fix(source-control): map babysit recommended_cadence to ScheduleWakeup.delaySeconds - #652
Conversation
…p.delaySeconds (#504) The babysit-prs snapshot engine emits `recommended_cadence`, and loop.md §5.3 told the orchestrator to derive the wake interval from it but never gave the string-to-seconds translation. Orchestrators fell back to the generic `/loop` skill's own 1200-1800s fallback-heartbeat range, overriding the domain skill's tighter adaptive-cadence contract so PRs with pending CI or blocking feedback sat unchecked 4-5x longer than intended. §5.3 now carries a deterministic mapping table (active->300, normal->900, quiet->3600, idle->3600) and states that this signal ALWAYS wins over the generic heuristic when a snapshot supplies it. The idle row is documented as a ceiling: ScheduleWakeup clamps delaySeconds to [60, 3600], so cadence.md's daily idle intent truncates to 3600s — a genuine daily cadence needs the durable /schedule cron mechanism, not a single-session /loop wakeup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KyLj6oaFVTE6xFuoYCC2KC
|
Claude finished @kyle-sexton's task in 1m 49s —— View job Code Review —
|
recommended_cadence |
cadence.md prose | Mapped seconds | Correct? |
|---|---|---|---|
active |
every 5 minutes | 300 | ✓ |
normal |
every 15 minutes | 900 | ✓ |
quiet |
hourly | 3600 | ✓ |
idle |
daily (86400s) | 3600 (ceiling) | ✓ documented |
All four values are arithmetically consistent with the source-of-truth descriptions in cadence.md. The ScheduleWakeup [60, 3600] clamp is confirmed by the live tool schema.
Priority statement — the ALWAYS wins phrasing is strong and unambiguous. No orchestrator following §5.3 can reasonably fall back to the generic /loop heuristic after reading this; the original silent override is closed.
Idle-ceiling caveat — the /schedule pointer gives a concrete escape hatch for genuine daily cadences. Documenting it as a known limitation rather than a silent truncation is the right call.
Python-free ladder untouched — the existing degrade table (lines 392–396 in the final file) correctly handles the no-snapshot case and is unaffected by this change. The boundary between "engine-backed" and "Python-free" sections is clear.
Permalink: loop.md §5.3 (new table)
Minor Observation (non-blocking)
SKILL.md step 9 phrasing is still accurate but slightly imprecise after this change. It reads:
Schedule the next wake from the snapshot's
recommended_cadence(reference/cadence.md); the static ladder inreference/loop.md§5.3 is the Python-free degrade.
After this PR, §5.3 now houses both the engine-backed mapping table and the Python-free ladder. The step 9 reference still works (it correctly points to loop.md §5.3 for the degrade ladder), but it no longer surfaces that §5.3 is also where the engine-backed mapping lives — a reader might still go to cadence.md first looking for the seconds. A future cleanup could tighten this to something like:
Schedule the next wake using the mapping table in
loop.md§5.3 (engine-backed) or the static degrade ladder in the same section (Python-free).
This is out of scope for this PR and not a blocker — the existing cross-reference is navigable.
plugin.json — Version Bump
0.13.1 → 0.13.2 is correct: patch semver for a behavioral fix with no interface change.
CHANGELOG.md — Entry
Well-written. The entry accurately describes the problem (silent fallback to 1200–1800s), the fix (deterministic table + priority statement), and the idle-ceiling nuance. Format follows the established Keep a Changelog style.
Verdict: approved. The fix is correct, internally consistent, and complete. No blocking issues.
Review classification (work-2 lane, post-green pass)
Verdict: approved, no blocking issues. Flipping to ready. |
|
Claude finished @kyle-sexton's task in 1m 20s —— View job Code Review —
|
recommended_cadence |
cadence.md definition |
Mapped seconds | Correct? |
|---|---|---|---|
active |
every 5 minutes | 300 | ✓ |
normal |
every 15 minutes | 900 | ✓ |
quiet |
hourly | 3600 | ✓ |
idle |
daily (86400s) | 3600 (ceiling, documented) | ✓ |
All four values are consistent with cadence.md. The ScheduleWakeup [60, 3600] clamp is confirmed by the live tool schema (the delaySeconds description in ScheduleWakeup states "Clamped to [60, 3600] by the runtime").
Priority statement (lines 376–381) — the ALWAYS wins phrasing is strong and unambiguous. No orchestrator following §5.3 can reasonably fall back to the generic /loop heuristic after reading this.
Idle ceiling caveat (lines 383–388) — the /schedule pointer gives a concrete escape hatch for genuine daily cadences. Documenting this as a known limitation rather than a silent truncation is the right call.
Python-free degrade ladder (lines 390–396) — untouched and unaffected. The boundary between "engine-backed" and "Python-free" sections is clear.
cadence.md cross-reference — cadence.md's own preamble ("the loop derives its wake interval from it — loop.md owns the wake mechanics") remains accurate after this change; the ownership language still correctly defers to loop.md.
plugin.json — Version Bump
0.13.1 → 0.13.2 is correct: patch semver for a behavioral fix with no interface change.
CHANGELOG.md — Entry
Well-written and accurate. The entry covers the problem (silent fallback to 1200–1800s), the fix (deterministic table + priority statement), and the idle-ceiling nuance. Format follows Keep a Changelog style consistently with prior entries.
Previously Noted (Non-Blocking)
SKILL.md step 9's cross-reference to loop.md §5.3 is slightly imprecise now that §5.3 houses both the engine-backed mapping table and the Python-free degrade ladder — a reader looking for the seconds mapping might still go to cadence.md first. Already filed as #653 and cited in ## Related; out of scope for this PR, not a blocker.
Verdict: approved. The fix is correct, internally consistent, and complete. No blocking issues.
main advanced source-control to 0.13.3 (#651, #666) while this branch bumped from an older 0.13.1 base; resolve the version and CHANGELOG conflict by re-homing this PR's entry as [0.13.4] on top of main's [0.13.3], keeping all main entries intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FM1RfM3jHkgenpdbMv4o64
|
Claude finished @kyle-sexton's task in 2m 24s —— View job Code Review —
|
recommended_cadence |
cadence.md definition |
Mapped seconds | Correct? |
|---|---|---|---|
active |
every 5 minutes | 300 | ✓ |
normal |
every 15 minutes | 900 | ✓ |
quiet |
hourly | 3600 | ✓ |
idle |
daily (86400s) | 3600 (ceiling, documented) | ✓ |
All four values are arithmetically consistent with cadence.md. The [60, 3600] ScheduleWakeup clamp is confirmed by the live tool schema.
Priority statement (lines 376–381) — the ALWAYS wins phrasing is unambiguous and directly addresses the root cause. An orchestrator following this cannot reasonably fall back to the generic heuristic.
Idle ceiling caveat (lines 383–388) — disclosed as a known limitation with a concrete /schedule escape hatch. Not a silent truncation.
Python-free degrade ladder (lines 390–396) — untouched and correct. Boundary between engine-backed and Python-free sections remains clear.
cadence.md preamble unchanged — its existing loop.md owns the wake mechanics ownership statement remains accurate.
Merge Conflict Resolution
The merge commit 0cbe3c1 correctly re-homes this PR's CHANGELOG entry as [0.13.4] on top of main's [0.13.3]. The commit message explains the version conflict precisely: main advanced source-control to 0.13.3 (#651, #666) while this branch bumped from an older 0.13.1 base. No content drift in loop.md between the original fix commit and the merged head.
plugin.json — Version
0.13.4 is correct after re-homing (the original bump was 0.13.1 → 0.13.2, but main's 0.13.2 and 0.13.3 landed first, so 0.13.4 is the right patch increment).
CHANGELOG.md — Entry
Well-written [0.13.4] entry covering the problem (silent fallback to 1200–1800s), the fix (deterministic table + priority statement), and the idle ceiling. Format is consistent with prior Keep a Changelog entries.
Previously Noted (Non-Blocking, Deferred as #653)
SKILL.md step 9's cross-reference to loop.md §5.3 is slightly imprecise now that §5.3 houses both the engine-backed mapping table and the Python-free degrade ladder. Already filed and cited in ## Related; out of scope for this PR.
Verdict: approved. The fix is correct, internally consistent, and the merge conflict was resolved cleanly. No blocking issues.
…oop.md §5.3 (#1307) *This was generated by AI during an autonomous work-loop execution session.* Closes #653 ## Summary `babysit-prs`'s SKILL.md described a cadence-ownership split that no longer exists. #652 added the engine-backed `recommended_cadence` → `ScheduleWakeup.delaySeconds` mapping table to `reference/loop.md` §5.3, alongside the static Python-free degrade ladder already living there. `reference/cadence.md` owns only the cadence *states and thresholds* — and has said so since #322 ("`loop.md` owns the wake mechanics, this file owns the states and thresholds behind the recommendation"). SKILL.md still routed readers the old way: | Surface | Before | After | |---|---|---| | Runbook step 9 | `recommended_cadence` per `cadence.md`; `loop.md` §5.3 named only as the "static ladder … Python-free degrade" | `Schedule the next wake per the cadence contract in reference/loop.md §5.3.` | | Reporting closing line | "Recommend the exact next interval per `cadence.md`" | same sentence, now citing `loop.md` §5.3 | | References entry for `loop.md` | "…checklist, static cadence ladder" | "…checklist, and the §5.3 cadence contract" | | Step-5 progressive-disclosure trigger | load `cadence.md` "only before **recommending cadence**" | load `cadence.md` "only before **interpreting a cadence state**" | The filed issue named step 9. The other three are the same defect: leaving them would have made the file internally inconsistent about which section owns the seconds. **The Reporting line was a live wrong-number risk, not just imprecision.** `cadence.md` states `idle` = **daily**; §5.3 documents `ScheduleWakeup` clamping `delaySeconds` to `[60, 3600]`, so inside `/loop` `idle` and `quiet` both wake hourly. "Recommend the exact next interval per `cadence.md`" could therefore surface an interval the loop will never schedule. The `cadence.md` link is dropped from step 9 rather than kept alongside `loop.md`: §5.3 already hops to `cadence.md` for the states in one line, so a second pointer here is the gratuitous cross-reference `CLAUDE.md`'s pointer-not-copy rule forbids. `cadence.md` keeps its own References entry and its step-5 disclosure trigger. The new step-9 wording matches vocabulary the file already uses at its Step 7 checklist line ("schedule the next wake per the cadence contract (§5.3)") and in the sibling `babysit-loop` SKILL.md ("that mapping owns the seconds"). Docs-only. No behavior change, no script or engine change. `source-control` `0.26.1` → `0.26.2` with the matching CHANGELOG entry. ## Test plan Every command below was run from the branch worktree; all passed. - `plugins/skill-quality/scripts/check-skill.sh babysit-prs` (`CHECK_SKILL_BASE_REF=origin/main`) — **exit 0**, `PASS — 0 errors`. This is the gate CI runs via `scripts/check-changed-skills.sh`. Includes the 500-line hard cap, all 9 base-ref trigger phrases preserved, broken-internal-ref check, and `scripts/engine.test.sh`. - **SKILL.md is now 497 lines** (base `origin/main`: 499). An intermediate revision of this branch landed at exactly 500 and *failed* the hard cap (`LINE_COUNT >= LINE_HARD_CAP`, cap 500) — caught by an independent reviewer before the PR was opened. Collapsing step 9 to a single pointer and the References bullet back to two lines both fixed the cap and made the diff better follow pointer-not-copy. The one remaining `WARN` is the pre-existing 200-line soft target, untouched by this change. - `scripts/check-changelog-parity.sh --check` — exit 0. - `scripts/check-changelog-parity.sh --check-bump origin/main` — exit 0 (the manifest bump has a matching `## [0.26.2]` entry). - `markdownlint-cli2` over both changed markdown files, repo config auto-discovered — exit 0, 0 errors. - `jq empty` on `plugins/source-control/.claude-plugin/plugin.json` — exit 0. - `typos` over both changed markdown files — exit 0. - Every relative link in the edited regions resolved by hand (`reference/loop.md`, `cadence.md`, `safety.md`, `orchestration.md`, `freshness.md`, `stuck-checks.md`, `feedback.md`, `review-trigger.md`); the `§5.3` anchor exists at `reference/loop.md:426`. No external URLs introduced, so lychee's absence changes nothing. - Repo-wide `grep` for any remaining reference sending a reader to `cadence.md` for wake seconds, or describing §5.3 as holding only the static ladder — none remain. The four surviving `cadence.md` citations in `plugins/source-control/` were each checked and are correct. Claims verified against the actual files rather than the issue text: §5.3 does contain both the mapping table (`loop.md:435-440`) and the degrade ladder (`loop.md:456-462`); `cadence.md:4-5` does disclaim the wake mechanics; `cadence.md:27` does state `idle` = daily against `loop.md:449-452`'s `[60, 3600]` clamp. The CHANGELOG entry's causal account was corrected after `git show --stat e9cef6e` showed #652 never touched `cadence.md`, and `git log -L4,5` dated that file's disclaimer to `fe78acb6` (#322) — so the references were always imprecise; #652 only made the correct target concrete. Per this repo's fresh-docs mandate, <https://code.claude.com/docs/en/skills> was fetched this session. It confirms the progressive-disclosure model this diff operates on — SKILL.md body plus bundled supporting files loaded on demand — and imposes no constraint on how a SKILL.md cites its own `reference/` files, so the choice of pointer is a repo-convention question, decided above by `CLAUDE.md`'s pointer-not-copy rule. ## Related - Refs #652 / #504 — the PR and issue that added the §5.3 mapping table this diff points at. - Refs #322 — where `reference/cadence.md` first disclaimed ownership of the wake mechanics. - Version note: open PRs #1285 and #1264 both claim `source-control` `0.27.0`. If either merges first this branch's `0.26.2` becomes a regression — it self-surfaces as a git conflict on both the manifest line and the CHANGELOG insert point, so rebase rather than force-merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
The
babysit-prssnapshot engine computes arecommended_cadencefield (reference/cadence.md: active=5m / normal=15m / quiet=1h / idle=1d), andreference/loop.md§5.3 told the orchestrator to "derive the wake interval" from it — but never gave the string-to-seconds translation for aScheduleWakeup.delaySecondscall. So orchestrators (including the babysit lane itself, historically) silently fell back to the generic/loopskill's own "lean 1200–1800s" fallback-heartbeat range, overriding the domain skill's tighter adaptive-cadence contract. Net effect: PRs with pending CI or blocking feedback sat unchecked 4–5x longer than the skill's own cadence contract intends.Fix
reference/loop.md§5.3 (engine-backed subsection) now carries:recommended_cadence→ScheduleWakeup.delaySecondsmapping table:active→300,normal→900,quiet→3600,idle→3600. The prior duration prose ("active 5 minutes, normal 15 minutes…") is replaced by the seconds table so the mechanics live in one place;cadence.mdremains the owner of the states/thresholds behind each value.ScheduleWakeupclampsdelaySecondsto[60, 3600](verified against the tool's live schema —delaySecondsis "Clamped to [60, 3600] by the runtime"), socadence.md'sidle= daily (86400s) truncates to the 3600s ceiling, the same interval asquiet. Documented as a known limitation with a pointer to the durable/schedulecron mechanism for a genuine daily cadence — not left as a silent truncation.recommended_cadenceALWAYS wins over the generic/loopheuristic whenever a snapshot supplies it: in babysit dynamic mode theScheduleWakeupdelay is the primary cadence signal, not a fallback heartbeat behind some other armed wake event — so the generic skill's 1200–1800s heartbeat framing does not apply.Version bump
0.13.1→0.13.2(patch, behavioral fix) plus a## [0.13.2]CHANGELOG entry.Verification
The added §5.3 engine-backed guidance reads:
Acceptance criteria:
recommended_cadence→delaySecondsmapping table present.activecycle at 300s, not the generic 1200–1800s range (stated explicitly)./schedule-cron pointer, not left as a silent gap.markdownlint-cli2andtypospass on both changed docs.Closes #504
Related
source-control(babysit-prs): SKILL.md step 9 cross-reference to loop.md §5.3 is imprecise after #504's cadence-mapping table landed #653 — post-green review finding (SKILL.md step 9 cross-reference imprecise after this PR), deferred, cosmetic-only
loops/skills: self-paced /loop idles between non-empty cycles — cadence should be work-state-driven (no delay when queue non-empty), not a guessed clock #500 — the generic sibling (triage lane's "no delay when queue non-empty"). Different skill; reference only, not fixed here.
source-control:babysit-prs + loop: /loop 15m fires overlap in-flight fan-out cycles when batch-cap-10 dispatch runs long #472 — fixed-interval
/loop Nmoverlap (a different failure mode). Reference pointer, not fixed.source-control(babysit-prs): absorb v4 loop-prompt operating rules into the skill #477 — absorption umbrella (lists pacing-vs-concurrency but not this specific
recommended_cadence-consumption gap). Reference pointer, not fixed.🤖 Generated with Claude Code
https://claude.ai/code/session_01KyLj6oaFVTE6xFuoYCC2KC