fix(watch): Rebuild the dependency graph after a filter change - #101
Merged
Conversation
refreshConfiguredFiles invalidated the dependency graph and left it that way, with a comment deferring the rebuild to "restart". Nothing rebuilds it lazily, so a single edit to .codemap/config.json — or a touch of any .gitignore, which filterControlEvent matches by basename anywhere in the tree — permanently stripped hub and importer intelligence from the running daemon. Every hook that reads daemon state silently degraded from that point on. Rebuild after invalidating, behind the same shouldComputeDependencyGraph guard Start uses so large repos still skip the work. computeDeps takes the graph lock itself, so it is called after the unlock. TestConfiguredFilterChangeInvalidatesDependencyState asserted the graph stayed nil, which encoded the defect rather than the intent. Its real invariant is that state computed under the old filters is discarded, so it now asserts the stale entries are gone; the published-state check is unchanged. A new test covers the rebuild, using a sentinel entry so a rebuilt graph is distinguishable from the startup graph. Follow-up to #95 by @reneleonhardt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8 tasks
There was a problem hiding this comment.
Pull request overview
Fixes a watch-daemon bug where changing configured filters (via .codemap/config.json or .gitignore) invalidated dependency state but never rebuilt it, causing hub/importer intelligence to silently degrade until restart.
Changes:
- Rebuild the dependency graph after
refreshConfiguredFilesinvalidates it, guarded byshouldComputeDependencyGraph. - Update
TestConfiguredFilterChangeInvalidatesDependencyStateto assert stale entries are removed (instead of pinningHasDeps == false/FileGraph == nil). - Add
TestConfiguredFilterChangeRebuildsDependencyGraphto ensure invalidation is followed by a rebuild.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
watch/daemon.go |
Recomputes dependency state after filter/config changes (under the same size guard used at startup). |
watch/more_test.go |
Adjusts invalidation assertions and adds a new regression test covering dependency rebuild. |
Suppressed comments (1)
watch/more_test.go:468
- This test relies on the dependency graph being rebuildable (it asserts
HasDepsbecomes true andFileGraphbecomes non-nil). In this repo, other tests that depend on dependency analysis skip when ast-grep isn't available; without a similar guard, this can fail in environments where ast-grep isn't installed/bundled.
func TestConfiguredFilterChangeRebuildsDependencyGraph(t *testing.T) {
root := t.TempDir()
if err := os.MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+315
to
+319
| if d.graph.FileGraph == nil { | ||
| return true | ||
| } | ||
| _, stale := d.graph.FileGraph.Importers["old.go"] | ||
| return !stale |
Comment on lines
+238
to
+244
| // Invalidation alone would leave the daemon serving no hub or importer | ||
| // intelligence until it restarts, so every hook reading daemon state would | ||
| // silently degrade after one config edit. Rebuild under the same size guard | ||
| // Start uses. computeDeps takes the lock itself, so call it unlocked. | ||
| if shouldComputeDependencyGraph(len(configured)) { | ||
| d.computeDeps() | ||
| } |
8 tasks
JordanCoin
added a commit
that referenced
this pull request
Aug 3, 2026
Addresses both Copilot findings on #101. Control events bypassed the debouncer: the event loop handled them and continued before takeDueBeforeEvent ran. That was cheap when the handler only re-walked the tree, but #101 made it rebuild the dependency graph, which runs ast-grep over the whole repo. fsnotify emits several events per save, so one config.json write triggered three full rebuilds back-to-back, serialized in the event loop. A test over a five-write burst reproduced exactly that count. Control events now get their own trailing-edge debounce, separate from the file-event debouncer so neither perturbs the other. The resetIgnoreCache flag is OR-ed across the burst, so a coalesced refresh still resets the ignore cache when any event in it was a .gitignore. The two filter-change tests waited on in-memory graph fields. The refresh nils the graph before rebuilding, so those conditions could be satisfied mid-refresh, before writeState ran, leaving the ReadState assertions racing the daemon. They now wait for the published state to advance past a captured baseline, which is the observable end of a refresh cycle. Verified with -count=8 and -race. Co-authored-by: Claude Opus 5 (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.
What does this PR do?
Follow-up to #95 by @reneleonhardt, landing the one review point from that PR that was still open. Sent as a separate PR rather than a push to his branch so his work merges intact under his own authorship.
refreshConfiguredFilesinvalidated the dependency graph and left it that way:Nothing rebuilds it lazily. So a single edit to
.codemap/config.json— or a touch of any.gitignore, sincefilterControlEventmatches that by basename anywhere in the tree — permanently stripped hub and importer intelligence from the running daemon. Every hook that reads daemon state silently degraded from that point until the daemon was restarted.The invalidation itself is correct: filters define dependency membership, so the old graph must not be published under a new configured-file count. The missing half is rebuilding afterward, behind the same
shouldComputeDependencyGraphguardStartalready uses so large repos still skip the work.computeDepstakes the graph lock itself, so it is called after the unlock.Type of change
Checklist
go build && ./codemap .CONTRIBUTING.md; this does not add a new language.A note on the modified test
TestConfiguredFilterChangeInvalidatesDependencyStateasserted!HasDeps && FileGraph == nil && len(DepCtx) == 0— it pinned the defect rather than the intent. Its real invariant is that state computed under the old filters is discarded, which this change preserves, so it now asserts the staleold.goentries are gone. The published-state assertion (state.Hubs/Imports/Importersall empty) is untouched.The new
TestConfiguredFilterChangeRebuildsDependencyGraphcovers the rebuild. It plants a sentinel entry first, because asserting onlyHasDeps && FileGraph != nilpasses vacuously — the daemon already satisfies that from startup, before any refresh runs. The sentinel makes a rebuilt graph distinguishable from both the startup graph and a dropped one.Verified with
go test ./...,go vet,staticcheck,gofmt,go test ./watch/ -race, andgo vetcross-compiled for windows/amd64 and linux/amd64.