fix: persist deleted-export advisories so check survives purge ordering - #2068
Closed
carlos-alm wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
checkNoDeletedExportsInUse (#1806) only ever queried the live nodes/edges tables, so once any codegraph build purged a deleted file's rows - detectChanges does this unconditionally, independent of whether codegraph check has run yet - the violation silently went undetected. Capture a durable snapshot of each deleted file's exported defs and their external consumers at the exact point detectChanges/the native orchestrator computes the removed-file set, BEFORE purging. check now falls back to this deleted_export_advisories table whenever the live nodes for a deleted file are already gone. Advisory rows are cleared whenever a file reappears under the same path, so a resolved deletion never misattributes a stale violation. Mirrored in the native Rust orchestrator (record/clear_deleted_export_ advisories in detect_changes.rs, wired into pipeline.rs's save_and_purge_changed), since the native incremental fast path bypasses the JS detectChanges stage entirely. Impact: 28 functions changed, 13 affected
Copy link
Copy Markdown
Contributor
Greptile SummaryThis PR persists deleted-export evidence so signature checks can survive graph purges. The main changes are:
Confidence Score: 4/5The advisory persistence path can still drop evidence after repeated builds of the same deleted file.
src/db/repository/deleted-export-advisories.ts and crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant B1 as Build N
participant DB as graph.db
participant B2 as Build N+1
participant C as codegraph check
B1->>DB: Capture advisory while deleted file nodes exist
B1->>DB: Purge deleted file nodes and edges
B2->>DB: Detect same removed file again
B2->>DB: Delete existing advisory
B2->>DB: Find no live nodes, insert no advisory
C->>DB: Live query returns empty
C->>DB: Advisory fallback returns empty
C-->>C: Deleted export violation is missed
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant B1 as Build N
participant DB as graph.db
participant B2 as Build N+1
participant C as codegraph check
B1->>DB: Capture advisory while deleted file nodes exist
B1->>DB: Purge deleted file nodes and edges
B2->>DB: Detect same removed file again
B2->>DB: Delete existing advisory
B2->>DB: Find no live nodes, insert no advisory
C->>DB: Live query returns empty
C->>DB: Advisory fallback returns empty
C-->>C: Deleted export violation is missed
Reviews (1): Last reviewed commit: "fix: persist deleted-export advisories s..." | Re-trigger Greptile |
Sorry, something went wrong.
Comment thread
src/db/repository/deleted-export-advisories.ts
| const tx = db.transaction(() => { | ||
| const now = Date.now(); | ||
| for (const file of removedFiles) { | ||
| deleteStmt.run(file); |
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Repeat Build Erases Advisory
When the same deleted file is seen by a later incremental build, the prior advisory is deleted before a replacement is known to exist. Because the first build already purged that file's live nodes, the second capture finds no exported definitions and inserts nothing, so a later codegraph check has neither live rows nor an advisory to report the deleted export.
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Codegraph Impact Analysis28 functions changed → 13 callers affected across 6 files
|
Sorry, something went wrong.
This was referenced Jul 12, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Reviewers
1 more reviewer1 participant
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
checkNoDeletedExportsInUse (#1806) detects an exported function/method/class lost when a file is deleted in its entirety, but only by querying the current DB for that file's rows. Once any codegraph build purges the deleted file's nodes/edges rows — detectChanges does this unconditionally for any file no longer found on disk, regardless of whether codegraph check has run yet — the predicate has nothing left to query and the violation silently goes undetected. This is purely a function of external invocation ordering (e.g. this repo's own update-graph.sh hook firing a plain codegraph build before pre-commit.sh's check runs), not something the predicate itself can control.
Closes #1938
Fix
Capture a durable, purge-order-independent snapshot instead of relying on live rows surviving until check runs:
Native engine parity: the native Rust orchestrator's incremental fast path (crates/codegraph-core/.../pipeline.rs's save_and_purge_changed) bypasses the JS detectChanges stage entirely for a plain incremental build — this is the default code path when the native engine is available. Mirrored the capture/clear logic there (record_deleted_export_advisories/clear_deleted_export_advisories in detect_changes.rs) so both engines produce identical results. Verified via a real buildGraph() end-to-end regression test that runs against both engines.
While implementing, found and fixed a real bug this surfaced: the Rust table_queryable existence-probe used query_row(...).is_ok(), which returns Err(QueryReturnedNoRows) — not Ok — for a table that exists but is still empty, causing the whole advisory capture to silently no-op on a freshly-migrated table. Caught by the new Rust unit tests; fixed by treating QueryReturnedNoRows as "exists".
Test plan
Out-of-scope findings filed separately
Stacked on #1937/#1929 (base branch fix/issue-1929-bare-super-constructor-calls-are-never) — only the check/detect-changes/migrations/test diff described above is this issue's change.