perf(watch): Build dependency graphs off the event loop

[reneleonhardt]

What does this PR do?

• Builds post-start dependency graphs in one cancellable worker, keeping event handling responsive during saves and configuration changes.
• Captures configured-file, filter, and generation snapshots, then applies results only when the snapshot still matches.
• Coalesces invalidations and preserves a follow-up rebuild when an earlier build fails.
• Waits for the worker during shutdown before closing the filesystem watcher.

This keeps the watcher responsive without publishing stale or mismatched dependency graphs.

Type of change

• Bug fix
• New feature
• New language support
• Documentation
• Other

Checklist

• I've tested this locally with go build && ./codemap .
• I've run go test ./... and go test -race ./...
• I've verified with go vet ./...
• I've run the repository linter; it reports the existing baseline findings, with no new findings from this diff.

Additional notes

Initial dependency graph construction remains synchronous; only post-start rebuilds move off the event loop. No new commands, arguments, or MCP tools are added.

[JordanCoin]

Went through this one carefully — it's the worker you sketched in #168, and the design holds up. No blocking findings. One request before I merge, at the bottom.

What I verified

The published graph matches disk. Live daemon on a three-package Go module:

baseline (truth 2)                    importers=[main.go pkgb/b.go]  status=available
drop pkgb's import (truth 1)          importers=[main.go]            status=available
burst of 40 rapid edits (truth 1)     importers=[main.go]            status=available
new dir + file importing pkga (t=2)   importers=[main.go pkgc/c.go]  status=available

Converges every time, no panics or races in the daemon log. The 40-edit burst is the case I most expected to expose a lost rebuild, and it coalesces correctly.

The #168-class permanent-degradation guard is load-bearing. This was my main worry — that dependencyBusy could stick true and the daemon never rebuild again. Dropping the pending-retry in handleDependencyGraphResult:

--- FAIL: TestDependencyRefreshCoalescesInvalidations
    graph_state_test.go:249: pending invalidation did not schedule a second build

So the retry path is genuinely covered rather than incidentally working. TestDependencyRefreshRejectsResultAfterInvalidation similarly fails when the generation/fingerprint check is disabled, and TestDependencyRefreshBuildsOffEventLoop fails when the build is made synchronous again. All four new tests are meaningful.

Concurrency. refreshDependencies and handleDependencyGraphResult are reachable only from inside eventLoop()'s select — via the handleEvent closure, the two handleConfiguredMembershipEvent branches, the new dependencyResults case, and refreshConfiguredFiles off controlTimerC. No goroutine, no time.AfterFunc, no off-loop test helper. So the plain dependencyBusy/dependencyPending bools are safe. go test -race ./watch/... 4/4 clean, go vet clean, full suite at baseline.

Shutdown: because the worker never starts a build until its previous result has been both sent and consumed, the cap-1 dependencyResults buffer is provably empty at every send, so Stop() can't wedge on it — and dependencyCancel() fires before either Wait().

Two non-blocking notes

• The goroutine-ownership invariant that makes those two bools safe is real but undocumented. A comment on the fields (and on the two methods) saying they're event-loop-only would keep the next person from calling refreshDependencies from a goroutine — -race catches that misuse instantly, so today the only guard is the call graph.
• The worker has no recover() around buildFileGraph. Not a regression — main's synchronous call had the same exposure — but it's now an unattended background goroutine, so a panic takes the process down instead of failing closed. recover → markGraphLifecycle(graphLifecycleFailed) → log would match the rest of the file's posture.

For the record, Stop() called twice panics with close of closed channel — I checked and it does the same on b713d5f, so that's pre-existing and not yours.

The one thing I need

CI is red on Test (ubuntu-latest, 1.26) with watch/TestDebounce: Expected 1-2 debounced events, got 3. That's #135, and normally I'd wave it through — but this PR changes the event loop (the new dependencyResults case calls armTimer), so watch/ being the failing package is exactly where I shouldn't assume.

I did try to attribute it. Compiling test binaries for both branches up front and alternating them under CPU contention: 0/20 failures on main and 0/20 on this branch, then 0/25 each under heavier load. So I have no evidence this PR worsens it — but I also couldn't reproduce the flake at all locally, which means that's an absence of evidence, not a clean bill of health.

rerun-failed-jobs returns 403 for me and I can't update a fork branch, so I can't get a fresh run myself. Could you rebase onto current main and push? #169 just merged so you're a commit behind anyway, and it'll give this a clean CI signal. If TestDebounce fails again on that fresh run I'll treat it as real and dig into the armTimer interaction properly rather than calling it a flake twice.

