feat(cadence): wire p3_velocity into Stop + UPS dispatchers (#876 PR 3/5) - #886
Conversation
PR 3 of 5 in the #876 stack. Builds on: - PR 1 (#884, 647953b): POLICY_P3_VELOCITY enum + resolver + pure should_fire_p3_velocity predicate. - PR 2 (#885, f1f9d7b): session_ring P3 state slots (bytes_at_last_fire, classifications) + writer helpers. Adds in this PR: 1. New session_ring slot: fire_idx_at_last_fire. The velocity predicate needs both bytes_at_last_fire AND a fire_idx anchor to compute turns_since_last_fire (= next_fire_idx - fire_idx_at_last_fire). Backward-compat handled in _normalize_for_session — pre-PR-3 rings default the field to 0. 2. update_p3_velocity_state(session_id, *, transcript_bytes, fire_idx) — new combined updater that writes both p3_velocity state slots atomically under the existing advisory lock. Keeps the two inputs in sync so the next predicate evaluation sees a consistent (bytes, fire_idx) snapshot. update_bytes_at_last_fire is kept as a thin wrapper for backward-compat with PR 2 callers. 3. POLICY_P3_VELOCITY dispatch branch in _maybe_fire_cadence_checkpoint (Stop) and _maybe_run_ups_cadence_checkpoint (UPS). Both branches: a. Read ring state — extract next_fire_idx, bytes_at_last_fire, fire_idx_at_last_fire (type-check each). b. Resolve p3_velocity_threshold + estimate transcript bytes via the existing P2 helper. c. Compute turns_since_last_fire; bail if non-positive (no valid window yet, e.g. first fire of a new session). d. Call should_fire_p3_velocity with the three state inputs. e. On fire: run the rebuilder, write resume cache (Stop only), call update_p3_velocity_state to advance both state slots atomically to the current (transcript_bytes, next_fire_idx). f. Emit stderr observability line with the computed density and configured threshold for operator visibility. Counter-sharing with Stop (post-#874 pattern): UPS reads ring state before this turn's _ring_append_ids, so it sees the same next_fire_idx Stop saw at end of the prior turn. Both fire on the same boundary by construction; the second updater is a benign overwrite with identical values. Tests: - tests/test_hook_stop_cadence_p3_velocity.py (6 tests): above- threshold fires, below skips, turns_since_last_fire == 0 short- circuit, default-OFF respected, env override, missing session state skips cleanly. - tests/test_hook_ups_cadence_p3_velocity.py (4 tests): UPS fires return body, UPS below skips, atomic state update on UPS fire, empty session_id skips. All cadence + session_ring suites pass: 277 / 277 (267 prior + 10 new). Full pytest pass pending in PR CI. What's left in the stack: - PR 4: wire p3_substantive into Stop + UPS + #881 shadow-mode companions for the two new policies. - PR 5: CHANGELOG + CONFIG.md + design-doc updates.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
robotrocketscience
added
attn:review
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
merge-train: blocked FF push to The |
github-actions
Bot
removed
the
ready-to-merge
CodeQL caught the unused symbol — the test exercises the policy via TOML config strings, not by referencing the constant directly. merge-train catch on PR #886.
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged af0306e → |
2 participants
Part of #876. PR 3 of 5 in the stack. Builds on #884 (PR 1) + #885 (PR 2).
Summary
Wires
p3_velocityinto both Stop and UPS cadence dispatchers. After this PR, an operator can opt into bytes-per-turn-density cadence by setting[cadence] enabled = true+[cadence] policy = "p3_velocity"in.aelfrice.toml.New in this PR
Session-ring slot
Added
fire_idx_at_last_fireto the ring schema. The velocity predicate needs bothbytes_at_last_fireAND a fire_idx anchor to computeturns_since_last_fire = next_fire_idx - fire_idx_at_last_fire. Backward-compat handled in_normalize_for_session— pre-PR-3 rings default the field to 0.Atomic combined updater
update_p3_velocity_state(session_id, *, transcript_bytes, fire_idx)writes both p3_velocity state slots atomically under the existing advisory lock. Keeps the two inputs in sync so the next predicate evaluation sees a consistent(bytes, fire_idx)snapshot.update_bytes_at_last_fireis kept as a thin wrapper for backward-compat with PR 2 callers.Dispatch wiring
POLICY_P3_VELOCITYbranch in_maybe_fire_cadence_checkpoint(Stop) and_maybe_run_ups_cadence_checkpoint(UPS). Both branches:next_fire_idx,bytes_at_last_fire,fire_idx_at_last_fire.p3_velocity_threshold+ estimate transcript bytes via the existing P2 helper.turns_since_last_fire; bail if non-positive.should_fire_p3_velocitywith the three state inputs.update_p3_velocity_stateto advance both state slots atomically.Counter-sharing post-#874 pattern preserved: UPS reads ring state before this turn's
_ring_append_ids, sees samenext_fire_idxStop saw at end of prior turn. Both fire on the same boundary; second updater is a benign overwrite.Determinism (#605)
Predicate inputs are filesystem state + config. Same inputs reproduce same fire decisions across replays.
Tests
10 new wiring tests across two files:
tests/test_hook_stop_cadence_p3_velocity.py— 6 tests: above-threshold fires, below skips,turns_since_last_fire == 0short-circuit, default-OFF respected, env override, missing session state skips cleanly.tests/test_hook_ups_cadence_p3_velocity.py— 4 tests: UPS fires returns body, UPS below skips, atomic state update on UPS fire, empty session_id short-circuit.All 277 cadence + session_ring tests pass locally. Full suite pass pending in CI.
Discretion
Diff clean against the pre-push deny-list.
What's left in the stack
p3_substantiveinto Stop + UPS + feat(cadence): shadow-evaluation mode for direct P1 vs P2 policy comparison (#875) #881 shadow-mode companions.