feat(loop): hot-file overlap guard + review-queue WIP limit#10
Merged
Conversation
Now that max_concurrent>1 runs features in parallel, two new back-pressure gates in _spawn_ready keep concurrency correct: - Hot-file guard: skip a ready candidate whose files_to_modify overlap an in-flight build's files — two parallel coders editing the same file are a guaranteed merge conflict. _spawn_ready now peeks the ready_queue (priority order) and claims a specific non-conflicting feature via the new store.claim(fid) instead of always taking the top; the files each drive holds are tracked in self._inflight_files and released by the drive's done-callback (so a deferred candidate can claim next). - Review-queue WIP limit (max_pending_reviews, default 5): pause claiming new work while that many PRs already await review, so the loop can't pile up PRs faster than they merge (flooding CI / reviewers). 0 = unlimited. store.claim(fid) atomically claims a chosen ready feature (vs claim_next_ready's top-of-queue), returning None if it changed state or lost the `br --claim` race. First of the P1 concurrency-correctness ports. Tests: +6 (104 total) — store.claim (specific / not-ready / claim-race); _spawn_ready caps at max_concurrent, defers a file-conflicting candidate, respects the review WIP limit, and releases files when a drive finishes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #10 | feat(loop): hot-file overlap guard + review-queue WIP limit
VERDICT: WARN ⏳ (CI queued — non-terminal)
CI Status
- test: queued
Diff Review
loop.py:_spawn_readyrewritten with hot-file guard (skip file-overlapping candidates viabusyset +store.claim(fid)) and review-queue WIP limit (max_pending_reviews, default 5)store.py: newclaim(fid)— atomic specific-feature claim viabr --claim, returnsNoneon race/state changeprotoagent.plugin.yaml: config docs for both new gatestests/test_loop.py:_ClaimStorerefactored toready_queue()/claim()/list_features()API; new helpers_ready()and_hold_drives()for test ergonomics
Observations
- LOW:
busyset computed once per_spawn_readycall; drives finishing mid-iteration don't update it until next call — conservative but safe (may defer a candidate unnecessarily, never allows a conflict) - GAP:
ready_queue()andlist_features()used in_spawn_readybut not visible in thestore.pydiff — unverified they exist on the productionBoardStore. Tests mock via_ClaimStoreso wouldn't catch a missing method. Likely pre-existing but worth confirming. - GAP: clawpatch structural review skipped — repo not in project registry. Cross-file analysis (call chain from
_spawn_ready→ store →brCLI) couldn't run. - Clean: no unresolved CodeRabbit threads, tests claim +6 all green,
ruffclean
— Quinn, QA Engineer
|
Submitted COMMENT review on |
This was referenced Jun 13, 2026
mabry1985
added a commit
that referenced
this pull request
Jun 13, 2026
…w) (#12) By default a dependent waits for every blocks-dependency to MERGE (done) — `br ready` excludes a feature with any open blocker, so this falls out for free and stays the safe default (dep_gate: merge). The new opt-in dep_gate: review relaxes it: a NON-foundation blocker releases its dependents once it reaches in_review (build on code that's in review, not yet merged — more parallelism). Foundation features (foundation=True) ALWAYS gate dependents on merge. - store.ready_queue(relaxed=False): the relaxed branch adds dep-blocked ready features whose every still-open blocker is a non-foundation feature at in_review, resolved in Python from `br list` labels (board_state + foundation) + `br show` deps (`_open_blockers`), since `br ready` only knows "blocker closed". - create_feature(foundation=…) + the `foundation` label + _project exposure; the board_create_feature tool + the decompose skill gain the flag (the skill already marked milestones foundation in frontmatter — now it's wired to the gate). - loop: dep_gate config → relaxed_gate → ready_queue(relaxed=…). Verified against br 0.1.23: `br list --json` carries labels; `br show` deps carry status; `br ready` waits for blockers to close. Default behaviour is unchanged. Completes the P1 concurrency-correctness batch (hot-file guard #10, health sweep #11, this). Tests: +6 (116 total) — _project foundation, create_feature labels foundation, _open_blockers filtering, ready_queue(relaxed) releases only non-foundation in_review blockers, dep_gate config, _spawn_ready passes the gate through. Co-authored-by: Claude Fable 5 <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
1 participant
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
The first P1 concurrency-correctness port — directly hardens the
max_concurrent>1parallel drives from #6. Two back-pressure gates in_spawn_ready:files_to_modifyoverlap an in-flight build's files. Two parallel coders editing the same file are a guaranteed merge conflict._spawn_readynow peeksready_queue()(priority order) and claims a specific non-conflicting feature via the newstore.claim(fid)instead of always taking the top; each drive's files are tracked inself._inflight_filesand released by its done-callback (so a deferred candidate can claim next).max_pending_reviews, default 5) — pause claiming new work while that many PRs already await review, so the loop can't pile up PRs faster than they merge (flooding CI / reviewers).0= unlimited.store.claim(fid)atomically claims a chosen ready feature (vsclaim_next_ready's top-of-queue), returningNoneif it changed state or lost thebr --claimrace.Tests
+6 (104 total), all green:
store.claim(specific / not-ready / claim-race);_spawn_readycaps atmax_concurrent, defers a file-conflicting candidate, respects the review WIP limit, and releases files when a drive finishes.ruffclean.P1 batch
🤖 Generated with Claude Code