feat(brainstorming): modernize spec refinement for capable models - #599
feat(brainstorming): modernize spec refinement for capable models#599SUaDtL wants to merge 2 commits into
Conversation
Five phases replace four: a split-before-you-spend bundling check in Phase 1, a new shape-the-approach phase (genuine alternatives with one recommendation, isolation + YAGNI lenses), a decide-or-ask question discipline replacing one-question-per-round (parameters decided and recorded in the spec, forks asked in full with a recommendation and batched when independent), a rationalization guard making closure a checklist instead of a feeling, a breaker for non-converging loops, and a Phase 5 review upgrade: mechanical self-review of the file as written plus an adversarial pass presented WITH the spec at approval. INDEX row and curated site page aligned; ca 2.11.5, ca-codex 0.4.4, ca-pi 0.2.4 (root manifest regenerated; ca-pi changelog entry per its release guard). Implements the accepted slice of #597; D1-D4 deferred there. CHANGELOG: the brainstorming skill now splits bundled ideas before refining, shapes the approach with a recommendation, decides routine parameters while asking genuine forks in full, and adversarially reviews its own spec before approval Claude-Session: https://claude.ai/code/session_01QjJeSbcwPHwMmd6CEZeagB
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
The version-advance surfaces the CI gates pin: CHANGELOG top section equals the ca manifest, README version badge matches, and the README Codex line names the shipped ca-codex version. CHANGELOG: (internal release-surface sync; no user-facing change beyond the 2.11.5 entry itself) Claude-Session: https://claude.ai/code/session_01QjJeSbcwPHwMmd6CEZeagB
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/surface/skills/brainstorming/SKILL.md`:
- Around line 43-63: Clarify the Phase 3 artifact lifecycle in the brainstorming
instructions: either define where the running spec draft exists before Phase 4,
or revise the references in the “Decide the parameters” bullet, the Phase 3
gate, and the later Phase 3 guidance to say decisions are tracked for inclusion
in the Phase 4 spec. Keep the wording consistent across all three references so
agents can satisfy the gate before “Write the spec.”
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 54761dd5-aee4-4a4c-a51a-c16a2a7ff350
⛔ Files ignored due to path filters (6)
plugins/ca-codex/routines/INDEX.mdis excluded by!plugins/ca-codex/routines/**plugins/ca-codex/routines/brainstorming/SKILL.mdis excluded by!plugins/ca-codex/routines/**plugins/ca-pi/routines/INDEX.mdis excluded by!plugins/ca-pi/routines/**plugins/ca-pi/routines/brainstorming/SKILL.mdis excluded by!plugins/ca-pi/routines/**plugins/ca/skills/INDEX.mdis excluded by!plugins/ca/skills/**plugins/ca/skills/brainstorming/SKILL.mdis excluded by!plugins/ca/skills/**
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdcore/surface/skills/INDEX.mdcore/surface/skills/brainstorming/SKILL.mdpackage.jsonplugins/ca-codex/.codex-plugin/plugin.jsonplugins/ca-pi/CHANGELOG.mdplugins/ca-pi/package.jsonplugins/ca/.claude-plugin/plugin.jsonsite/src/curated/skills/brainstorming.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
core/**
⚙️ CodeRabbit configuration file
This is the single source of truth for all three host plugins. tools/sync-core.py copies core/pysrc/*.py byte-identically into each plugin's hooks/ directory, and tools/build-surface.py renders core/surface/ into each plugin's commands, skills, routines, agents and includes, following core/hosts.json. CI fails the build if any copy drifts from its source. Never suggest editing a file under plugins/ to fix a defect — the fix belongs in core/ and the copies are regenerated from it. Report each defect once, against the core/ file, and do not note that the same issue appears in a vendored copy.
Files:
core/surface/skills/INDEX.mdcore/surface/skills/brainstorming/SKILL.md
**/*.md
⚙️ CodeRabbit configuration file
Prose in this repo is part of the product — skills and commands are read and executed by agents, so an ambiguous instruction is a defect, not a style nit. Prioritise findings where prose and the mechanism it describes disagree, where a documented command or flag no longer exists, or where an instruction cannot be followed literally without a contradiction. Deprioritise wording, tone and formatting preferences.
Files:
core/surface/skills/INDEX.mdCHANGELOG.mdREADME.mdsite/src/curated/skills/brainstorming.mdplugins/ca-pi/CHANGELOG.mdcore/surface/skills/brainstorming/SKILL.md
🪛 LanguageTool
core/surface/skills/brainstorming/SKILL.md
[style] ~96-~96: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... before any question budget is spent. - MUST NOT manufacture alternatives in Phase 2...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~97-~97: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...sane approach exists, say so and why. - MUST NOT write an acceptance criterion that ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~98-~98: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... cannot be verified by a single test. - MUST decide parameter-level choices and reco...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~99-~99: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e an open decision without asking it. - MUST stop a non-converging refinement loop a...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~100-~100: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...is a breaker, not a prompt to circle. - MUST run the Phase 5 mechanical self-review ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...the adversarial findings at approval. - MUST NOT resolve a [CONFIRM-NN] by guessin...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~102-~102: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...and record it in open-questions.md. - MUST NOT auto-approve a spec carrying a bloc...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~103-~103: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: .../sprint — it escalates to the user. - MUST NOT contradict the NOT-building list or...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~104-~104: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...a conflict to surface, not reconcile. - MUST NOT run decompose's six-layer whole-pro...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 SkillSpector (2.4.4)
core/surface/skills/brainstorming/SKILL.md
[warning] 98: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 102: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 98: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 102: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
🔇 Additional comments (10)
core/surface/skills/brainstorming/SKILL.md (1)
1-42: LGTM!Also applies to: 64-104
core/surface/skills/INDEX.md (1)
17-17: LGTM!site/src/curated/skills/brainstorming.md (1)
7-10: LGTM!Also applies to: 17-23, 27-40
CHANGELOG.md (1)
15-24: LGTM!README.md (1)
13-13: LGTM!Also applies to: 122-122
package.json (1)
3-3: LGTM!plugins/ca-codex/.codex-plugin/plugin.json (1)
4-4: LGTM!plugins/ca-pi/CHANGELOG.md (1)
7-12: LGTM!plugins/ca-pi/package.json (1)
3-3: LGTM!plugins/ca/.claude-plugin/plugin.json (1)
5-5: LGTM!
| ## Phase 3 — Socratic refinement loop · gate: BLOCK | ||
|
|
||
| Turn the chosen approach concrete. The discipline is decide-or-ask, never drift: | ||
|
|
||
| - **Decide the parameters; ask the forks.** A parameter-level choice — a naming, a default, a threshold, an ordering with one sensible answer — is yours to make: decide it, and record it in the spec where the user will see it at approval. A genuine fork — a real trade-off, a scope boundary, anything irreversible or user-visible — is the user's: ask it IN FULL the first time, leading with your recommendation and its reasoning. Never merely mention that an open decision exists; a named-but-unasked question is an omission wearing a disclaimer. | ||
| - **Batch what is independent.** Forks whose answers do not depend on each other are asked together, not serialized into rounds. One round of three real questions beats three rounds of one. | ||
| - Run every answer through three lenses: | ||
| - **Vague language** — force concrete nouns, numbers, and verbs. "Manage", "handle", "support" are not verbs. "Fast", "secure", "scalable" are not specifications. "We'll figure it out later" is not an answer — every "later" becomes a `[CONFIRM-NN]`. | ||
| - **Hidden complexity** — name what the user assumes is easy but is hard: state, concurrency, edge cases, failure modes, validation, idempotency, migration of existing data. Surface it now or it surfaces in `tdd`. | ||
| - **Trade-off forcing** — when a real decision exists, frame it with a recommendation and let the user rule. Do not pick for the user; do not hide that a pick exists. | ||
| - **The loop has a breaker.** When refinement stops converging — an answer contradicts an earlier one, or the same fork resurfaces after being answered — STOP the loop: restate the contradiction plainly, force one adjudication (a user ruling, or a recorded `[CONFIRM-NN]`), and only then continue. A loop that circles is not being thorough; it is failing to close. | ||
| - **Closure is a checklist, not a feeling.** The reasons to stop asking that do not count, each with why: | ||
| - *"The feature is simple enough."* — Simplicity shortens the spec; it does not waive it. The five-sentence spec still names its criteria. | ||
| - *"I've asked enough questions."* — Question fatigue is not an exit condition. The exit is the Phase 5 review passing. | ||
| - *"The user seems eager to move on."* — Eagerness is a signal to compress the asking (decide more parameters, batch harder), never to skip an unresolved fork. | ||
| - *"We can settle it during implementation."* — Every deferral is a `[CONFIRM-NN]` on record, or it is a guess that `tdd` will faithfully implement. | ||
|
|
||
| Record every genuinely-unresolved unknown as `[CONFIRM-NN]` in `{{PROJECT_DIR}}/.codearbiter/open-questions.md`, numbered sequentially. A finding that belongs to a different feature or a future scope gets an inline `[NEEDS-TRIAGE]` marker in the notes — never route it to a ticket. | ||
|
|
||
| Gate: every vague term made concrete; every forced trade-off resolved or recorded as `[CONFIRM-NN]`; no unresolved "later" outside a `[CONFIRM-NN]`. A blocking `[CONFIRM-NN]` that gates the spec's core stops the loop — surface it and STOP. | ||
| Gate: every vague term made concrete; every fork resolved by the user or recorded as `[CONFIRM-NN]`; every parameter decision recorded in the spec draft; no unresolved "later" outside a `[CONFIRM-NN]`. A blocking `[CONFIRM-NN]` that gates the spec's core stops the loop — surface it and STOP. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify "spec draft" reference in Phase 3 before Phase 4 writes the spec.
Line 47 says to record parameter decisions "in the spec where the user will see it at approval." Line 62 gates on "every parameter decision recorded in the spec draft." Line 98 repeats "record them in the spec." But Phase 4 (Line 64) is titled "Write the spec," and no earlier phase creates a spec file or draft on disk.
An agent following Phase 3 literally cannot satisfy the gate on Line 62, because the artifact it names does not exist until Phase 4. State explicitly whether Phase 3 maintains an informal running draft (and where it lives), or reword Line 47, Line 62, and Line 98 to say the decisions are tracked for inclusion in the Phase 4 spec, rather than implying an existing spec draft.
Also applies to: 98-98
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 98: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 102: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/surface/skills/brainstorming/SKILL.md` around lines 43 - 63, Clarify the
Phase 3 artifact lifecycle in the brainstorming instructions: either define
where the running spec draft exists before Phase 4, or revise the references in
the “Decide the parameters” bullet, the Phase 3 gate, and the later Phase 3
guidance to say decisions are tracked for inclusion in the Phase 4 spec. Keep
the wording consistent across all three references so agents can satisfy the
gate before “Write the spec.”
Source: Path instructions
Implements the accepted slice of #597 (full design rationale and deferred decisions D1-D4 live there).
What changed
The brainstorming skill predated several standing steers and read as written for a weaker model. Five phases now replace four:
Existing mechanics preserved unchanged: CONFIRM-NN, NEEDS-TRIAGE + harvest, Governs enrollment, sprint SMARTS auto-approval + logging, blocking-CONFIRM escalation, tdd handoff.
Surfaces
core/surface/skills/brainstorming/SKILL.md(canonical) → rendered to all three hosts via build-surfacecore/surface/skills/INDEX.mdrow updated (was pinning "one question at a time")site/src/curated/skills/brainstorming.md(hand-written page, no drift gate) alignedVerification
build-surface --check,sync-core --check: cleantest_build_surface.py: 40/40https://claude.ai/code/session_01QjJeSbcwPHwMmd6CEZeagB