Skip to content

fix(watcher): pace re-indexes by the cost of the last successful run - #6

Open
halindrome wants to merge 1 commit into
mainfrom
fix/watcher-reindex-cooldown
Open

halindrome wants to merge 1 commit into
mainfrom
fix/watcher-reindex-cooldown

Conversation

@halindrome

Copy link
Copy Markdown
Owner

Summary

Pace watcher re-indexes by how long the last successful one took, so a large project whose full rebuild takes minutes no longer re-indexes back-to-back while an agent session keeps writing files.

Upstream issue: DeusData#2835

Today the next poll after a successful re-index is the normal interval (5 s + 1 s per 500 files, max 60 s). Any write changes the dirty signature, and a new file forces a full rebuild, so on a large monorepo indexing becomes nearly continuous. It competes for CPU with interactive queries the whole time.

Change

  • cbm_watcher_success_delay_ms(interval_ms, index_ms) (src/watcher/watcher.c/.h) is a pure helper that returns max(interval, 3 × index duration), capped at 30 minutes. It never returns less than the interval, treats negative inputs as zero, and divides instead of multiplying so it cannot overflow.
  • poll_project times the index_fn call with now_ns() and, after a success, schedules the next poll at now + success_delay. It logs watcher.cooldown (project, delay_ms) only when the delay is extended.
  • Failure backoff and busy-skip are unchanged. Sub-second indexes keep today's cadence.
  • Reviewer decision: the cap is a new INDEX_COOLDOWN_CEILING_MS (30 min), not the existing INDEX_FAIL_CEILING_MS (5 min). The delay counts from when the index starts, so with a 5-minute cap any rebuild longer than about 100 s would restart right after it finishes. That is the behaviour being fixed. The cap bounds how stale an index can get after one very slow run.

Acceptance criteria

  1. After a successful watcher re-index, the next poll of that project waits at least 3 × index duration, and never less than the normal interval.
  2. The delay is capped, so one very slow run cannot stop the watcher for an unbounded time.
  3. Small or fast repos keep their current poll cadence.
  4. Failure backoff and busy-skip behaviour are unchanged.
  5. The delay computation cannot overflow and handles degenerate inputs (zero or negative).
  6. Existing watcher and daemon suites stay green.

Testing

  • tests/test_watcher.c: four unit tests for the helper (fast index, scaling, clamp/degenerate, monotonic) and one behavioural test, watcher_success_cooldown_measures_index_issue2835. The behavioural test uses a real git repo and a log sink. An instant index produces no cooldown line. A 1.8 s index produces exactly one, because 3 × 1800 ms exceeds the 5000 ms interval.
  • Red-first check: with the helper stubbed to return the plain interval, 3 tests fail. With the real code, all 96 watcher tests pass.
  • scripts/test.sh --suites watcher,daemon,daemon_runtime,daemon_application,daemon_frontend,daemon_bootstrap,daemon_ipc,httpd,mcp: 841 passed, 0 failed, 7 skipped.
  • Not covered: the exact scheduled poll time. The watcher has no injectable clock, so the unit tests cover the size of the delay and the behavioural test covers the wiring.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CDDUoWEZMNZ9fLvzp2pPGT

@halindrome

halindrome commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner Author

QA Round 1

Contract Verification

Contract: DeusData#2835 + PR acceptance criteria (cost-proportional cooldown only).

# Criterion Verdict Evidence
1 After a successful re-index, the next poll waits at least 3x the index duration, and never less than the poll interval ⚠️ PARTIAL cbm_watcher_success_delay_ms (watcher.c:217-229) correctly returns max(interval, 3×elapsed). But poll_project adds the delay to ctx->now (watcher.c:1761), which is the cycle timestamp taken once in cbm_watcher_poll_once (watcher.c:1811), not to the time this index started. See Finding 1.
2 The delay is capped at 30 min ✅ PASS INDEX_COOLDOWN_CEILING_MS 1800000. success_delay_clamped_and_degenerate checks 600000, 600001 and INT64_MAX, and all three give 1800000.
3 Small/fast repos keep their poll cadence ✅ PASS When 3×elapsed <= interval the function returns interval. Tested at the 1666/1667 boundary, and the wire-up test asserts fast_hits == 0.
4 Failure backoff and busy-skip behaviour are unchanged ✅ PASS delay_ms stays at -1 for rc>0, rc<0 and a missing index_fn, so those cases fall through to the unchanged cbm_watcher_index_backoff_ms(...).
5 The delay computation cannot overflow and handles degenerate inputs ✅ PASS The ceiling/factor comparison runs before the multiply, a negative interval clamps to 0, and elapsed <= 0 returns the interval. Tested with INT64_MAX, INT64_MIN, -1, 0 and INT_MAX.
6 Existing suites stay green, and the new tests are red without the change ⚠️ PARTIAL / not verified The suites were not run in this round (the lenses are read-only). Without the change the pure-function tests do not link. The wire-up test stays green if only the next_poll_ns scheduling hunk is reverted. See Finding 2.
7 Trust boundary: no weakening of authn/authz, input validation or secret handling ✅ PASS The diff changes scheduling arithmetic and adds one info log line (project name plus an integer). It adds no new input surface.

