Skip to content

fix(dev): machine-recognize review-claim and author_decision states (#7508) - #7519

Merged
ll7 merged 1 commit into
mainfrom
fix/issue-7508-review-claim-20260818
Aug 18, 2026
Merged

fix(dev): machine-recognize review-claim and author_decision states (#7508)#7519
ll7 merged 1 commit into
mainfrom
fix/issue-7508-review-claim-20260818

Conversation

@ll7

@ll7 ll7 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Summary

Make scripts/dev/pr_loop_policy.py machine-recognize the two workflow markers that PR #7500 introduced to goal-pr-review (docs-only): the advisory review-claim: <lane> @ <head> until <UTC> comment marker and the author_decision parking state (decision-required label + ### Decision packet comment at the live head). PRs parked by either are classified active_writer / author_decision with recommended action no_action/stop, distinct from blocked_preflight, so reviewer lanes and the factory stop racing on the same branch.

Linked Issues

  • Closes #7508
  • Refs #7500 (defines the marker formats), #7491 (not-ready-sentinel half, intentionally NOT implemented here)

Stack / Dependency

What Changed

  • scripts/dev/pr_loop_policy.py:
    • VALID_STATES gains active_writer and author_decision.
    • New marker parsing: _REVIEW_CLAIM_RE (review-claim: <lane> @ <sha> until <UTC>), _REVIEW_CLAIM_RELEASED_RE (review-claim: released @ <sha>), _DECISION_PACKET_HEADING_RE (### Decision packet), plus a ReviewClaim dataclass.
    • New pure helpers: _parse_review_claim_marker, _review_claim_released_shas, _has_decision_packet_heading, _trusted_marker_comments (OWNER/MEMBER/COLLABORATOR only, reading both comments and reviews bodies), active_review_claim (unexpired + unreleased + head-bound, now=None -> datetime.now(UTC)), has_author_decision_packet (live-head binding; review-body packets must name the live head).
    • classify_pr_state takes an explicit now kwarg; draft/error/closed still return no_action first; active_writer/author_decision are checked before blocked_preflight and the rest of the pipeline (precedence per issue).
    • recommend_action maps both new states to no_action with named reasons; flow decision is stop for both.
    • Module docstring documents the lane-identity limitation (snapshot cannot distinguish lanes, so any unexpired unreleased trusted marker parks) and links friction(merge-gate): merge-ready + stale 'not merge-ready' body narrative reaches main (#7374, #7435) #7491 as explicitly out of scope — no not-ready-sentinel implementation was added.
  • tests/dev/test_pr_loop_policy.py: 24 new tests with fixed timestamps (no monkeypatching): park on unexpired trusted claim, untrusted does not park, released clears, expiry, now == until is expiry, head mismatch, unparseable timestamp fails closed, claim in review body, parser round-trips, author_decision with/without label and packet, live vs stale head, untrusted packet, precedence over blocked_preflight and over ready-to-merge, draft still no_action, VALID_STATES contract, queue-level evaluation.
  • CHANGELOG.md: Unreleased -> Changed entry.

Why It Matters

Research Result Guidance

  • Target claim / hypothesis / blocker this should affect: NA - support/tooling-only; no research claim.
  • Comparator or baseline, if applicable: NA
  • Evidence tier: NA - support helper.
  • Result classification: NA
  • Decision or stop rule, if applicable: NA
  • Parent issue, claim map, registry, context note, or synthesis surface to update: NA
  • New research/benchmark/metric/paper-facing analysis tool, if any: NA - support helper.

Domain-Aware Approval

  • Required for this PR: no - workflow/tooling classification change; no evidence classification, comparison methodology, figure eligibility, benchmark interpretation, or paper-facing claim change.
  • Domains reviewed: NA
  • Status: not required
  • Approver/review source or waiver: NA
  • Validity checklist (machine-detected labels unchanged):
    • Target claim/hypothesis: NA
    • Comparator or split/evidence validity: NA
    • Fallback/degraded exclusions: NA
    • Claim boundary: NA
    • Implementation integrity vs experimental validity: implementation integrity only.

Falsification / Non-Transfer Check

  • Did the mechanism activate? NA
  • Did the intervention change command source, selected command, trajectory, or route progress? NA
  • Did the scenario actually contain the targeted failure mode? NA
  • Result route: NA
  • Follow-up question or issue for weak, negative, or non-transfer results: NA

Next Empirical Action

  • Rerun needed: no
  • Extractor or analysis tool needed: no
  • Artifact missing or unavailable: none
  • Stop / revise / continue decision: NA
  • Proposed child issue or existing follow-up: none

Validation / Proof

  • Commands run (worktree root):
    • uv run pytest tests/dev/test_pr_loop_policy.py -q — 174 passed (150 pre-existing + 24 new).
    • uv run pytest tests/dev/test_snapshot_pr_queue.py -q — 55 passed (companion).
    • uv run ruff check scripts/dev/pr_loop_policy.py tests/dev/test_pr_loop_policy.py — clean.
    • uv run ruff format --check scripts/dev/pr_loop_policy.py tests/dev/test_pr_loop_policy.py — clean.
    • Smoke: real snapshot_pr_queue --active | pr_loop_policy --stdin --json — exit 0, no crash; synthetic snapshot emits active_writer and author_decision states via the CLI.
  • Evidence that the change works here: 24 focused tests cover the marker grammar, expiry/release semantics, live-head binding, precedence, and queue-level behavior with fixed timestamps.
  • Benchmarks or smoke tests, if applicable: none (workflow tooling).

Performance Evidence

  • (Not a perf change; section deleted per template guidance.)

Risks / Rollout

  • Compatibility risks: additive states and an explicit now kwarg (default preserves current behavior); existing classifications re-tested (draft still no_action, ready_to_merge precedence preserved).
  • Failure modes: unparseable claim timestamps fail closed (documented); lane identity is not distinguishable in the snapshot, so any unexpired unreleased trusted marker parks (documented limitation).
  • Rollback or fallback plan: revert the single commit; the new states are purely additive.

Docs / Provenance

Downstream Propagation

  • Parent issue updated (yes/no/NA): yes - issue friction: machine-recognize review-claim marker and author_decision state from goal-pr-review (#7500) #7508 will be closed by this PR.
  • Claim map / benchmark report updated (yes/no/NA): NA
  • Leaderboard / artifact catalog updated (yes/no/NA): NA
  • Registry or config index updated (yes/no/NA): NA
  • Context index / memory note updated (yes/no/NA): NA
  • Follow-up issue opened for deferred propagation (yes/no/NA): no
  • Not applicable because: workflow/tooling-only change with no research-facing downstream surface.

Follow-Up Issues

Reviewer Notes

  • Anything a reviewer should verify closely: the marker regex grammar matches the docs(skills): harden goal-pr-review for concurrent review lanes #7500 docs exactly (review-claim: <lane-id> @ <head-sha> until <UTC>); precedence of active_writer/author_decision over blocked_preflight; the live-head binding for author_decision packets.
  • Any known limitations: snapshot does not carry lane identity, so the active-writer check cannot exclude the claiming lane itself; documented in the module docstring.
  • Shared-helper migration: NA - single-file CLI helper change.

@github-actions github-actions Bot added the review-bot-auto Managed label that routes code-bearing PRs to CodeRabbit. label Aug 18, 2026
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Important

Review available on request

  • 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d98fb6f-34cc-450b-a315-29275cbd3687


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🔍 PR Contract Check Summary

Overall Status: 🟢 PASSED

Check Status Description
1. Closes-discipline ✅ PASSED Demand Refs #N for epic issues
2. Closure declaration ⚠️ WARNING Require Closes/Refs for title issues
3. State-refresh-only ✅ PASSED Reject docs/context state updates
4. Evidence hygiene ✅ PASSED Checks markers and provenance fields
5. Evidence writer usage ✅ PASSED Require the shared marked writer path
6. Successor discipline ⚠️ WARNING Require successor statement on multi-PR issues
7. Worker-lane label ⚪ None Label PRs from cheap worker lane
8. Placeholder docstring ratchet ✅ PASSED Reject NEW TODO/empty docstrings in added diff lines

⚠️ Warnings

ℹ️ Info

  • No worker-lane provenance detected.

This check is mechanized. Please resolve any blockers to pass CI.

@ll7 ll7 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Exact-head review evidence

Reviewed head e2800b70c26ae367a506f1b6ea4c696e095e52ab (current PR head, verified against GitHub).

Validation performed (parent-run, at head):

  • pytest tests/dev/test_pr_loop_policy.py -q — 174 passed (150 pre-existing + 24 new).
  • pytest tests/dev/test_snapshot_pr_queue.py -q — 55 passed (companion).
  • ruff check and ruff format --check on changed files — clean.
  • CI rollup on head: all 32 checks success/skipped, no failures.
  • Smoke: real snapshot through pr_loop_policy --stdin --json exits 0; synthetic snapshot emits both active_writer and author_decision states.

Intended design: pr_loop_policy.py machine-recognizes the #7500 review-claim marker (review-claim: <lane> @ <sha> until <UTC>, review-claim: released @ <sha>) as active_writer (park, no_action/stop) and the decision-required + ### Decision packet combination as author_decision (park, no_action/stop), both taking precedence over blocked_preflight. The #7491 not-ready-sentinel half is explicitly out of scope and linked. Live-head binding, expiry, and release semantics are tested with fixed timestamps.

Findings disposition: No actionable review threads; no fixable findings.

Scope check: Matches issue #7508 contract (Closes #7508). Workflow/tooling only; no benchmark/planner/evidence claim. No follow-up issues required.

Base policy: ordinary-cas @ e2800b70c26a (not base-sensitive; guarded merger performs current-main CAS).

Single-account waiver: Repository operated through one effective account; internal review recorded for exact head above.

pr-metadata: reconciled @ 394210fd62322f60e0f2583ac7fb6877f343b166ebba6e372f6d9410788caabb
gate-verdict: accepted @ e2800b7

@ll7 ll7 added the merge-ready Verified by the PR gate and ready to merge label Aug 18, 2026
@ll7
ll7 merged commit b6a75a4 into main Aug 18, 2026
33 checks passed
@ll7
ll7 deleted the fix/issue-7508-review-claim-20260818 branch August 18, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-ready Verified by the PR gate and ready to merge review-bot-auto Managed label that routes code-bearing PRs to CodeRabbit.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant