Repository navigation
Fix dynamic ignored-directory watcher replanning and degraded status - #973
7 commits merged into
Conversation
code-review-graph reviewOverall risk: 0.55 (MEDIUM) — 31 changed function(s)/class(es), 0 affected flow(s), 12 test gap(s) Risk-scored changes
Test gaps
Token savings: this graph-backed report used ~43,019 fewer tokens (~89%) than reading every changed file in full (estimated, chars/4 approximation). Powered by code-review-graph — local-first analysis; no code leaves the CI runner. |
|
Hey @tirth8205 just wanted to add on one note on validation: I added the regression tests, but I wasn't able to run the full pytest suite locally because the current environment is missing The changes do pass Could you please run the full test suite in the project environment to verify the new watcher regression tests and the existing tests? |
Bakul2006
changed the title
|
Changes required: pending replans are never processed when the watcher has only a recursive root and no shallow watches. Run |
|
Hey @tirth8205 just did the same as you suggested me now the desired test are successfully passed |
|
008051b fixes the original recursive-root replan tests, but existing source coverage is still lost when replacement registration raises |
|
Hey @tirth8205 just addressed the failed replan coverage regression. |
|
This no longer merges into git fetch origin && git merge origin/staging
# resolve, then
uv run pytest tests/ -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
git pushWhen resolving:
A large integration branch landed on PRs now target |
|
This no longer merges into Worth knowing before you resolve:
git fetch origin
git merge origin/staging
# resolve, then
uv run pytest tests/ -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
git pushI have not reviewed the change itself yet. That comes once it merges and the checks run against the merged state, since staging has moved a long way and the result is what matters. |
|
Thank you for this PR. I tested your branch, and a copy with the conflict markers removed. Before I can merge it, please fix these:
Please update your branch with the latest staging. There are conflicts in After you push the fix, I will test it again. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved merge-conflict markers in incremental.py make it invalid Python, preventing watch mode and tests that import the module from running.
4 open issues
What changed in this PR
This PR addresses #908 by updating watcher plans as ignored directories appear or grow, and reporting startup budget fallback as degraded coverage.
Changes:
- Requests replanning on directory events and detects ignored-tree threshold crossings during synchronization.
- Adds replacement-watch registration and rollback handling to preserve coverage when replanning fails.
- Adds regression tests for dynamic ignored trees, failed replans, and startup health reporting.
| File | Description |
|---|---|
tests/test_watch_robustness.py |
Adds replanning, failure-recovery, and degraded-status regression tests. |
code_review_graph/incremental.py |
Adds dynamic watch replanning, rollback handling, and startup fallback status. |
Review effort: Balanced
| else: | ||
| self._shallow.add(key) | ||
|
|
||
| <<<<<<< HEAD |
| plan, fallback = _plan_watch_paths_with_status( | ||
| self._repo_root, | ||
| self._ignore_patterns, | ||
| max_schedules=self._max_schedules, | ||
| ) |
| f"rollback watch registration {key}", | ||
| timeout=_WATCH_STOP_TIMEOUT, | ||
| ) | ||
| return |
| if fallback: | ||
| self._degraded = True |
tirth8205
closed this pull request by merging all changes
into
tirth8205:staging
in
0160eb0
…ew of tirth8205#973) A failed replan now stays pending and retries after a cool-down. A successful one clears degraded status and any refusals it covers. When no plan fits the budget, the watches in place stay rather than a recursive root taking over. Removes the conflict markers and the unused _replace_watch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pull Request
Linked issue
Closes #908
What & why
Fixes watcher planning when ignored directories are created or grow after the initial watch plan is installed.
Previously, the watch plan was calculated at startup. If an ignored directory such as
node_modules/ortarget/did not exist at startup, its parent could receive a recursive watch. If the ignored directory was created later and grew beyond the existing split threshold, the watch plan was not reconsidered, allowing the ignored tree to fall under the recursive watch and potentially contribute to watcher/resource exhaustion.This PR:
degraded, socrg-daemon statuscorrectly reports degraded coverage.Regression tests were added for dynamically created/growing ignored directories and the startup degraded-status behavior.
How it was tested
The following validations were run successfully:
Language/static diagnostics reported no errors.
The full pytest/lint/type-check commands could not be run in the current environment because
pytest,uv, andnetworkxare unavailable.These commands should be run in a fully provisioned development environment before merging.
Checklist
uv run pytest tests/ --tb=short -quv run ruff check code_review_graph/uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optionaldocs/, docstrings)