mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(db): guard synthesis metadata JSON and keep calls ahead of imports (#2038)
* fix(db): guard synthesis metadata and migrate the expression index Bisect identifies2a2f71ec(#2033) as the malformed JSON regression. Guard index expressions, ownership predicates, wiring lookups, and the legacy Go backfill; schema v11 replaces existing v10 indexes. Cover malformed base edges, staged publication, upgrade replay, and indexed range lookup. * fix(db): preserve call precedence in deterministic edge traversal Independent bisect identifies2a2f71ec(#2033): source-first ordering puts importing file IDs ahead of function callers and consumes the default caller limit. Restore kind-first incoming and outgoing traversal with deterministic endpoint/position ties. Keep both original regression tests unchanged and cover filtered, cached, and provenance-filtered queries across reverse insertion order. Validation: 25 test files passed (564 tests, 3 skipped), including security, MCP caller truncation, sync, migration, graph, and DB/query coverage; npx tsc --noEmit passed. Both bisects rebuilt tsc and copied assets at every step with CODEGRAPH_KERNEL=0 to avoid using the HEAD kernel on historical extraction code. Final validation used the normal kernel setting. * chore: no changelog entry for an unreleased regression Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -363,3 +363,54 @@ describe('migration v6: dedup edges + add identity index on upgrade (#1034)', ()
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge traversal order', () => {
|
||||
let dir: string;
|
||||
let db: DatabaseConnection;
|
||||
let q: QueryBuilder;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'db-edge-order-'));
|
||||
db = DatabaseConnection.initialize(path.join(dir, 'test.db'));
|
||||
q = new QueryBuilder(db.getDb());
|
||||
q.insertNodes(['hub', 'file:a', 'function:b', 'function:z'].map(id => makeNode(id)));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
db.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it.each(['incoming', 'outgoing'] as const)
|
||||
('keeps calls ahead of imports with deterministic %s ties across rewrites', direction => {
|
||||
const peerKey = direction === 'incoming' ? 'source' : 'target';
|
||||
const hubKey = direction === 'incoming' ? 'target' : 'source';
|
||||
const shapes: Array<[string, Edge['kind'], number, number]> = [
|
||||
['function:z', 'calls', 4, 2],
|
||||
['file:a', 'imports', 1, 0],
|
||||
['function:b', 'calls', 7, 2],
|
||||
['file:a', 'references', 2, 0],
|
||||
['function:b', 'calls', 3, 2],
|
||||
['function:b', 'calls', 3, 1],
|
||||
];
|
||||
const edges: Edge[] = shapes.map(([peer, kind, line, column]) => ({
|
||||
source: '', target: '', [hubKey]: 'hub', [peerKey]: peer,
|
||||
kind, line, column, provenance: 'tree-sitter',
|
||||
}));
|
||||
const expected = [shapes[5], shapes[4], shapes[2], shapes[0], shapes[1], shapes[3]];
|
||||
for (const insertionOrder of [edges, [...edges].reverse()]) {
|
||||
db.getDb().exec('DELETE FROM edges');
|
||||
q.insertEdges(insertionOrder);
|
||||
// Exercise both the cached unfiltered query and the dynamic filtered one.
|
||||
for (const kinds of [undefined, ['references', 'imports', 'calls'] as Edge['kind'][]]) {
|
||||
const result = direction === 'incoming'
|
||||
? q.getIncomingEdges('hub', kinds) : q.getOutgoingEdges('hub', kinds);
|
||||
expect(result.map(edge => [edge[peerKey], edge.kind, edge.line, edge.column])).toEqual(expected);
|
||||
}
|
||||
if (direction === 'outgoing') {
|
||||
expect(q.getOutgoingEdges('hub', undefined, 'tree-sitter')
|
||||
.map(edge => [edge[peerKey], edge.kind, edge.line, edge.column])).toEqual(expected);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { DatabaseConnection } from '../src/db';
|
||||
import { CURRENT_SCHEMA_VERSION, getCurrentVersion, runMigrations } from '../src/db/migrations';
|
||||
import { QueryBuilder } from '../src/db/queries';
|
||||
import { SynthesisStage } from '../src/db/synthesis-stage';
|
||||
|
||||
describe('synthesis metadata JSON boundaries and migration', () => {
|
||||
let dir: string;
|
||||
let connection: DatabaseConnection;
|
||||
let queries: QueryBuilder;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-synthesis-json-'));
|
||||
connection = DatabaseConnection.initialize(path.join(dir, 'test.db'));
|
||||
queries = new QueryBuilder(connection.getDb());
|
||||
queries.insertNodes(['a', 'b'].map(id => ({
|
||||
id, name: id, qualifiedName: id, kind: 'function', language: 'typescript',
|
||||
filePath: `${id}.ts`, startLine: 1, endLine: 1, startColumn: 0, endColumn: 1,
|
||||
updatedAt: 0,
|
||||
})));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
connection.close();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const insertRaw = (metadata: string | null, line = 1) => connection.getDb().prepare(
|
||||
"INSERT INTO edges(source, target, kind, metadata, line) VALUES ('a', 'b', 'calls', ?, ?)"
|
||||
).run(metadata, line);
|
||||
|
||||
it.each([null, 'broken json {{{', 'null', '[]', '{}', '{"registeredAt":"wiring.ts:1"}'])
|
||||
('treats unowned metadata %s as an ordinary edge', metadata => {
|
||||
insertRaw(metadata);
|
||||
expect(queries.getOutgoingEdges('a')).toHaveLength(1);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('a.ts')).toBe(false);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('b.ts')).toBe(false);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('wiring.ts')).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the third-file range lookup indexed while tolerating malformed neighbors', () => {
|
||||
insertRaw('broken json {{{');
|
||||
queries.insertEdge({ source: 'a', target: 'b', kind: 'calls', line: 2,
|
||||
metadata: { synthesizedBy: 'event-emitter', registeredAt: 'wiring.ts:12' } });
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('a.ts')).toBe(true);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('b.ts')).toBe(true);
|
||||
const prepare = vi.spyOn(connection.getDb(), 'prepare');
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('wiring.ts')).toBe(true);
|
||||
const sql = prepare.mock.calls.map(([sql]) => sql).find(sql => sql.includes('FROM edges e WHERE'))!;
|
||||
expect(sql).toBeDefined();
|
||||
prepare.mockRestore();
|
||||
const plan = connection.getDb().prepare(`EXPLAIN QUERY PLAN ${sql}`).all('wiring.ts:', 'wiring.ts;');
|
||||
expect(plan.map(row => row.detail).join('\n'))
|
||||
.toMatch(/SEARCH e USING INDEX idx_edges_synthesis_site/);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('wiring.tsx')).toBe(false);
|
||||
});
|
||||
|
||||
it('preserves malformed base edges while staging and publishing owned replacements', async () => {
|
||||
insertRaw('broken json {{{');
|
||||
queries.insertEdge({ source: 'a', target: 'b', kind: 'calls', line: 2,
|
||||
metadata: { synthesizedBy: 'old-pass', registeredAt: 'old.ts:1' } });
|
||||
const stage = new SynthesisStage(connection.getPath());
|
||||
try {
|
||||
expect(stage.queries.getOutgoingEdges('a').map(edge => edge.line)).toEqual([1]);
|
||||
// The overlay must not shadow an existing base edge, even if its JSON is bad.
|
||||
stage.queries.insertEdge({ source: 'a', target: 'b', kind: 'calls', line: 1,
|
||||
metadata: { synthesizedBy: 'new-pass' } });
|
||||
stage.queries.insertEdge({ source: 'a', target: 'b', kind: 'calls', line: 3,
|
||||
metadata: { synthesizedBy: 'new-pass', registeredAt: 'new.ts:1' } });
|
||||
await stage.publish();
|
||||
} finally { stage.close(); }
|
||||
expect(connection.getDb().prepare('SELECT line, metadata FROM edges ORDER BY line').all()).toEqual([
|
||||
{ line: 1, metadata: 'broken json {{{' },
|
||||
{ line: 3, metadata: JSON.stringify({ synthesizedBy: 'new-pass', registeredAt: 'new.ts:1' }) },
|
||||
]);
|
||||
});
|
||||
|
||||
it('upgrades pre-v10 malformed metadata, including legacy Go containment', () => {
|
||||
const raw = connection.getDb();
|
||||
raw.exec(`DROP INDEX idx_edges_synthesis_site;
|
||||
DROP TABLE synthesis_inputs;
|
||||
DELETE FROM schema_versions WHERE version >= 10;
|
||||
UPDATE nodes SET language = 'go', kind = CASE id WHEN 'a' THEN 'struct' ELSE 'method' END;`);
|
||||
insertRaw('broken json {{{');
|
||||
raw.prepare("INSERT INTO edges(source, target, kind, metadata) VALUES ('a', 'b', 'contains', ?)")
|
||||
.run('broken containment {{{');
|
||||
runMigrations(raw, 9);
|
||||
expect(getCurrentVersion(raw)).toBe(CURRENT_SCHEMA_VERSION);
|
||||
expect(queries.getOutgoingEdges('a', ['calls'])[0].metadata).toBeUndefined();
|
||||
expect(queries.getOutgoingEdges('a', ['contains'])[0].metadata?.synthesizedBy).toBe('go-method-contains');
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('a.ts')).toBe(true);
|
||||
insertRaw('still malformed', 2);
|
||||
});
|
||||
|
||||
it('replaces the v10 index on open and safely replays the replacement', () => {
|
||||
const raw = connection.getDb();
|
||||
raw.exec(`DROP INDEX idx_edges_synthesis_site;
|
||||
CREATE INDEX idx_edges_synthesis_site ON edges(json_extract(metadata, '$.registeredAt'))
|
||||
WHERE json_extract(metadata, '$.synthesizedBy') IS NOT NULL;
|
||||
DELETE FROM schema_versions WHERE version >= 11;
|
||||
INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (10, 0, 'legacy fixture');`);
|
||||
queries.insertEdge({ source: 'a', target: 'b', kind: 'calls', line: 2,
|
||||
metadata: { synthesizedBy: 'event-emitter', registeredAt: 'wiring.ts:12' } });
|
||||
connection.close();
|
||||
connection = DatabaseConnection.open(path.join(dir, 'test.db'));
|
||||
queries = new QueryBuilder(connection.getDb());
|
||||
expect(getCurrentVersion(connection.getDb())).toBe(CURRENT_SCHEMA_VERSION);
|
||||
insertRaw('broken json {{{');
|
||||
const before = connection.getDb().prepare('SELECT * FROM edges ORDER BY id').all();
|
||||
// Replay the DDL over its own output with bad JSON already present.
|
||||
connection.getDb().exec('DELETE FROM schema_versions WHERE version >= 11');
|
||||
runMigrations(connection.getDb(), 10);
|
||||
runMigrations(connection.getDb(), getCurrentVersion(connection.getDb()));
|
||||
expect(connection.getDb().prepare('SELECT * FROM edges ORDER BY id').all()).toEqual(before);
|
||||
expect(queries.hasSynthesizedEdgesTouchingFile('wiring.ts')).toBe(true);
|
||||
});
|
||||
});
|
||||
+19
-4
@@ -9,7 +9,7 @@ import { SqliteDatabase } from './sqlite-adapter';
|
||||
/**
|
||||
* Current schema version
|
||||
*/
|
||||
export const CURRENT_SCHEMA_VERSION = 10;
|
||||
export const CURRENT_SCHEMA_VERSION = 11;
|
||||
|
||||
/**
|
||||
* Migration definition
|
||||
@@ -187,9 +187,9 @@ const migrations: Migration[] = [
|
||||
CREATE TABLE IF NOT EXISTS synthesis_inputs (
|
||||
file_path TEXT PRIMARY KEY REFERENCES files(path) ON DELETE CASCADE
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS idx_edges_synthesis_site ON edges(json_extract(metadata, '$.registeredAt'))
|
||||
WHERE json_extract(metadata, '$.synthesizedBy') IS NOT NULL;
|
||||
UPDATE edges SET metadata = json_set(COALESCE(metadata, '{}'), '$.synthesizedBy', 'go-method-contains')
|
||||
CREATE INDEX IF NOT EXISTS idx_edges_synthesis_site ON edges(CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.registeredAt') END)
|
||||
WHERE CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.synthesizedBy') END IS NOT NULL;
|
||||
UPDATE edges SET metadata = json_set(CASE WHEN json_valid(metadata) THEN metadata ELSE '{}' END, '$.synthesizedBy', 'go-method-contains')
|
||||
WHERE kind = 'contains' AND provenance IS NULL AND EXISTS (
|
||||
SELECT 1 FROM nodes s JOIN nodes t ON t.id = edges.target
|
||||
WHERE s.id = edges.source AND s.language = 'go' AND t.language = 'go'
|
||||
@@ -201,6 +201,21 @@ const migrations: Migration[] = [
|
||||
`);
|
||||
},
|
||||
},
|
||||
{
|
||||
version: 11,
|
||||
description: 'Guard synthesis metadata lookups against malformed JSON',
|
||||
up: (db) => {
|
||||
// Existing v10 indexes keep their old expression under IF NOT EXISTS.
|
||||
// Rebuild transactionally; the guarded v10 definition also lets older
|
||||
// databases containing malformed metadata reach this migration safely.
|
||||
db.exec(`
|
||||
DROP INDEX IF EXISTS idx_edges_synthesis_site;
|
||||
CREATE INDEX idx_edges_synthesis_site
|
||||
ON edges(CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.registeredAt') END)
|
||||
WHERE CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.synthesizedBy') END IS NOT NULL;
|
||||
`);
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
|
||||
+13
-9
@@ -1878,15 +1878,15 @@ export class QueryBuilder {
|
||||
|
||||
/** Must run before file replacement/deletion cascades the endpoint edges. */
|
||||
hasSynthesizedEdgesTouchingFile(filePath: string): boolean {
|
||||
const owned = "json_extract(e.metadata, '$.synthesizedBy') IS NOT NULL";
|
||||
const owned = "CASE WHEN json_valid(e.metadata) THEN json_extract(e.metadata, '$.synthesizedBy') END IS NOT NULL";
|
||||
for (const endpoint of ['source', 'target']) {
|
||||
if (this.db.prepare(`SELECT 1 FROM nodes n JOIN edges e ON e.${endpoint} = n.id
|
||||
WHERE n.file_path = ? AND ${owned} LIMIT 1`).get(filePath)) return true;
|
||||
}
|
||||
// Wiring often lives in a third file, with neither endpoint in it.
|
||||
return !!this.db.prepare(`SELECT 1 FROM edges e WHERE ${owned}
|
||||
AND json_extract(e.metadata, '$.registeredAt') >= ?
|
||||
AND json_extract(e.metadata, '$.registeredAt') < ? LIMIT 1`
|
||||
AND CASE WHEN json_valid(e.metadata) THEN json_extract(e.metadata, '$.registeredAt') END >= ?
|
||||
AND CASE WHEN json_valid(e.metadata) THEN json_extract(e.metadata, '$.registeredAt') END < ? LIMIT 1`
|
||||
).get(`${filePath}:`, `${filePath};`);
|
||||
}
|
||||
|
||||
@@ -1973,7 +1973,8 @@ export class QueryBuilder {
|
||||
}
|
||||
|
||||
/**
|
||||
* Get outgoing edges from a node
|
||||
* Get outgoing edges from a node. Preserve the source/kind index order
|
||||
* (calls before imports/references), then break ties deterministically.
|
||||
*/
|
||||
getOutgoingEdges(sourceId: string, kinds?: EdgeKind[], provenance?: string): Edge[] {
|
||||
if ((kinds && kinds.length > 0) || provenance) {
|
||||
@@ -1990,30 +1991,33 @@ export class QueryBuilder {
|
||||
params.push(provenance);
|
||||
}
|
||||
|
||||
sql += ' ORDER BY target, kind, line, col';
|
||||
sql += ' ORDER BY kind, target, line, col';
|
||||
const rows = this.db.prepare(sql).all(...params) as EdgeRow[];
|
||||
return rows.map(rowToEdge);
|
||||
}
|
||||
|
||||
if (!this.stmts.getEdgesBySource) {
|
||||
this.stmts.getEdgesBySource = this.db.prepare('SELECT * FROM edges WHERE source = ? ORDER BY target, kind, line, col');
|
||||
this.stmts.getEdgesBySource = this.db.prepare('SELECT * FROM edges WHERE source = ? ORDER BY kind, target, line, col');
|
||||
}
|
||||
const rows = this.stmts.getEdgesBySource.all(sourceId) as EdgeRow[];
|
||||
return rows.map(rowToEdge);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get incoming edges to a node
|
||||
* Get incoming edges to a node. Kind must precede opaque source IDs:
|
||||
* file IDs sort before function IDs, so source-first ordering lets imports
|
||||
* displace actual calls in capped caller lists. Keep deterministic ties
|
||||
* without changing the target/kind index's established kind precedence.
|
||||
*/
|
||||
getIncomingEdges(targetId: string, kinds?: EdgeKind[]): Edge[] {
|
||||
if (kinds && kinds.length > 0) {
|
||||
const sql = `SELECT * FROM edges WHERE target = ? AND kind IN (${kinds.map(() => '?').join(',')}) ORDER BY source, kind, line, col`;
|
||||
const sql = `SELECT * FROM edges WHERE target = ? AND kind IN (${kinds.map(() => '?').join(',')}) ORDER BY kind, source, line, col`;
|
||||
const rows = this.db.prepare(sql).all(targetId, ...kinds) as EdgeRow[];
|
||||
return rows.map(rowToEdge);
|
||||
}
|
||||
|
||||
if (!this.stmts.getEdgesByTarget) {
|
||||
this.stmts.getEdgesByTarget = this.db.prepare('SELECT * FROM edges WHERE target = ? ORDER BY source, kind, line, col');
|
||||
this.stmts.getEdgesByTarget = this.db.prepare('SELECT * FROM edges WHERE target = ? ORDER BY kind, source, line, col');
|
||||
}
|
||||
const rows = this.stmts.getEdgesByTarget.all(targetId) as EdgeRow[];
|
||||
return rows.map(rowToEdge);
|
||||
|
||||
+4
-2
@@ -199,8 +199,10 @@ CREATE INDEX IF NOT EXISTS idx_unresolved_status ON unresolved_refs(status);
|
||||
CREATE INDEX IF NOT EXISTS idx_unresolved_failed_tail ON unresolved_refs(name_tail) WHERE status = 'failed';
|
||||
CREATE INDEX IF NOT EXISTS idx_edges_provenance ON edges(provenance);
|
||||
-- Sync's third-file wiring lookup must not scan every synthesized edge.
|
||||
CREATE INDEX IF NOT EXISTS idx_edges_synthesis_site ON edges(json_extract(metadata, '$.registeredAt'))
|
||||
WHERE json_extract(metadata, '$.synthesizedBy') IS NOT NULL;
|
||||
-- CASE short-circuits malformed metadata; keep these expressions identical
|
||||
-- in migrations and synthesis queries so SQLite can use the partial index.
|
||||
CREATE INDEX IF NOT EXISTS idx_edges_synthesis_site ON edges(CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.registeredAt') END)
|
||||
WHERE CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.synthesizedBy') END IS NOT NULL;
|
||||
|
||||
-- Retain the cheap source-gate verdict when a later sync deletes the source.
|
||||
CREATE TABLE IF NOT EXISTS synthesis_inputs (
|
||||
|
||||
@@ -3,7 +3,7 @@ import { QueryBuilder } from './queries';
|
||||
import { createYielder } from '../resolution/cooperative-yield';
|
||||
|
||||
// Ownership is independent of provenance: Go method containment is structural.
|
||||
export const SYNTHESIZED_EDGE = "json_extract(metadata, '$.synthesizedBy') IS NOT NULL";
|
||||
export const SYNTHESIZED_EDGE = "CASE WHEN json_valid(metadata) THEN json_extract(metadata, '$.synthesizedBy') END IS NOT NULL";
|
||||
|
||||
/** A private edge overlay: passes see base edges plus their new Go prerequisites. */
|
||||
export class SynthesisStage {
|
||||
|
||||
Reference in New Issue
Block a user