feat(beagle-analysis): four planning guardrails from issue-124 retention post-mortem#140
Conversation
…tention post-mortem Two bugs from a single shipped feature (Osprey issue-124 retention, built through the beagle planning workflow) exposed four guardrail gaps in the write-plan and brainstorm-beagle skills. All four are fail-the-plan/spec checks, not advisory notes. - G1 Consumer/YAGNI gate: every new public API surface a plan introduces must name a production consumer in the same plan (a test is not a consumer); brainstorm gets the parallel spec-level check. - G2 Discriminating-assertion gate: construct-a-false-pass-impl check, plus a payload-preservation carve-out so YAGNI-for-tests no longer collapses structurally-distinct producers and the corrupted region. - G3 Composition check: brainstorm Prior Art Check step 5 escalates upstream/downstream data-transforming mechanisms to Key Decision + spike. - G4 Input-shape spike trigger: write-plan Task 0 list gains the upstream-data-shape assumption trigger. Closes #139 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughFive skill-guide documents and templates receive coordinated updates to enforce consumer-identification requirements and payload-preservation testing discipline. The brainstorm-beagle SKILL.md gains a composition-check step in Prior Art Check, a Consumer (brownfield) self-review item, and a Key Principles bullet requiring every capability have a named consumer. The brainstorm-beagle spec-reviewer.md adds checklist rules for consumer naming and composition validation. The resolve-beagle SKILL.md expands its latent-gap taxonomy to include Unconsumed surface and Unresolved composition gaps, and updates its self-review checklist with consumer and composition requirements. The write-plan SKILL.md adds an input-shape assumption example to spike-candidate guidance, extends the payload-preservation invariants exception to require all structurally-distinct producers and sentinel checks in the corrupted region, adds Consumer check and Discriminating assertion self-review items, and restores the preserve/recover/transform carve-out in YAGNI-for-tests. Finally, plan-reviewer.md and plan-template.md add corresponding reviewer guidance, calibration criteria, and self-review checklist items. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![daydream-review[bot]](https://avatars.githubusercontent.com/in/4014446?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
📋 Non-inline findings (8)
plugins/beagle-analysis/skills/brainstorm-beagle/SKILL.md (2)
🔵 Spec follow-up review paths do not enforce the new brownfield checks | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
brainstorm-beagle/SKILL.mdadds the composition and consumer checks, but the detailed spec reviewer still only covers the older reinvention check. That matters becausebrainstorm-beagleexplicitly points self-review toreferences/spec-reviewer.md, andresolve-beagleimports that same checklist when rewriting existing specs. A spec can therefore be resolved or detailed-reviewed without catching the new failure mode this patch is meant to prevent: an upstream/downstream mechanism composing incorrectly, or must-have external surface with no named consumer.Recommendation: Update
plugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.mdwith explicit Composition and Consumer rows, and updateresolve-beaglelatent-gap extraction/self-review language to treat unconsumed external surface and unresolved pipeline composition as gaps before planning.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/brainstorm-beagle/SKILL.md`, Spec follow-up review paths do not enforce the new brownfield checks: **Severity:** medium
💡 Composition spike handoff should use the existing spike marker | severity:
low· confidence:HIGHSeverity: low
Confidence: HIGH
The new composition check says to “flag it as a Task 0 spike candidate for the plan,” but the skill already has a concrete handoff marker for this class of issue:
needs-spike-before-planningin Key Decision rationale. Leaving the new instruction as prose creates two spike dialects inside the same spec workflow, making it easier forwrite-planor a human planner to miss that the composition question is mandatory before plan lock.Recommendation: Change the composition check to require the same
needs-spike-before-planningmarker used by the Key Decisions tool-behavior section, with wording specific to upstream/downstream data shape. That keeps the brainstorm-to-plan handoff uniform and easier to audit.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/brainstorm-beagle/SKILL.md`, Composition spike handoff should use the existing spike marker: **Severity:** lowplugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.md (1)
🔵 Spec follow-up review paths do not enforce the new brownfield checks | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
brainstorm-beagle/SKILL.mdadds the composition and consumer checks, but the detailed spec reviewer still only covers the older reinvention check. That matters becausebrainstorm-beagleexplicitly points self-review toreferences/spec-reviewer.md, andresolve-beagleimports that same checklist when rewriting existing specs. A spec can therefore be resolved or detailed-reviewed without catching the new failure mode this patch is meant to prevent: an upstream/downstream mechanism composing incorrectly, or must-have external surface with no named consumer.Recommendation: Update
plugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.mdwith explicit Composition and Consumer rows, and updateresolve-beaglelatent-gap extraction/self-review language to treat unconsumed external surface and unresolved pipeline composition as gaps before planning.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.md`, Spec follow-up review paths do not enforce the new brownfield checks: **Severity:** mediumplugins/beagle-analysis/skills/resolve-beagle/SKILL.md (1)
🔵 Spec follow-up review paths do not enforce the new brownfield checks | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
brainstorm-beagle/SKILL.mdadds the composition and consumer checks, but the detailed spec reviewer still only covers the older reinvention check. That matters becausebrainstorm-beagleexplicitly points self-review toreferences/spec-reviewer.md, andresolve-beagleimports that same checklist when rewriting existing specs. A spec can therefore be resolved or detailed-reviewed without catching the new failure mode this patch is meant to prevent: an upstream/downstream mechanism composing incorrectly, or must-have external surface with no named consumer.Recommendation: Update
plugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.mdwith explicit Composition and Consumer rows, and updateresolve-beaglelatent-gap extraction/self-review language to treat unconsumed external surface and unresolved pipeline composition as gaps before planning.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/resolve-beagle/SKILL.md`, Spec follow-up review paths do not enforce the new brownfield checks: **Severity:** mediumplugins/beagle-analysis/skills/write-plan/SKILL.md (2)
🔵 Plan reviewer/template still encode the old test and surface rules | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
write-plan/SKILL.mdadds strong gates for production consumers and discriminating payload-preservation assertions, but the imported plan reviewer prompt does not check either category. The plan template also still says plain “YAGNI for tests” in its task guidelines without the new payload-preservation exception, and its self-review outcome does not prompt authors to report the new gates. For long or high-stakes plans, the optional reviewer can now approve a plan that the main skill’s own self-review should fail.Recommendation: Update
plan-reviewer.mdto include Consumer check and Discriminating assertion categories, including the payload-preservation producer/region rule. Updateplan-template.mdtask guidelines and Self-Review Outcome bullets so generated plans carry the same contract the main skill now requires.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/write-plan/SKILL.md`, Plan reviewer/template still encode the old test and surface rules: **Severity:** medium
💡 Composition spike handoff should use the existing spike marker | severity:
low· confidence:HIGHSeverity: low
Confidence: HIGH
The new composition check says to “flag it as a Task 0 spike candidate for the plan,” but the skill already has a concrete handoff marker for this class of issue:
needs-spike-before-planningin Key Decision rationale. Leaving the new instruction as prose creates two spike dialects inside the same spec workflow, making it easier forwrite-planor a human planner to miss that the composition question is mandatory before plan lock.Recommendation: Change the composition check to require the same
needs-spike-before-planningmarker used by the Key Decisions tool-behavior section, with wording specific to upstream/downstream data shape. That keeps the brainstorm-to-plan handoff uniform and easier to audit.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/write-plan/SKILL.md`, Composition spike handoff should use the existing spike marker: **Severity:** lowplugins/beagle-analysis/skills/write-plan/references/plan-reviewer.md (1)
🔵 Plan reviewer/template still encode the old test and surface rules | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
write-plan/SKILL.mdadds strong gates for production consumers and discriminating payload-preservation assertions, but the imported plan reviewer prompt does not check either category. The plan template also still says plain “YAGNI for tests” in its task guidelines without the new payload-preservation exception, and its self-review outcome does not prompt authors to report the new gates. For long or high-stakes plans, the optional reviewer can now approve a plan that the main skill’s own self-review should fail.Recommendation: Update
plan-reviewer.mdto include Consumer check and Discriminating assertion categories, including the payload-preservation producer/region rule. Updateplan-template.mdtask guidelines and Self-Review Outcome bullets so generated plans carry the same contract the main skill now requires.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/write-plan/references/plan-reviewer.md`, Plan reviewer/template still encode the old test and surface rules: **Severity:** mediumplugins/beagle-analysis/skills/write-plan/references/plan-template.md (1)
🔵 Plan reviewer/template still encode the old test and surface rules | severity:
medium· confidence:HIGHSeverity: medium
Confidence: HIGH
write-plan/SKILL.mdadds strong gates for production consumers and discriminating payload-preservation assertions, but the imported plan reviewer prompt does not check either category. The plan template also still says plain “YAGNI for tests” in its task guidelines without the new payload-preservation exception, and its self-review outcome does not prompt authors to report the new gates. For long or high-stakes plans, the optional reviewer can now approve a plan that the main skill’s own self-review should fail.Recommendation: Update
plan-reviewer.mdto include Consumer check and Discriminating assertion categories, including the payload-preservation producer/region rule. Updateplan-template.mdtask guidelines and Self-Review Outcome bullets so generated plans carry the same contract the main skill now requires.🔮 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `plugins/beagle-analysis/skills/write-plan/references/plan-template.md`, Plan reviewer/template still encode the old test and surface rules: **Severity:** medium
🔮 Prompt for all review comments with AI agents
Fix the 8 review comment(s) posted on this PR.
If the /beagle-core:fetch-pr-feedback skill is available, run:
/beagle-core:fetch-pr-feedback --pr 140
Otherwise, fetch the comments manually:
1. gh api repos/existential-birds/beagle/pulls/140/comments
2. gh api repos/existential-birds/beagle/issues/140/comments
These endpoints return all comments on the PR. Focus on the most
recent review — ignore older review threads that have already been
addressed. For each comment: read the referenced file, verify the
finding against the current code, and fix it if valid. Skip false
positives. Commit all fixes when done.
ℹ️ Review info
- Model: gpt-5.5
- Cost: $1.29
- Tokens: 698,117 in (561,920 cached, 80% hit) → 10,786 out
- Steps / tool calls: 4 / 45
- Duration: 2m 53s
Per-phase breakdown
| Phase | Model | Tools | Input (cached) | Output | Cost | Latency |
|---|---|---|---|---|---|---|
| Exploration | gpt-5.5 | 24 | 453,336 (84%) | 6,012 | $0.73 | 1m 32s |
| Understand Intent | gpt-5.5 | 6 | 53,881 (74%) | 867 | $0.12 | 17s |
| Alternatives | gpt-5.5 | 15 | 190,900 (74%) | 3,907 | $0.44 | 1m 3s |
Generated by daydream v0.19.0
- Severity: 6 medium, 2 low
- Confidence: 8 HIGH
🧙 Posted by daydream v0.19.0
The issue-139 composition/consumer and discriminating-assertion gates were added inline to brainstorm-beagle and write-plan SKILL.md, but the shared checklists those skills delegate to lagged behind: - spec-reviewer.md (imported by brainstorm self-review AND resolve-beagle) gains Consumer (§8) and Composition (§9) review dimensions. - resolve-beagle gains the two gap types in its latent-gap extraction table and self-review checklist. - brainstorm composition check now emits the structured needs-spike-before-planning marker instead of loose prose, unifying the spike-handoff dialect. - plan-reviewer.md gains Consumer check and Discriminating assertion rows (incl. payload-preservation producer/region rule). - plan-template.md carries the payload-preservation YAGNI carve-out and new Self-Review Outcome bullets. Closes the gap where a detailed review or resolve/reviewer pass could approve a spec or plan the originating skill's own self-review would fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Two bugs from a single shipped feature (Osprey's issue-124 retention, built through the beagle planning workflow) revealed four distinct guardrail gaps in the
write-planandbrainstorm-beagleskills. This PR adds all four as fail-the-plan/spec checks (not advisory notes), editing only the two skill files so they land together without conflicting edits.Changes
Added
write-plan/SKILL.mdbrainstorm-beagle/SKILL.mdChanged
Motivation
The two bugs (a dead
archive_sessionprimitive with zero production callers, and a tool-output recovery path that shipped green because its test asserted only the surviving head sentinel) share one throughline: the project's prior lesson ("grep all crates before specing") was half-absorbed. The skills got the reinvention guardrail but never the composition, discriminating-assertion, consumer, or input-shape altitude. These four close that gap.Testing
This is a pure-markdown plugin marketplace with no build system or automated tests (per CLAUDE.md). Verification was manual.
Manual Testing Steps
Related Issues
Checklist
🤖 Generated with Claude Code