Skip to content

feat: Log loop claim decisions: which candidate was picked and why others were skipped (#124) - #127

Merged
mabry1985 merged 1 commit into
mainfrom
feat/bd-1uj
Jul 25, 2026
Merged

feat: Log loop claim decisions: which candidate was picked and why others were skipped (#124)#127
mabry1985 merged 1 commit into
mainfrom
feat/bd-1uj

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Summary

The claim loop was silent about which Ready feature it picked and why it passed over others, leaving no way to tell a lost claim race from the hot-file guard from a ready_queue mis-ordering. _spawn_ready now emits exactly one structured claim_decision log.info per tick: a JSON payload with the selected fid(s) and, for each candidate skipped, the reason — hot-file (naming the in-flight fid and files it collides with), claim-race (claim() returned None), blocked, or state=<x>. The JSON is parseable without grepping log levels, so a future observer can trace exactly why a lower-priority card claimed ahead of a higher one. Two tests in tests/test_loop.py assert the line's content — that the selected fid and the passed-over candidate's skip reason (hot-file overlap and, separately, a claim race) are both recorded.

Fixes #124

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

QA panel review — PASS

code-review · head bca95629776b · formal

[review-synthesizer completed: workflow code-review:report]

Brief

Overall risk: Low. The only finding from the panel (a correctness concern about intra-tick file collisions in loop.py) was refuted by verification — the verifier confirmed that busy |= files is still executed at the end of each successful iteration, so the guard continues to refresh per iteration as intended. No defects survive the verify pass.

Fix-first item: None — the finding was refuted, not retained.

Panel disagreement: The panel flagged a correctness issue with busy not being updated within the loop, but the verifier demonstrated that the busy |= files line at the end of each iteration (after selected.append(...)) does refresh busy before the next candidate is processed. The panel missed this line in their evidence review.

Verification changes: The verifier marked the sole finding as refuted with a clear note showing the diff evidence (busy |= files at line ~877, after selected.append(claimed['id'])) that contradicts the panel's claim that busy is never refreshed. No uncertain findings remain.

Coverage note: This was the only finding submitted by the panel. With no other findings to cross-check, I cannot confirm whether other heavily-changed files were under-read — the panel's report was singular in scope.

[]

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Promoting the PASS verdict for head bca95629776b: all checks terminal-green, zero unresolved review threads. (approve-on-green)

@mabry1985

Copy link
Copy Markdown
Member Author

Merge verdict — protoEngineer adjudication

Verdict: MERGE. squash, delete branch.

CI: All SUCCESS on the current head. Operator independently verified the merged state against new main (which carries #126's loop.py sweep change — two in-flight cards editing the same file, clean rebase): ruff clean, ruff format clean, 572 tests pass, no conflict.

Review gate: NOT RUN. This board's review_gate is false (advisory).

The tests assert content, not existence. The hot-file skip carries overlaps (which in-flight feature owns the file) and files (the actual overlap), not just a label — the difference between "it was skipped" and "it was skipped because bd-xyz is editing store.py." A "log line exists" assertion would pass forever while the reason silently went empty; these assert the reason field is populated.

Manual merge — no --auto in this repo.

@mabry1985
mabry1985 merged commit 16670ee into main Jul 25, 2026
1 check passed
@mabry1985
mabry1985 deleted the feat/bd-1uj branch July 25, 2026 19:50
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.

Loop never logs its claim decision — priority inversions are undiagnosable

1 participant