Repository navigation
Sort query results by exactly one, deliberately chosen property (#129) - #153
Merged
Merged
Conversation
The KG query API allows "sort": true on only one root-level property, but generate_query() flagged every property named name, fullName or lookupLabel, so classes with both name and lookup_label sent two sort keys and the KG used whichever came first. Each generated class now has a sort_property attribute, chosen by the builder from a priority order (name, full_name, lookup_label, family_name) with per-class exceptions: ParcellationEntity and ParcellationEntityVersion sort by lookup_label, which keeps entities from the same atlas together. Query.serialize() now rejects queries with more than one sort key. As a result, SlicingDevice, Electrode, ElectrodeArray and Pipette are sorted by name instead of lookup_label, and Person is now sorted by family_name. Adds offline tests over all v4 and v5 classes, live tests of the KG's ordering (case-insensitive, missing values first), and documentation of result ordering, including that the core API, used by default for unfiltered list(), does not sort. Implementation made use of Claude Opus 5.5
Member
Author
|
Note that the |
apdavison
marked this pull request as ready for review
October 17, 2026 19:13
Member
Author
|
Both this and #154 ended up with very similar solutions; here the implementation is a bit more elegant, and is very close to what I would have implemented by hand (probably with better tests). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #129
This PR is part of an experiment comparing the performance of different AI coding agents. The other PR addressing the same issue is #154.
This implementation made use of Claude Opus 5.5 running in Claude Code. The initial prompt, in a checkout of my fork of fairgraph, was "Please make a plan to address issue #129 from the upstream HumanBrainProject/fairgraph repository".
One notable feature of the planning process was that Claude asked what priority should be given to different properties when deciding which to sort on ("name", "lookupLabel", etc.), and one if its suggestions was a per-class priority. This was the suggestion I went with, although I suggested a different implementation to the one Claude proposed.