Detect and purge graph nodes whose files no longer exist on disk - #458
Merged
Conversation
Every freshness signal index_health publishes is derived from the files the daemon already tracks: the mtime ledger answers staleness, the parse list answers extraction, the skip rollup answers exclusion. None of them can see a node whose file left the disk without the daemon witnessing the departure — a subtree deleted while the daemon was down, an mtime record pruned without its nodes, a repo whose checkout was removed. Those symbols keep coming back from search_symbols and find_usages while the probe reports 100% and stale=0, which is the worst shape a health signal can take: confidently wrong. Add the one check that can see them — stat the paths the graph itself claims. It folds into the file-node walk the payload already runs, and reports a path_liveness block (checked / orphan_files / orphan_rate / orphans_by_repo, with samples). A non-zero orphan count caps health_score, worst-of against the parse ratio rather than blended: the two answer different questions and health should track whichever is failing. The audit refuses to infer deletion from failure. A prefix with no resolvable local root, a node whose path does not sit under the repo it claims, or a stat that fails for any reason other than absence all abstain rather than count as orphans. Synthetic attribution paths (external::, external-call::) are outside the question entirely, for the same reason they are outside the ownership audit. The stat loop is capped so a liveness probe stays a probe. Past the cap the block reports truncated:true and an indicative extrapolation, since what it sees is a prefix of the walk rather than a uniform sample.
Deletion detection seeded its candidates from the mtime ledger alone, so it could only evict files it was still tracking. A file whose mtime record was pruned without its nodes — or never restored on a warm start — had nothing left pointing at it, so every reconcile walked straight past it and its symbols stayed in the graph indefinitely, answering searches with code that is no longer there. On a full-tree pass the disk walk is authoritative for the whole repository, so diff the graph's own file inventory against it too. The compact file projection is written per indexed file and dropped on eviction, which makes it the one inventory that tracks the node set rather than the ledger. This only contributes candidates. Every one still passes the existing stat gate, so a file that merely fell out of the walk — an unrecognised language, an artifact, a path the walk spells differently — is preserved exactly as before; only a confirmed absence evicts. Scoped passes are excluded, since their disk set covers one subtree and would read the rest of the repository as deleted. So is a full walk that found nothing at all: a vanished root is a likelier explanation than an emptied repository, and that is not evidence worth taking a whole repo out of the index on.
…orphaned-file-nodes * origin/main: fix(savings): price Zhipu GLM models instead of reporting $0 avoided hooks: narrow the access policy to the surface its message describes fix(daemon): flag tracked repos whose directory was deleted Tidy extraction_gap.go alongside the classification change Stop the GCX caveat message from destroying its own payload Caveat a usage result whose every row is a name-only match Weigh usage-edge provenance before calling a symbol used # Conflicts: # docs/mcp.md # internal/mcp/tools_enhancements.go
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.
Fixes #311.
The bug, restated
index_healthreportshealth=100.0% stale=0while the graph holds nodes for files that no longer exist on disk. Those symbols keep coming back fromsearch_symbolsandfind_usages, and nothing in the payload contradicts them.The reason is structural, not a missed case: every freshness signal the payload publishes is derived from the daemon's tracked-file set.
stale_fileswalksIndexer.FileMtimes(),parse_failureslists files that were indexed, the skip rollup counts synthetic file nodes. All three are keyed by files the daemon still knows about — so a node whose file left the disk without the daemon recording the departure is invisible to all of them. The same gap on the write side means the nodes are never evicted either: deletion detection seeded its candidates from the mtime ledger alone, so a file with no ledger entry had nothing left pointing at it and survived every reconcile.store_sqlite.OrphanRepoPrefixesalready covers prefix-level orphans. Nothing covered path-level orphans inside a still-tracked repo.What this changes
1. Detect —
index_healthstats the paths the graph itself claimsinternal/graph/orphan_diagnostics.goadds a bounded path-liveness audit, modelled on the existingPrefixDiagnosticsownership audit. It folds into theNodesByKind(KindFile)walkbuildIndexHealthPayloadCtxalready runs, so it costs stats rather than a second graph pass, and emits:A non-zero orphan count caps
health_score— worst-of against the parse ratio rather than blended, since the two answer different questions ("did the files we found parse" vs "are the files we hold still there") and health should track whichever is failing. The recommendation names the remediation and says explicitly thatstale_filescannot cover this population.The block is emitted on a clean workspace too (
checked: 2581, clean: true). That is the point: it is the evidence that a 100% means what it says, and its absence tells a caller the probe could not run.The audit refuses to infer deletion from failure. A prefix with no resolvable local root, a node whose path does not sit under the repo it claims, and a stat that fails for any reason other than absence all abstain into
unresolvablerather than count as orphans. Synthetic attribution paths (external::,external-call::) are outside the question entirely, via the sameIsAuditableRepoSourcePathpredicate the ownership audit uses.The stat loop is capped (20k). Past the cap the block reports
truncated: trueplus an indicative extrapolation — what it saw is a prefix of the node walk, not a uniform sample. That is stated in the code and in the payload rather than dressed up as a census.2. Repair — a full-tree reconcile evicts what the walk no longer finds
Indexer.indexedFilesAbsentFromDiskdiffs the graph's own file inventory (the compactfilesprojection, written per indexed file and dropped on eviction — the one inventory that tracks the node set rather than the ledger) against the full-tree disk walk, and seeds the existing deletion-candidate set with the difference.This only contributes candidates. Every one still passes the existing stat gate, so a file that merely fell out of the walk — an unrecognised language, an artifact, a path the walk spells differently — is preserved exactly as before; only a confirmed absence evicts. Two exclusions:
Scope and boundaries
ReconcileAll(default hourly), warm-restart routes that take the full-tree pipeline, andreindex_repositorywithout apathsargument. The warm-restartcensus_noopfast path deliberately skips the walk, so it stays untouched — orphans there clear on the next janitor tick.unresolvable, not orphans, and does not lower the score. Deciding "the repo is dead" from "I could not look" belongs to bug:daemon statusandgortex reposdisagree about tracked repos; a repo whose directory was deleted is never flagged #312, not here.gortex reindexverb (item 3) is not in this PR. It is a CLI lifecycle ask (Add gortex uninstall and gortex upgrade commands (especially for Windows) #298), andreindex_repository/gortex call workspace_admin --arg operation=reindexalready reach the pass that now purges.Verification
TestFullTreeReconcileEvictsFilesTheMtimeLedgerForgotfail withDeletedFileCount 0 != 1; the safety tests assert the converse (a file still on disk, and a walk that found nothing, are never swept).internal/graph, 4 ininternal/indexer, 5 ininternal/mcp(clean workspace, orphans reported + score capped, synthetic paths ignored, unresolvable roots abstain, orphans surviving a zero-stalereport).
go test -racegreen forinternal/graph,internal/indexer,internal/mcp, and the full./...suite.golangci-lint run ./...— 0 issues.path_liveness {checked: 2, clean: true}athealth_score 100; deleting a watched subtree drops it tochecked: 1, still clean. Worth noting for whoever revisits the report: the fsnotify deletion path on currentmainis healthy, so the identity-doubling that originally manufactured these orphans no longer reproduces. What remains is the class of defect — the graph having no way to notice a departure it did not witness — which is what the regression tests construct directly.