diff --git a/gitnexus/src/core/group/extractors/manifest-extractor.ts b/gitnexus/src/core/group/extractors/manifest-extractor.ts index 53e65c20f..4462f51f9 100644 --- a/gitnexus/src/core/group/extractors/manifest-extractor.ts +++ b/gitnexus/src/core/group/extractors/manifest-extractor.ts @@ -15,6 +15,10 @@ export interface ManifestExtractResult { // reserved-keyword labels `Macro` and `Union`, and LadybugDB's parser rejects // a disjunction that names a reserved keyword (#2325) — which the resolver's // try/catch then swallowed. `labels(n) IN` has no such collision. +// This list overlaps `ingestion/utils/symbol-labels.ts` (SYMBOL_NODE_LABELS) but +// is a deliberate SUBSET — it omits `Namespace`/`Variable`/`Module`. Unifying the +// two would widen which nodes resolve as contract symbols and must update the +// #2325 test, so they are intentionally kept separate for now. export const CUSTOM_CONTRACT_RESOLVE_QUERY = `MATCH (n) WHERE labels(n) IN ['Function','Method','Class','Interface','Struct','Enum','Trait','Constructor','TypeAlias','Impl','Macro','Union','Typedef','Property','Record','Delegate','Annotation','Template','Const','Static','CodeElement'] AND n.name = $symbolName diff --git a/gitnexus/src/core/ingestion/cobol-processor.ts b/gitnexus/src/core/ingestion/cobol-processor.ts index e7aa064de..8963ea893 100644 --- a/gitnexus/src/core/ingestion/cobol-processor.ts +++ b/gitnexus/src/core/ingestion/cobol-processor.ts @@ -15,6 +15,7 @@ import path from 'node:path'; import { generateId } from '../../lib/utils.js'; +import { toZeroBasedLine } from './utils/line-base.js'; import { SupportedLanguages } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../graph/types.js'; import { @@ -359,8 +360,8 @@ function mapToGraph( properties: { name: extracted.programName, filePath, - startLine: 1, - endLine: lines.length, + startLine: toZeroBasedLine(1), + endLine: toZeroBasedLine(lines.length), language: SupportedLanguages.Cobol, isExported: true, description: metaDesc || undefined, @@ -394,8 +395,8 @@ function mapToGraph( properties: { name: prog.name, filePath, - startLine: prog.startLine, - endLine: prog.endLine, + startLine: toZeroBasedLine(prog.startLine), + endLine: toZeroBasedLine(prog.endLine), language: SupportedLanguages.Cobol, isExported: true, description: `nested-program${prog.isCommon ? ' common' : ''}`, @@ -442,8 +443,8 @@ function mapToGraph( properties: { name: sec.name, filePath, - startLine: sec.line, - endLine: nextLine, + startLine: toZeroBasedLine(sec.line), + endLine: toZeroBasedLine(nextLine), language: SupportedLanguages.Cobol, isExported: true, }, @@ -477,8 +478,8 @@ function mapToGraph( properties: { name: para.name, filePath, - startLine: para.line, - endLine: nextLine, + startLine: toZeroBasedLine(para.line), + endLine: toZeroBasedLine(nextLine), language: SupportedLanguages.Cobol, isExported: true, }, @@ -511,8 +512,8 @@ function mapToGraph( properties: { name: item.name, filePath, - startLine: item.line, - endLine: item.line, + startLine: toZeroBasedLine(item.line), + endLine: toZeroBasedLine(item.line), language: SupportedLanguages.Cobol, description: `level:${item.level} section:${item.section}${item.pic ? ` pic:${item.pic}` : ''}`, }, @@ -614,8 +615,8 @@ function mapToGraph( properties: { name: `CALL ${call.target}`, filePath, - startLine: call.line, - endLine: call.line, + startLine: toZeroBasedLine(call.line), + endLine: toZeroBasedLine(call.line), language: SupportedLanguages.Cobol, description: 'dynamic-call (target is a data item, not resolvable statically)', }, @@ -742,8 +743,8 @@ function mapToGraph( properties: { name: `EXEC SQL ${sql.operation}`, filePath, - startLine: sql.line, - endLine: sql.line, + startLine: toZeroBasedLine(sql.line), + endLine: toZeroBasedLine(sql.line), language: SupportedLanguages.Cobol, description: `tables:[${sql.tables.join(',')}] cursors:[${sql.cursors.join(',')}]`, }, @@ -817,8 +818,8 @@ function mapToGraph( properties: { name: `EXEC CICS ${cics.command}`, filePath, - startLine: cics.line, - endLine: cics.line, + startLine: toZeroBasedLine(cics.line), + endLine: toZeroBasedLine(cics.line), language: SupportedLanguages.Cobol, description: [ @@ -856,8 +857,8 @@ function mapToGraph( properties: { name: `CICS ${cics.command} ${cics.programName}`, filePath, - startLine: cics.line, - endLine: cics.line, + startLine: toZeroBasedLine(cics.line), + endLine: toZeroBasedLine(cics.line), language: SupportedLanguages.Cobol, description: `cics-dynamic-program (target is data item ${cics.programName})`, }, @@ -1029,8 +1030,8 @@ function mapToGraph( properties: { name: entry.name, filePath, - startLine: entry.line, - endLine: entry.line, + startLine: toZeroBasedLine(entry.line), + endLine: toZeroBasedLine(entry.line), language: SupportedLanguages.Cobol, isExported: true, description: @@ -1176,8 +1177,8 @@ function mapToGraph( properties: { name: `EXEC DLI ${dli.verb}`, filePath, - startLine: dli.line, - endLine: dli.line, + startLine: toZeroBasedLine(dli.line), + endLine: toZeroBasedLine(dli.line), language: SupportedLanguages.Cobol, description: [ @@ -1316,8 +1317,8 @@ function mapToGraph( properties: { name: fd.selectName, filePath, - startLine: fd.line, - endLine: fd.line, + startLine: toZeroBasedLine(fd.line), + endLine: toZeroBasedLine(fd.line), language: SupportedLanguages.Cobol, description: `assign:${fd.assignTo}${fd.isOptional ? ' optional' : ''}${fd.organization ? ` org:${fd.organization}` : ''}${fd.access ? ` access:${fd.access}` : ''}`, }, @@ -1406,8 +1407,8 @@ function mapToGraph( properties: { name: `CANCEL ${cancel.target}`, filePath, - startLine: cancel.line, - endLine: cancel.line, + startLine: toZeroBasedLine(cancel.line), + endLine: toZeroBasedLine(cancel.line), language: SupportedLanguages.Cobol, description: 'dynamic-cancel (target is a data item, not resolvable statically)', }, diff --git a/gitnexus/src/core/ingestion/cobol/jcl-processor.ts b/gitnexus/src/core/ingestion/cobol/jcl-processor.ts index 9f4eecfe0..1ed54f42d 100644 --- a/gitnexus/src/core/ingestion/cobol/jcl-processor.ts +++ b/gitnexus/src/core/ingestion/cobol/jcl-processor.ts @@ -19,6 +19,7 @@ import { parseJcl, type JclParseResults } from './jcl-parser.js'; import type { KnowledgeGraph } from '../../graph/types.js'; import { generateId } from '../../../lib/utils.js'; +import { toZeroBasedLine } from '../utils/line-base.js'; export interface JclProcessResult { jobCount: number; @@ -98,8 +99,8 @@ function integrateJclResults( properties: { name: job.name, filePath, - startLine: job.line, - endLine: job.line, + startLine: toZeroBasedLine(job.line), + endLine: toZeroBasedLine(job.line), description: `jcl-job${classPart}${msgPart}`, }, }); @@ -137,8 +138,8 @@ function integrateJclResults( properties: { name: step.name, filePath, - startLine: step.line, - endLine: step.line, + startLine: toZeroBasedLine(step.line), + endLine: toZeroBasedLine(step.line), description: `jcl-step${pgmPart}${procPart}`, }, }); @@ -209,8 +210,8 @@ function integrateJclResults( properties: { name: dd.dataset, filePath, - startLine: dd.line, - endLine: dd.line, + startLine: toZeroBasedLine(dd.line), + endLine: toZeroBasedLine(dd.line), description: `jcl-dataset${dispPart}`, }, @@ -244,8 +245,8 @@ function integrateJclResults( properties: { name: proc.name, filePath, - startLine: proc.line, - endLine: proc.line, + startLine: toZeroBasedLine(proc.line), + endLine: toZeroBasedLine(proc.line), description: 'jcl-proc-instream', }, }); diff --git a/gitnexus/src/core/ingestion/emit-references.ts b/gitnexus/src/core/ingestion/emit-references.ts index 8c1145874..f03819c02 100644 --- a/gitnexus/src/core/ingestion/emit-references.ts +++ b/gitnexus/src/core/ingestion/emit-references.ts @@ -57,6 +57,7 @@ import type { } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../graph/types.js'; import type { ScopeResolutionIndexes } from './model/scope-resolution-indexes.js'; +import { toZeroBasedLine } from './utils/line-base.js'; // ─── Public API ───────────────────────────────────────────────────────────── @@ -140,8 +141,8 @@ export function emitScopeGraph(input: { properties: { name: scope.kind, filePath: scope.filePath, - startLine: scope.range.startLine, - endLine: scope.range.endLine, + startLine: toZeroBasedLine(scope.range.startLine), + endLine: toZeroBasedLine(scope.range.endLine), description: `Scope: ${scope.kind}`, } as unknown as Parameters[0]['properties'], }); diff --git a/gitnexus/src/core/ingestion/markdown-processor.ts b/gitnexus/src/core/ingestion/markdown-processor.ts index b2013ee7a..2372f20e3 100644 --- a/gitnexus/src/core/ingestion/markdown-processor.ts +++ b/gitnexus/src/core/ingestion/markdown-processor.ts @@ -8,6 +8,7 @@ import path from 'node:path'; import { generateId } from '../../lib/utils.js'; +import { toZeroBasedLine } from './utils/line-base.js'; import type { GraphNode } from 'gitnexus-shared'; import { KnowledgeGraph } from '../graph/types.js'; @@ -81,8 +82,8 @@ export const processMarkdown = ( properties: { name: heading, filePath: file.path, - startLine: lineNum, - endLine, + startLine: toZeroBasedLine(lineNum), + endLine: toZeroBasedLine(endLine), level, description: `h${level}`, }, diff --git a/gitnexus/src/core/ingestion/utils/line-base.ts b/gitnexus/src/core/ingestion/utils/line-base.ts new file mode 100644 index 000000000..3fe684ab5 --- /dev/null +++ b/gitnexus/src/core/ingestion/utils/line-base.ts @@ -0,0 +1,20 @@ +/** + * Convert a 1-based source line number to the 0-based convention used by + * GraphNode `startLine`/`endLine`. + * + * The graph layer stores line numbers 0-based (tree-sitter `startPosition.row`), + * and this is load-bearing: the taint/PDG/CFG join and the MCP consumers all add + * `+ 1` to recover 1-based (see `summary-harvest-driver.ts` — "Function/Method + * node startLine is 0-based"). Most emitters get 0-based for free from + * tree-sitter. The exceptions are the regex-based COBOL/JCL processors (their + * parsers use `lineNum = i + 1`) and the scope-capture path (`Capture` ranges + * are 1-based per RFC §2.1). Those must convert to 0-based when they build a + * graph node, or the exact-content slice in `csv-generator.ts` drops the + * symbol's declaration line (#2379) and reported line numbers are off (#2377). + * + * Apply this ONLY at the graph-node `startLine:`/`endLine:` assignment. The + * parser-internal 1-based values (`.line`, `prog.startLine`) stay 1-based — + * they feed `L${line}` node/edge IDs and line-range containment checks that + * must not shift. The clamp guards degenerate inputs (line 0 / empty files). + */ +export const toZeroBasedLine = (oneBasedLine: number): number => Math.max(0, oneBasedLine - 1); diff --git a/gitnexus/src/core/ingestion/utils/symbol-labels.ts b/gitnexus/src/core/ingestion/utils/symbol-labels.ts new file mode 100644 index 000000000..a21df10b6 --- /dev/null +++ b/gitnexus/src/core/ingestion/utils/symbol-labels.ts @@ -0,0 +1,47 @@ +import type { NodeLabel } from 'gitnexus-shared'; + +/** + * Graph-node labels that represent a resolvable code symbol — a definition with + * its own source span (function, type, member, module-like container). + * + * These get EXACT source-span content in the FTS index: `csv-generator.ts` + * slices exactly `[startLine, endLine]` for them (no ±2 padding), while every + * other label keeps the context window. That exactness depends on the 0-based + * `startLine`/`endLine` invariant enforced by `line-base.ts` — the slice is only + * correct because all emitters store 0-based lines. Keep the two together. + * + * Single source of truth so the set can't silently drift the way the inline copy + * did in #2379. + * + * NOTE: `group/extractors/manifest-extractor.ts`'s `CUSTOM_CONTRACT_RESOLVE_QUERY` + * carries a near-identical hand-list that is intentionally a SUBSET — it excludes + * `Namespace`, `Variable`, `Module`. Unifying the two needs a contract-resolution + * behavior check (would widen which nodes resolve as contract symbols), so it is + * deliberately left separate for now. + */ +export const SYMBOL_NODE_LABELS: ReadonlySet = new Set([ + 'Function', + 'Method', + 'Class', + 'Interface', + 'CodeElement', + 'Struct', + 'Enum', + 'Macro', + 'Typedef', + 'Union', + 'Namespace', + 'Trait', + 'Impl', + 'TypeAlias', + 'Const', + 'Static', + 'Variable', + 'Property', + 'Record', + 'Delegate', + 'Annotation', + 'Constructor', + 'Template', + 'Module', +]); diff --git a/gitnexus/src/core/lbug/csv-generator.ts b/gitnexus/src/core/lbug/csv-generator.ts index aef7bff74..b9cf8e309 100644 --- a/gitnexus/src/core/lbug/csv-generator.ts +++ b/gitnexus/src/core/lbug/csv-generator.ts @@ -20,6 +20,7 @@ import { KnowledgeGraph } from '../graph/types.js'; import { NodeTableName, NODE_TABLES } from './schema.js'; import { RelPairRouter } from './rel-pair-routing.js'; import { parseTruthyEnv } from '../ingestion/utils/env.js'; +import { SYMBOL_NODE_LABELS } from '../ingestion/utils/symbol-labels.js'; import { applyCjkSegmentationIfEnabled } from '../search/cjk-segmentation.js'; /** @@ -212,6 +213,11 @@ export const normalizeFtsText = (text: string): string => text.replace(/[\r\n\t] const formatFtsDescription = (description: string): string => normalizeFtsText(applyCjkSegmentationIfEnabled(description)); +// Labels that get exact source-span content (no ±2 window). Single source of +// truth in `symbol-labels.ts` — see there for why the exactness depends on the +// 0-based line invariant. Kept as a named alias to read intent at the use site. +const EXACT_SYMBOL_CONTENT_LABELS = SYMBOL_NODE_LABELS; + const extractContent = async (node: GraphNode, contentCache: FileContentCache): Promise => { const filePath = node.properties.filePath; const content = await contentCache.get(filePath); @@ -233,8 +239,9 @@ const extractContent = async (node: GraphNode, contentCache: FileContentCache): if (startLine === undefined || endLine === undefined) return ''; const lines = content.split('\n'); - const start = Math.max(0, startLine - 2); - const end = Math.min(lines.length - 1, endLine + 2); + const exactSymbolContent = EXACT_SYMBOL_CONTENT_LABELS.has(node.label); + const start = Math.max(0, exactSymbolContent ? startLine : startLine - 2); + const end = Math.min(lines.length - 1, exactSymbolContent ? endLine : endLine + 2); const snippet = lines.slice(start, end + 1).join('\n'); const MAX_SNIPPET = 5000; const capped = diff --git a/gitnexus/src/mcp/local/line-display.ts b/gitnexus/src/mcp/local/line-display.ts new file mode 100644 index 000000000..ec2cc2006 --- /dev/null +++ b/gitnexus/src/mcp/local/line-display.ts @@ -0,0 +1,25 @@ +/** + * Convert a 0-based GraphNode `startLine`/`endLine` to the 1-based line number + * shown to humans and LLMs in MCP tool output. + * + * Storage is 0-based (tree-sitter `startPosition.row`; see + * `ingestion/utils/line-base.ts`), which matches editors/`sed`/`less -N` only + * after `+ 1`. The `context`, `query`, and `impact` tools present line numbers a + * user cross-references against source, so they convert here at the response + * boundary (#2377). + * + * Apply ONLY to a symbol node's 0-based `startLine`/`endLine`. Do NOT apply to: + * - BasicBlock / CFG `functionStartLine` and PDG statement lines — already + * 1-based (they use `startPosition.row + 1`); + * - the internal `sym.startLine + 1` join params that target the 1-based + * BasicBlock id space; + * - raw `cypher` results, which pass LadybugDB columns through verbatim and + * stay 0-based (documented). + * + * `undefined`/`null` pass through so optional line fields stay absent. + */ +export function toDisplayLine(zeroBasedLine: number): number; +export function toDisplayLine(zeroBasedLine: number | null | undefined): number | undefined; +export function toDisplayLine(zeroBasedLine: number | null | undefined): number | undefined { + return typeof zeroBasedLine === 'number' ? zeroBasedLine + 1 : undefined; +} diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index fe32bc960..0956b4071 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -17,6 +17,7 @@ import { isLbugReady, } from '../../core/lbug/pool-adapter.js'; import { isValidQueryParams } from '../../core/lbug/query-params.js'; +import { toDisplayLine } from './line-display.js'; import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/lbug-config.js'; // Embedding imports are lazy (dynamic import) to avoid loading onnxruntime-node // at MCP server startup — crashes on unsupported Node ABI versions (#89) @@ -702,7 +703,7 @@ export class LocalBackend { query: (r, p) => this.query(r as RepoHandle, p), impactByUid: (id, uid, d, o) => this.impactByUid(id, uid, d, o), context: (r, p) => this.context(r as RepoHandle, p), - trace: (r, p) => this.trace(r as RepoHandle, p), + trace: (r, p) => this.traceForGroup(r as RepoHandle, p), resolveSymbol: (r, q) => this.resolveSymbolForGroup(r as RepoHandle, q), pdgFlows: (r, anchor, opts) => this.pdgFlowsForGroup(r as RepoHandle, anchor, opts), }; @@ -711,6 +712,24 @@ export class LocalBackend { return this.groupToolSvc; } + /** + * Adapt local `trace` to the group port. The assembled group/cross-repo trace + * presents 1-based endpoints (via resolveSymbolForGroup), so convert the hop + * lines here too — otherwise one response mixes 1-based endpoints with 0-based + * hops (#2380). Single-repo `trace` dispatches directly (not through this + * port) and stays 0-based (documented full-parity follow-up). + */ + private async traceForGroup(repo: RepoHandle, params: TraceParams): Promise { + const result = await this.trace(repo, params); + const hops = (result as { hops?: Array<{ startLine?: number | null }> }).hops; + if (Array.isArray(hops)) { + for (const hop of hops) { + hop.startLine = toDisplayLine(hop.startLine); + } + } + return result; + } + /** * Adapt the shared symbol resolver to the GroupToolPort contract. Used by the * cross-repo trace path to locate which member repo an endpoint lives in and @@ -735,8 +754,8 @@ export class LocalBackend { name: s.name, type: s.type, filePath: s.filePath, - startLine: s.startLine, - endLine: s.endLine, + startLine: toDisplayLine(s.startLine), + endLine: toDisplayLine(s.endLine), }, }; } @@ -748,7 +767,7 @@ export class LocalBackend { name: c.name, type: c.type, filePath: c.filePath, - startLine: c.startLine, + startLine: toDisplayLine(c.startLine), })), }; } @@ -1987,8 +2006,8 @@ export class LocalBackend { name: sym.name, type: sym.type, filePath: sym.filePath, - startLine: sym.startLine, - endLine: sym.endLine, + startLine: toDisplayLine(sym.startLine), + endLine: toDisplayLine(sym.endLine), ...(module ? { module } : {}), ...(includeContent && content ? { content } : {}), }; @@ -2255,8 +2274,11 @@ export class LocalBackend { name: sym.name || sym[1], type: sym.type || sym[2], filePath: sym.filePath || sym[3], - startLine: sym.startLine || sym[4], - endLine: sym.endLine || sym[5], + // Raw 0-based here — `bm25Search` is only called from `query()`, + // whose aggregation loop applies `toDisplayLine` once (see below). + // Converting here too would double-shift BM25-matched lines (#2380). + startLine: sym.startLine ?? sym[4], + endLine: sym.endLine ?? sym[5], bm25Score: bm25Result.score, }); } @@ -2989,7 +3011,7 @@ export class LocalBackend { name: c.name, kind: c.type, filePath: c.filePath, - line: c.startLine, + line: toDisplayLine(c.startLine), score: Number(c.score.toFixed(2)), })), }; @@ -3246,8 +3268,8 @@ export class LocalBackend { name: sym.name || sym[1], kind: symKind, filePath: sym.filePath || sym[3], - startLine: sym.startLine || sym[4], - endLine: sym.endLine || sym[5], + startLine: toDisplayLine(sym.startLine ?? sym[4]), + endLine: toDisplayLine(sym.endLine ?? sym[5]), ...(include_content && (sym.content || sym[6]) ? { content: sym.content || sym[6] } : {}), ...(methodMetadata ? { methodMetadata } : {}), }, @@ -3334,7 +3356,7 @@ export class LocalBackend { name: c.name, kind: c.type, filePath: c.filePath, - line: c.startLine, + line: toDisplayLine(c.startLine), score: Number(c.score.toFixed(2)), })), }, @@ -3355,11 +3377,14 @@ export class LocalBackend { anchorClause: 'a.id STARTS WITH $idPrefix AND a.startLine >= $symStart AND a.startLine <= $symEnd', queryParams: { idPrefix, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, + // Display anchor is 1-based, matching the ambiguous-candidate branch and + // the context/query/impact tools (#2380). This is display-only — the + // BasicBlock join above uses the raw `sym.startLine + 1` in `symStart`. anchor: { file: sym.filePath, symbol: sym.name, - startLine: sym.startLine, - endLine: sym.endLine, + startLine: toDisplayLine(sym.startLine), + endLine: toDisplayLine(sym.endLine), }, }; } @@ -4951,7 +4976,7 @@ export class LocalBackend { name: c.name, kind: c.type, filePath: c.filePath, - line: c.startLine, + line: toDisplayLine(c.startLine), score: Number(c.score.toFixed(2)), })), }; @@ -5016,7 +5041,7 @@ export class LocalBackend { name: c.name, kind: c.type, filePath: c.filePath, - line: c.startLine, + line: toDisplayLine(c.startLine), score: Number(c.score.toFixed(2)), impactedCount: summary?.impactedCount ?? 0, risk: summary?.risk ?? 'UNKNOWN', diff --git a/gitnexus/src/mcp/local/pdg-impact.ts b/gitnexus/src/mcp/local/pdg-impact.ts index 434d03a5a..6a25ab19b 100644 --- a/gitnexus/src/mcp/local/pdg-impact.ts +++ b/gitnexus/src/mcp/local/pdg-impact.ts @@ -11,6 +11,7 @@ import type { executeParameterized } from '../../core/lbug/pool-adapter.js'; import { loadMeta } from '../../storage/repo-manager.js'; import { IMPACT_MAX_DEPTH, PDG_QUERY_DEFAULT_LIMIT, PDG_QUERY_MAX_LIMIT } from '../tools.js'; import { CALLEES_TRUNCATED_SENTINEL, CALLEE_ID_SEP } from '../../core/ingestion/cfg/emit.js'; +import { toDisplayLine } from './line-display.js'; import { decodeCallSummary } from '../../core/ingestion/taint/call-summary-codec.js'; import { decodeReachingDefReason } from '../../core/ingestion/cfg/reaching-def-reason-codec.js'; import { getProviderForFile } from '../../core/ingestion/languages/index.js'; @@ -90,8 +91,10 @@ export function splitCalleeIds(raw: unknown): string[] { * Contract version of the mode:'pdg' impact result shape. A stable discriminator * for external MCP/agent consumers — distinct from the DB INCREMENTAL_SCHEMA_VERSION. * Bump on any breaking change to the PDG result fields. + * v2: `startLine` in the result is now 1-based display (#2380), matching the + * context/query/impact tools (was 0-based). */ -export const PDG_RESULT_VERSION = 1 as const; +export const PDG_RESULT_VERSION = 2 as const; /** A reachable dependence block resolved to its source statement. */ export interface PdgStatement { @@ -582,7 +585,7 @@ export interface PdgInterproceduralImpact { export interface PdgImpactBaseResult extends PdgImpactParityFields { mode: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 1; + pdgResultVersion: 2; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; impactedCount: number; @@ -655,7 +658,7 @@ export interface PdgImpactDegradedResult extends PdgImpactBaseResult { export interface PdgImpactErrorResult { mode?: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 1; + pdgResultVersion: 2; error: string; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; @@ -809,7 +812,7 @@ function assemblePdgImpactResult(input: { name: s.name, type: s.type, filePath: s.filePath, - ...(s.startLine !== undefined ? { startLine: s.startLine } : {}), + ...(s.startLine !== undefined ? { startLine: toDisplayLine(s.startLine) } : {}), ...(s.ambiguous ? { ambiguous: true } : {}), ...(s.id === null ? { unresolved: true } : {}), pdgEvidence: (s.id === null ? 'degraded' : 'owner-projection') as PdgImpactEvidence, diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index 5bf81a772..48cd89b57 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -450,6 +450,7 @@ additional_node_types: "Multi-language: Struct, Enum, Macro, Typedef, Union, Nam node_properties: common: "name (STRING), filePath (STRING), startLine (INT32), endLine (INT32)" + line_numbers: "startLine/endLine on symbol nodes are 0-BASED (tree-sitter rows) in storage AND in raw Cypher results. The context, query, impact, group/cross-repo trace, and explain/pdg_query (symbol anchor) tools present them 1-BASED (editor / sed / less -N aligned), so a symbol spans editor lines (startLine+1)..(endLine+1) — e.g. sed ',!d' . Single-repo trace symbol lines stay 0-BASED for now (full-parity follow-up). content holds the exact symbol span. (BasicBlock / PDG statement lines are separately 1-based.) (#2377, #2380)" Method: "parameterCount (INT32), returnType (STRING), isVariadic (BOOL), visibility (STRING), isStatic (BOOL), isAbstract (BOOL), isFinal (BOOL), isVirtual (BOOL), isOverride (BOOL), isAsync (BOOL), isPartial (BOOL), requiredParameterCount (INT32), parameterTypes (STRING[]), annotations (STRING[])" Function: "parameterCount (INT32), returnType (STRING), isVariadic (BOOL), visibility (STRING), isStatic (BOOL), isAbstract (BOOL), isFinal (BOOL), isAsync (BOOL), parameterTypes (STRING[]), annotations (STRING[])" Property: "declaredType (STRING) — the field's type annotation (e.g., 'Address', 'City'). Used for field-access chain resolution." diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index f118c8c7b..704b553c8 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -427,7 +427,7 @@ MODE (opt-in): "callgraph" (default) walks symbol→symbol edges (CALLS/IMPORTS/ STATEMENT-ANCHORED PDG SLICE: with mode:'pdg', pass "line" (1-based source line within the target symbol) to seed the dependence slice on the statement at that line and return what depends on it in affectedStatements (line + text). Inter-procedural symbols are still reported through interproceduralByDepth/pdgInterprocedural and the compatibility byDepth bucket. Without "line", pdg returns whole-symbol inter-procedural reach plus local whole-symbol PDG diagnostics. -PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:1 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:1, target metadata when the target resolves, risk:'UNKNOWN', note/remediation, and empty byDepth parity fields — never a false-safe zero. If depth and limit both bound the slice, truncatedByReasons reports both causes while truncatedBy remains scalar. +PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:2 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:2, target metadata when the target resolves, risk:'UNKNOWN', note/remediation, and empty byDepth parity fields — never a false-safe zero. If depth and limit both bound the slice, truncatedByReasons reports both causes while truncatedBy remains scalar. WHEN TO USE: Before making code changes — especially refactoring, renaming, or modifying shared code. Shows what would break. AFTER THIS: Review d=1 items (WILL BREAK). Use context() on high-risk symbols. diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 0f721c76b..ea2dd0e9c 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -271,8 +271,13 @@ export interface RepoMeta { * URL-only id). The incremental writeback preserves unchanged-file rows, so a * top-up against a pre-v5 index would strand old url-keyed Route nodes alongside * new composite-keyed ones — force a full re-analyze instead. + * v6: line-number storage flipped to uniform 0-based for the last 1-based + * GraphNode emitters — COBOL/JCL/markdown/scope (#2377/#2379/#2380). Incremental + * writeback preserves unchanged-file rows, so a top-up against a pre-v6 index + * would MIX old 1-based rows with new 0-based ones — and the 1-based MCP display + * would render the stale rows one line too high — so force a full re-analyze. */ -export const INCREMENTAL_SCHEMA_VERSION = 5; +export const INCREMENTAL_SCHEMA_VERSION = 6; export interface IndexedRepo { repoPath: string; diff --git a/gitnexus/test/integration/csv-pipeline.test.ts b/gitnexus/test/integration/csv-pipeline.test.ts index 497db9241..6f76bfeaf 100644 --- a/gitnexus/test/integration/csv-pipeline.test.ts +++ b/gitnexus/test/integration/csv-pipeline.test.ts @@ -171,6 +171,109 @@ describe('streamAllCSVsToDisk', () => { expect(content).toContain('"index.ts"'); }); + it('stores exact symbol content, pinned against a ±1 boundary shift', async () => { + // Neighbors sit DIRECTLY adjacent to the [2,4] span (no blank buffer), so a + // one-line slice shift at either edge — the #2379 COBOL/JCL failure mode — + // pulls a guard line into the snippet and fails an assertion. + await fs.writeFile( + path.join(repoDir, 'src', 'symbol-window.ts'), + [ + 'const guardTop = 0;', + 'const before = 1;', + 'export function target() {', + ' return before;', + '}', + 'const after = 2;', + 'const guardBottom = 3;', + ].join('\n'), + ); + const graph = buildTestGraph([ + { + id: 'func:target', + label: 'Function', + name: 'target', + filePath: 'src/symbol-window.ts', + startLine: 2, + endLine: 4, + isExported: true, + }, + ]); + + const result = await streamAllCSVsToDisk(graph, repoDir, csvDir); + const functionCsv = result.nodeFiles.get('Function'); + expect(functionCsv).toBeDefined(); + const content = await fs.readFile(functionCsv!.csvPath, 'utf-8'); + expect(content).toContain('export function target()'); + expect(content).toContain('return before;'); + // Directly-adjacent neighbors must NOT leak — catches an off-by-one either way. + expect(content).not.toContain('const before = 1;'); + expect(content).not.toContain('const after = 2;'); + }); + + it('stores exact content for a one-line symbol (startLine === endLine)', async () => { + await fs.writeFile( + path.join(repoDir, 'src', 'one-line.ts'), + ['AAA_TOP', 'BBB_BEFORE', 'const only = 1;', 'CCC_AFTER', 'DDD_BOTTOM'].join('\n'), + ); + const graph = buildTestGraph([ + { + id: 'func:only', + label: 'Function', + name: 'only', + filePath: 'src/one-line.ts', + startLine: 2, + endLine: 2, + isExported: true, + }, + ]); + + const result = await streamAllCSVsToDisk(graph, repoDir, csvDir); + const functionCsv = result.nodeFiles.get('Function'); + expect(functionCsv).toBeDefined(); + const content = await fs.readFile(functionCsv!.csvPath, 'utf-8'); + expect(content).toContain('const only = 1;'); + expect(content).not.toContain('BBB_BEFORE'); + expect(content).not.toContain('CCC_AFTER'); + }); + + it('keeps ±2 neighbor context for non-exact labels (Section)', async () => { + // `Section` is NOT in EXACT_SYMBOL_CONTENT_LABELS, so it retains the ±2 + // context window — the fallback branch the exact-content change left in place. + await fs.writeFile( + path.join(repoDir, 'src', 'section-window.ts'), + [ + 's0_alpha', + 's1_bravo', + 's2_charlie', + 's3_delta', + 's4_echo', + 's5_foxtrot', + 's6_golf', + 's7_hotel', + ].join('\n'), + ); + const graph = buildTestGraph([ + { + id: 'sec:s', + label: 'Section', + name: 's', + filePath: 'src/section-window.ts', + startLine: 4, + endLine: 4, + }, + ]); + + const result = await streamAllCSVsToDisk(graph, repoDir, csvDir); + const sectionCsv = result.nodeFiles.get('Section'); + expect(sectionCsv).toBeDefined(); + const content = await fs.readFile(sectionCsv!.csvPath, 'utf-8'); + expect(content).toContain('s4_echo'); // the section's own line + expect(content).toContain('s2_charlie'); // startLine - 2 + expect(content).toContain('s6_golf'); // endLine + 2 + expect(content).not.toContain('s1_bravo'); // outside the ±2 window + expect(content).not.toContain('s7_hotel'); + }); + it('keeps full text file content searchable past 10KB', async () => { const lateNeedle = 'late_text_file_needle_after_10kb'; await fs.writeFile( diff --git a/gitnexus/test/integration/group/cross-trace-e2e.test.ts b/gitnexus/test/integration/group/cross-trace-e2e.test.ts index 11a89e364..9890af72a 100644 --- a/gitnexus/test/integration/group/cross-trace-e2e.test.ts +++ b/gitnexus/test/integration/group/cross-trace-e2e.test.ts @@ -321,6 +321,15 @@ matching: { name: 'getUsers', repo: 'app/backend' }, ]); + // #2380: the whole group-trace response is 1-based — the hops share the same + // base as the endpoints (before the fix, endpoints were 1-based via + // resolveSymbol while hops stayed 0-based, mixing bases in one response). + const hopLines = (result.hops as Array<{ startLine: number }>).map((h) => h.startLine); + expect(hopLines[0]).toBe(11); // checkout stored 10 -> display 11 + expect(hopLines[3]).toBe(2); // getUsers stored 1 -> display 2 + expect((result.from as { startLine: number }).startLine).toBe(hopLines[0]); + expect((result.to as { startLine: number }).startLine).toBe(hopLines[3]); + // The boundary hop carries the CONTRACT_LINK edge. const edgeTypes = (result.edges as Array<{ relType: string }>).map((e) => e.relType); expect(edgeTypes).toContain('CONTRACT_LINK'); diff --git a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts index a32facb01..ee9b31c92 100644 --- a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts +++ b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts @@ -17,7 +17,7 @@ * "complete" result. * * This golden asserts the EXACT degraded envelope (not just non-crash): - * - the result is still mode:'pdg' with pdgResultVersion:1 (the contract + * - the result is still mode:'pdg' with pdgResultVersion:2 (the contract * discriminator); * - the intra slice is PRESENT (CALL_SUMMARY is NOT a required sub-layer — the * index is `ready`, pdgLayer is undefined, risk is UNKNOWN, epistemic is the @@ -75,14 +75,14 @@ withTestLbugDB( }); describe('CALL_SUMMARY-absent (v3 / pre-FU-C index): the ascent is silent but the user is TOLD', () => { - it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:1, intra slice present, risk UNKNOWN', async () => { + it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:2, intra slice present, risk UNKNOWN', async () => { const result = await slice(); // Golden envelope: the index is `ready` (CALL_SUMMARY is NOT a required // sub-layer), so this is a real traversal result — NOT a pdgLayer // degradation early-return. The intra slice ran and risk stays UNKNOWN. expect(result).toMatchObject({ mode: 'pdg', - pdgResultVersion: 1, + pdgResultVersion: 2, risk: 'UNKNOWN', epistemic: 'pdg-intra-procedural', target: { id: 'func:fnA', name: 'fnA' }, diff --git a/gitnexus/test/integration/markdown-processor-crlf.test.ts b/gitnexus/test/integration/markdown-processor-crlf.test.ts index 7a3e91a95..2a84aae8e 100644 --- a/gitnexus/test/integration/markdown-processor-crlf.test.ts +++ b/gitnexus/test/integration/markdown-processor-crlf.test.ts @@ -58,8 +58,8 @@ describe('markdown-processor CRLF tolerance', () => { const sections = getMarkdownSections(graph, filePath); expect(sections.map((s) => s.properties.name)).toEqual(['Title', 'Sub', 'SubSub']); expect(sections.map((s) => s.properties.level)).toEqual([1, 2, 3]); - expect(sections.map((s) => s.properties.startLine)).toEqual([1, 3, 5]); - expect(sections.map((s) => s.properties.endLine)).toEqual([7, 7, 7]); + expect(sections.map((s) => s.properties.startLine)).toEqual([0, 2, 4]); + expect(sections.map((s) => s.properties.endLine)).toEqual([6, 6, 6]); for (const s of sections) { expect(String(s.properties.name)).not.toMatch(/\r/); } @@ -81,8 +81,8 @@ describe('markdown-processor CRLF tolerance', () => { const sections = getMarkdownSections(graph, filePath); expect(sections.map((s) => s.properties.name)).toEqual(['Title', 'Sub', 'SubSub']); expect(sections.map((s) => s.properties.level)).toEqual([1, 2, 3]); - expect(sections.map((s) => s.properties.startLine)).toEqual([1, 3, 5]); - expect(sections.map((s) => s.properties.endLine)).toEqual([7, 7, 7]); + expect(sections.map((s) => s.properties.startLine)).toEqual([0, 2, 4]); + expect(sections.map((s) => s.properties.endLine)).toEqual([6, 6, 6]); for (const s of sections) { expect(String(s.properties.name)).not.toMatch(/\r/); } @@ -103,8 +103,8 @@ describe('markdown-processor CRLF tolerance', () => { const sections = getMarkdownSections(graph, filePath); expect(sections.map((s) => s.properties.name)).toEqual(['Title', 'Sub']); expect(sections.map((s) => s.properties.level)).toEqual([1, 2]); - expect(sections.map((s) => s.properties.startLine)).toEqual([1, 3]); - expect(sections.map((s) => s.properties.endLine)).toEqual([5, 5]); + expect(sections.map((s) => s.properties.startLine)).toEqual([0, 2]); + expect(sections.map((s) => s.properties.endLine)).toEqual([4, 4]); for (const s of sections) { expect(String(s.properties.name)).not.toMatch(/\r/); } @@ -124,8 +124,8 @@ describe('markdown-processor CRLF tolerance', () => { const sections = getMarkdownSections(graph, filePath); expect(sections.map((s) => s.properties.name)).toEqual(['LF Title', 'CRLF Sub', 'Trailing LF']); expect(sections.map((s) => s.properties.level)).toEqual([1, 2, 3]); - expect(sections.map((s) => s.properties.startLine)).toEqual([1, 3, 5]); - expect(sections.map((s) => s.properties.endLine)).toEqual([7, 7, 7]); + expect(sections.map((s) => s.properties.startLine)).toEqual([0, 2, 4]); + expect(sections.map((s) => s.properties.endLine)).toEqual([6, 6, 6]); for (const s of sections) { expect(String(s.properties.name)).not.toMatch(/\r/); } @@ -138,7 +138,8 @@ describe('markdown-processor CRLF tolerance', () => { it('reports correct startLine and endLine for CRLF content', () => { const filePath = 'crlf-lines.md'; const graph = setupGraphWithFile(filePath); - // Lines 1, 3, 5 are headings (1-indexed) + // Headings sit on physical lines 1, 3, 5; graph nodes store 0-based + // startLine/endLine (the GraphNode convention, #2377) — so 0, 2, 4. const content = '# T\r\nbody\r\n## Sub\r\nmore\r\n### SubSub\r\ntail\r\n'; processMarkdown(graph, [{ path: filePath, content }], new Set([filePath])); @@ -148,11 +149,11 @@ describe('markdown-processor CRLF tolerance', () => { const subSection = sections.find((s) => s.properties.name === 'Sub'); const subSubSection = sections.find((s) => s.properties.name === 'SubSub'); - expect(titleSection?.properties.startLine).toBe(1); - expect(titleSection?.properties.endLine).toBe(7); - expect(subSection?.properties.startLine).toBe(3); - expect(subSection?.properties.endLine).toBe(7); - expect(subSubSection?.properties.startLine).toBe(5); - expect(subSubSection?.properties.endLine).toBe(7); + expect(titleSection?.properties.startLine).toBe(0); + expect(titleSection?.properties.endLine).toBe(6); + expect(subSection?.properties.startLine).toBe(2); + expect(subSection?.properties.endLine).toBe(6); + expect(subSubSection?.properties.startLine).toBe(4); + expect(subSubSection?.properties.endLine).toBe(6); }); }); diff --git a/gitnexus/test/integration/mcp-line-display.test.ts b/gitnexus/test/integration/mcp-line-display.test.ts new file mode 100644 index 000000000..ae16fbfbe --- /dev/null +++ b/gitnexus/test/integration/mcp-line-display.test.ts @@ -0,0 +1,126 @@ +/** + * Integration test: MCP tools present 1-based line numbers (#2377), while raw + * `cypher` returns the stored 0-based value unchanged. + * + * GraphNode startLine/endLine are stored 0-based (the tree-sitter convention; + * see ingestion/utils/line-base.ts). Human/LLM-facing tools (context, query, + * impact) add 1 at the response boundary so the numbers line up with editors / + * `sed`; the raw `cypher` passthrough stays 0-based and is documented. + * + * One shared LadybugDB (with FTS) backs every case so query()'s BM25 path is + * exercised without a second full DB+FTS setup. + */ +import { describe, expect, it, vi } from 'vitest'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; +import { FTS_INDEXES } from '../../src/core/search/fts-schema.js'; + +const PRODUCTION_FTS_INDEXES = FTS_INDEXES.map((i) => ({ + table: i.table, + indexName: i.indexName, + columns: [...i.properties], +})); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), + }; +}); + +// Stored 0-based: App occupies 0-based lines 41..58 (editor lines 42..59). +// TopFn sits on the file's first line (stored 0-based 0) — the #2380 falsy-`||` +// case where `sym.startLine || sym[4]` would drop the line entirely. +// Two DupFn symbols force impact()'s ambiguous branch, the only impact response +// that surfaces a per-candidate line. Zqxwvbm carries a distinctive content +// token so query()'s BM25/FTS retriever surfaces it (the #2380 P1 path). +const SEED = [ + `CREATE (c:Class {id:'Class:src/app.ts:App', name:'App', filePath:'src/app.ts', startLine:41, endLine:58, content:'class App {}', description:''})`, + `CREATE (c:Class {id:'Class:src/top.ts:TopFn', name:'TopFn', filePath:'src/top.ts', startLine:0, endLine:0, content:'class TopFn {}', description:''})`, + `CREATE (f:Function {id:'Function:src/a.ts:DupFn', name:'DupFn', filePath:'src/a.ts', startLine:41, endLine:50, content:'function DupFn() {}', description:''})`, + `CREATE (f:Function {id:'Function:src/b.ts:DupFn', name:'DupFn', filePath:'src/b.ts', startLine:7, endLine:12, content:'function DupFn() {}', description:''})`, + `CREATE (c:Class {id:'Class:src/svc.ts:Zqxwvbm', name:'Zqxwvbm', filePath:'src/svc.ts', startLine:41, endLine:58, content:'class Zqxwvbm zqxwvbmtoken', description:'zqxwvbmtoken service'})`, +]; + +let backend: LocalBackend; + +withTestLbugDB( + 'mcp-line-display', + () => { + describe('MCP line-number display (#2377): tools 1-based, raw cypher 0-based', () => { + it('context() reports 1-based startLine/endLine (editor / sed aligned)', async () => { + const result = await backend.callTool('context', { uid: 'Class:src/app.ts:App' }); + expect(result.status).toBe('found'); + expect(result.symbol.startLine).toBe(42); // stored 0-based 41 -> display 42 + expect(result.symbol.endLine).toBe(59); // stored 0-based 58 -> display 59 + }); + + it('context() keeps a 0-based first-line symbol (startLine:0 -> 1, not dropped)', async () => { + // Before #2380 the falsy `sym.startLine || sym[4]` collapsed a valid 0 to + // undefined, so context() omitted startLine/endLine for first-line symbols + // (every COBOL Module, markdown h1). `??` preserves the 0. + const result = await backend.callTool('context', { uid: 'Class:src/top.ts:TopFn' }); + expect(result.status).toBe('found'); + expect(result.symbol.startLine).toBe(1); // stored 0-based 0 -> display 1 + expect(result.symbol.endLine).toBe(1); + }); + + it('impact() ambiguous candidates report 1-based line (stored 41 -> 42)', async () => { + const result = await backend.callTool('impact', { target: 'DupFn' }); + expect(result.status).toBe('ambiguous'); + const cand = (result.candidates as Array<{ filePath: string; line: number }>).find( + (c) => c.filePath === 'src/a.ts', + ); + expect(cand).toBeDefined(); + expect(cand!.line).toBe(42); // stored 0-based 41 -> display 42 + }); + + it('query() BM25 path converts the line exactly once (stored 41 -> 42, not 43)', async () => { + // bm25Search returns raw 0-based rows; query()'s aggregation applies + // toDisplayLine once. Before #2380 both converted -> 43 (#2380 P1). + type QuerySymbol = { id: string; startLine?: number; endLine?: number }; + type QueryResult = { definitions?: QuerySymbol[]; process_symbols?: QuerySymbol[] }; + const result: QueryResult = await backend.callTool('query', { query: 'zqxwvbmtoken' }); + const sym = [...(result.process_symbols ?? []), ...(result.definitions ?? [])].find( + (s) => s.id === 'Class:src/svc.ts:Zqxwvbm', + ); + expect(sym).toBeDefined(); + expect(sym!.startLine).toBe(42); // 41 + 1, converted exactly once + expect(sym!.endLine).toBe(59); // 58 + 1 + }); + + it('raw cypher returns the stored 0-based value unchanged', async () => { + const result = await backend.callTool('cypher', { + statement: "MATCH (n:Class {name:'App'}) RETURN n.startLine AS startLine", + }); + expect(result).toHaveProperty('markdown'); + // If display-conversion leaked into raw cypher this would read 42. + expect(result.markdown).toContain('41'); + expect(result.markdown).not.toContain('42'); + }); + }); + }, + { + seed: SEED, + ftsIndexes: PRODUCTION_FTS_INDEXES, + poolAdapter: true, + afterSetup: async (handle) => { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'test-repo', + path: '/test/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 1, nodes: 5, communities: 0, processes: 0 }, + }, + ]); + backend = new LocalBackend(); + await backend.init(); + }, + }, +); diff --git a/gitnexus/test/integration/pdg-query.test.ts b/gitnexus/test/integration/pdg-query.test.ts index 74bf232d0..de5477d8a 100644 --- a/gitnexus/test/integration/pdg-query.test.ts +++ b/gitnexus/test/integration/pdg-query.test.ts @@ -315,6 +315,9 @@ withTestLbugDB( target: 'targetFn', }); expect(result).not.toHaveProperty('error'); + // #2380: the display anchor is 1-based, matching context/query/impact — + // targetFn stored 0-based 10 -> 11 (the BasicBlock join is unaffected). + expect((result.anchor as { startLine: number }).startLine).toBe(11); // Only targetFn's own control edge — the neighbor's line-10 edge is out // of the [11,15] window after the lower-bound +1 fix. expect(result.results).toHaveLength(1); diff --git a/gitnexus/test/integration/resolvers/cobol.test.ts b/gitnexus/test/integration/resolvers/cobol.test.ts index 0d89090d8..71d8908fc 100644 --- a/gitnexus/test/integration/resolvers/cobol.test.ts +++ b/gitnexus/test/integration/resolvers/cobol.test.ts @@ -9,11 +9,13 @@ * CUSTDAT.cpy, COPYLIB.cpy, RUNJOBS.jcl */ import { describe, it, expect, beforeAll } from 'vitest'; +import fs from 'fs/promises'; import path from 'path'; import { FIXTURES, getRelationships, getNodesByLabel, + getNodesByLabelFull, edgeSet, runPipelineFromRepo, type PipelineResult, @@ -752,4 +754,44 @@ describe('COBOL full system extraction', () => { expect(parsedFile!.moduleScope.length).toBeGreaterThan(0); }); }); + + // --------------------------------------------------------------------- + // LINE-BASE CONVENTION — COBOL/JCL emit 0-based startLine (#2377 / #2379) + // Regex-based processors carry 1-based line numbers; they must convert to + // the 0-based GraphNode convention at emission or the exact-content slice + // drops each symbol's declaration line. These lock that in. + // --------------------------------------------------------------------- + describe('line-base convention: 0-based startLine (#2377 / #2379)', () => { + it('primary program Module starts at 0-based line 0', () => { + const custupdt = getNodesByLabelFull(result, 'Module').find((m) => m.name === 'CUSTUPDT'); + expect(custupdt).toBeDefined(); + expect(custupdt!.properties.startLine).toBe(0); + }); + + // NOTE: COBOL paragraph lines can't be cross-checked against the raw file — + // the preprocessor expands COPY statements, so `startLine` is in expanded + // coordinates (a separate, pre-existing content-alignment concern, out of + // scope for the 0-based conversion). JCL has no such expansion, so a JCL + // step gives a clean 0-based proof for a NON-line-0 symbol — ruling out a + // "conversion always yields 0" false pass. + it('JCL step CodeElement startLine is the 0-based declaration line', async () => { + const source = await fs.readFile(path.join(FIXTURES, 'cobol-app', 'RUNJOBS.jcl'), 'utf-8'); + const lines = source.split('\n'); + const expectedIdx = lines.findIndex((l) => l.includes('STEP1')); + expect(expectedIdx).toBeGreaterThan(0); // not line 0 — proves a real conversion + const step1 = getNodesByLabelFull(result, 'CodeElement').find((n) => n.name === 'STEP1'); + expect(step1).toBeDefined(); + expect(step1!.properties.startLine).toBe(expectedIdx); + expect(lines[step1!.properties.startLine]).toContain('STEP1'); + }); + + it('JCL job CodeElement starts at 0-based line 0', async () => { + const source = await fs.readFile(path.join(FIXTURES, 'cobol-app', 'RUNJOBS.jcl'), 'utf-8'); + const lines = source.split('\n'); + const custjob = getNodesByLabelFull(result, 'CodeElement').find((n) => n.name === 'CUSTJOB'); + expect(custjob).toBeDefined(); + expect(custjob!.properties.startLine).toBe(0); + expect(lines[custjob!.properties.startLine]).toContain('CUSTJOB'); + }); + }); }); diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index a3edc9685..723cf3439 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -73,8 +73,8 @@ describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => { }); describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { - it('INCREMENTAL_SCHEMA_VERSION is bumped to 5 (multi-verb Route identity re-index window)', () => { - expect(INCREMENTAL_SCHEMA_VERSION).toBe(5); + it('INCREMENTAL_SCHEMA_VERSION is bumped to 6 (uniform 0-based line storage re-index window)', () => { + expect(INCREMENTAL_SCHEMA_VERSION).toBe(6); }); it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => { @@ -91,7 +91,11 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { expect(passesReuseGate(4)).toBe(false); // A legacy stamp with no schemaVersion at all is likewise rejected. expect(passesReuseGate(undefined)).toBe(false); + // A pre-v6 (v5) index predates the uniform 0-based line-storage flip → its + // COBOL/JCL/markdown/scope rows are still 1-based, so an incremental top-up + // would mix bases → must NOT reuse. + expect(passesReuseGate(5)).toBe(false); // A current-version stamp passes the gate (incremental top-up eligible). - expect(passesReuseGate(5)).toBe(true); + expect(passesReuseGate(6)).toBe(true); }); }); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index da5f6a560..afee977fd 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -1352,9 +1352,14 @@ describe('LocalBackend.callTool', () => { backend = new LocalBackend(); await backend.init(); + // The symbol is stored at 0-based startLine 1; context() presents it 1-based + // (line 2) and rename subtracts 1 to recover the 0-based file index (1), so + // `oldName` must sit on the file's 0-based line 1 for the definition edit to + // fire. (#2380: the mock previously put it on line 0, which stopped matching + // once context() went 1-based.) const readSpy = vi .spyOn(fsPromises, 'readFile') - .mockResolvedValue('function oldName() {}\n' as unknown as Buffer); + .mockResolvedValue('\nfunction oldName() {}\n' as unknown as Buffer); const writeSpy = vi .spyOn(fsPromises, 'writeFile') .mockRejectedValue(new Error('EACCES: permission denied')); diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index 8bfe49ed4..cb17487a5 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -38,7 +38,7 @@ function pdgFindings(overrides: Record = {}): Record { // The PDG result family advertises a contract version (FIX #2) so external // MCP/agent consumers can version against future shape evolution. It is a // mode:'pdg'-only field — never on the default callgraph result. - expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 1 }); + expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 2 }); }); it('surfaces ambiguous-projection and unresolved block counts honestly', () => { diff --git a/gitnexus/test/unit/group/manifest-label-drift.test.ts b/gitnexus/test/unit/group/manifest-label-drift.test.ts new file mode 100644 index 000000000..18802e8c5 --- /dev/null +++ b/gitnexus/test/unit/group/manifest-label-drift.test.ts @@ -0,0 +1,34 @@ +/** + * #2380: manifest-extractor's CUSTOM_CONTRACT_RESOLVE_QUERY hand-lists the graph + * labels that resolve as contract symbols. It is a deliberate SUBSET of the + * shared SYMBOL_NODE_LABELS (ingestion/utils/symbol-labels.ts) — omitting + * Namespace/Variable/Module, which would widen contract resolution and is + * #2325-test-locked. A comment asserts that relationship but nothing enforced + * it, so adding a label to SYMBOL_NODE_LABELS could silently diverge the two. + * This locks it: the query string stays literal; the test derives its label set. + */ +import { describe, it, expect } from 'vitest'; +import { CUSTOM_CONTRACT_RESOLVE_QUERY } from '../../../src/core/group/extractors/manifest-extractor.js'; +import { SYMBOL_NODE_LABELS } from '../../../src/core/ingestion/utils/symbol-labels.js'; + +describe('manifest contract-resolve label list vs SYMBOL_NODE_LABELS (#2380)', () => { + const match = CUSTOM_CONTRACT_RESOLVE_QUERY.match(/labels\(n\) IN \[([^\]]+)\]/); + const manifestLabels = new Set( + (match?.[1] ?? '').split(',').map((t) => t.trim().replace(/^'|'$/g, '')), + ); + const symbolLabels = new Set(SYMBOL_NODE_LABELS); + + it('extracts a non-empty label allowlist from the query', () => { + expect(manifestLabels.size).toBeGreaterThan(0); + }); + + it('every manifest label is a member of SYMBOL_NODE_LABELS (strict subset)', () => { + const extra = [...manifestLabels].filter((l) => !symbolLabels.has(l)); + expect(extra).toEqual([]); + }); + + it('the difference is exactly {Namespace, Variable, Module}', () => { + const diff = [...symbolLabels].filter((l) => !manifestLabels.has(l)).sort(); + expect(diff).toEqual(['Module', 'Namespace', 'Variable']); + }); +}); diff --git a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts index 7b4f0c010..ecf1ce9e5 100644 --- a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts +++ b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts @@ -36,7 +36,7 @@ const local = ( impactedCount: number, ): PdgImpactSuccessResult => ({ mode: 'pdg', - pdgResultVersion: 1, + pdgResultVersion: 2, target: { id: 'T', name: 'criterion', type: 'Function', filePath: 'src/a.ts' }, direction: 'downstream', risk: 'UNKNOWN',