Finding 1 — [claude] Cooldown is anchored to the stale poll-cycle timestamp, so a project indexed after another in the same cycle can be rebuilt back-to-back

  • Severity: major · Status: confirmed · Relevance: contract (AC 1) · Category: logic-error
  • Location: src/watcher/watcher.c:1722-1761 (and :1811)

poll_project measures the index from index_start = now_ns() (L1722) and computes delay_ms = 3 × elapsed (L1729). It then sets s->next_poll_ns = ctx->now + delay_ms (L1761). ctx->now is taken once per cycle in cbm_watcher_poll_once (L1811), before any project is checked or indexed. Projects are polled one after another, so time spent earlier in the cycle comes out of the cooldown. That includes git checks and the index runs of projects polled earlier. The idle time left after the index finishes is roughly 2×elapsed minus that earlier time, and it can reach zero.

  • Two watched projects A and B, both changed. A indexes for 10 min and then B for 5 min. B's next_poll is T0 + 15 min, which is exactly when B's index finishes, so B is due again on the next cycle.
  • A single project whose index takes 30 min or more hits the cap. Its next_poll is T0 + 30 min, which is at or before the end of its own index, so it can rebuild back-to-back. That contradicts the comment that the ceiling prevents this.

Expected: after a successful index, the next poll is at least index_start + max(interval, 3×elapsed), however long other projects took earlier in the cycle. On the success path, use index_start (or the index-end time) as the base instead of ctx->now. In the common case of one project indexing in under 10 min the effect is small, but this is the scenario DeusData#2835 describes.

Finding 2 — [claude] Wire-up test checks the cooldown log line, not that the next poll is actually deferred

  • Severity: minor · Status: confirmed · Relevance: regression · Category: test-gap
  • Location: tests/test_watcher.c:2326-2378

watcher_success_cooldown_measures_index_issue2835 calls cbm_watcher_touch before every poll, and touch resets next_poll_ns = 0, which bypasses the gate this change sets. The only assertion is cooldown_log_hits == 1, and that line is logged before next_poll_ns is assigned. If the final scheduling hunk were reverted to the plain backoff, the test would still pass. A 1000× unit error in the elapsed conversion would also pass. Compare watcher_index_failure_backoff_gates_repolling_issue2015, which runs a poll without touch and checks that the gate holds. The production code is correct today, so this is a mutation gap rather than a defect. It is acceptable to defer it to a follow-up.

Summary

Severity Confirmed
Critical 0
Major 1
Minor 1

Panel: contract-security, regression-edges and test-quality all landed on claude-opus-5-5 with navigation cmm+ctx. The working tree was unchanged after the panel. No schema change.

Observations

None.


SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.


QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

  • Fixed (major): the cooldown was anchored to the poll cycle's start time (ctx->now), so time spent earlier in the cycle, or an index longer than the 30-minute cap, could cancel it. After a successful index the next poll is now index end + max(interval, 3 × duration). Failure and busy-skip paths still anchor to ctx->now, as before. Comments updated: indexing now uses at most about 1/(K+1) of wall time.
  • Not fixed (carried forward, minor): the wiring test asserts the watcher.cooldown log line, not next_poll_ns. The watcher has no injectable clock or next-poll accessor, so asserting the schedule needs new test infrastructure, which is bigger than the fix. Consequently, this round's fix has no red-first test; it was verified by reading only.

Verification

CC=/usr/bin/clang CXX=/usr/bin/clang++ SDKROOT=… scripts/test.sh --suites "watcher daemon daemon_runtime daemon_application" → 248 passed, 0 failed (exit 0). This is narrower than CI's default leg; the local default scripts/test.sh stops at contract Step 0t, which unmodified main also fails in this environment.

QA-Fix-Commit: a83e75b


Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

@halindrome

Copy link
Copy Markdown
Owner Author

QA Round 2

Contract Verification

Contract: DeusData#2835 + PR acceptance criteria (cost-proportional cooldown only). Reviewed at the head that includes round 1's fix a83e75b7.

