diff --git a/__tests__/db-perf.test.ts b/__tests__/db-perf.test.ts index 941fb50f5..e1cdea7a5 100644 --- a/__tests__/db-perf.test.ts +++ b/__tests__/db-perf.test.ts @@ -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); + } + } + }); +}); diff --git a/__tests__/migrations-synthesis-json.test.ts b/__tests__/migrations-synthesis-json.test.ts new file mode 100644 index 000000000..f10c5f59a --- /dev/null +++ b/__tests__/migrations-synthesis-json.test.ts @@ -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); + }); +}); diff --git a/src/db/migrations.ts b/src/db/migrations.ts index b3b0a1805..bb7b84bf5 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -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; + `); + }, + }, ]; /** diff --git a/src/db/queries.ts b/src/db/queries.ts index 5acfbf971..14a664341 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -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); diff --git a/src/db/schema.sql b/src/db/schema.sql index e49bf6937..91ae4805b 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -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 ( diff --git a/src/db/synthesis-stage.ts b/src/db/synthesis-stage.ts index 7764a1c06..23c914d2a 100644 --- a/src/db/synthesis-stage.ts +++ b/src/db/synthesis-stage.ts @@ -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 {