Happy to fold the two notes above into that same push, or leave them for later — your call.

---

Generated by Claude Code

[reneleonhardt]

Complied

• Implemented the PR 169 routing review fixes.
• Added focused ordering and boundary regression coverage.

Not fully complied

• Ubuntu 1.26 CI failed in the existing readiness test; the implementation itself was not implicated.

Other changes

• The test helper writes readiness JSON with shell redirection, allowing an unexpected EOF race; production uses atomic rename.
• Reproduced locally 100 times without failure.

[JordanCoin]

Re-verified at 5f1d0a9. watch passed on the fresh run — that was the thing I was actually holding for, so the event-loop concern is closed.

The remaining red check is not this PR's, and I can prove it

Test (ubuntu-latest, 1.26) now fails on mcp/TestRustGraphContextHandlersDisclosePartialCoverage with Failed to build file graph: context deadline exceeded — a different test, in a package this diff doesn't touch.

The proof it's a flake rather than a code failure: #169's PR head 6f6e9ce and the squashed main commit 1c02105 have byte-identical trees (1d84a54b4c07088eed4bd1cce3da78d73835c5ab for both). The PR run failed this exact test on ubuntu-1.26; the main run on the same tree passed all 12 checks. Same code, same workflow, opposite outcomes.

And the mechanism, since it's now cost two PRs a red check — scanner/rustcargo.go:

const cargoMetadataTimeout = 3 * time.Second
...
ctx, cancel := context.WithTimeout(ctx, cargoMetadataTimeout)
output, err := cmd.Output()
if ctx.Err() != nil {
    return nil, ctx.Err()          // raw "context deadline exceeded"
}

That raw ctx.Err() is the literal string in the CI output, and it's the only place in the graph build that produces it — the ast-grep path reports ast-grep timed out after 30s instead. The test fixture is a Rust crate, so it shells out to cargo metadata, and 3s is tight for a cold runner: the two failures took 3.63s and 6.19s, consistent with the timeout firing once, then again on the fallback call at rustcargo.go:118. Worth a look separately — I'm not touching it here, and I haven't opened an issue for it.

On the extra changes in this push

The rebase also folded in both nits from my #169 review — dropping the redundant early duplicate check, and replacing the case-insensitive materialize-then-sort with a precomputed prefixPaths map. I did say your call, so no complaint, but it means this PR now carries context-routing changes under a watch-perf title, and I re-reviewed them rather than assuming.

They're correct. Output is identical to main on the same boundary fixture I used for #169 (scanner.go still isn't swept into the scanner/ prefix), and raw-path emission order still holds on the case-insensitive path:

[SCANNER/Beta.go Scanner/Deep/delta.go Scanner/Zeta.go
 scanner/alpha.go scanner/deep/Nu.go scanner/mu.go]   == sort.Strings(raw)

That works because the map is built from sortedPaths while it's still in raw-path order, before the key sort — worth a comment, because reordering those two blocks would silently break it and no test would obviously point at why.

One thing I'd flag: prefixPaths indexes every path under every ancestor prefix, eagerly, on every context call on case-insensitive platforms. Measured:

n=5000    keys=15168    entries=25000     heap +3 MB
n=20000   keys=55785    entries=100000    heap +10 MB
n=50000   keys=115785   entries=250000    heap +28 MB

The old code did two binary searches and materialized only matches, so this trades a lazy O(matches) for an eager O(files × depth). It's transient and Linux skips it entirely, so I'm not blocking — but on a 50k-file macOS monorepo that's 28 MB per editor-hook invocation, which is a worse deal than the per-prefix sort it replaced. Bounding it by topK, or building it lazily on first use, would keep the win without the eager cost.

Verification on 5f1d0a9

go vet clean. Full suite at baseline (only the three root-permission failures). go test -race ./watch/... 3/3 clean. Both CI-flaky tests pass locally (mcp 5/5, TestDebounce 10/10). Live daemon:

baseline (truth 2)                   importers=[main.go pkgb/b.go]  available
drop pkgb's import (truth 1)         importers=[main.go]            available
40-edit burst (truth 1)              importers=[main.go]            available
new dir + file (truth 2)             importers=[main.go pkgc/c.go]  available

No panics or races in the daemon log.

Merging. The worker came out clean, and thanks for the fast rebase — that's what settled the TestDebounce question.

---

Generated by Claude Code