fix(search): break intent ties on identity instead of node id - #110
Merged
Conversation
seedTiedIntentFixture always wrote the tied declarations in ascending order, which makes file order and id order the same sequence. A test that wants to prove the answer does not depend on which id a declaration got needs to seed the same corpus the other way round. Split the loop out as seedTiedIntentNodes(indexes) and keep the ascending sequence as tiedIntentNames(count). No test changes behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two recorded reasons that score the same were ordered by node id, so the same declarations re-indexed under different ids answered in a different order. Rerank already ordered by identity — file path, qualified name, kind, namespace, start line — so the two layers of one answer disagreed about what "same score" means. Move that key into the domain as graph.Identity and graph.CompareIdentity, have rank.compareIdentity delegate to it, and widen intentrank.Doc with the identity fields so intent scoring breaks its ties the same way. Both MatchIntent queries now join nodes to carry the identity, which also drops reason rows whose node is gone rather than spending a candidate slot on one that cannot be scored. Measured on the live graphs, comparing the two tie-breaks over every golden question: ccg (1740 recorded reasons) answers 78 of 91 questions, 19 of those in a different order, 18 of the 19 keeping the same top hit; context-diary (110 reasons) answers 18 of 21, none of them differently. The golden report does not move, and cannot: intent_candidates.json freezes the post-Rank answer, so `make search-eval` never runs intentrank.Rank. ccg stays at ALL 0.747 (127/170) 48 62 0.649, ANSWERABLE 0.840. Retrieval's matchRows keeps its own key order on purpose — flipping it decides which tied rows survive the LIMIT and so moves measured numbers. Both comments now say what #106 did and what a later change still has to recapture and re-judge. Closes #106 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #106.
What changed
internal/domain/graph/identity.go(new):IdentityandCompareIdentity— file path, qualified name, kind, namespace, start line. Those five are also the node uniqueness index, so the key is total.rank.compareIdentitynow delegates to it; the reranker's order is unchanged.intentrank.Doccarries the identity fields and the score-tie sort usesgraph.CompareIdentityinstead of node id.MatchIntentqueries joinnodesto carry that identity, which also drops reason rows whose node is gone instead of spending a candidate slot on one that cannot be scored.Why
The same declarations re-indexed under different ids answered in a different order, and rerank and intent scoring disagreed about what "same score" means.
Measurement
Comparing the two tie-breaks over every golden question against the live graphs:
The golden report does not move, and cannot:
intent_candidates.jsonfreezes the post-Rank answer, somake search-evalnever runsintentrank.Rank. ccg stays at ALL0.747 (127/170) 48 62 0.649, ANSWERABLE0.840.Deliberately out of scope
Retrieval's
matchRowskeeps its own key order (rank, qualified name, file path, id). Flipping it decides which tied rows survive theLIMIT, so it moves measured ranking numbers and needs a-capture-goldenrecapture plus a re-judgment landing as its own change. Both comments now say what this change did and what that later one still owes.Verification
CGO_ENABLED=1 go test -tags "fts5" ./... -count=1— greengo vet -tags fts5 ./...— cleangofmt -l .— only the two pre-existingtestdata/binding_gap/gofixturesmake search-eval— unchanged on all four corpora🤖 Generated with Claude Code