mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-14 04:08:05 +00:00
fix(ingestion): re-point Go struct member edges when struct is declared in a sibling file (#3558)
findEnclosingClassInfo mints a Go method's owner id from the file containing the method (Struct:<methodFile>:<Type>), 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).
This commit is contained in:
parent
191a954bf6
commit
cc17925548
3 changed files with 208 additions and 0 deletions
|
|
@ -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:<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>:<name>` 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:<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) };
|
||||
};
|
||||
|
||||
/** 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<string, GraphNode[]>();
|
||||
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 `<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;
|
||||
};
|
||||
|
||||
/** Unique same-package same-name Struct node for a dangling owner id, else null. */
|
||||
const resolveOwner = (
|
||||
graph: KnowledgeGraph,
|
||||
structByKey: Map<string, GraphNode[]>,
|
||||
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;
|
||||
};
|
||||
|
|
@ -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
|
||||
|
|
|
|||
87
gitnexus/test/integration/go-cross-file-struct-owner.test.ts
Normal file
87
gitnexus/test/integration/go-cross-file-struct-owner.test.ts
Normal file
|
|
@ -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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue