diff --git a/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts b/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts new file mode 100644 index 000000000..6503dec7d --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts @@ -0,0 +1,220 @@ +/** + * Go cross-file struct-owner reconciliation (#3558, #3559). + * + * `findEnclosingClassInfo`'s `method_declaration` branch mints the member + * owner id from the file CONTAINING THE METHOD (`Struct::`), + * while the Struct node itself is keyed by the file DECLARING the struct + * (`Struct::`). Same Go package, different file → the + * HAS_METHOD / HAS_PROPERTY edge points at a node that does not exist and is + * silently dropped at COPY time. + * + * This pass runs after ALL worker chunks are merged into the graph (so every + * package's structs are visible), and rewrites only edges that would + * otherwise dangle: + * + * - sourceId looks like `Struct:.go:` AND no such node exists + * AND the id names a Go source file — cross-language `Struct:` owner ids + * (Rust, Swift, …) never re-point; + * - candidate Struct nodes are Go (`properties.language === 'go'`, the + * marker the worker stamps on every declared symbol); + * - exactly ONE Struct node shares the owner's package directory, declared + * package clause, and simple name. The clause matters because one + * directory can carry `package pkg` AND external-test `package pkg_test` + * files declaring the same type name — the same (dir, package) bucket + * `method-owners.ts` uses. 0 or ≥2 candidates: leave the edge alone — a + * wrong owner is worse than a dropped edge. + * + * Cheap by default: the per-directory index is built only when a dangling Go + * `Struct:` owner source actually exists, and package clauses are read only + * from that directory's Go files. Struct fields never dangle (they live in + * the struct's own file), so the common case pays two per-type edge scans and + * nothing else. Idempotent: a second run finds no dangling sources and + * changes nothing. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import type { KnowledgeGraph } from '../../../graph/types.js'; +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { SupportedLanguages } from 'gitnexus-shared'; +import { logger } from '../../../logger.js'; +import { goPackageDir, inferGoPackageName } from './package-clause.js'; + +const OWNER_LABEL = 'Struct'; +const GO_LANGUAGE = SupportedLanguages.Go as string; + +/** `Struct::` → { filePath, name }, or null for other id shapes. */ +const parseStructOwnerId = ( + id: string, +): { readonly filePath: string; readonly name: string } | null => { + if (!id.startsWith(`${OWNER_LABEL}:`)) return null; + const rest = id.slice(OWNER_LABEL.length + 1); + const sep = rest.lastIndexOf(':'); + if (sep === -1 || sep === 0 || sep === rest.length - 1) return null; + return { filePath: rest.slice(0, sep), name: rest.slice(sep + 1) }; +}; + +const isGoSourcePath = (filePath: string): boolean => filePath.endsWith('.go'); + +const isGoNode = (node: GraphNode): boolean => node.properties?.language === GO_LANGUAGE; + +/** Rewrites to apply, collected before mutation (the type-index iterator must not be mutated mid-walk). */ +interface OwnerRewrite { + readonly rel: GraphRelationship; + readonly newSourceId: string; +} + +/** `package\0name` → candidate Struct nodes in that package. */ +type StructOwnerIndex = Map; + +interface DirIndex { + readonly byPackageAndName: StructOwnerIndex; +} + +/** + * Package-clause reads for one reconcile call. A file's clause is immutable + * within a run, and the dangling-owner scan plus the index build both consult + * it; the cache is call-scoped so concurrent pipelines over different repos + * never collide on a relative path. Only `.go` files in dangling-owner + * directories are ever read. + */ +const clauseReader = (repoPath: string): ((filePath: string) => string | null) => { + const packageClauses = new Map(); + return (filePath: string): string | null => { + const hit = packageClauses.get(filePath); + if (hit !== undefined) return hit; + let clause: string | null = null; + try { + clause = inferGoPackageName(fs.readFileSync(path.join(repoPath, filePath), 'utf-8')); + } catch { + clause = null; + } + packageClauses.set(filePath, clause); + return clause; + }; +}; + +/** Go Struct nodes in `dir`, from the FULL merged graph. */ +const goStructsInDir = (graph: KnowledgeGraph, dir: string): GraphNode[] => + [...graph.iterNodes()].filter((node) => { + if (node.label !== OWNER_LABEL || !isGoNode(node)) return false; + const filePath = node.properties?.filePath; + return ( + typeof filePath === 'string' && + isGoSourcePath(filePath) && + goPackageDir(filePath) === dir && + typeof node.properties?.name === 'string' + ); + }); + +/** + * `(clause, name)` index for one directory's Go structs, or null when the + * directory has none. Keyed by the DECLARED clause — `pkg` and `pkg_test` + * are different packages, and only same-package files can own each other's + * receivers (same rule `populateGoWorkspaceOwners` buckets by). + */ +const buildDirIndex = ( + graph: KnowledgeGraph, + readClause: (filePath: string) => string | null, + dir: string, +): DirIndex | null => { + const structs = goStructsInDir(graph, dir); + if (structs.length === 0) return null; + + const byPackageAndName: StructOwnerIndex = new Map(); + for (const node of structs) { + const filePath = node.properties.filePath as string; + const declared = readClause(filePath); + // A struct whose file has no resolvable clause cannot be keyed by + // package — the same isolation `populateGoWorkspaceOwners` applies. + if (declared === null) continue; + const key = `${declared}\0${node.properties.name as string}`; + const bucket = byPackageAndName.get(key); + if (bucket) bucket.push(node); + else byPackageAndName.set(key, [node]); + } + return { byPackageAndName }; +}; + +/** + * Rewrite cross-file Go struct-owner edges in place. Returns the number of + * edges re-pointed. `repoPath` resolves the repo-relative + * `properties.filePath` for package-clause reads. + */ +export const reconcileGoStructOwnerEdges = (graph: KnowledgeGraph, repoPath: string): number => { + const readClause = clauseReader(repoPath); + + const dangling: GraphRelationship[] = []; + for (const type of ['HAS_METHOD', 'HAS_PROPERTY'] as const) { + for (const rel of graph.iterRelationshipsByType(type)) { + if (graph.getNode(rel.sourceId) === undefined) dangling.push(rel); + } + } + if (dangling.length === 0) return 0; + + // Go-owner edges only. A non-Go `Struct::` owner id — or one + // whose file is not Go source — never re-points: a cross-language guess is + // exactly the wrong-owner case this pass refuses. + const goDangling = dangling.filter((rel) => { + const parsed = parseStructOwnerId(rel.sourceId); + return parsed !== null && isGoSourcePath(parsed.filePath); + }); + if (goDangling.length === 0) return 0; + + // One index per directory that actually has a dangling owner edge. Built + // from the FULL merged graph, so the declaring file may sit in any + // already-merged chunk. + const indexByDir = new Map(); + const dirIndex = (dir: string): DirIndex | null => { + let index = indexByDir.get(dir); + if (index === undefined) { + index = buildDirIndex(graph, readClause, dir); + indexByDir.set(dir, index); + } + return index; + }; + + const resolveOwner = (danglingId: string): string | null => { + const parsed = parseStructOwnerId(danglingId); + if (parsed === null) return null; + const dir = goPackageDir(parsed.filePath); + const index = dirIndex(dir); + if (index === null) return null; + const declared = readClause(parsed.filePath); + // Owner file unreadable or clauseless: refuse, keep the drop behavior. + if (declared === null) return null; + const bucket = index.byPackageAndName.get(`${declared}\0${parsed.name}`); + // No candidate (struct never parsed / outside the repo slice) or an + // ambiguous same-name pair in the same package: keep the old drop + // behavior rather than guess. + if (bucket === undefined || bucket.length !== 1) return null; + const owner = bucket[0]!; + if (graph.getNode(owner.id) === undefined) return null; + return owner.id; + }; + + const rewrites: OwnerRewrite[] = []; + for (const rel of goDangling) { + const newSourceId = resolveOwner(rel.sourceId); + if (newSourceId === null || newSourceId === rel.sourceId) continue; + rewrites.push({ rel, newSourceId }); + } + + let rekeyed = 0; + for (const { rel, newSourceId } of rewrites) { + graph.removeRelationship(rel.id); + // Edge ids are minted as `:->` by the worker path. + graph.addRelationship({ + ...rel, + id: `${rel.type}:${newSourceId}->${rel.targetId}`, + sourceId: newSourceId, + }); + rekeyed++; + } + if (rekeyed > 0) { + logger.info( + `go: re-pointed ${rekeyed} cross-file struct member edge(s) to their declaring file(s)`, + ); + } + return rekeyed; +}; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index cc72a04d5..6ca134f83 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -71,6 +71,7 @@ import { } from '../languages/index.js'; import { classifyContentLanguages } from '../content-language-classification.js'; import { SCOPE_RESOLVERS } from '../scope-resolution/pipeline/registry.js'; +import { reconcileGoStructOwnerEdges } from '../languages/go/struct-owner-reconcile.js'; import { DATA_ROUTE_TABLE_SOURCE } from '../route-extractors/data-route-table.js'; import type Parser from 'tree-sitter'; import { @@ -1524,6 +1525,17 @@ export async function runChunkedParseAndResolve( await workerPool?.terminate(); } + // Cross-file Go struct-owner repair (#3558): every chunk is merged, so all + // packages' Struct nodes are in the graph. Owner edges minted by + // `findEnclosingClassInfo` from the METHOD's file dangle whenever the struct + // is declared in a sibling file; re-point them before any consumer walks. + // TODO(#3559): this is a graph-level post-merge pass, but every + // `LanguageProvider`/`ScopeResolver` hook today runs per-language on + // `ParsedFile`s before merge — there is no post-merge provider hook to route + // through. Call stays direct until such a hook exists; introducing one is + // shared-pipeline surface this fix deliberately does not grow. + reconcileGoStructOwnerEdges(graph, repoPath); + // Fetch calls + ORM queries were already extracted inside each worker // (returned in ParseWorkerResult, aggregated per chunk in applyChunkResults). // With sequential parsing removed there is no post-loop drain to run — only diff --git a/gitnexus/test/integration/go-cross-file-struct-owner.test.ts b/gitnexus/test/integration/go-cross-file-struct-owner.test.ts new file mode 100644 index 000000000..238384069 --- /dev/null +++ b/gitnexus/test/integration/go-cross-file-struct-owner.test.ts @@ -0,0 +1,282 @@ +/** + * Regression test for #3558: Go methods declared in a file OTHER than the one + * declaring their receiver struct must still get HAS_METHOD edges to the + * struct node keyed by the DECLARING file. + * + * Before the fix, `findEnclosingClassInfo` minted the owner id from the + * method's own file (`Struct:methods.go:Widget`), while the Struct node lives + * at `Struct:decl.go:Widget` — the edge dangled and was silently dropped at + * COPY into LadybugDB. + */ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { runPipelineFromRepo } from './resolvers/helpers.js'; +import { reconcileGoStructOwnerEdges } from '../../src/core/ingestion/languages/go/struct-owner-reconcile.js'; +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; + +function createGoRepo(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'go-cross-file-struct-')); + fs.writeFileSync(path.join(dir, 'go.mod'), 'module example.com/pkgtest\n\ngo 1.22\n'); + // Struct declared here. + fs.writeFileSync( + path.join(dir, 'decl.go'), + [ + 'package pkgtest', + '', + 'type Widget struct {', + '\tName string', + '}', + '', + '// Same-file method: worked before the fix (baseline).', + 'func (w *Widget) Describe() string { return w.Name }', + '', + ].join('\n'), + ); + // Methods on the same struct, DIFFERENT file, same package: dangled before. + fs.writeFileSync( + path.join(dir, 'methods.go'), + [ + 'package pkgtest', + '', + 'func (w *Widget) Rename(n string) { w.Name = n }', + '', + 'func (w *Widget) Clone() *Widget { return &Widget{Name: w.Name} }', + '', + ].join('\n'), + ); + return dir; +} + +function hasMethodEdgeFrom( + graph: { + iterRelationshipsByType(t: string): IterableIterator<{ sourceId: string; targetId: string }>; + }, + structNodeId: string, +): string[] { + const methods: string[] = []; + for (const rel of graph.iterRelationshipsByType('HAS_METHOD')) { + if (rel.sourceId === structNodeId) methods.push(rel.targetId); + } + return methods; +} + +describe('Go cross-file struct method ownership (#3558)', () => { + it('attaches methods from sibling files to the struct node of the declaring file', async () => { + const repo = createGoRepo(); + try { + const result = await runPipelineFromRepo(repo, () => {}, { skipGraphPhases: true }); + + const widgetNode = [...result.graph.iterNodes()].find( + (n) => n.label === 'Struct' && n.properties.name === 'Widget', + ); + expect(widgetNode).toBeDefined(); + // Sanity: the struct node is keyed by the file that DECLARES it. + expect(widgetNode!.properties.filePath).toContain('decl.go'); + + const methodTargets = hasMethodEdgeFrom(result.graph, widgetNode!.id); + // All three methods own edges: the same-file one AND the two from + // methods.go (Rename value-receiver-less pointer receiver, Clone). + expect(methodTargets.length).toBe(3); + expect(methodTargets.some((id) => id.includes('Describe'))).toBe(true); + expect(methodTargets.some((id) => id.includes('Rename'))).toBe(true); + expect(methodTargets.some((id) => id.includes('Clone'))).toBe(true); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); +}); + +// Graph-level units for the #3559 review findings: the pass must be scoped to +// Go (never re-point a Rust/Swift dangling owner onto a Go struct) and must +// key candidates by (dir, package clause, name), because one directory can +// carry `package pkg` AND external-test `package pkg_test` files declaring +// the same type name. + +const structNode = (id: string, filePath: string, language: string): GraphNode => ({ + id, + label: 'Struct', + properties: { name: id.split(':').pop()!, filePath, language }, +}); + +const danglingHasMethod = (sourceId: string, targetId: string): GraphRelationship => ({ + id: `HAS_METHOD:${sourceId}->${targetId}`, + type: 'HAS_METHOD', + sourceId, + targetId, + confidence: 1, + reason: 'test fixture', +}); + +const methodNode = (id: string): GraphNode => ({ + id, + label: 'Method', + properties: { name: id.split(':').pop()!, filePath: 'x.go', language: 'go' }, +}); + +describe('Go struct-owner reconcile scoping (#3559)', () => { + it('never re-points a dangling non-Go Struct owner edge, even with a same-name Go struct', () => { + const graph = createKnowledgeGraph(); + // Go struct Widget in the same directory the Rust owner id points at. + graph.addNode(structNode('Struct:src:Widget', 'src/decl.go', 'go')); + graph.addNode(methodNode('Method:src:describe')); + // Rust owner id — same dir, same simple name, file NOT .go. + graph.addRelationship(danglingHasMethod('Struct:src/widget.rs:Widget', 'Method:src:describe')); + + expect(reconcileGoStructOwnerEdges(graph, '/nonexistent')).toBe(0); + // The edge must be untouched (still dangling, not re-pointed, not dropped). + const edges = [...graph.iterRelationshipsByType('HAS_METHOD')]; + expect(edges.length).toBe(1); + expect(edges[0]!.sourceId).toBe('Struct:src/widget.rs:Widget'); + }); + + it('re-points a dangling Go owner edge to the Go struct with the same package clause', () => { + const graph = createKnowledgeGraph(); + graph.addNode(structNode('Struct:src/decl.go:Widget', 'src/decl.go', 'go')); + graph.addNode(methodNode('Method:src/methods.go:Describe')); + + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'go-reconcile-')); + try { + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'src', 'decl.go'), + 'package pkgtest\n\ntype Widget struct { Name string }\n', + ); + fs.writeFileSync(path.join(repo, 'src', 'methods.go'), 'package pkgtest\n'); + graph.addRelationship( + danglingHasMethod('Struct:src/methods.go:Widget', 'Method:src/methods.go:Describe'), + ); + + expect(reconcileGoStructOwnerEdges(graph, repo)).toBe(1); + const edges = [...graph.iterRelationshipsByType('HAS_METHOD')]; + expect(edges.length).toBe(1); + expect(edges[0]!.sourceId).toBe('Struct:src/decl.go:Widget'); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('distinguishes `package pkg` from external-test `package pkg_test` structs in one directory', () => { + const graph = createKnowledgeGraph(); + // Two same-name structs in ONE directory, different packages: production + // `pkg.Widget` and external-test `pkg_test.Widget`. The old (dir, name) + // key saw 2 candidates and dropped the edge fail-closed. + graph.addNode(structNode('Struct:src/widget.go:Widget', 'src/widget.go', 'go')); + graph.addNode( + structNode('Struct:src/widget_ext_test.go:Widget', 'src/widget_ext_test.go', 'go'), + ); + graph.addNode(methodNode('Method:src/external_methods_test.go:Describe')); + + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'go-reconcile-pkg-')); + try { + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'src', 'widget.go'), + 'package pkg\n\ntype Widget struct { Name string }\n', + ); + fs.writeFileSync( + path.join(repo, 'src', 'widget_ext_test.go'), + 'package pkg_test\n\ntype Widget struct { Name string }\n', + ); + // Owner file declares `package pkg_test` → must resolve to the + // pkg_test.Widget node, not drop, not pick pkg.Widget. + fs.writeFileSync(path.join(repo, 'src', 'external_methods_test.go'), 'package pkg_test\n'); + graph.addRelationship( + danglingHasMethod( + 'Struct:src/external_methods_test.go:Widget', + 'Method:src/external_methods_test.go:Describe', + ), + ); + + expect(reconcileGoStructOwnerEdges(graph, repo)).toBe(1); + const edges = [...graph.iterRelationshipsByType('HAS_METHOD')]; + expect(edges.length).toBe(1); + expect(edges[0]!.sourceId).toBe('Struct:src/widget_ext_test.go:Widget'); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('drops (fail-closed) when the same package declares the same name twice', () => { + const graph = createKnowledgeGraph(); + graph.addNode(structNode('Struct:src/a.go:Widget', 'src/a.go', 'go')); + graph.addNode(structNode('Struct:src/b.go:Widget', 'src/b.go', 'go')); + graph.addNode(methodNode('Method:src/methods.go:Describe')); + + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'go-reconcile-amb-')); + try { + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'src', 'a.go'), 'package pkg\n'); + fs.writeFileSync(path.join(repo, 'src', 'b.go'), 'package pkg\n'); + fs.writeFileSync(path.join(repo, 'src', 'methods.go'), 'package pkg\n'); + graph.addRelationship( + danglingHasMethod('Struct:src/methods.go:Widget', 'Method:src/methods.go:Describe'), + ); + + expect(reconcileGoStructOwnerEdges(graph, repo)).toBe(0); + // Edge untouched — the old drop behavior for ambiguity. + const edges = [...graph.iterRelationshipsByType('HAS_METHOD')]; + expect(edges.length).toBe(1); + expect(edges[0]!.sourceId).toBe('Struct:src/methods.go:Widget'); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); +}); + +describe('Go external-test-package method ownership through the pipeline (#3559)', () => { + it('re-points a pkg_test method edge onto the pkg_test struct, not the pkg struct and not a drop', async () => { + // Layout: one directory, TWO packages declaring the same type name — + // `package pkg` (widget.go) and external-test `package pkg_test` + // (widget_ext_test.go) — plus external_methods_test.go declaring a method + // on pkg_test.Widget in yet another file. The (dir, name) key saw two + // candidates and dropped the edge fail-closed; the (dir, package, name) + // key resolves it. + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'go-ext-test-owner-')); + try { + fs.writeFileSync(path.join(repo, 'go.mod'), 'module example.com/pkgtest\n\ngo 1.22\n'); + fs.writeFileSync( + path.join(repo, 'widget.go'), + ['package pkg', '', 'type Widget struct {', '\tName string', '}', ''].join('\n'), + ); + fs.writeFileSync( + path.join(repo, 'widget_ext_test.go'), + ['package pkg_test', '', 'type Widget struct {', '\tName string', '}', ''].join('\n'), + ); + fs.writeFileSync( + path.join(repo, 'external_methods_test.go'), + ['package pkg_test', '', 'func (w *Widget) Describe() string { return w.Name }', ''].join( + '\n', + ), + ); + + const result = await runPipelineFromRepo(repo, () => {}, { skipGraphPhases: true }); + + // BOTH structs exist. + const widgets = [...result.graph.iterNodes()].filter( + (n) => n.label === 'Struct' && n.properties.name === 'Widget', + ); + expect(widgets.length).toBe(2); + const pkgWidget = widgets.find((n) => n.properties.filePath.includes('widget.go')); + const pkgTestWidget = widgets.find((n) => + n.properties.filePath.includes('widget_ext_test.go'), + ); + expect(pkgWidget).toBeDefined(); + expect(pkgTestWidget).toBeDefined(); + + // The Describe edge from external_methods_test.go must land on the + // pkg_test.Widget node (it dangles at `Struct:external_methods_test.go:Widget` + // before the reconcile pass). + const describeTargets = hasMethodEdgeFrom(result.graph, pkgTestWidget!.id); + expect(describeTargets.some((id) => id.includes('Describe'))).toBe(true); + // And must NOT have been re-pointed onto the production pkg.Widget. + expect( + hasMethodEdgeFrom(result.graph, pkgWidget!.id).some((id) => id.includes('Describe')), + ).toBe(false); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); +});