Skip to content

fix(watch): preserve final rapid-write state#78

Merged
JordanCoin merged 1 commit into
JordanCoin:mainfrom
reneleonhardt:fix/watch-debounce
Jul 22, 2026
Merged

fix(watch): preserve final rapid-write state#78
JordanCoin merged 1 commit into
JordanCoin:mainfrom
reneleonhardt:fix/watch-debounce

Conversation

@reneleonhardt

@reneleonhardt reneleonhardt commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Preserve the final state from rapid successive file writes.
  • Skip a debounced write only when the cached file size already matches the file on disk.
  • Refresh stale graph state instead of treating every write inside the debounce window as a duplicate.
  • Strengthen the daemon test to require the final source-file event and a positive delta.

Type of change

  • Bug fix
  • New feature
  • New language support
  • Documentation
  • Other (describe below)

Checklist

  • I've tested this locally with go build && ./codemap .
  • I've read CONTRIBUTING.md (for new language support)
  • I've updated documentation if needed

Additional notes

Filesystem watchers can emit multiple write notifications for one logical save. Time-based debouncing alone could discard the final notification while the cached graph still described an earlier file state. The debounce decision now checks the current file size against the cached state before skipping a rapid write.

Focused debounce/daemon tests pass with the full race suite, formatting,
module verification, vet, staticcheck, and a debug build/smoke run.

Reviewer setup:

set -euo pipefail

ROOT=$PWD
BRANCH=fix/watch-debounce
REVIEW_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/codemap-watch-debounce.XXXXXX")
WT="$REVIEW_ROOT/worktree"
DEBUG_BIN="$REVIEW_ROOT/codemap-debug"
COVERAGE_OUT="$REVIEW_ROOT/coverage.out"

git worktree add --detach "$WT" "$BRANCH"

cleanup() {
  cd "$ROOT"
  git worktree remove --force "$WT"
  rm -rf "$REVIEW_ROOT"
}
trap cleanup EXIT

cd "$WT"
go fmt ./...
git diff --exit-code
go mod verify
go vet ./...
staticcheck ./...
go test ./watch -run 'TestDaemonDoesNotDebounceWriteWhenCachedSizeIsStale|TestEventDebouncerSkipsRapidWrites|TestDaemonStartTracksWriteEventsAndState' -count=1
go test -race -coverprofile="$COVERAGE_OUT" ./...
go build -gcflags='all=-N -l' -o "$DEBUG_BIN" .
"$DEBUG_BIN" .
cleanup
trap - EXIT

Developed with carefully directed, manually reviewed AI assistance. Allow edits by maintainers is enabled, so maintainers are welcome to fix wording or make minor cleanup directly; broader review feedback will be incorporated.

Co-Authored-By: GPT-5.6 Sol codex@openai.com

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
watch/events.go 73.33% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@JordanCoin JordanCoin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — confirmed the bug is real: the debouncer is leading-edge only with no trailing flush, so the final write of a rapid burst could be dropped while the cached graph state still described an earlier file state. The fix is correct, locking is right (RLock on graph.mu, single-goroutine event loop), error paths fail open in the safe direction, and it composes correctly with the temp-file churn filtering since isTransientFile runs before shouldSkipEvent. Full suite passes with -race, and the new test genuinely exercises all three branches of the new logic.

Two non-blocking thoughts for a follow-up (not asking for changes here):

  1. Same-size content changes are still droppedFileState only stores Lines+Size, so a burst whose final write leaves size unchanged (e.g. s/foo/bar/) still loses its last event. Not a regression (main drops it too), but storing ModTime in FileState and comparing it would close the hole cheaply.
  2. This effectively turns "skip writes within 100ms" into "skip writes within 100ms whose size hasn't changed" — a file being written in chunks now runs full handleEvent per event (countLines + a git diff subprocess + writeState), which is the burst load the debouncer exists to absorb. A trailing-edge flush timer would preserve both coalescing and final-state accuracy if this ever shows up in profiles.

Minor: checking the State cache before os.Stat would save a syscall for untracked/gitignored paths.

@JordanCoin
JordanCoin merged commit afdd593 into JordanCoin:main Jul 22, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants