fix(db): enforce UNIQUE on edges so INSERT OR IGNORE actually dedupes#34
Open
mschreib28 wants to merge 1 commit into
Open
fix(db): enforce UNIQUE on edges so INSERT OR IGNORE actually dedupes#34mschreib28 wants to merge 1 commit into
mschreib28 wants to merge 1 commit into
Conversation
The edges table has `id INTEGER PRIMARY KEY AUTOINCREMENT` and no other
UNIQUE constraint. The codebase uses
INSERT OR IGNORE INTO edges (source, target, kind, ...) VALUES (...)
clearly intending dedup, but the only candidate key for OR IGNORE was
the autoincrement id (which never conflicts) — so the OR IGNORE was a
silent no-op. Any code path that re-emits the same edge (resolver retries,
partial-failure re-runs, framework extractors that double-emit) silently
inserted duplicates, inflating call graphs in codegraph_callers/callees.
This change adds a real UNIQUE index on the natural key:
UNIQUE INDEX idx_edges_unique
ON edges(source, target, kind, COALESCE(line, -1), COALESCE(col, -1))
COALESCE keeps two NULL line/col values comparable as equal — SQLite
treats raw NULLs in a UNIQUE index as distinct, which would otherwise
defeat dedup for edges that don't carry a line/col (1-indexed everywhere
in this codebase, so -1 is a safe sentinel).
Migration v4 first deduplicates pre-existing rows (DELETE ... WHERE id
NOT IN (SELECT MIN(id) FROM edges GROUP BY source, target, kind,
COALESCE(line, -1), COALESCE(col, -1))) then creates the index. Both run
inside the migration transaction wrapper so a crash leaves the DB
consistent.
CURRENT_SCHEMA_VERSION bumped to 4. Two existing version-pinned tests
updated to match.
## Files changed
| File | Change |
|---|---|
| src/db/schema.sql | Add UNIQUE INDEX idx_edges_unique for fresh installs |
| src/db/migrations.ts | Bump version to 4; add migration v4 (dedup + index) |
| __tests__/edges-unique.test.ts | 7 regression tests |
| __tests__/foundation.test.ts | Update expected schema version |
| __tests__/pr19-improvements.test.ts | Update expected schema version |
## Test plan
- [x] npm test: 387/387 pass on macOS (one pre-existing fs.watch flake under parallel load, passes in isolation)
- [x] npx tsc --noEmit clean
- [x] Independent reviewer pass before pushing — APPROVE; nits-only
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n\nThe
edgestable hasid INTEGER PRIMARY KEY AUTOINCREMENTand no other UNIQUE constraint. The codebase usesINSERT OR IGNORE INTO edges ...clearly intending dedup, but autoincrement IDs never conflict, so theOR IGNOREwas silently a no-op. Any path that re-emits the same edge (resolver retries, partial-failure re-runs, framework extractors that double-emit) accumulated duplicates → inflated call graphs incodegraph_callers/codegraph_calleesresults.\n\n## What changed\n\n- Schema:src/db/schema.sqladdsUNIQUE INDEX idx_edges_unique ON edges(source, target, kind, COALESCE(line, -1), COALESCE(col, -1))for fresh installs.\n- Migration v4 (src/db/migrations.ts): existing databases first dedupe pre-existing duplicates (DELETE ... WHERE id NOT IN (SELECT MIN(id) ... GROUP BY ...)) then create the index. Both run inside the migration transaction wrapper, so a crash leaves the DB consistent.\n-COALESCE(line, -1), COALESCE(col, -1)in the unique key: SQLite treats raw NULLs in a UNIQUE index as distinct, which would defeat dedup for edges that don't carry a line/column. The codebase's line numbers are 1-indexed and column numbers are non-negative everywhere —-1is a safe sentinel (verified via grep acrosssrc/).\n-CURRENT_SCHEMA_VERSIONbumped 3 → 4. Two existing version-pinned tests updated.\n\n## Files changed\n\n| File | Change |\n|---|---|\n|src/db/schema.sql| AddUNIQUE INDEX idx_edges_uniquefor fresh installs |\n|src/db/migrations.ts| Bump version to 4; add migration v4 (dedup + index) |\n|__tests__/edges-unique.test.ts| 7 regression tests |\n|__tests__/foundation.test.ts| Update expected schema version |\n|__tests__/pr19-improvements.test.ts| Update expected schema version |\n\n## Test coverage\n\n- Exact duplicates (same source/target/kind/line/col) collapse to one row\n- NULL line edges with same source/target/kind collapse viaCOALESCE\n- Different lines / different kinds remain distinct\n- BatchinsertEdgesdedupes within one call\n- Re-emission across 100 cycles leaves a single row\n- Migration test: simulate a v3-shaped DB, insert duplicates via raw SQL, runrunMigrations(db, 3), assert dedup occurred and the constraint is now enforced\n\n## Test plan\n\n- [x]npm test: 387/387 pass on macOS (one pre-existing fs.watch flake under parallel load, passes in isolation — unrelated to this change)\n- [x]npx tsc --noEmitclean\n- [x] Independent reviewer pass before pushing — verdict APPROVE (informational nits only: schema.sql header comment still says "Version 1", and the migration test simulates the v3 state by surgically rolling back rather than reproducing the historical schema.sql; both deemed acceptable)\n\n🤖 Generated with Claude Code\nCopied from colbymchenry/codegraph#102