From 334c6316e2862d3e4d04f4b2499c8f440f8e168c Mon Sep 17 00:00:00 2001 From: tae2089 Date: Tue, 11 Aug 2026 16:39:50 +0900 Subject: [PATCH 1/2] test: let a caller choose the seeding order of the tied intent fixture 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 --- .../adapters/outbound/searchsql/intent_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/adapters/outbound/searchsql/intent_test.go b/internal/adapters/outbound/searchsql/intent_test.go index c9a512fd..a15f5193 100644 --- a/internal/adapters/outbound/searchsql/intent_test.go +++ b/internal/adapters/outbound/searchsql/intent_test.go @@ -54,7 +54,25 @@ func seedIntentFixture(t *testing.T, db *gorm.DB) (reasoned, namesake graph.Node // every one of them scores identically and only the tiebreak decides the order. func seedTiedIntentFixture(t *testing.T, db *gorm.DB, count int) { t.Helper() + seedTiedIntentNodes(t, db, tiedIntentNames(count)) +} + +// tiedIntentNames is the fixture's declarations in ascending order, which is +// both their file order and, when they are seeded in this sequence, their id +// order. A test that seeds them in some other sequence takes the two apart. +func tiedIntentNames(count int) []int { + indexes := make([]int, 0, count) for i := range count { + indexes = append(indexes, i) + } + return indexes +} + +// seedTiedIntentNodes writes the tied declarations in the sequence given, so a +// caller can decide which declaration gets the lowest id. +func seedTiedIntentNodes(t *testing.T, db *gorm.DB, indexes []int) { + t.Helper() + for _, i := range indexes { node := graph.Node{ QualifiedName: fmt.Sprintf("tied.decl%02d", i), Kind: graph.NodeKindFunction, From 1e63a713535428b11b4e40083cc8705e2cdc8faa Mon Sep 17 00:00:00 2001 From: tae2089 Date: Tue, 11 Aug 2026 16:40:12 +0900 Subject: [PATCH 2/2] fix(search): break intent ties on identity instead of node id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../outbound/searchsql/intent_order_test.go | 92 +++++++++++++++++++ .../outbound/searchsql/intent_reasons_test.go | 20 ++-- .../adapters/outbound/searchsql/postgres.go | 20 ++-- .../adapters/outbound/searchsql/sqlite.go | 36 +++++--- internal/app/search/intentrank/rank.go | 51 ++++++++-- internal/app/search/intentrank/rank_test.go | 60 ++++++++++-- internal/app/search/rank/rank.go | 19 ++-- internal/domain/graph/identity.go | 60 ++++++++++++ 8 files changed, 300 insertions(+), 58 deletions(-) create mode 100644 internal/adapters/outbound/searchsql/intent_order_test.go create mode 100644 internal/domain/graph/identity.go diff --git a/internal/adapters/outbound/searchsql/intent_order_test.go b/internal/adapters/outbound/searchsql/intent_order_test.go new file mode 100644 index 00000000..0e6108a7 --- /dev/null +++ b/internal/adapters/outbound/searchsql/intent_order_test.go @@ -0,0 +1,92 @@ +//go:build fts5 + +package searchsql + +import ( + "slices" + "testing" + + intentapp "github.com/tae2089/code-context-graph/internal/app/search/intent" + requestctx "github.com/tae2089/code-context-graph/internal/ctx" +) + +// The same corpus, indexed in a different sequence, has to answer the same +// question the same way. +// +// Recorded reasons are one sentence long, so exact score ties are the normal +// case rather than the edge case, and whatever breaks those ties decides the +// answer. Breaking them by node id makes the answer a function of the order the +// rows happened to be written: re-index a repository from a clean checkout, or +// let a webhook rebuild arrive in a different order, and the same question comes +// back with a different top hit. Nobody can act on an answer like that, and +// nobody can measure one either. +// +// The two orders here are the same twelve declarations, seeded ascending and +// descending, so every declaration's id is different between the two databases +// while its name, path and reason are identical. +func TestQueryIntent_SameCorpusSeededEitherWayGivesTheSameAnswer(t *testing.T) { + const count = 12 + ascending := tiedIntentNames(count) + descending := slices.Clone(ascending) + slices.Reverse(descending) + + forward := answerTiedIntent(t, ascending, count) + backward := answerTiedIntent(t, descending, count) + + if len(forward) != count || len(backward) != count { + t.Fatalf("got %d and %d answers, want %d each", len(forward), len(backward), count) + } + for i := range forward { + if forward[i] != backward[i] { + t.Fatalf("row %d is %s when the corpus is seeded ascending and %s when it is seeded descending;"+ + " the answer depends on insertion order", i, forward[i], backward[i]) + } + } +} + +// Asking for one more row has to extend the answer, not reshuffle it — the +// promise the tie-break exists to keep. This is checked against a corpus seeded +// backwards, because a tie-break that happens to agree with insertion order +// keeps that promise for the wrong reason. +func TestQueryIntent_ExtendsRatherThanReshufflesOnABackwardsSeededCorpus(t *testing.T) { + const count = 12 + descending := slices.Clone(tiedIntentNames(count)) + slices.Reverse(descending) + + short := answerTiedIntent(t, descending, 4) + long := answerTiedIntent(t, descending, count) + + if len(short) != 4 || len(long) != count { + t.Fatalf("got %d and %d answers, want 4 and %d", len(short), len(long), count) + } + for i, name := range short { + if long[i] != name { + t.Fatalf("row %d is %s at limit 4 but %s at limit %d", i, name, long[i], count) + } + } +} + +// answerTiedIntent seeds one database in the given sequence and returns the +// answer as qualified names, which are the same in every seeding while the ids +// are not. +func answerTiedIntent(t *testing.T, indexes []int, limit int) []string { + t.Helper() + db := setupTestDB(t) + seedTiedIntentNodes(t, db, indexes) + backend := buildIntentIndex(t, db) + ctx := requestctx.WithNamespace(t.Context(), requestctx.DefaultNamespace) + + result, err := NewReader(db, backend).QueryIntent(ctx, "what keeps the queue draining", limit) + if err != nil { + t.Fatalf("QueryIntent: %v", err) + } + return qualifiedNames(result) +} + +func qualifiedNames(result intentapp.Result) []string { + names := make([]string, 0, len(result.Hits)) + for _, hit := range result.Hits { + names = append(names, hit.Node.QualifiedName) + } + return names +} diff --git a/internal/adapters/outbound/searchsql/intent_reasons_test.go b/internal/adapters/outbound/searchsql/intent_reasons_test.go index 5cb61ce0..710d40c5 100644 --- a/internal/adapters/outbound/searchsql/intent_reasons_test.go +++ b/internal/adapters/outbound/searchsql/intent_reasons_test.go @@ -82,16 +82,18 @@ func TestIntentIndex_HoldsOneDocumentPerReasonTag(t *testing.T) { func TestQueryIntent_ExtraRulesDoNotSinkTheDeclarationThatWroteThem(t *testing.T) { db := setupTestDB(t) const shared = "verify the signature so a push from anywhere else is rejected" - // The loaded declaration is seeded first so it holds the lower node id. Ties - // break on id, so on equal footing it must come first — and under the joined - // index it cannot, because its three rules lengthened its one document. - loaded := seedReasoned(t, db, "verifyLoaded", + // Ties break on identity, and `reasons/verifyAll.go` sorts before + // `reasons/verifyBare.go`, so on equal footing the loaded declaration must come + // first — and under the joined index it cannot, because its three rules + // lengthened its one document. It is seeded second on purpose: it holds the + // higher node id, so an answer that still leans on id order gets this backwards. + bare := seedReasoned(t, db, "verifyBare", graph.DocTag{Kind: graph.TagIntent, Value: shared}) + loaded := seedReasoned(t, db, "verifyAll", graph.DocTag{Kind: graph.TagIntent, Value: shared}, graph.DocTag{Kind: graph.TagDomainRule, Value: "the shared secret is read from the environment and never logged"}, graph.DocTag{Kind: graph.TagDomainRule, Value: "an unsigned request is refused before the body is parsed"}, graph.DocTag{Kind: graph.TagDomainRule, Value: "a signature that does not compare in constant time is a defect"}, ) - bare := seedReasoned(t, db, "verifyBare", graph.DocTag{Kind: graph.TagIntent, Value: shared}) backend := buildIntentIndex(t, db) ctx := requestctx.WithNamespace(t.Context(), requestctx.DefaultNamespace) @@ -108,10 +110,10 @@ func TestQueryIntent_ExtraRulesDoNotSinkTheDeclarationThatWroteThem(t *testing.T if !bareSeen || !loadedSeen { t.Fatalf("both declarations must answer; got %v", answeringNodes(result)) } - // Same reason, same words, so neither may be ranked below the other. Ties - // break on node id, which is the only order the two can legitimately differ in. - if (bareRank < loadedRank) != (bare.ID < loaded.ID) { - t.Errorf("rank order (%d before %d) does not follow the tiebreak; the three domain rules moved the score", + // Same reason, same words, so the score cannot separate them and the identity + // tiebreak decides: file path first, which puts verifyAll ahead of verifyBare. + if loadedRank > bareRank { + t.Errorf("verifyBare answered at %d and verifyAll at %d, want the file-path order; the three domain rules moved the score", bareRank, loadedRank) } } diff --git a/internal/adapters/outbound/searchsql/postgres.go b/internal/adapters/outbound/searchsql/postgres.go index 6fc6dd18..8a1ff86d 100644 --- a/internal/adapters/outbound/searchsql/postgres.go +++ b/internal/adapters/outbound/searchsql/postgres.go @@ -113,10 +113,10 @@ type resultRow struct { // rather than reordering them — so the same repository indexed twice could answer // differently. `n.id` last makes the key total. // -// The key order is deliberately NOT rank.compareIdentity's, which compares file -// path before qualified name (and then kind and namespace). See the SQLite -// matchRows for why that difference matters and why aligning them belongs to #106 -// rather than here. +// The key order is deliberately NOT graph.CompareIdentity's, which compares file +// path before qualified name (and then kind, namespace and start line). See the +// SQLite matchRows for why that difference matters and why #106 left retrieval on +// this order while giving rerank and intent scoring the shared one. // // @intent let Query run the same retrieval twice with a different expression. func (p *PostgresBackend) matchRows(ctx context.Context, db *gorm.DB, tsQuery, ns string, limit int) ([]resultRow, error) { @@ -184,7 +184,13 @@ func (p *PostgresBackend) Query(ctx context.Context, db *gorm.DB, query string, // them, so it could not tell a distinctive word from a filler one. Retrieval is // what the GIN index is genuinely good at; scoring moved to intentrank, which // counts the corpus and gives both backends the same answer. -// @intent hand every candidate reason to shared scoring, in whatever order the index produced. +// +// The join onto nodes carries each candidate's identity — path, qualified name, +// kind, namespace, start line — because that is what intentrank breaks its score +// ties on, and it drops reason rows whose node is gone rather than spending a row +// of the candidate cap on one that cannot be scored. The SQLite twin does the +// same; both have to, or the two backends tie-break on different information. +// @intent hand every candidate reason to shared scoring, with the identity that scoring breaks ties on. // @requires maxCandidates must be greater than 0. // @return returns unordered candidates with the exact text that was indexed for each. func (p *PostgresBackend) MatchIntent(ctx context.Context, db *gorm.DB, query string, maxCandidates int) ([]intentrank.Doc, error) { @@ -198,8 +204,10 @@ func (p *PostgresBackend) MatchIntent(ctx context.Context, db *gorm.DB, query st var docs []intentrank.Doc if err := db.WithContext(ctx).Raw(` - SELECT sr.node_id, sr.content + SELECT sr.node_id, sr.content, + n.file_path, n.qualified_name, n.kind, n.namespace, n.start_line FROM search_reasons sr + JOIN nodes n ON n.id = sr.node_id WHERE sr.reason_tsv @@ to_tsquery('simple', ?) AND sr.namespace = ? LIMIT ?`, tsQuery, requestctx.FromContext(ctx), maxCandidates).Scan(&docs).Error; err != nil { diff --git a/internal/adapters/outbound/searchsql/sqlite.go b/internal/adapters/outbound/searchsql/sqlite.go index 2f1f21cc..b7f0435b 100644 --- a/internal/adapters/outbound/searchsql/sqlite.go +++ b/internal/adapters/outbound/searchsql/sqlite.go @@ -287,15 +287,19 @@ type ftsRow struct { // tied rows past it are not reordered but dropped, so the same repository indexed // twice could answer differently. `n.id` last makes the key total. // -// The key order here is deliberately NOT rank.compareIdentity's, which compares -// file path before qualified name (and then kind and namespace). The difference -// is not cosmetic: it decides which tied rows survive the LIMIT. File-path-first -// would cut on file boundaries more often, which is closer to what wire.FileGroup -// promises one layer up — that a file appearing in an answer appears whole. -// Aligning the two is #106's job, where compareIdentity is reused across -// retrieval, rerank and intent together and one measurement covers all three. -// Do not flip these keys on their own: every ranking number and every recapture -// figure recorded for #103 was measured with this order. +// The key order here is deliberately NOT graph.CompareIdentity's, which compares +// file path before qualified name (and then kind, namespace and start line). The +// difference is not cosmetic: it decides which tied rows survive the LIMIT. +// File-path-first would cut on file boundaries more often, which is closer to what +// wire.FileGroup promises one layer up — that a file appearing in an answer appears +// whole. +// +// #106 gave rerank and intent scoring that one shared key; retrieval is the layer +// it left alone, because flipping these keys changes which rows the LIMIT keeps and +// so moves measured ranking numbers. Do not flip them on their own: every ranking +// number and every recapture figure recorded for #103 was measured with this order, +// so the flip needs a -capture-golden recapture and a re-judgment landing as its own +// change. // // @intent let Query run the same retrieval twice with a different expression. func (s *SQLiteBackend) matchRows(ctx context.Context, db *gorm.DB, ftsQuery, ns string, limit int) ([]ftsRow, error) { @@ -362,7 +366,13 @@ func (s *SQLiteBackend) Query(ctx context.Context, db *gorm.DB, query string, li // intentrank so that this backend and the PostgreSQL one answer the same // question the same way. What is left here is retrieval, which is what the index // is for. -// @intent hand every candidate reason to shared scoring, in whatever order the index produced. +// +// The join onto nodes carries each candidate's identity — path, qualified name, +// kind, namespace, start line — because that is what intentrank breaks its score +// ties on. It also drops index rows whose node is gone, the same way matchRows +// does: an orphan cannot be scored and would otherwise spend a row of the +// candidate cap. +// @intent hand every candidate reason to shared scoring, with the identity that scoring breaks ties on. // @requires maxCandidates must be greater than 0. // @return returns unordered candidates with the exact text that was indexed for each. func (s *SQLiteBackend) MatchIntent(ctx context.Context, db *gorm.DB, query string, maxCandidates int) ([]intentrank.Doc, error) { @@ -376,9 +386,11 @@ func (s *SQLiteBackend) MatchIntent(ctx context.Context, db *gorm.DB, query stri var docs []intentrank.Doc if err := db.WithContext(ctx).Raw( - `SELECT CAST(node_id AS INTEGER) AS node_id, content + `SELECT CAST(intent_fts.node_id AS INTEGER) AS node_id, intent_fts.content, + n.file_path, n.qualified_name, n.kind, n.namespace, n.start_line FROM intent_fts - WHERE intent_fts MATCH ? AND namespace = ? + JOIN nodes n ON n.id = CAST(intent_fts.node_id AS INTEGER) + WHERE intent_fts MATCH ? AND intent_fts.namespace = ? LIMIT ?`, ftsQuery, requestctx.FromContext(ctx), maxCandidates).Scan(&docs).Error; err != nil { return nil, trace.Wrap(err, "intent fts query") } diff --git a/internal/app/search/intentrank/rank.go b/internal/app/search/intentrank/rank.go index 6a76bd84..41ffac7b 100644 --- a/internal/app/search/intentrank/rank.go +++ b/internal/app/search/intentrank/rank.go @@ -10,15 +10,38 @@ import ( "github.com/tae2089/code-context-graph/internal/app/search/identtoken" "github.com/tae2089/code-context-graph/internal/app/search/queryterm" + "github.com/tae2089/code-context-graph/internal/domain/graph" ) // Doc is one candidate the index admitted: a node and one recorded reason that // was indexed for it. A node that recorded several reasons arrives as several // Docs sharing a node id. +// +// It carries the node's identity alongside its id because scoring ties are the +// normal case here, and the tie-break has to mean the same thing after a +// re-index. An id cannot: it is handed out in the order rows were written. // @intent carry the exact indexed text into scoring so the score is computed over what was matched. type Doc struct { - NodeID uint - Content string + NodeID uint + Content string + FilePath string + QualifiedName string + Kind graph.NodeKind + Namespace string + StartLine int +} + +// identity is who this candidate's node is, in the form every layer of search +// breaks ties on. +// @intent keep the fields that make up the tie-break named in one place. +func (d Doc) identity() graph.Identity { + return graph.Identity{ + FilePath: d.FilePath, + QualifiedName: d.QualifiedName, + Kind: d.Kind, + Namespace: d.Namespace, + StartLine: d.StartLine, + } } // Match is one declaration the question reached, and the terms of the question @@ -132,9 +155,10 @@ func Rank(question string, docs []Doc, corpusSize, limit int) Result { // term, summed over terms, charges each term exactly the length of the reason // it was written in and still counts how much of the question was answered. type scored struct { - nodeID uint - score float64 - best []float64 + nodeID uint + identity graph.Identity + score float64 + best []float64 } grouped := make([]scored, 0, len(docs)) position := make(map[uint]int, len(docs)) @@ -143,7 +167,7 @@ func Rank(question string, docs []Doc, corpusSize, limit int) Result { if !seen { at = len(grouped) position[doc.NodeID] = at - grouped = append(grouped, scored{nodeID: doc.NodeID, best: make([]float64, len(groups))}) + grouped = append(grouped, scored{nodeID: doc.NodeID, identity: doc.identity(), best: make([]float64, len(groups))}) } node := &grouped[at] for g := range groups { @@ -165,14 +189,21 @@ func Rank(question string, docs []Doc, corpusSize, limit int) Result { results = append(results, node) } - // The node id is not a preference, it is the promise that asking for one more - // row extends the answer instead of reshuffling it. Reasons are one sentence - // long, so exact ties are common rather than rare. + // The tie-break is not a preference, it is the promise that asking for one + // more row extends the answer instead of reshuffling it. Reasons are one + // sentence long, so exact ties are common rather than rare. + // + // It breaks on identity and not on the node id, which used to sit here. An id + // is handed out in the order rows were written, so it kept that promise for a + // single database and broke a bigger one: index the same repository twice in a + // different order and the same question came back with a different top hit. + // Identity is the same key the reranker uses, so the two layers of one search + // cannot disagree about who comes first. sort.SliceStable(results, func(a, b int) bool { if results[a].score != results[b].score { return results[a].score > results[b].score } - return results[a].nodeID < results[b].nodeID + return graph.CompareIdentity(results[a].identity, results[b].identity) < 0 }) matches := make([]Match, 0, min(limit, len(results))) diff --git a/internal/app/search/intentrank/rank_test.go b/internal/app/search/intentrank/rank_test.go index d0ff3f02..1b6af4dd 100644 --- a/internal/app/search/intentrank/rank_test.go +++ b/internal/app/search/intentrank/rank_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/tae2089/code-context-graph/internal/app/search/intentrank" + "github.com/tae2089/code-context-graph/internal/domain/graph" ) // rank is the shorthand every test here uses: score these documents against this @@ -44,17 +45,60 @@ func TestRank_PrefersTheRarerWord(t *testing.T) { // Two documents that earn exactly the same score must always come back in the // same order, or asking for one more row reshuffles the answer. -func TestRank_BreaksTiesByNodeID(t *testing.T) { +// +// The order is the identity order, and the ids here run against it on purpose: +// ascending file path is descending node id, so a tie broken by id would answer +// 3, 5, 7 while a tie broken by identity answers 7, 5, 3. +func TestRank_BreaksTiesByIdentityNotByNodeID(t *testing.T) { + docs := tiedDocs() + got := rank(t, "what keeps the queue draining", len(docs), docs...) + if !slices.Equal(got, []uint{7, 5, 3}) { + t.Fatalf("got %v, want the file-path order 7, 5, 3 on a three-way tie", got) + } +} + +// The same corpus with its ids handed out differently is the same answer. This +// is the unit-level form of what re-indexing a repository does: same +// declarations, same reasons, different ids. +func TestRank_TiedAnswerDoesNotDependOnWhichIDsWereHandedOut(t *testing.T) { + docs := tiedDocs() + reassigned := make([]intentrank.Doc, 0, len(docs)) + for i, doc := range docs { + doc.NodeID = uint(100 - i) + reassigned = append(reassigned, doc) + } + + first := namesInAnswerOrder(rank(t, "what keeps the queue draining", len(docs), docs...), docs) + second := namesInAnswerOrder(rank(t, "what keeps the queue draining", len(reassigned), reassigned...), reassigned) + if !slices.Equal(first, second) { + t.Fatalf("answer is %v with one set of ids and %v with another", first, second) + } +} + +// tiedDocs is three declarations whose recorded reason is byte-identical, so all +// three score the same and only the tie-break decides the order. +func tiedDocs() []intentrank.Doc { same := "keep the queue draining under backpressure" - docs := []intentrank.Doc{ - {NodeID: 7, Content: same}, - {NodeID: 3, Content: same}, - {NodeID: 5, Content: same}, + return []intentrank.Doc{ + {NodeID: 7, Content: same, FilePath: "queue/drain.go", QualifiedName: "queue.Drain", Kind: graph.NodeKindFunction}, + {NodeID: 3, Content: same, FilePath: "queue/worker.go", QualifiedName: "queue.Worker", Kind: graph.NodeKindFunction}, + {NodeID: 5, Content: same, FilePath: "queue/pump.go", QualifiedName: "queue.Pump", Kind: graph.NodeKindFunction}, } - got := rank(t, "what keeps the queue draining", len(docs), docs...) - if !slices.Equal(got, []uint{3, 5, 7}) { - t.Fatalf("got %v, want ascending node ids on a three-way tie", got) +} + +// namesInAnswerOrder reads an answer back as qualified names, which stay the +// same across two seedings while the ids do not. +func namesInAnswerOrder(answer []uint, docs []intentrank.Doc) []string { + names := make([]string, 0, len(answer)) + for _, id := range answer { + for _, doc := range docs { + if doc.NodeID == id { + names = append(names, doc.QualifiedName) + break + } + } } + return names } // A short Latin term matches whole words only. `get` reaching `getAnnotation` diff --git a/internal/app/search/rank/rank.go b/internal/app/search/rank/rank.go index 9282c328..df6a3b84 100644 --- a/internal/app/search/rank/rank.go +++ b/internal/app/search/rank/rank.go @@ -231,21 +231,14 @@ func RerankGroups(query string, groups [][]graph.Node, limit int) []graph.Node { } // compareIdentity orders two structurally tied candidates by who they are. -// File path comes first so a tie group reads as whole files, matching how the -// evidence list will group it anyway; namespace is included because federated -// search can hold the same file in two repositories. +// +// The key itself lives in the domain, as graph.CompareIdentity, because the +// intent scorer has to break its ties the same way. Two layers of one search +// disagreeing about who comes first is how an answer starts depending on which +// layer produced it. // @intent break structural ties by node identity so the order never depends on which backend retrieved the pool. func compareIdentity(a, b graph.Node) int { - if by := cmp.Compare(a.FilePath, b.FilePath); by != 0 { - return by - } - if by := cmp.Compare(a.QualifiedName, b.QualifiedName); by != 0 { - return by - } - if by := cmp.Compare(a.Kind, b.Kind); by != 0 { - return by - } - return cmp.Compare(a.Namespace, b.Namespace) + return graph.CompareIdentity(a.Identity(), b.Identity()) } // applyLimit bounds the result slice, treating a non-positive limit as unbounded. diff --git a/internal/domain/graph/identity.go b/internal/domain/graph/identity.go new file mode 100644 index 00000000..80c0a872 --- /dev/null +++ b/internal/domain/graph/identity.go @@ -0,0 +1,60 @@ +package graph + +import "cmp" + +// Identity is who a node is, told apart from which row it happens to be. +// +// The database gives every node an id in the order the rows were written, so an +// id says when a node was indexed, not what it is. Re-index the same repository +// from a clean checkout and every id can differ while every Identity here stays +// the same. Anything that has to produce the same answer twice — a tie-break, a +// stable sort, a comparison across two databases — belongs on this and not on +// the id. +// +// The four fields are the columns of the node table's uniqueness index +// (namespace, qualified_name, file_path, start_line) plus the kind, so two +// different nodes cannot share one Identity. +// @intent give ranking a key that survives re-indexing, which the node id does not. +type Identity struct { + FilePath string + QualifiedName string + Kind NodeKind + Namespace string + StartLine int +} + +// Identity says who this node is. +// @intent read a node's stable identity without repeating which fields make it up. +func (n Node) Identity() Identity { + return Identity{ + FilePath: n.FilePath, + QualifiedName: n.QualifiedName, + Kind: n.Kind, + Namespace: n.Namespace, + StartLine: n.StartLine, + } +} + +// CompareIdentity orders two nodes by who they are, for the callers that have +// run out of reasons to prefer one over the other. +// +// File path comes first so a tie group reads as whole files, matching how an +// evidence list groups it anyway. Namespace is in there because federated search +// can hold the same file in two repositories, and start line because one file +// can declare the same qualified name twice. +// @intent give every layer of search one tie-break, so two layers cannot disagree about who comes first. +func CompareIdentity(a, b Identity) int { + if by := cmp.Compare(a.FilePath, b.FilePath); by != 0 { + return by + } + if by := cmp.Compare(a.QualifiedName, b.QualifiedName); by != 0 { + return by + } + if by := cmp.Compare(a.Kind, b.Kind); by != 0 { + return by + } + if by := cmp.Compare(a.Namespace, b.Namespace); by != 0 { + return by + } + return cmp.Compare(a.StartLine, b.StartLine) +}