fix(search): break intent ties on identity instead of node id

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
+}

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)
 	}
 }

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,

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 {

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")
 	}

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)))