fix(core): prune stale changed_files rows on refresh (#100) - #106
Conversation
RefreshChangedFiles recomputed the changed set every 2s and UPSERTed every file, but never deleted rows whose paths had left the set. Stale rows therefore persisted forever: after gitignoring a large untracked directory (e.g. a .venv) or deleting untracked files, the changed_files table kept the obsolete rows even though the TUI had moved on. Each file was also a separate auto-committed write every cycle, producing a write storm under repos with thousands of changes. Replace the per-file upsert loop with a single transactional replace (ReplaceChangedFiles): within one transaction it deletes the session's rows and batch-inserts the current set, carrying each file's already-merged reviewed state. This prunes files no longer present and collapses the writes into one commit. The engine's post-refresh snapshot path (filesRelativeToSnapshot) still runs after this and re-adds reverted-but-in-snapshot files, so that behavior is unaffected. Closes #100 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Greptile SummaryFixes stale
Confidence Score: 5/5Safe to merge. The transactional replace is correctly implemented, the previously-flagged reviewed-state loss for reverted files is resolved, and the new test suite covers the core scenarios end-to-end. The change is narrowly scoped: one new DB method with a straightforward DELETE+INSERT transaction, a signature change on RefreshChangedFiles to return the pre-replace reviewed map, and a one-parameter addition to filesRelativeToSnapshot that uses it. Reading from a nil map in Go is safe, so existing test call-sites that pass nil are correct. No new data races or loss scenarios were found. No files require special attention.
|
| Filename | Overview |
|---|---|
| internal/db/queries.go | Adds ReplaceChangedFiles: atomically DELETE + batch-INSERT within a single transaction; deferred Rollback + Commit pattern is correct. |
| internal/core/session.go | RefreshChangedFiles now returns the pre-replace reviewed map alongside the file list; switches from per-file UpsertChangedFile to a single ReplaceChangedFiles call. |
| internal/core/engine_impl.go | Threads priorReviewed through filesRelativeToSnapshot so reverted-file rows re-inserted after the transactional replace preserve the user's reviewed flag; nil-map read in Go is safe for callers that pass nil. |
| internal/core/engine_test.go | Adds regression test TestFilesRelativeToSnapshot_RevertedFilePreservesReviewedState; updates three existing call-sites to pass nil for priorReviewed. |
| internal/core/session_test.go | Adds two new tests covering stale-row pruning and .gitignore-triggered prune; updates existing call-sites for the new return signature. |
| internal/db/db_test.go | Adds TestReplaceChangedFiles verifying the delete+insert behaviour, stale-row pruning, and reviewed-flag preservation. |
Sequence Diagram
sequenceDiagram
participant BG as Background ticker (2s)
participant E as Engine.RefreshChangedFiles
participant SM as SessionManager.RefreshChangedFiles
participant DB as DB
participant Git as git.Diff
BG->>E: tick
E->>SM: RefreshChangedFiles(session)
SM->>Git: Diff(baseRef)
Git-->>SM: currentFiles[]
SM->>DB: GetChangedFiles(sessionID)
DB-->>SM: existingRows (priorReviewed map)
note over SM: merge reviewed flags onto currentFiles
SM->>DB: ReplaceChangedFiles(sessionID, ptrs) BEGIN TX / DELETE all rows / INSERT currentFiles / COMMIT
DB-->>SM: ok
SM-->>E: files, priorReviewed, nil
E->>E: autoUnmarkChangedFiles(files, latestSnap)
alt "reviewBase != nil"
E->>E: filesRelativeToSnapshot(session, files, reviewBase, priorReviewed)
note over E: for each reverted file: Reviewed = priorReviewed[path]
E->>DB: UpsertChangedFile(revertedFile)
end
E-->>BG: updatedFiles
Reviews (2): Last reviewed commit: "fix(core): preserve reviewed state for r..." | Re-trigger Greptile
…resh PR #106 replaced the per-file upsert loop in RefreshChangedFiles with a transactional ReplaceChangedFiles (delete-all + batch-insert) to prune stale changed_files rows. Greptile flagged a P1 regression: that replace deletes the row for a reverted file (in the snapshot but no longer in the git diff) before filesRelativeToSnapshot re-adds it via UpsertChangedFile. With the row gone, the upsert takes the INSERT path and hard-coded Reviewed: false, so a file the user marked reviewed was silently un-reviewed within ~2s on the next refresh. The reviewed bit is lost from the DB by the time filesRelativeToSnapshot runs, so capture it earlier: SessionManager.RefreshChangedFiles now returns the reviewed map it reads from the DB *before* ReplaceChangedFiles deletes rows (this map still includes snapshot-only/reverted files). The engine threads that priorReviewed map into filesRelativeToSnapshot, which uses it instead of hard-coding false when re-adding reverted files. Adds a regression test asserting a reviewed reverted file stays reviewed both in-memory and in the DB after the refresh path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bug
In repos with many untracked files (e.g. a
.venvnot yet in.gitignore, thousands of files), Monocle freezes/slows and thechanged_filestable fills with thousands of rows. After adding the directory to.gitignorethe TUI recovers, but stale rows remain inchanged_files. Likewise, deleting untracked files leaves their rows behind.Root cause
SessionManager.RefreshChangedFiles(internal/core/session.go) runs every 2s. It computed the current changed set viagit.Diff()and UPSERTed every file (UpsertChangedFile— an additiveINSERT ... ON CONFLICT DO UPDATE), but never deleted rows whose paths were no longer in the current set.DeleteChangedFilesexisted but was only called from tests, so stale rows persisted forever.Secondary perf issue: each file was a separate auto-committed write every 2s (WAL, no batching) — a write storm under thousands of files.
Fix (sync + batched transaction)
Added
DB.ReplaceChangedFiles(sessionID, files)(internal/db/queries.go): within a single*sql.TxitDELETEs the session'schanged_filesrows then batch-INSERTs the current set (carrying each file's already-mergedReviewedstate), committing once and rolling back on error.RefreshChangedFilesnow reads existing reviewed state from the DB (as before), merges it onto the current set, and callsReplaceChangedFilesonce instead of looping per-file upserts. This:The engine's post-refresh snapshot path (
Engine.filesRelativeToSnapshotinengine_impl.go) runs afterRefreshChangedFilesreturns and re-adds reverted-but-in-snapshot files via its own upsert, so that behavior is preserved — the prune happens before, not inside, that path.Tests
internal/db/db_test.go—TestReplaceChangedFiles: seed 3 files (one reviewed), replace with a 2-file set → third row pruned, surviving reviewed flag preserved.internal/core/session_test.go:TestRefreshChangedFiles_PrunesStaleRows(gitStub): A,B,C changed → 3 rows; C leaves the set → C's row gone, A,B retain reviewed state.TestRefreshChangedFiles_GitignorePrune(setupTestRepo+ real git client): untracked file row appears, add to.gitignore, refresh → row removed.All of
go build ./...,go test ./internal/..., andgo vet ./...pass.Closes #100
🤖 Generated with Claude Code