# Criterion Verdict Evidence
1 After a successful re-index, the next poll waits at least 3x the index duration, and never less than the poll interval ✅ PASS poll_project now sets anchor_ns = now_ns() after index_fn and next_poll_ns = anchor_ns + cbm_watcher_success_delay_ms(interval, elapsed) × US_PER_MS. The function returns max(interval, 3×elapsed). Round 1's Finding 1 (stale ctx->now anchor) is resolved.
2 The delay is capped at 30 min ✅ PASS INDEX_COOLDOWN_CEILING_MS 1800000 (watcher.c:165). success_delay_clamped_and_degenerate checks 600000, 600001 and INT64_MAX, and all three give 1800000.
3 Small/fast repos keep their poll cadence ✅ PASS When 3×elapsed <= interval the function returns interval (success_delay_fast_index_keeps_interval). The wire-up test asserts that an instant index logs no cooldown.
4 Failure backoff and busy-skip behaviour are unchanged ✅ PASS On rc>0, rc<0 or a missing index_fn, delay_ms stays at -1 and anchor_ns stays at ctx->now, so those paths compute the same ctx->now + cbm_watcher_index_backoff_ms(...) expression as the base branch.
5 The delay computation cannot overflow and handles degenerate inputs ✅ PASS The ceiling/factor comparison runs before the multiply, a negative interval clamps to 0, and elapsed <= 0 returns the interval. Tested with INT64_MAX, INT64_MIN, -1, (-1,0) and INT_MAX.
6 Existing suites stay green, and the new tests are red without the change ⚠️ PARTIAL The lenses are read-only, so they did not run the suites. The orchestrator ran scripts/test.sh --suites "watcher daemon daemon_runtime daemon_application": 248 passed, 0 failed. Without the change the new tests do not link. Round 1's anchor fix has no red-first test; that is the deferred item recorded under Observations.

Finding 1 — [claude] Header doc and test comment say the cooldown is measured from the index start; the code measures from the index finish

  • Severity: minor · Status: confirmed · Relevance: regression · Category: documentation
  • Location: src/watcher/watcher.h:198-204, plus the comment in tests/test_watcher.c success_delay_slow_index_scales_by_factor
  • Lenses: contract-security, regression-edges and test-quality (3 of 3 raised it)

Round 1's fix (a83e75b7) moved the cooldown anchor to the index finish and updated the .c comment at watcher.c:157-163, which now reads "counted from when the index finished … ~1/(K+1) of wall time". Two comments were not updated:

  • The public doc comment for cbm_watcher_success_delay_ms still says "Measured from the index start, so the watcher spends at most ~1/3 of wall time indexing a project".
  • The test comment still says "5-minute rebuild → next poll 15 min after its start, ~10 min idle".

The header now contradicts both the code and the .c comment.

Expected: both comments say the delay counts from the index finish: indexing takes about 1/(K+1) ≈ 1/4 of wall time, and a 5-minute rebuild idles for 15 minutes. There is no runtime effect, but a maintainer who trusts the header could move the anchor back, and no test would catch it (see Observations). The fix is a two-comment edit.

Summary

Severity Confirmed
Critical 0
Major 0
Minor 1

Panel: contract-security, regression-edges and test-quality all ran on claude-opus-5-5 with navigation cmm+ctx. The working tree was unchanged after the panel. No schema change. Attribution: the finding is on lines round 1's fix commit did not write, so qa_introduced=false. It is a comment that round 1 left stale.

Observations

  • [claude] The cooldown's scheduling (next_poll_ns) and round 1's finish-time anchor are not pinned by any test (minor, tests/test_watcher.c:2328-2378). This is round 1's deferred item, raised again by test-quality. The extra point this round: a83e75b7 changed only src/watcher/watcher.c, so reverting that anchor would also pass the suite. The finding stays deferred because the watcher has no injectable clock or next-poll accessor.

SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.


QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

  • Fixed (minor, documentation): the watcher.h doc for cbm_watcher_success_delay_ms and a test comment still described the round-1 behaviour (counted from the index start, about 1/3 duty cycle). Both now say the cooldown counts from the index finish (about 1/4 duty cycle; a 5-minute rebuild is followed by 15 minutes idle). Comment-only; clang-format --dry-run --Werror is clean. No test changed, so no red-first run applies.
  • Fix review (Step 3B.5): clean, reviewed_sha 40966a20237f75d1cd69371578589c304e34d81b. Q1: the diff does what the finding asked. Q2: no sibling comment in watcher.c, watcher.h or test_watcher.c still asserts the old anchor.
  • Carried forward: next_poll_ns and the finish-time anchor are not pinned by any test (no injectable clock). This is the same item as round 1.

Cycle outcome: no critical or major findings remain after round 2. The round-1 major (cooldown anchor) is fixed and verified. Not approved: no QA-agent token is available, and approval is the human reviewer's call.

QA-Fix-Commit: 40966a2


Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

After a successful re-index the next poll waits max(interval, 3 x index
duration), capped at 30 min, counted from when the index finished. A
project whose full rebuild takes minutes no longer re-indexes
back-to-back under continuous writes; indexing spends at most ~1/4 of
wall time. Sub-second indexes keep the existing cadence; failure backoff
and busy-skip are unchanged. Logs watcher.cooldown when the delay is
extended.

Refs DeusData#2835

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>

This branch has not been deployed

No deployments
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.

2 participants