This commit is contained in:
Nguyễn Đăng Minh Lực 2026-10-13 00:05:43 +00:00 • committed by GitHub
commit c0b01fbd98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 514 additions and 0 deletions

View file

@ -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:<methodFile>:<Type>`),
* while the Struct node itself is keyed by the file DECLARING the struct
* (`Struct:<declFile>:<Type>`). 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:<file>.go:<name>` 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:<file>:<name>` → { 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<string, GraphNode[]>;
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<string, string | null>();
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:<file>:<name>` 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<string, DirIndex | null>();
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 `<TYPE>:<source>-><target>` 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;
};

View file

@ -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

View file

@ -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 });
}
});
});