From cc17925548d92eefbcd17e4283fbf8b3fcb8f272 Mon Sep 17 00:00:00 2001 From: System administrator Date: Sun, 11 Oct 2026 13:19:35 +0700 Subject: [PATCH 1/2] fix(ingestion): re-point Go struct member edges when struct is declared in a sibling file (#3558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findEnclosingClassInfo mints a Go method's owner id from the file containing the method (Struct::), while the Struct node is keyed by the file declaring the struct (parse-worker uses declFile). Same package, different file => the HAS_METHOD edge's sourceId points at a node that does not exist and the edge is silently dropped at COPY into LadybugDB. Add reconcileGoStructOwnerEdges, run at the end of the parse phase after ALL worker chunks are merged so every package's structs are visible. It rewrites only edges that would otherwise dangle, and only when exactly one Struct node shares the dangling id's package directory and simple name; 0 or >=2 candidates keep the old drop behavior (a wrong owner is worse than a dropped edge). Index build is lazy: dangling Struct sources are rare (struct fields never dangle — they live in the struct's own file), so the common non-Go or fully-same-file case pays two per-type edge scans and no whole-node pass. Regression test: test/integration/go-cross-file-struct-owner.test.ts (fails-before: 1 HAS_METHOD edge; passes-after: 3). --- .../languages/go/struct-owner-reconcile.ts | 114 ++++++++++++++++++ .../ingestion/pipeline-phases/parse-impl.ts | 7 ++ .../go-cross-file-struct-owner.test.ts | 87 +++++++++++++ 3 files changed, 208 insertions(+) create mode 100644 gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts create mode 100644 gitnexus/test/integration/go-cross-file-struct-owner.test.ts 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..9498845bf --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts @@ -0,0 +1,114 @@ +/** + * Go cross-file struct-owner reconciliation (#3558). + * + * `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::` AND no such node exists; + * - exactly ONE Struct node shares the same package directory and simple + * name. 0 or ≥2 candidates: leave the edge alone — a wrong owner is worse + * than a dropped edge. + * + * Cheap by default: the whole-node package index is built only when a + * dangling `Struct:` owner source actually exists. 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 type { KnowledgeGraph } from '../../../graph/types.js'; +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { logger } from '../../../logger.js'; +import { goPackageDir } from './package-clause.js'; + +const OWNER_LABEL = 'Struct'; + +/** `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) }; +}; + +/** Rewrites to apply, collected before mutation (the type-index iterator must not be mutated mid-walk). */ +interface OwnerRewrite { + readonly rel: GraphRelationship; + readonly newSourceId: string; +} + +/** + * Rewrite cross-file Go struct-owner edges in place. Returns the number of + * edges re-pointed. + */ +export const reconcileGoStructOwnerEdges = (graph: KnowledgeGraph): number => { + 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; + + // Package-dir + simple-name → candidate Struct nodes. Built from the FULL + // merged graph, so the declaring file may sit in any already-merged chunk. + const structByKey = new Map(); + for (const node of graph.iterNodes()) { + if (node.label !== OWNER_LABEL) continue; + const filePath = node.properties?.filePath; + const name = node.properties?.name; + if (typeof filePath !== 'string' || typeof name !== 'string') continue; + const key = `${goPackageDir(filePath)}\0${name}`; + const bucket = structByKey.get(key); + if (bucket) bucket.push(node); + else structByKey.set(key, [node]); + } + + let rekeyed = 0; + for (const rel of dangling) { + const newSourceId = resolveOwner(graph, structByKey, rel.sourceId); + if (newSourceId === null || newSourceId === rel.sourceId) continue; + 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; +}; + +/** Unique same-package same-name Struct node for a dangling owner id, else null. */ +const resolveOwner = ( + graph: KnowledgeGraph, + structByKey: Map, + danglingId: string, +): string | null => { + const parsed = parseStructOwnerId(danglingId); + if (parsed === null) return null; + const bucket = structByKey.get(`${goPackageDir(parsed.filePath)}\0${parsed.name}`); + // No candidate (struct never parsed / outside the repo slice) or an + // ambiguous same-name pair: 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; +}; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index cc72a04d5..f8dde7afe 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,12 @@ 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. + reconcileGoStructOwnerEdges(graph); + // 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..7c9de17b0 --- /dev/null +++ b/gitnexus/test/integration/go-cross-file-struct-owner.test.ts @@ -0,0 +1,87 @@ +/** + * 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 { runPipelineFromRepo } from './resolvers/helpers.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 }); + } + }); +}); From cb30af3229a8193416fe8deeaab929958fcf7d54 Mon Sep 17 00:00:00 2001 From: System administrator Date: Tue, 13 Oct 2026 07:05:30 +0700 Subject: [PATCH 2/2] fix(ingestion): scope Go struct-owner reconcile by language and package Address PR #3559 review findings: - Scope the pass to Go: dangling HAS_METHOD/HAS_PROPERTY owner ids whose Struct:: file is not Go source (Rust, Swift, ...) are never re-pointed, and only Struct nodes stamped language 'go' (the marker the worker sets on every declared symbol) are candidates. Negative test pins a Rust-shaped dangling edge staying untouched beside a same-name Go struct. - Key candidate structs by (dir, declared package clause, name) instead of (dir, name): one directory can carry 'package pkg' and external-test 'package pkg_test' files declaring the same type name, which the old key saw as ambiguous and dropped fail-closed. Clauses are read via the shared inferGoPackageName derivation with a call-scoped cache. Integration test covers the 3-file layout (pkg decl, pkg_test decl, pkg_test methods file) and asserts the edge lands on the pkg_test.Widget node. - Keep the direct call in parse-impl with a TODO: the pass needs a graph-level post-merge hook, which no LanguageProvider/ScopeResolver hook provides today. --- .../languages/go/struct-owner-reconcile.ts | 196 ++++++++++++++---- .../ingestion/pipeline-phases/parse-impl.ts | 7 +- .../go-cross-file-struct-owner.test.ts | 195 +++++++++++++++++ 3 files changed, 352 insertions(+), 46 deletions(-) diff --git a/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts b/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts index 9498845bf..6503dec7d 100644 --- a/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts +++ b/gitnexus/src/core/ingestion/languages/go/struct-owner-reconcile.ts @@ -1,5 +1,5 @@ /** - * Go cross-file struct-owner reconciliation (#3558). + * 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::`), @@ -12,24 +12,36 @@ * package's structs are visible), and rewrites only edges that would * otherwise dangle: * - * - sourceId looks like `Struct::` AND no such node exists; - * - exactly ONE Struct node shares the same package directory and simple - * name. 0 or ≥2 candidates: leave the edge alone — a wrong owner is worse - * than a dropped edge. + * - 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 whole-node package index is built only when a - * dangling `Struct:` owner source actually exists. 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. + * 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 } from './package-clause.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 = ( @@ -42,17 +54,96 @@ const parseStructOwnerId = ( 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. + * edges re-pointed. `repoPath` resolves the repo-relative + * `properties.filePath` for package-clause reads. */ -export const reconcileGoStructOwnerEdges = (graph: KnowledgeGraph): number => { +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)) { @@ -61,24 +152,56 @@ export const reconcileGoStructOwnerEdges = (graph: KnowledgeGraph): number => { } if (dangling.length === 0) return 0; - // Package-dir + simple-name → candidate Struct nodes. Built from the FULL - // merged graph, so the declaring file may sit in any already-merged chunk. - const structByKey = new Map(); - for (const node of graph.iterNodes()) { - if (node.label !== OWNER_LABEL) continue; - const filePath = node.properties?.filePath; - const name = node.properties?.name; - if (typeof filePath !== 'string' || typeof name !== 'string') continue; - const key = `${goPackageDir(filePath)}\0${name}`; - const bucket = structByKey.get(key); - if (bucket) bucket.push(node); - else structByKey.set(key, [node]); + // 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 of dangling) { - const newSourceId = resolveOwner(graph, structByKey, rel.sourceId); - if (newSourceId === null || newSourceId === rel.sourceId) continue; + for (const { rel, newSourceId } of rewrites) { graph.removeRelationship(rel.id); // Edge ids are minted as `:->` by the worker path. graph.addRelationship({ @@ -95,20 +218,3 @@ export const reconcileGoStructOwnerEdges = (graph: KnowledgeGraph): number => { } return rekeyed; }; - -/** Unique same-package same-name Struct node for a dangling owner id, else null. */ -const resolveOwner = ( - graph: KnowledgeGraph, - structByKey: Map, - danglingId: string, -): string | null => { - const parsed = parseStructOwnerId(danglingId); - if (parsed === null) return null; - const bucket = structByKey.get(`${goPackageDir(parsed.filePath)}\0${parsed.name}`); - // No candidate (struct never parsed / outside the repo slice) or an - // ambiguous same-name pair: 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; -}; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index f8dde7afe..6ca134f83 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -1529,7 +1529,12 @@ export async function runChunkedParseAndResolve( // 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. - reconcileGoStructOwnerEdges(graph); + // 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). diff --git a/gitnexus/test/integration/go-cross-file-struct-owner.test.ts b/gitnexus/test/integration/go-cross-file-struct-owner.test.ts index 7c9de17b0..238384069 100644 --- a/gitnexus/test/integration/go-cross-file-struct-owner.test.ts +++ b/gitnexus/test/integration/go-cross-file-struct-owner.test.ts @@ -12,7 +12,10 @@ 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-')); @@ -85,3 +88,195 @@ describe('Go cross-file struct method ownership (#3558)', () => { } }); }); + +// 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 }); + } + }); +});