From 084ac4514d049cad6caa7894ff908de10f6780da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Thu, 24 Sep 2026 07:58:51 +0100 Subject: [PATCH] fix(query): send hub content once across process_symbols rows (#3356) * fix(query): send hub content once across process_symbols rows With include_content, a symbol in several execution flows carried its full source text on every (id, process_id) row. Keep content on the first row for each symbol id and omit it from later rows. Membership fields, is_entry_point, and symbol_count are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) * fix(query): point agents to the content row and lock the dedup in tests The query tool text now says content is kept once per symbol id across the whole process_symbols array, possibly under a different process_id, and names context({uid, include_content: true}) as the fallback. Tests: the integration suite asserts func:validate has two rows with content on exactly one. Unit tests cover a later entry-point row that is flagged and stripped, a hub in three processes, and that the shaper does not mutate its input. Co-Authored-By: Claude Opus 5.5 (1M context) * fix(query): state the content-once rule on include_content and in the query hint The query tool's include_content property now says content is sent once per symbol id, on its first process_symbols row, and that context({uid: "", include_content: true}) returns it for any row. When a query asked for content, the Next hint adds that same fallback; without include_content the hint is unchanged. The example call now uses the "" placeholder style used elsewhere in the tool text. Tests: pin the property sentence, check the hint with and without include_content, and cover a max_symbols slice that moves the content row to a later process and a first row that is also the entry point. Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Gergo Magyar Co-authored-by: Claude Opus 5.5 (1M context) --- .../src/mcp/local/query-process-attaches.ts | 15 ++- gitnexus/src/mcp/server.ts | 2 +- gitnexus/src/mcp/tools.ts | 6 +- .../local-backend-calltool.test.ts | 13 ++ .../unit/mcp/query-process-attaches.test.ts | 126 +++++++++++++++++- gitnexus/test/unit/server.test.ts | 31 +++++ gitnexus/test/unit/tools.test.ts | 17 +++ 7 files changed, 200 insertions(+), 10 deletions(-) diff --git a/gitnexus/src/mcp/local/query-process-attaches.ts b/gitnexus/src/mcp/local/query-process-attaches.ts index 963978118..bb81531c7 100644 --- a/gitnexus/src/mcp/local/query-process-attaches.ts +++ b/gitnexus/src/mcp/local/query-process-attaches.ts @@ -5,6 +5,8 @@ * Aggregation already pushes a hub onto every owning process. This helper * is the last shaping step: slice each process to `max_symbols`, keep one * row per `(id, process_id)`, then set `symbol_count` from those rows. + * `content` stays only on the first row for each symbol id so a hub in + * many flows does not repeat its source text. */ export type QueryProcessAttach = { @@ -55,6 +57,7 @@ export function shapeQueryProcessAttaches( ): { processes: QueryProcessCard[]; process_symbols: S[] } { const { maxSymbolsPerProcess, chainByProcessId } = options; const seen = new Set(); + const contentSeen = new Set(); const process_symbols: S[] = []; const countByProcess = new Map(); @@ -63,8 +66,16 @@ export function shapeQueryProcessAttaches( const key = attachPairKey(s.id, s.process_id); if (seen.has(key)) continue; seen.add(key); - const row = - p.entryPointId && s.id === p.entryPointId ? ({ ...s, is_entry_point: true } as S) : s; + let row: S = s; + if (s.content !== undefined) { + if (contentSeen.has(s.id)) { + const { content: _content, ...rest } = s; + row = rest as S; + } else { + contentSeen.add(s.id); + } + } + if (p.entryPointId && s.id === p.entryPointId) row = { ...row, is_entry_point: true }; process_symbols.push(row); countByProcess.set(s.process_id, (countByProcess.get(s.process_id) ?? 0) + 1); } diff --git a/gitnexus/src/mcp/server.ts b/gitnexus/src/mcp/server.ts index d44f8c0f2..d2353b1cc 100644 --- a/gitnexus/src/mcp/server.ts +++ b/gitnexus/src/mcp/server.ts @@ -63,7 +63,7 @@ function getNextStepHint(toolName: string, args: Record | undefined return `\n\n---\n**Next:** READ gitnexus://repo/{name}/context for any repo above to get its overview and check staleness. If pagination.hasMore is true, call list_repos again with offset set to pagination.nextOffset to fetch the rest.`; case 'query': - return `\n\n---\n**Next:** To understand a specific symbol in depth, use context({name: ""${repoParam}}) to see categorized refs and process participation.`; + return `\n\n---\n**Next:** To understand a specific symbol in depth, use context({name: ""${repoParam}}) to see categorized refs and process participation.${args?.include_content === true ? ` For source of a process_symbols row without content, use context({uid: "", include_content: true${repoParam}}).` : ''}`; case 'context': return `\n\n---\n**Next:** If planning changes, use impact({target: "${args?.name || ''}", direction: "upstream"${repoParam}}) to check blast radius. To see execution flows, READ gitnexus://repo/${repoPath}/processes.`; diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 4daaf27d4..2bdda85be 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -144,11 +144,11 @@ specify the "repo" parameter explicitly.`, Returns ranked processes plus a flat process_symbols list. Join processes[].id to process_symbols[].process_id. WHEN TO USE: Understanding how code works together. Use this when you need execution flows and relationships, not just file matches. Complements grep/IDE search. -AFTER THIS: Use context() on a specific symbol for 360-degree view (callers, callees, categorized refs). +AFTER THIS: Use context() on a specific symbol for 360-degree view (callers, callees, categorized refs). With include_content, context() also returns that symbol's source. Returns results grouped by process (execution flow): - processes: ranked execution flows with relevance priority. When a process has an HTTP endpoint, each item includes route and method string aliases plus routes: [{ url, method? }] (same shape as context). When chain_depth > 0, each item also includes chain — layered upstream callers + downstream callees from the process entry symbol (same BFS as context({chain_depth})). -- process_symbols: search-hit symbols in those flows with file locations and module (functional area). On the single-repo envelope { processes, process_symbols, definitions }: One row per (id, process_id) — the same symbol id may appear under more than one process. Join a process to its rows by process_id; symbol_count is the number of those rows. When the process entry is among those hits, it is marked is_entry_point: true. A repo of "@" returns { group, query, results, per_repo } and does not include process_symbols. results[].symbol_count is the member's post-slice attach count; when service is set, it counts only attaches under that prefix. To get process_symbols for one member, query again with repo "@/" (member path from group.yaml, or results[]._repo). +- process_symbols: search-hit symbols in those flows with file locations and module (functional area). On the single-repo envelope { processes, process_symbols, definitions }: One row per (id, process_id) — the same symbol id may appear under more than one process. Join a process to its rows by process_id; symbol_count is the number of those rows. When the process entry is among those hits, it is marked is_entry_point: true. With include_content, content appears only on the first row for each symbol id across the whole process_symbols array, not per process; later rows for that id omit it, even under a different process_id. To get content for a row without it, find the earlier row with the same id, or call context({uid: "", include_content: true}). A repo of "@" returns { group, query, results, per_repo } and does not include process_symbols. results[].symbol_count is the member's post-slice attach count; when service is set, it counts only attaches under that prefix. To get process_symbols for one member, query again with repo "@/" (member path from group.yaml, or results[]._repo). - definitions: standalone types/interfaces not in any process. Keyword hits on Route URLs (route_fts) are bridged to their handler via HANDLES_ROUTE (handlerSymbolId, routes) when the edge exists; use route_map({route}) for the full HTTP surface. Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank Fusion. @@ -196,7 +196,7 @@ ${HOT_READ_STALENESS_NOTE}`, include_content: { type: 'boolean', description: - 'Include source text retained for matching symbols (default: false). The response reports contentAvailability; indexes built with content retention "none" explicitly report unavailable content.', + 'Include source text retained for matching symbols (default: false). The response reports contentAvailability; indexes built with content retention "none" explicitly report unavailable content. Content is sent once per symbol id, on its first process_symbols row; context({uid: "", include_content: true}) returns it for any row.', default: false, }, chain_depth: { diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index d76b3b716..f618a3c5b 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -207,6 +207,19 @@ withTestLbugDB( // leaked some other node's community onto it. It must have none. expect(validate.module).toBeUndefined(); expect(validate.content).toBe('function validate() {}'); + // validate is a step in two processes, so it has two process_symbols + // rows; content is emitted once per symbol id — only the first row + // carries it, the sibling row omits the key entirely. + const validateRows = (validateRes.process_symbols ?? []).filter( + (s: { id: string }) => s.id === 'func:validate', + ); + expect(validateRows).toHaveLength(2); + const withContent = validateRows.filter( + (s: { content?: string }) => s.content === 'function validate() {}', + ); + expect(withContent).toHaveLength(1); + const withoutContent = validateRows.filter((s: object) => !('content' in s)); + expect(withoutContent).toHaveLength(1); }); it('reports content capability for the default full profile', async () => { diff --git a/gitnexus/test/unit/mcp/query-process-attaches.test.ts b/gitnexus/test/unit/mcp/query-process-attaches.test.ts index 23e368303..51040c942 100644 --- a/gitnexus/test/unit/mcp/query-process-attaches.test.ts +++ b/gitnexus/test/unit/mcp/query-process-attaches.test.ts @@ -102,7 +102,7 @@ describe('shapeQueryProcessAttaches', () => { expect(processes[0]?.symbol_count).toBe(1); }); - it('keeps include_content on every emitted hub attach (KTD5)', () => { + it('keeps include_content only on the first row for a hub id', () => { const { process_symbols } = shapeQueryProcessAttaches( [ ranked('proc:login-flow', [ @@ -115,10 +115,128 @@ describe('shapeQueryProcessAttaches', () => { { maxSymbolsPerProcess: 25 }, ); - expect(process_symbols.map((s) => s.content)).toEqual([ - 'function validate() {}', - 'function validate() {}', + expect(process_symbols.map((s) => s.process_id)).toEqual(['proc:login-flow', 'proc:beta-flow']); + expect(process_symbols[0]?.content).toBe('function validate() {}'); + expect(process_symbols[1]).not.toHaveProperty('content'); + }); + + it('strips content from a later entry-point row while flagging it', () => { + const { process_symbols } = shapeQueryProcessAttaches( + [ + ranked('proc:login-flow', [ + attach('func:validate', 'proc:login-flow', { content: 'function validate() {}' }), + ]), + ranked( + 'proc:beta-flow', + [attach('func:validate', 'proc:beta-flow', { content: 'function validate() {}' })], + { entryPointId: 'func:validate' }, + ), + ], + { maxSymbolsPerProcess: 25 }, + ); + + expect(process_symbols.map((s) => s.process_id)).toEqual(['proc:login-flow', 'proc:beta-flow']); + expect(process_symbols[0]?.content).toBe('function validate() {}'); + expect(process_symbols[0]).not.toHaveProperty('is_entry_point'); + expect(process_symbols[1]?.is_entry_point).toBe(true); + expect(process_symbols[1]).not.toHaveProperty('content'); + }); + + it('keeps content only on the first of three rows for a hub id', () => { + const { processes, process_symbols } = shapeQueryProcessAttaches( + [ + ranked('proc:login-flow', [ + attach('func:validate', 'proc:login-flow', { content: 'function validate() {}' }), + ]), + ranked('proc:beta-flow', [ + attach('func:validate', 'proc:beta-flow', { content: 'function validate() {}' }), + ]), + ranked('proc:gamma-flow', [ + attach('func:validate', 'proc:gamma-flow', { content: 'function validate() {}' }), + ]), + ], + { maxSymbolsPerProcess: 25 }, + ); + + expect(process_symbols.map((s) => s.process_id)).toEqual([ + 'proc:login-flow', + 'proc:beta-flow', + 'proc:gamma-flow', ]); + expect(process_symbols.map((s) => Object.hasOwn(s, 'content'))).toEqual([true, false, false]); + expect(process_symbols[0]?.content).toBe('function validate() {}'); + expect(processes.map((p) => [p.id, p.symbol_count])).toEqual([ + ['proc:login-flow', 1], + ['proc:beta-flow', 1], + ['proc:gamma-flow', 1], + ]); + }); + + it('moves content to the first emitted row when max_symbols drops an earlier hub row', () => { + const { processes, process_symbols } = shapeQueryProcessAttaches( + [ + ranked('proc:alpha-flow', [ + attach('func:other', 'proc:alpha-flow', { content: 'function other() {}' }), + attach('func:validate', 'proc:alpha-flow', { content: 'function validate() {}' }), + ]), + ranked('proc:beta-flow', [ + attach('func:validate', 'proc:beta-flow', { content: 'function validate() {}' }), + ]), + ], + { maxSymbolsPerProcess: 1 }, + ); + + expect(process_symbols.map((s) => [s.id, s.process_id])).toEqual([ + ['func:other', 'proc:alpha-flow'], + ['func:validate', 'proc:beta-flow'], + ]); + const hubRows = process_symbols.filter((s) => s.id === 'func:validate'); + expect(hubRows).toHaveLength(1); + expect(hubRows[0]?.content).toBe('function validate() {}'); + expect(processes.map((p) => [p.id, p.symbol_count])).toEqual([ + ['proc:alpha-flow', 1], + ['proc:beta-flow', 1], + ]); + }); + + it('keeps content on a first row that is also the entry point', () => { + const { process_symbols } = shapeQueryProcessAttaches( + [ + ranked( + 'proc:login-flow', + [attach('func:validate', 'proc:login-flow', { content: 'function validate() {}' })], + { entryPointId: 'func:validate' }, + ), + ranked('proc:beta-flow', [ + attach('func:validate', 'proc:beta-flow', { content: 'function validate() {}' }), + ]), + ], + { maxSymbolsPerProcess: 25 }, + ); + + expect(process_symbols.map((s) => s.process_id)).toEqual(['proc:login-flow', 'proc:beta-flow']); + expect(process_symbols[0]?.content).toBe('function validate() {}'); + expect(process_symbols[0]?.is_entry_point).toBe(true); + expect(process_symbols[1]).not.toHaveProperty('content'); + expect(process_symbols[1]).not.toHaveProperty('is_entry_point'); + }); + + it('does not mutate the input attach objects', () => { + const first = attach('func:validate', 'proc:login-flow', { content: 'function validate() {}' }); + const second = attach('func:validate', 'proc:beta-flow', { content: 'function validate() {}' }); + const before = structuredClone([first, second]); + + shapeQueryProcessAttaches( + [ + ranked('proc:login-flow', [first]), + ranked('proc:beta-flow', [second], { entryPointId: 'func:validate' }), + ], + { maxSymbolsPerProcess: 25 }, + ); + + expect([first, second]).toEqual(before); + expect(second.content).toBe('function validate() {}'); + expect(second).not.toHaveProperty('is_entry_point'); }); it('marks the entry-point hit and preserves process card extras', () => { diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts index c1b2a63df..544987e09 100644 --- a/gitnexus/test/unit/server.test.ts +++ b/gitnexus/test/unit/server.test.ts @@ -252,6 +252,37 @@ describe('getNextStepHint (via tool call response)', () => { // The actual hint logic is tested via the integration path. expect(backend.callTool).not.toHaveBeenCalled(); // not called until request }); + + const QUERY_HINT_BASE = + '\n\n---\n**Next:** To understand a specific symbol in depth, use context({name: "", repo: "demo"}) to see categorized refs and process participation.'; + + it('query hint is unchanged when include_content is not set', async () => { + const backend = createMockBackend({ + callTool: vi.fn().mockResolvedValue({ processes: [] }), + }); + const { text } = await callToolThroughServer(backend, 'query', { + search_query: 'auth', + repo: 'demo', + }); + expect(text.endsWith(QUERY_HINT_BASE)).toBe(true); + expect(text).not.toContain('context({uid:'); + }); + + it('query hint names the context({uid, include_content}) fallback when include_content is true', async () => { + const backend = createMockBackend({ + callTool: vi.fn().mockResolvedValue({ processes: [] }), + }); + const { text } = await callToolThroughServer(backend, 'query', { + search_query: 'auth', + repo: 'demo', + include_content: true, + }); + expect( + text.endsWith( + `${QUERY_HINT_BASE} For source of a process_symbols row without content, use context({uid: "", include_content: true, repo: "demo"}).`, + ), + ).toBe(true); + }); }); describe('MCP output budgets', () => { diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index aba1c7fd8..3cb464ef8 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -136,6 +136,23 @@ describe('GITNEXUS_TOOLS', () => { 'when service is set, it counts only attaches under that prefix', ); expect(queryTool.description).toContain('query again with repo "@/"'); + expect(queryTool.description).toContain( + 'content appears only on the first row for each symbol id across the whole process_symbols array, not per process', + ); + expect(queryTool.description).toContain('even under a different process_id'); + expect(queryTool.description).toContain('context({uid: "", include_content: true})'); + expect(queryTool.description).toContain( + "With include_content, context() also returns that symbol's source.", + ); + }); + + it('query include_content property states the once-per-id rule and the context() fallback', () => { + const queryTool = GITNEXUS_TOOLS.find((t) => t.name === 'query')!; + const description = queryTool.inputSchema.properties.include_content.description; + expect(description).toContain('Include source text retained for matching symbols'); + expect(description).toContain( + 'Content is sent once per symbol id, on its first process_symbols row; context({uid: "", include_content: true}) returns it for any row.', + ); }); it('query tool requires "search_query" parameter (renamed from "query" for #2175)', () => {