diff --git a/src/handoff/schema.ts b/src/handoff/schema.ts index 9771946..049ac9c 100644 --- a/src/handoff/schema.ts +++ b/src/handoff/schema.ts @@ -12,6 +12,7 @@ export interface PreparedStatements { /** Sessions whose retrieval units do not reach their last message. */ selectSessionsMissingUnits: Statement; selectSessionMessages: Statement; + messageOffsetInSession: Statement; selectSessionTool: Statement; selectUnitIds: Statement; insertUnit: Statement; @@ -258,6 +259,34 @@ export function prepareStatements(db: DatabaseHandle): PreparedStatements { WHERE session_ref = ? ORDER BY timestamp ASC, message_index ASC, id ASC`, ), + /** + * How many messages of a session sort before a given `message_index`. + * + * `getSessionDetail` pages by POSITION — `LIMIT ? OFFSET ?` over this + * same ordering — while a retrieval unit records the `message_index` + * values at its edges. Those two coincide only while a session's + * numbering is dense and monotonic in timestamp order, and real + * transcripts are neither: one session here carries 828 duplicate + * messages and 862 places where index order disagrees with time order, + * which is what made a match point somewhere unrelated. + * + * The ordering below is character-for-character the one + * `selectSessionMessages` and `getSessionDetail` use. If any of the + * three changes, all three must. + */ + messageOffsetInSession: db.prepare( + `WITH target AS ( + SELECT timestamp, message_index, id FROM messages + WHERE session_ref = ? AND message_index = ? + ) + SELECT COUNT(*) AS count + FROM messages m, target t + WHERE m.session_ref = ? + AND (m.timestamp < t.timestamp + OR (m.timestamp = t.timestamp AND m.message_index < t.message_index) + OR (m.timestamp = t.timestamp AND m.message_index = t.message_index + AND m.id < t.id))`, + ), selectSessionTool: db.prepare("SELECT tool FROM sessions WHERE session_ref = ?"), selectUnitIds: db.prepare("SELECT id FROM retrieval_units WHERE session_ref = ?"), insertUnit: db.prepare( diff --git a/src/handoff/sqlite-index.ts b/src/handoff/sqlite-index.ts index 08ba835..9c2170b 100644 --- a/src/handoff/sqlite-index.ts +++ b/src/handoff/sqlite-index.ts @@ -816,7 +816,9 @@ export class SqliteHandoffIndex implements SessionService { ): Promise { const rows = this.queryKeywordUnits(query, limit, toolFilter, branchFilter); // Rank by BM25 position so relevance, not recency, dominates ordering. - return groupUnits(rows, rankKeywordRows(rows), "keyword", normalizeLimit(limit, DEFAULT_LIMIT)); + return this.withDetailOffsets( + groupUnits(rows, rankKeywordRows(rows), "keyword", normalizeLimit(limit, DEFAULT_LIMIT)), + ); } private async semanticSearch( @@ -861,15 +863,62 @@ export class SqliteHandoffIndex implements SessionService { // Only needed to score vectors; skip the model entirely when there are none. const queryVector = rows.length > 0 ? await this.embeddingProvider.embed(query) : null; - return rankSearchCandidates({ - rows, - keywordRows, - queryVector, - mode, - limit: normalizedLimit, - cosineSimilarity, - deserializeVector, - }); + return this.withDetailOffsets( + rankSearchCandidates({ + rows, + keywordRows, + queryVector, + mode, + limit: normalizedLimit, + cosineSimilarity, + deserializeVector, + }), + ); + } + + /** + * Give every match the `offset` that actually reaches it. + * + * A window records the `message_index` values at its edges, and those are + * not positions. `getSessionDetail` pages by position, so handing an agent + * the index values — which is what the rendered `Match 5987-2108` was — + * lands it wherever those numbers happen to fall. On a live index 430 of + * 9,728 windows (4.4%) even had an end index BELOW their start, because one + * session carried 828 duplicate messages and 862 disagreements between index + * order and time order. + * + * Resolved on read rather than stored: the stored edges are part of a + * window's id, so changing them would orphan every vector in every existing + * index and cost each user a full re-embed — measured at 92 minutes on the + * project that exposed this. At most three matches per session over five + * sessions, each an indexed count, is the cheaper end of that trade. + */ + private withDetailOffsets(sessions: SessionSummary[]): SessionSummary[] { + for (const session of sessions) { + for (const match of session.matches ?? []) { + const offset = this.resolveDetailOffset(session.session_ref, match.message_start_index); + if (offset !== null) { + match.detail_offset = offset; + } + } + } + return sessions; + } + + private resolveDetailOffset(sessionRef: string, messageIndex: number): number | null { + try { + const row = this.prepared().messageOffsetInSession.get( + sessionRef, + messageIndex, + sessionRef, + ) as { count: number } | undefined; + return row?.count ?? null; + } catch { + // A pointer is an aid, not the answer: a session whose messages this + // cannot locate still returns its match, preview and all. Failing the + // whole search over a missing signpost would be the worse trade. + return null; + } } private queryKeywordUnits( diff --git a/src/handoff/types.ts b/src/handoff/types.ts index 53714f7..981ec08 100644 --- a/src/handoff/types.ts +++ b/src/handoff/types.ts @@ -202,6 +202,20 @@ export interface RetrievalMatch { started_at: string; ended_at: string; preview: string; + /** + * The `offset` to pass to `xtctx_session_detail` to land on this match. + * + * Not the same number as `message_start_index`, and that difference is the + * bug this exists to fix. Detail pages by POSITION in the session's + * timestamp ordering; a window records the `message_index` VALUES at its + * edges. The two agree only while numbering is dense and monotonic in time + * order, which real transcripts are not — measured on one live index, 430 of + * 9,728 windows (4.4%) had an end index BELOW their start, and following the + * rendered range as an offset landed three weeks away from the match. + * + * Absent for a literal match, which has no indexed session to page through. + */ + detail_offset?: number; /** * How similar this window is to the query, on the same absolute scale as the * session's score. Absent for keyword-only matches, whose keyword score is diff --git a/src/mcp/tools/sessions.ts b/src/mcp/tools/sessions.ts index 9700c36..51d9c46 100644 --- a/src/mcp/tools/sessions.ts +++ b/src/mcp/tools/sessions.ts @@ -271,9 +271,18 @@ function formatRecentSessionsMarkdown( lines.push(`- Preview: ${inlineSafe(session.preview)}`); } for (const match of session.matches ?? []) { - lines.push( - `- Match ${match.message_start_index}-${match.message_end_index}: ${inlineSafe(match.preview)}`, - ); + // Says what to do with the number rather than printing a bare pair. + // + // This rendered `Match ${start}-${end}` from the window's message_index + // values, next to a tool whose parameter is called "Message offset" — + // an invitation an agent took literally, and those are not offsets. On a + // live index 4.4% of windows even printed backwards (`Match 5987-2108`), + // and following one landed three weeks from the match. + const pointer = + match.detail_offset === undefined + ? "" + : ` (xtctx_session_detail offset=${match.detail_offset})`; + lines.push(`- Match${pointer}: ${inlineSafe(match.preview)}`); } lines.push(""); } diff --git a/tests/handoff/match-detail-offset.test.ts b/tests/handoff/match-detail-offset.test.ts new file mode 100644 index 0000000..e6b675c --- /dev/null +++ b/tests/handoff/match-detail-offset.test.ts @@ -0,0 +1,143 @@ +/** + * A match has to say where it is in terms the detail tool understands. + * + * `xtctx_search_sessions` rendered `Match ${message_start_index}-${end}` next + * to a tool whose parameter is documented as "Message offset for pagination". + * An agent auditing this project took that invitation literally — and those + * are not offsets. `getSessionDetail` pages by POSITION in the session's + * timestamp ordering; a window stores the `message_index` VALUES at its edges. + * + * The two agree only while a session's numbering is dense and monotonic in + * time order, and real transcripts are neither. Measured on a live index: 430 + * of 9,728 windows (4.4%) had an end index BELOW their start — the rendered + * range read backwards, `Match 5987-2108` — because one session carried 828 + * duplicate messages and 862 places where index order disagrees with time + * order. Following such a pointer landed three weeks away from the match. + * + * The fixture below reproduces that shape deliberately: message indices that + * do not ascend with time, so position and index cannot be confused for one + * another. + */ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { SqliteHandoffIndex } from "@xtctx/handoff/sqlite-index"; +import type { ConversationChunk, ConversationScraper, ScraperState } from "@xtctx/types/scraper"; + +class FixtureScraper implements ConversationScraper { + readonly tool = "codex"; + constructor(private readonly chunks: ConversationChunk[]) {} + async detect(): Promise { + return true; + } + getStorePaths(): string[] { + return ["fixture://codex"]; + } + async *scrape(): AsyncIterable { + yield* this.chunks; + } + async *fullSync(): AsyncIterable { + yield* this.chunks; + } + async getLastScrapedPosition(): Promise { + return { lastTimestamp: new Date(0) }; + } + async saveScrapedPosition(): Promise { + return; + } +} + +/** + * Messages whose `message_index` deliberately does not ascend with time. + * + * The second half is numbered far above the first but timestamped after it — + * the signature of a transcript that was re-ingested under fresh numbering, + * which is what a Claude Code `/compact` produces. + */ +function skewedConversation(): ConversationChunk[] { + const chunks: ConversationChunk[] = []; + const at = (minute: number) => new Date(Date.UTC(2026, 0, 1, 0, minute)); + for (let i = 0; i < 12; i++) { + chunks.push({ + tool: "codex", + sessionId: "skewed", + timestamp: at(i), + role: i % 2 === 0 ? "user" : "assistant", + content: `early message ${i} about the parser fallback`, + metadata: { messageIndex: i, tokenEstimate: 1, layer: 0 }, + }); + } + for (let i = 0; i < 12; i++) { + chunks.push({ + tool: "codex", + sessionId: "skewed", + timestamp: at(12 + i), + // Numbered 500+ while sorting after the block above. + content: `late message ${i} about the parser fallback`, + role: i % 2 === 0 ? "user" : "assistant", + metadata: { messageIndex: 500 + i, tokenEstimate: 1, layer: 0 }, + }); + } + return chunks; +} + +describe("a match's detail offset", () => { + let tempDir = ""; + let index: SqliteHandoffIndex; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "xtctx-offset-")); + index = new SqliteHandoffIndex( + join(tempDir, "xtctx.db"), + tempDir, + [{ tool: "codex", scraper: new FixtureScraper(skewedConversation()) }], + { windowSize: 4, windowStride: 2 }, + ); + await index.listRecentSessions(1); + await index.whenScanSettled(); + }); + + afterEach(async () => { + await index.close().catch(() => {}); + await rm(tempDir, { recursive: true, force: true }); + }); + + it("lands on the match, not wherever the index numbers happen to fall", async () => { + const results = await index.searchSessions("parser fallback", 5, undefined, "keyword"); + const matches = results.flatMap((session) => + (session.matches ?? []).map((match) => ({ ref: session.session_ref, match })), + ); + expect(matches.length).toBeGreaterThan(0); + + for (const { ref, match } of matches) { + expect(match.detail_offset, "every indexed match should carry a pointer").toBeDefined(); + + // The message the pointer reaches must be the window's own first + // message. Read through the public detail path, exactly as an agent + // would follow it. + const [landed] = await index.getSessionDetail(ref, match.detail_offset as number, 1); + const [expected] = await index.getSessionDetail(ref, 0, 100).then((all) => + all.slice(match.detail_offset as number, (match.detail_offset as number) + 1), + ); + + expect(landed).toBeDefined(); + expect(landed.content).toBe(expected.content); + } + }, 60_000); + + it("does not hand back the raw index values, which are not offsets", async () => { + // The specific confusion: on this fixture the later windows carry indices + // of 500+, while the session holds only 24 messages. An offset of 500 + // reaches nothing at all. + const results = await index.searchSessions("late message", 5, undefined, "keyword"); + const matches = results.flatMap((session) => session.matches ?? []); + const skewed = matches.filter((match) => match.message_start_index >= 500); + + expect(skewed.length, "fixture should produce high-numbered windows").toBeGreaterThan(0); + for (const match of skewed) { + expect(match.detail_offset).toBeLessThan(24); + expect(match.detail_offset).not.toBe(match.message_start_index); + } + }, 60_000); +});