Refactor stop-token policy and workflow entry requirements - #8
Merged
Conversation
- Updated stop-token policy documentation to clarify requirements and actions for various prompts. - Enhanced clarity in the rules for matching stop tokens, ensuring case-insensitive and exact matches. - Improved workflow entry points by enforcing the loading of SKILL.md and related components before proceeding. - Standardized the use of REQUIRE and RUN directives across workflows for consistency. - Added structured error handling and user prompts for various decision points in the workspace setup process. - Ensured that all phases of workspace configuration and validation adhere to the new requirements for confirmation and error handling. Signed-off-by: ainetx <viator@via-net.org>
📝 WalkthroughWalkthroughThe PR standardizes PDSL wording and structure across specs, controller contracts, agent prompts, and workflow files, adds a repository-wide test that validates fenced ChangesPDSL contract normalization
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
workflows/generate/phase-6/post-write-handoff.md (1)
43-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNumber all top-level
OPTIONSentries to match execution-card contract.
W2 ->andW3 ->no longer conform to the new rule that every top-level menu option starts with a decimal number. This creates an internal contract mismatch in the same PR.🤖 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 `@workflows/generate/phase-6/post-write-handoff.md` around lines 43 - 55, The top-level OPTIONS menu entries in post-write-handoff.md (specifically the entries labeled "W2 ->" and "W3 ->") must be renumbered to follow the execution-card contract (decimal numbered options); replace the nonconforming "W2 ->" and "W3 ->" labels with numbered OPTION entries (e.g., "2. ->" and "3. ->" or "2." and "3.") so every top-level OPTIONS item uses a decimal number and keep the existing text for each option unchanged.workflows/explore.md (1)
116-125:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExploreSaveMenu has duplicate option number
3.Line 116 and Line 123 both use selector
3, so one branch can shadow the other depending on parser behavior. This breaks deterministic menu routing.Suggested fix
- 3 folder: <path> -> + 3 | folder: <path> -> SAVE_BUNDLE folder=<user path> ... - 3 | skip | no -> + 4 | skip | no -> EMIT "Skipped saving. No files were written." CONTINUE ExploreNextActions - 4 | cancel -> + 5 | cancel -> EMIT "Explore save canceled. No files were written." STOP_TURN ... - EMIT "Reply with 1-4, `save`, `skip`, or `folder: <path>`." + EMIT "Reply with 1-5, `save`, `skip`, or `folder: <path>`."🤖 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 `@workflows/explore.md` around lines 116 - 125, The ExploreSaveMenu contains duplicate selector '3' causing ambiguous routing; update one of the conflicting menu selectors so each branch has a unique numeric choice (e.g., change the selector for the "skip | no" branch or the "SAVE_BUNDLE folder=<user path>" branch), and then update any routing/dispatch logic that expects selector '3' to use the new number; ensure the labels referenced in the diff (SAVE_BUNDLE, "EMIT \"Saved exploration bundle to <user path>.\"", and the skip | no branch with "EMIT \"Skipped saving. No files were written.\"") are preserved and wired to the new unique selector.workflows/analyze/phase-3-semantic.md (1)
217-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRule declaration is misplaced inside a
MENUblock.Line 226 adds an
ALWAYSrule underMENU SemanticPlanRevalidationFailureMenu, which mixes unit-level rules into menu structure. This can invalidate the menu schema and make the invariant non-enforceable. Move it to the enclosing unitRULES:/INVARIANTS:section.Suggested fix
MENU SemanticPlanRevalidationFailureMenu: TITLE: "Reviewer execution plan failed Phase 3 re-validation. Route back to phase-2.5-reviewer-plan.md or stop." OPTIONS: 1 rerun -> CONTINUE {cf-studio-path}/.core/workflows/analyze/phase-2.5-reviewer-plan.md 2 stop -> STOP_TURN INVALID: EMIT "Reply `1` to rerun the planner or `2` to stop." WAIT user.reply STOP_TURN - - ALWAYS enforce all PlannedMultiReviewerDispatch INVARIANTS (see above) when REVIEWER_EXECUTION_PLAN is non-null and dispatch is active. + +RULES: + - ALWAYS enforce all PlannedMultiReviewerDispatch INVARIANTS (see above) when REVIEWER_EXECUTION_PLAN is non-null and dispatch is active.🤖 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 `@workflows/analyze/phase-3-semantic.md` around lines 217 - 227, The ALWAYS rule is incorrectly placed inside the MENU block "SemanticPlanRevalidationFailureMenu"; move the line "ALWAYS enforce all PlannedMultiReviewerDispatch INVARIANTS (see above) when REVIEWER_EXECUTION_PLAN is non-null and dispatch is active." out of the MENU and into the enclosing unit-level RULES:/INVARIANTS: section so the invariant is declared at unit scope rather than inside the menu; ensure the rule references the same symbols (PlannedMultiReviewerDispatch, REVIEWER_EXECUTION_PLAN, dispatch) and retains its condition and enforcement text verbatim when relocated.workflows/analyze/preamble.md (1)
140-142:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore remediation prompts when user chooses prompt-review over storytelling.
Option 2 flips
EXPLAIN_MODEto false but leavesenforceRemediationPromptsat false if it was set in the storytelling-intent branch. That violates your invariant and can suppress required remediation prompting in analyze mode.Suggested fix
OPTIONS: 1 -> SET EXPLAIN_MODE = true; proceed with storytelling walkthrough - 2 -> SET PROMPT_REVIEW = true; SET EXPLAIN_MODE = false; proceed with prompt engineering review + 2 -> SET PROMPT_REVIEW = true; SET EXPLAIN_MODE = false; SET enforceRemediationPrompts = true; proceed with prompt engineering reviewAlso applies to: 163-164
🤖 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 `@workflows/analyze/preamble.md` around lines 140 - 142, The prompt-review branch currently sets PROMPT_REVIEW=true and EXPLAIN_MODE=false but fails to restore or preserve enforceRemediationPrompts set earlier in the storytelling branch; update the logic around the branch that sets PROMPT_REVIEW (and the analogous block at lines ~163-164) to explicitly preserve or re-enable enforceRemediationPrompts when switching off EXPLAIN_MODE so the analyze-mode invariant is maintained—i.e., after setting PROMPT_REVIEW=true and EXPLAIN_MODE=false, copy or set enforceRemediationPrompts to the true value it had for storytelling, or compute it from the same source used by the storytelling branch.workflows/generate/phase-0.7/wrap-handoff.md (1)
55-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate menu prompt/invalid text to include option 6.
After adding
6 stop_token, the title and invalid-reply hint still say1..5, which gives incorrect instructions. Update both strings to reflect1..6(or remove numeric mention if token-only behavior is intended).Also applies to: 91-97
🤖 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 `@workflows/generate/phase-0.7/wrap-handoff.md` around lines 55 - 56, The menu text still references "1..5" but you've added option 6; update the TITLE line ("Brainstorm complete — choose next step (reply 1, 2, 3, 4, or 5)") and the invalid-reply hint(s) in this file to include option 6 (e.g., "reply 1, 2, 3, 4, 5, or 6" and "1..6") or alternatively remove explicit numeric ranges so token-only behavior is unambiguous; make the same change for the other occurrence around the 91-97 block so all user-facing prompts consistently reflect the new option.skills/studio/migrate-from-cypilot.md (1)
257-265:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync INVALID guidance with available menu options.
Line 264 still says “Reply with 1, 2, or 3.” but this menu now includes option 4 (
stop_token).Proposed fix
- INVALID: - EMIT "Reply with 1, 2, or 3." + INVALID: + EMIT "Reply with 1, 2, 3, or 4." WAIT user.reply STOP_TURN🤖 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 `@skills/studio/migrate-from-cypilot.md` around lines 257 - 265, The user-facing guidance is out of sync with MENU E3_MigratorMenu (which now includes option 4/stop_token); locate the prompt text that says "Reply with 1, 2, or 3." and update it to match the menu options (e.g., "Reply with 1, 2, 3, or 4." or replace the hardcoded list with a generated list from MENU E3_MigratorMenu), making sure references to selection and the E3_RunMigrator flow remain consistent with the new stop_token option.
🟡 Minor comments (3)
workflows/plan/phase-0-discover.md-26-27 (1)
26-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix malformed requirement sentence in NOTES.
Line 26 currently says “where they ALWAYS be”, which makes the requirement ambiguous. Use “ALWAYS are” (or equivalent) to keep the checkpoint instruction unambiguous.
🤖 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 `@workflows/plan/phase-0-discover.md` around lines 26 - 27, Update the malformed sentence in the NOTES paragraph so it reads unambiguously: replace "where they ALWAYS be written into the [meta] TOML table of plan.toml" with "where they ALWAYS are written into the [meta] TOML table of plan.toml" (keeping the reference to Phase 3.1 and the [meta] table intact).skills/studio/agents/cf-pdsl-author.md-18-20 (1)
18-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify ambiguous contract wording in section constraints.
“The final prompt also requires generated
STATE,WHEN,DO,RULES, andINVARIANTSsections...” reads awkwardly in a normative instruction and can be interpreted inconsistently. Prefer “requires the generated ... sections to be ...”.Suggested edit
-The final prompt also requires generated `STATE`, `WHEN`, `DO`, `RULES`, -and `INVARIANTS` sections to be list blocks whose top-level items start with +The final prompt also requires the generated `STATE`, `WHEN`, `DO`, `RULES`, +and `INVARIANTS` sections to be list blocks whose top-level items start with the starter keywords allowed by `architecture/specs/PDSL.md`:🤖 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 `@skills/studio/agents/cf-pdsl-author.md` around lines 18 - 20, Replace the awkward normative sentence in the cf-pdsl-author.md draft so it reads "requires the generated `STATE`, `WHEN`, `DO`, `RULES`, and `INVARIANTS` sections to be list blocks whose top-level items start with the starter keywords allowed by `architecture/specs/PDSL.md`" instead of the current phrasing; update the sentence that currently begins "The final prompt also requires generated ..." to this revised wording to remove ambiguity and ensure it clearly mandates the format.skills/studio/agents/cf-ralphex.md-80-80 (1)
80-80:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix duplicated modal keyword in CLI rule text.
ALWAYS alwaysis a typo in a normative rule line. Keep oneALWAYSto avoid ambiguity in contract wording.🤖 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 `@skills/studio/agents/cf-ralphex.md` at line 80, Fix the duplicated modal keyword in the CLI rule text by removing the extra "always" so the rule reads "ALWAYS invoke as `{cfs_cmd} delegate ...` without `--json` (replace the current line that says `ALWAYS always invoke as `{cfs_cmd} delegate ...` without `--json`) in skills/studio/agents/cf-ralphex.md; update only that text token to a single "ALWAYS" to preserve normative contract wording.
🧹 Nitpick comments (2)
skills/studio/agents/cf-brainstorm-facilitator.md (1)
104-106: ⚡ Quick winNormalize duplicated modal wording in completion rules.
These lines use repeated control tokens (
ALWAYS ... ALWAYS be), which makes constraints harder to read and audit. Keep one modal per rule (e.g., “ALWAYS proposed_panel length be 3..6”).🤖 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 `@skills/studio/agents/cf-brainstorm-facilitator.md` around lines 104 - 106, The three completion-rule lines use duplicated modal tokens; update them to use a single modal per rule for clarity—change "ALWAYS proposed_panel length ALWAYS be 3..6" to "ALWAYS proposed_panel length be 3..6", "ALWAYS proposed_panel[*].id values ALWAYS be unique" to "ALWAYS proposed_panel[*].id values be unique", and "ALWAYS proposed_panel[*].focus lists NEVER overlap materially" to "NEVER proposed_panel[*].focus lists overlap materially" (or consistently keep the modal as ALWAYS/NEVER at the start) so each rule contains only one modal and the identifiers proposed_panel, proposed_panel[*].id, and proposed_panel[*].focus remain unchanged.architecture/specs/shared-context-pack.md (1)
199-203: ⚡ Quick winSimplify execution-boundary rules to one modal per line.
The repeated pattern (
ALWAYS A dispatched sub-agent ALWAYS ...) is harder to parse than needed in a normative spec. Use one modal keyword per rule to keep controller/leaf boundary constraints crisp.🤖 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 `@architecture/specs/shared-context-pack.md` around lines 199 - 203, Replace the repeated double-modal phrasing like "ALWAYS A dispatched sub-agent ALWAYS ..." with single-modal rules per line so each constraint starts with one modal keyword (e.g., "ALWAYS") followed by the statement; specifically update the lines that currently read "ALWAYS A dispatched sub-agent ALWAYS receive a fully materialized final prompt", "ALWAYS A dispatched sub-agent NEVER load workflow, skill, requirement, spec, or AGENTS prompt files from disk", "ALWAYS A dispatched sub-agent NEVER discover prompt dependencies at runtime", and "ALWAYS Missing instruction context is an orchestration failure, not a leaf-agent" so that each rule begins with a single modal token and the remainder is the clear constraint (preserve intent and wording but remove duplicate modal words).
🤖 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 `@skills/studio/agents/storytelling-preflight.md`:
- Around line 143-145: The fallback prompt in DetermineAccessTier is split
across two bullets causing a malformed block; combine the split EMIT and RUN
lines into a single EMIT statement that contains the full message ("I cannot
access the file at the path you provided. Please paste the RUN content directly
into the chat so I can proceed." or similar), remove the stray "- RUN" bullet,
and ensure the block follows the same EMIT/CONTINUE structure used elsewhere so
the parser can consume it correctly.
---
Outside diff comments:
In `@skills/studio/migrate-from-cypilot.md`:
- Around line 257-265: The user-facing guidance is out of sync with MENU
E3_MigratorMenu (which now includes option 4/stop_token); locate the prompt text
that says "Reply with 1, 2, or 3." and update it to match the menu options
(e.g., "Reply with 1, 2, 3, or 4." or replace the hardcoded list with a
generated list from MENU E3_MigratorMenu), making sure references to selection
and the E3_RunMigrator flow remain consistent with the new stop_token option.
In `@workflows/analyze/phase-3-semantic.md`:
- Around line 217-227: The ALWAYS rule is incorrectly placed inside the MENU
block "SemanticPlanRevalidationFailureMenu"; move the line "ALWAYS enforce all
PlannedMultiReviewerDispatch INVARIANTS (see above) when REVIEWER_EXECUTION_PLAN
is non-null and dispatch is active." out of the MENU and into the enclosing
unit-level RULES:/INVARIANTS: section so the invariant is declared at unit scope
rather than inside the menu; ensure the rule references the same symbols
(PlannedMultiReviewerDispatch, REVIEWER_EXECUTION_PLAN, dispatch) and retains
its condition and enforcement text verbatim when relocated.
In `@workflows/analyze/preamble.md`:
- Around line 140-142: The prompt-review branch currently sets
PROMPT_REVIEW=true and EXPLAIN_MODE=false but fails to restore or preserve
enforceRemediationPrompts set earlier in the storytelling branch; update the
logic around the branch that sets PROMPT_REVIEW (and the analogous block at
lines ~163-164) to explicitly preserve or re-enable enforceRemediationPrompts
when switching off EXPLAIN_MODE so the analyze-mode invariant is
maintained—i.e., after setting PROMPT_REVIEW=true and EXPLAIN_MODE=false, copy
or set enforceRemediationPrompts to the true value it had for storytelling, or
compute it from the same source used by the storytelling branch.
In `@workflows/explore.md`:
- Around line 116-125: The ExploreSaveMenu contains duplicate selector '3'
causing ambiguous routing; update one of the conflicting menu selectors so each
branch has a unique numeric choice (e.g., change the selector for the "skip |
no" branch or the "SAVE_BUNDLE folder=<user path>" branch), and then update any
routing/dispatch logic that expects selector '3' to use the new number; ensure
the labels referenced in the diff (SAVE_BUNDLE, "EMIT \"Saved exploration bundle
to <user path>.\"", and the skip | no branch with "EMIT \"Skipped saving. No
files were written.\"") are preserved and wired to the new unique selector.
In `@workflows/generate/phase-0.7/wrap-handoff.md`:
- Around line 55-56: The menu text still references "1..5" but you've added
option 6; update the TITLE line ("Brainstorm complete — choose next step (reply
1, 2, 3, 4, or 5)") and the invalid-reply hint(s) in this file to include option
6 (e.g., "reply 1, 2, 3, 4, 5, or 6" and "1..6") or alternatively remove
explicit numeric ranges so token-only behavior is unambiguous; make the same
change for the other occurrence around the 91-97 block so all user-facing
prompts consistently reflect the new option.
In `@workflows/generate/phase-6/post-write-handoff.md`:
- Around line 43-55: The top-level OPTIONS menu entries in post-write-handoff.md
(specifically the entries labeled "W2 ->" and "W3 ->") must be renumbered to
follow the execution-card contract (decimal numbered options); replace the
nonconforming "W2 ->" and "W3 ->" labels with numbered OPTION entries (e.g., "2.
->" and "3. ->" or "2." and "3.") so every top-level OPTIONS item uses a decimal
number and keep the existing text for each option unchanged.
---
Major comments:
In `@skills/studio/agents/cf-analyze-planner.md`:
- Around line 212-216: The completion gate currently contradicts itself: allow
tasks=[] when mode=explain but still enforce the rule "ALWAYS have at least one
task per active methodology" unconditionally; update the gate to make the
per-methodology task requirement conditional on mode != "explain" (i.e., only
enforce the "ALWAYS have at least one task per active methodology" rule when
mode is not "explain"), and mention that when mode=explain the global "tasks=[]
allowed" exception supersedes the per-methodology requirement; ensure references
to "tasks", "mode=explain", and "methodology_flags" in cf-analyze-planner.md are
adjusted accordingly.
In `@skills/studio/agents/storytelling-context-pack.md`:
- Around line 203-208: The hot-anchor selection rule under "RUN WHEN strategy ==
\"hybrid\"" currently references plan_anchor_map before it is constructed; move
the construction of plan_anchor_map earlier (so it's built before the hybrid RUN
WHEN block) or modify the RUN WHEN logic to only rely on data available prior to
Step 5. Specifically, ensure plan_anchor_map (the map built in Step 5) is
created before the hot-anchor selection that sets is_hot based on anchor being
referenced by >= 2 plan items, or change that condition to use only anchor.index
== 0 and anchor.byte_count > hot_threshold_bytes until plan_anchor_map exists.
In `@skills/studio/agents/storytelling-export.md`:
- Around line 231-234: The RULES block has outdented SEE_ALSO bullets (e.g.,
"SEE_ALSO: Step3_WriteIndex") that may be parsed outside the RULES section and
break the PDSL shape; fix by indenting the SEE_ALSO lines to match the other
RULES bullets under the RULES header (ensure "SEE_ALSO: Step3_WriteIndex" and
any similar entries at lines ~372-385 are prefixed with the same
bullet/indentation level as "ALWAYS use only relative paths..." so they remain
children of RULES), verify formatting consistency for all RULES entries so
keyword/shape validation succeeds.
In `@skills/studio/agents/storytelling-gate.md`:
- Around line 587-589: Unify the invalid-parse failure mode to use a single
canonical next_action value (choose "re-render") across the code: update the
EMIT/RETURN that currently returns next_action="render" for strict-gate invalid
replies to return next_action="re-render", and update any other spots that check
or emit next_action (including Phase2Parse, ResponseCompletionGate, and the
output contract that references selected_tag/next_action) so they all expect
"re-render" instead of "render"; ensure the emitted message text ("Invalid reply
— please enter a number between 1 and N.") and the returned selected_tag=null
remain unchanged.
In `@skills/studio/migrate-from-cypilot.md`:
- Around line 77-79: The precondition question was split across an EMIT and a
separate RUN, causing the prompt text to be emitted incorrectly; update the
sequence so the full question is emitted as a single EMIT action (combine the
text "Has the deterministic migration (cfs init --migrate-from-cypilot=yes or
equivalent) RUN completed successfully? [y/N]" into one EMIT) and remove the
extraneous separate RUN that only contains user.reply so the prompt appears as
one complete message; adjust the EMIT/RUN pair around the precondition check to
use EMIT for the entire question and then WAIT or RUN user.reply as before.
In `@tests/test_pdsl_keywords.py`:
- Around line 99-120: The _iter_pdsl_blocks function currently drops an
unterminated pdsl block at EOF causing false-negatives; modify _iter_pdsl_blocks
so that after the for-loop completes it checks if in_pdsl is still True and, if
so, appends the pending block (use the existing start_line and current) to
blocks before returning; ensure you only append when current has content and
preserve the same tuple shape list[tuple[int, list[str]]] so downstream
validators see the unterminated block.
In `@workflows/analyze/context-budget.md`:
- Around line 19-21: The WHEN predicate "REQUIRE AnalyzePhase0 has completed
dependency resolution AND Phase 0.1 or later analysis is about to load large
documents OR estimated total context would exceed 1200 retained
instruction/input lines" allows activation via the right-hand clause before
AnalyzePhase0 finishes; change the logic to require AnalyzePhase0 first by
parenthesizing the latter two conditions (i.e. REQUIRE AnalyzePhase0 has
completed dependency resolution AND (Phase 0.1 or later analysis is about to
load large documents OR estimated total context would exceed 1200 retained
instruction/input lines)), and make the same adjustment where the same predicate
appears at lines noted (applies to 51-51).
In `@workflows/analyze/phase-2.5-reviewer-plan.md`:
- Around line 63-67: StorageChoiceMenu currently maps the literal token "3" to
two different targets (the rule starting "3 stop_token ->" which loads
stop-token-policy.md and triggers STOP_TURN, and the rule "4 no|skip|3 ->" which
emits the decomposition-required message), causing ambiguous routing for input
"3"; fix by making the menu tokens mutually exclusive — either change the first
rule's trigger from "3" to an explicit distinct token (e.g., "stop" or
"stop_token") or remove "3" from the second rule's alternation (no|skip|3),
ensuring only one rule references "3" and preserving the behavior of STOP_TURN
in the rule that should handle stopping (refer to the rules containing
stop_token, no|skip|3, STOP_TURN, and the loaded stop-token-policy.md).
In `@workflows/analyze/phase-3-semantic.md`:
- Around line 19-23: The WHEN block contains a non-boolean explanatory line
("REQUIRE Note: SEMANTIC_ONLY==true bypasses...") which breaks PDSL shape;
remove that explanatory line from the WHEN/REQUIRE list and relocate it to a
NOTES: or RUN NOTE section instead, keeping the declarative REQUIRE entries
(e.g., REQUIRE deterministic gate is PASS or SKIPPED (with validator
availability proof) OR SEMANTIC_ONLY == true and the existing
REVIEWER_PLAN_RESOLVED requirement) unchanged; ensure references to
SEMANTIC_ONLY and REVIEWER_PLAN_RESOLVED remain as boolean guards and add the
explanatory prose under NOTES or RUN NOTE so the parser only sees boolean
predicates in WHEN.
In `@workflows/explain.md`:
- Around line 42-51: Consolidate the fragmented RUN directives into one clear,
executable clause: replace the multiple partial RUN lines with a single cohesive
instruction that invokes the shared gate
({cf-studio-path}/.core/workflows/shared/explore-brainstorm-gate.md), requires
cf-explore when explanation targets are implicit, and mandates emitting the
EXPLAIN_RESULT envelope (with fields type, status, session_id, progress,
resume_path) on every exit
(complete/checkpointed/cancelled/deterministic-failure/wrap); ensure
deterministic validation failures map to status="checkpointed" with
failure/resume metadata and that EXPLAIN_MODE can be overridden into remediation
output before storytelling begins (references: EXPLAIN_RESULT, EXPLAIN_MODE,
cf-explore, explore-brainstorm-gate.md, analyze.md).
In `@workflows/generate/phase-0-git-commit-mode.md`:
- Around line 32-49: The EMIT exactly block is malformed because the `- RUN ...`
lines are at the top level instead of being nested under the `EMIT exactly` list
item; fix by moving all `RUN` lines (the whole git-permission table, suggested
line, reply prompt and the `WAIT user.reply`) to be indented under the `EMIT
exactly` item so they are a child block of that list entry (i.e., ensure `EMIT
exactly:` is the parent and the `RUN ...` lines are nested beneath it as its
content).
In `@workflows/generate/phase-0.7/panel-selection.md`:
- Around line 45-50: The rendered menu numbers and the parser routing are
inconsistent: the emitted menu shows "2=seed" while PanelEditLoop's MENU routes
2=>accept and 6=>seed, causing numeric replies to hit wrong branches; update the
MENU mapping inside PanelEditLoop (and any related handlers referenced in the
same block/section) so that option 2 routes to the seed-edit handler and option
6 routes to accept (or vice versa if you prefer the other canonical mapping),
and ensure the emitted menu text (the lines around the emission logic) and the
parser branches for actions like start, accept, seed, drop, swap, add, wrap
(referenced by symbol PanelEditLoop and MENU) are consistent across lines ~57-87
so numeric choices execute the intended handlers.
In `@workflows/generate/phase-0.7/round-loop.md`:
- Around line 99-104: The EMIT line uses {agent} before it is deterministically
set, so define and set an explicit agent variable (e.g., determine agent from
panel_mode and pending_round_kind: for panel_mode == "resolved" set
agent="cf-brainstorm-panel", otherwise set agent="cf-brainstorm-expert" or
follow mode_source rules) before calling Phase07AgentAvailabilityCheck and
before emitting the checkpoint; update the block that dispatches to
Phase07TopicDispatch/Phase07ChallengeDispatch (and the similar occurrence around
Phase07AgentAvailabilityCheck elsewhere) to reference this explicit agent and
ensure panel_size is computed from state.panel consistently.
- Around line 258-260: The repair payload construction uses undefined symbols
(repair_feedback = { mode, panel_mode, protocol, violations, prior_contributions
}) which will break the retry dispatch; update the payload to use defined
variables or explicit fields present in this flow (for example replace mode with
the current flow variable like current_mode or derive it from context, replace
violations with detected_violations or validation_errors, ensure
prior_contributions is populated from the prior_contributions variable in state,
and keep panel_mode/protocol only if they exist), then pass that corrected
object into the DISPATCH previous validation target WITH repair_feedback signal
and keep SET attempts_used = 2 unchanged; ensure the final payload shape matches
the retry dispatch contract expected by the receiver.
In `@workflows/generate/phase-1.5/offer-dispatch.md`:
- Around line 41-53: The EMIT exactly block currently converts display text into
executable RUN actions; change the two occurrences (the EMIT exactly section
starting around the “Author plan (mandatory — sub-agents approved): pick
storage.” block and the Phase15OptionalOffer block referenced in the review) so
that the prompt body remains plain emitted text rather than `RUN` lines—remove
the `RUN` prefixes and restore the text as literal emission (keeping the same
wording about "enter", "memory", "disk" and guidance) for both the Phase 1.5
EMIT block and the Phase15OptionalOffer block so the UX contract emits static
prompt text instead of actionable directives.
In `@workflows/generate/phase-1.5/state-contract.md`:
- Around line 65-68: The auto-skip rules are inconsistent with the declared
auto_skip_condition: update the contract so the ALWAYS auto-skip predicates
match the OR derivation of auto_skip_condition (auto_skip_condition should
reflect whether ANY of the predicates is true) or change the ALWAYS list to
require both predicates and update the derivation to an AND; specifically,
reconcile the symbols auto_skip_condition, the ALWAYS Auto-skip conditions
block, the CLI flag --no-author-plan, and KIND's rules.md author_plan =
"disabled" by either (A) making the ALWAYS section assert that any one of the
predicates can trigger skipping (i.e., list them as independent ALWAYS triggers)
and leave auto_skip_condition as OR, or (B) change auto_skip_condition to an AND
if both predicates must be true—pick one consistent approach and update the
contract text so auto_skip_condition and the ALWAYS conditions use the same
logical connective.
In `@workflows/generate/phase-5/index.md`:
- Around line 91-99: The `- RUN ...` lines after the "EMIT exactly:" heading are
incorrectly outdented and therefore parsed as top-level items; indent those `-
RUN How many automatic review iterations...`, `- RUN Each iteration:
validate...`, and `- RUN Reply with a number...` lines so they are nested under
the "EMIT exactly:" block in both occurrences (the block at lines 91-99 and the
similar block at 137-146), ensuring they are treated as the literal payload to
emit rather than separate actions; locate the "EMIT exactly:" headings and
adjust the child `- RUN` lines' indentation to match the intended block nesting.
In `@workflows/generate/phase-5/phase-5.1-det-gate.md`:
- Around line 22-25: The gate currently reads "REQUIRE validator source contract
is not loaded, unreadable, ambiguous, or not reflected..." which inverts the
intended precondition and causes valid states to fail; update the REQUIRE to
assert the good state instead (e.g., "REQUIRE validator source contract is
loaded, readable, unambiguous, and reflected in the final dispatch prompt") or
alternatively split into two branches: a positive branch that continues to
dispatch and an explicit failure branch that references SubAgentContractReadGate
and the existing "FAIL ... - NEVER dispatch" outcome; ensure the edited clause
replacing the inverted text still references SubAgentContractReadGate and
preserves the "NEVER dispatch" failure path for actual bad states.
In `@workflows/generate/phase-5/phase-5.5-final.md`:
- Around line 24-29: The lines in the "RUN APPEND Semantic Review summary block"
are modeled as procedural steps but should be literal canonical output; change
the leading keyword from RUN to EMIT for the summary block and its inner lines
(e.g., replace "RUN APPEND Semantic Review summary block:", "RUN ---", "RUN
Semantic Review: ...", "RUN remaining_findings = ...", and "RUN
det_gate_final_result = ..." with EMIT equivalents) so these lines are emitted
as the canonical Validation Results body rather than executed as steps.
In `@workflows/generate/phase-6/index.md`:
- Around line 119-127: The numbered-reply contract is broken because the EMIT
options under the "EMIT exactly:" block are not labeled with option numbers
while the prompt asks the user to "Reply with the option number"; update the
emitted menu text in workflows/generate/phase-6/index.md (the EMIT exactly block
and the alternate branch at lines ~132-138) to prefix each choice with an
explicit numeric label (e.g., "1. RUN ---", "2. RUN What would you like to do
next?", etc.), ensure numbering is consistent across both branches, and keep the
final prompt text unchanged so users can reliably reply using the option number.
In `@workflows/generate/validation-criteria.md`:
- Around line 168-171: AgentSelfTestRelaxed currently models the skip message as
three separate RUN steps which prevents emitting the required exact payload;
change the implementation so AgentSelfTestRelaxed emits the entire literal skip
block as a single emission (one payload) containing the three lines "RUN ---",
"RUN ⚠️ Self-test skipped (RELAXED mode — no Constructor Studio rules)", and
"RUN ---" exactly, rather than three separate RUN events, so the validator
receives the exact string as specified.
In `@workflows/pdsl/new.md`:
- Around line 27-29: The REQUIRE block is misindented and sits outside the WHEN
block; move the "- REQUIRE:" group so it is indented under the "WHEN:" clause to
follow the PDSL "Core Shape" (so that REQUIRE conditions like "target_paths
contains exactly one output path" and "user intent or source context is
available" are parsed as part of WHEN). Ensure the "- REQUIRE:" line and its
nested bullets (target_paths and user intent/source context) are indented to the
same level as other clauses under WHEN so the grouped requirements are scoped
correctly to the WHEN block.
In `@workflows/pdsl/review.md`:
- Around line 28-31: The REQUIRE precondition block (checking target_paths
non-empty and SUB_AGENT_SESSION_APPROVED == true OR INLINE_FALLBACK == true) is
currently outside any WHEN/DO section and therefore won't be enforced; move or
nest this REQUIRE block under the appropriate WHEN or DO section so it executes
as a guard for the related step. Specifically, locate the REQUIRE lines
referencing target_paths, SUB_AGENT_SESSION_APPROVED, and INLINE_FALLBACK and
place them inside the relevant WHEN/DO block that governs the operation they
protect, ensuring the conditions run before the action they guard.
In `@workflows/pdsl/transform.md`:
- Around line 28-29: The REQUIRE block is malformed because it nests a second
REQUIRE entry; replace the nested form so there's a single flattened requirement
entry (e.g., change the two lines "- REQUIRE:" and "- REQUIRE target_paths is
non-empty" into a single line like "- REQUIRE: target_paths is non-empty") to
remove the ambiguous/nested section for the REQUIRE directive and ensure only
one REQUIRE entry referencing target_paths appears.
In `@workflows/plan/phase-1-assess.md`:
- Around line 200-206: The workflow currently halts with STOP_TURN when the
manifest input_signature matches the dry-run signature; change this so the flow
does not terminate but continues the run. Replace the STOP_TURN action in the
matching branch with the appropriate continuation action (e.g., proceed/advance
the plan path or invoke the existing continueRun()/advancePhase()/NEXT_PHASE
transition used elsewhere) so the run advances past Phase 1 and resumes normal
routing instead of exiting; keep the “REMAIN on plan path and reuse that
authoritative raw-input package” behavior but ensure the state transition
advances rather than stops.
- Around line 124-131: The dry-run chunk-input command uses {task-slug} in its
output path before {task-slug} is computed; move the "RUN COMPUTE {task-slug}"
step to occur before the "RUN COMPUTE plan.input_signature" / before invoking
the chunk-input dry-run so the output-dir path
{cf-studio-path}/.plans/{task-slug}/input is resolved; update the sequence so
{task-slug} is derived first, then run the chunk-input dry-run and set
plan.input_signature from its returned signature.
In `@workflows/plan/phase-4-finalize.md`:
- Line 40: The line "SET CONTINUE Phase4StatusMapping" is outdented and must be
moved inside the enclosing "STATE" block; update the indentation/scope so that
"SET CONTINUE Phase4StatusMapping" is indented to match the other entries under
"STATE" (so it is parsed as part of that block) to ensure the continuation
mapping runs during Phase 4 initialization.
In `@workflows/plan/plan-lifecycle.md`:
- Around line 98-100: Update the lifecycle semantics so they are consistent:
keep the implemented behavior that setting plan.lifecycle_status = "failed"
occurs when cleanup is attempted and errors (e.g., file
removal/permission/unexpected state), and change the contradictory rule text
that currently defines "failed" as “cleanup never attempted” to instead state
"failed = cleanup attempted and failed"; update the other mention(s) of the old
definition (the occurrence that says cleanup never attempted) to use a different
status or wording (e.g., "cleanup_not_attempted" or "pending_cleanup") if
needed, and ensure all references to plan.lifecycle_status and "failed" in the
doc reflect this single, consistent semantics.
- Around line 88-90: The sentence is split into two separate RUN directives
causing ambiguity; combine the split lines into a single RUN directive so the
sentence reads continuously (e.g., "RUN RESERVE a final Cleanup phase now so
total_phases, dependencies, briefs, and RUN budget estimates are structurally
correct before plan.toml is written" should be rewritten to a single coherent
RUN statement), ensuring the RUN keyword appears only once and the terms
total_phases, dependencies, briefs, and budget estimates remain in the same
clause for PDSL execution.
In `@workflows/shared/mode-resolution.md`:
- Around line 87-90: The warning text for ValidationFailWarning is mismatched:
it triggers when rules_mode == STRICT but the emitted message says "(RELAXED
mode)"; update the emitted string in the DO example so the message reads "⚠️
Validated — FAIL (STRICT mode): rules applied but validation could not reach
PASS" (i.e., replace "RELAXED" with "STRICT" in the ValidationFailWarning
emission).
In `@workflows/workspace/phase-2-configure.md`:
- Around line 82-93: The current flow uses "REQUIRE ... / RUN otherwise" which
treats REQUIRE as a precondition and can short-circuit the normal path; replace
that pattern with an explicit conditional branch: check if the user's updated
location is inline AND the current source uses a Git URL, then EMIT "inline
config does not support Git URL sources — a standalone location is required.",
reset location to standalone, re-show the current source proposal, WAIT
user.reply and STOP_TURN; otherwise (else branch) run the existing "Update
global location choice", re-show the current source proposal, WAIT user.reply
and STOP_TURN. Ensure you update the branch structure wherever "REQUIRE" and
"RUN otherwise" appear in the location update flow so the non-inline path
executes deterministically.
---
Minor comments:
In `@skills/studio/agents/cf-pdsl-author.md`:
- Around line 18-20: Replace the awkward normative sentence in the
cf-pdsl-author.md draft so it reads "requires the generated `STATE`, `WHEN`,
`DO`, `RULES`, and `INVARIANTS` sections to be list blocks whose top-level items
start with the starter keywords allowed by `architecture/specs/PDSL.md`" instead
of the current phrasing; update the sentence that currently begins "The final
prompt also requires generated ..." to this revised wording to remove ambiguity
and ensure it clearly mandates the format.
In `@skills/studio/agents/cf-ralphex.md`:
- Line 80: Fix the duplicated modal keyword in the CLI rule text by removing the
extra "always" so the rule reads "ALWAYS invoke as `{cfs_cmd} delegate ...`
without `--json` (replace the current line that says `ALWAYS always invoke as
`{cfs_cmd} delegate ...` without `--json`) in
skills/studio/agents/cf-ralphex.md; update only that text token to a single
"ALWAYS" to preserve normative contract wording.
In `@workflows/plan/phase-0-discover.md`:
- Around line 26-27: Update the malformed sentence in the NOTES paragraph so it
reads unambiguously: replace "where they ALWAYS be written into the [meta] TOML
table of plan.toml" with "where they ALWAYS are written into the [meta] TOML
table of plan.toml" (keeping the reference to Phase 3.1 and the [meta] table
intact).
---
Nitpick comments:
In `@architecture/specs/shared-context-pack.md`:
- Around line 199-203: Replace the repeated double-modal phrasing like "ALWAYS A
dispatched sub-agent ALWAYS ..." with single-modal rules per line so each
constraint starts with one modal keyword (e.g., "ALWAYS") followed by the
statement; specifically update the lines that currently read "ALWAYS A
dispatched sub-agent ALWAYS receive a fully materialized final prompt", "ALWAYS
A dispatched sub-agent NEVER load workflow, skill, requirement, spec, or AGENTS
prompt files from disk", "ALWAYS A dispatched sub-agent NEVER discover prompt
dependencies at runtime", and "ALWAYS Missing instruction context is an
orchestration failure, not a leaf-agent" so that each rule begins with a single
modal token and the remainder is the clear constraint (preserve intent and
wording but remove duplicate modal words).
In `@skills/studio/agents/cf-brainstorm-facilitator.md`:
- Around line 104-106: The three completion-rule lines use duplicated modal
tokens; update them to use a single modal per rule for clarity—change "ALWAYS
proposed_panel length ALWAYS be 3..6" to "ALWAYS proposed_panel length be 3..6",
"ALWAYS proposed_panel[*].id values ALWAYS be unique" to "ALWAYS
proposed_panel[*].id values be unique", and "ALWAYS proposed_panel[*].focus
lists NEVER overlap materially" to "NEVER proposed_panel[*].focus lists overlap
materially" (or consistently keep the modal as ALWAYS/NEVER at the start) so
each rule contains only one modal and the identifiers proposed_panel,
proposed_panel[*].id, and proposed_panel[*].focus remain unchanged.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8195ac64-4014-41c2-80cf-427c61ce65ef
📒 Files selected for processing (145)
.gitignorearchitecture/specs/PDSL.mdarchitecture/specs/artifacts-registry.mdarchitecture/specs/shared-context-pack.mdarchitecture/specs/sysprompts.mdrequirements/auto-config.mdrequirements/pdsl-execution-card.mdrequirements/reverse-engineering.mdrequirements/storytelling.mdskills/studio/SKILL.mdskills/studio/agents/author-production-rules.mdskills/studio/agents/cf-analyze-planner.mdskills/studio/agents/cf-brainstorm-expert.mdskills/studio/agents/cf-brainstorm-facilitator.mdskills/studio/agents/cf-brainstorm-panel.mdskills/studio/agents/cf-code-bug-finder.mdskills/studio/agents/cf-codegen.mdskills/studio/agents/cf-deterministic-validator.mdskills/studio/agents/cf-diff-scope-resolver.mdskills/studio/agents/cf-explorer.mdskills/studio/agents/cf-generate-author-worker.mdskills/studio/agents/cf-generate-author.mdskills/studio/agents/cf-generate-collector.mdskills/studio/agents/cf-generate-planner.mdskills/studio/agents/cf-migrate-migrator.mdskills/studio/agents/cf-migrate-planner.mdskills/studio/agents/cf-migrate-scanner.mdskills/studio/agents/cf-migrate-verifier.mdskills/studio/agents/cf-pdsl-author.mdskills/studio/agents/cf-pdsl-reviewer.mdskills/studio/agents/cf-pdsl-transformer.mdskills/studio/agents/cf-phase-compiler.mdskills/studio/agents/cf-phase-runner.mdskills/studio/agents/cf-pr-review.mdskills/studio/agents/cf-prompt-bug-finder.mdskills/studio/agents/cf-ralphex.mdskills/studio/agents/cf-semantic-reviewer-artifact.mdskills/studio/agents/cf-semantic-reviewer-code.mdskills/studio/agents/cf-semantic-reviewer-consistency.mdskills/studio/agents/cf-semantic-reviewer-prompt.mdskills/studio/agents/storytelling-context-pack.mdskills/studio/agents/storytelling-export.mdskills/studio/agents/storytelling-gate.mdskills/studio/agents/storytelling-preflight.mdskills/studio/agents/storytelling-wrap.mdskills/studio/migrate-from-cypilot.mdskills/studio/protocol.mdskills/studio/routing.mdskills/studio/sub-agent-dispatch.mdtests/test_pdsl_keywords.pytests/test_pdsl_transform_equivalence.pytests/test_workflow_lazy_loading.pytests/test_workflow_subagents_dispatch.pyworkflows/analyze.mdworkflows/analyze/agent-self-test.mdworkflows/analyze/context-budget.mdworkflows/analyze/key-principles.mdworkflows/analyze/overview.mdworkflows/analyze/phase-0-change-review-scope.mdworkflows/analyze/phase-0-dependencies.mdworkflows/analyze/phase-0.1-plan-escalation-gate.mdworkflows/analyze/phase-0.5-scope.mdworkflows/analyze/phase-1-file-check.mdworkflows/analyze/phase-2-det-gate.mdworkflows/analyze/phase-2.5-reviewer-plan.mdworkflows/analyze/phase-3-semantic.mdworkflows/analyze/phase-3-to-4-checkpoint.mdworkflows/analyze/phase-4-output/fix-prompt-template.mdworkflows/analyze/phase-4-output/index.mdworkflows/analyze/phase-4-output/output-prompt-review.mdworkflows/analyze/phase-4-output/output-standard.mdworkflows/analyze/phase-4-output/output-storytelling.mdworkflows/analyze/phase-4-output/plan-prompt-template.mdworkflows/analyze/phase-4-output/remediation-handoff.mdworkflows/analyze/phase-5-next-steps.mdworkflows/analyze/preamble.mdworkflows/analyze/rules.mdworkflows/analyze/validation-criteria.mdworkflows/auto-config.mdworkflows/brainstorm.mdworkflows/explain.mdworkflows/explore.mdworkflows/generate.mdworkflows/generate/error-handling.mdworkflows/generate/phase-0-dependencies.mdworkflows/generate/phase-0-git-commit-mode.mdworkflows/generate/phase-0.2-review-loop-cfg.mdworkflows/generate/phase-0.5-clarify.mdworkflows/generate/phase-0.7/index.mdworkflows/generate/phase-0.7/offer.mdworkflows/generate/phase-0.7/panel-selection.mdworkflows/generate/phase-0.7/round-loop.mdworkflows/generate/phase-0.7/save-and-rules.mdworkflows/generate/phase-0.7/state-schema.mdworkflows/generate/phase-0.7/wrap-handoff.mdworkflows/generate/phase-1-collect.mdworkflows/generate/phase-1.5-author-plan.mdworkflows/generate/phase-1.5/disk-mode.mdworkflows/generate/phase-1.5/offer-dispatch.mdworkflows/generate/phase-1.5/state-contract.mdworkflows/generate/phase-2-checkpoint.mdworkflows/generate/phase-3-summary.mdworkflows/generate/phase-4-write.mdworkflows/generate/phase-5/index.mdworkflows/generate/phase-5/phase-5.1-det-gate.mdworkflows/generate/phase-5/phase-5.2-semantic.mdworkflows/generate/phase-5/phase-5.3-findings.mdworkflows/generate/phase-5/phase-5.4-approval.mdworkflows/generate/phase-5/phase-5.5-final.mdworkflows/generate/phase-6/index.mdworkflows/generate/phase-6/post-write-handoff.mdworkflows/generate/phase-6/prompt-template-direct-review.mdworkflows/generate/phase-6/prompt-template-fix.mdworkflows/generate/phase-6/prompt-template-plan-review.mdworkflows/generate/phase-6/prompt-template-plan.mdworkflows/generate/phase-6/prompt-templates.mdworkflows/generate/phase-6/remediation-handoff.mdworkflows/generate/reverse-engineering.mdworkflows/generate/validation-criteria.mdworkflows/help.mdworkflows/map.mdworkflows/pdsl.mdworkflows/pdsl/new.mdworkflows/pdsl/review.mdworkflows/pdsl/transform.mdworkflows/plan.mdworkflows/plan/phase-0-discover.mdworkflows/plan/phase-1-assess.mdworkflows/plan/phase-2-decompose.mdworkflows/plan/phase-3-compile.mdworkflows/plan/phase-4-finalize.mdworkflows/plan/plan-lifecycle.mdworkflows/plan/plan-reference.mdworkflows/shared/explore-brainstorm-gate.mdworkflows/shared/inline-fallback-probe.mdworkflows/shared/mode-resolution.mdworkflows/shared/plan-escalation-gate.mdworkflows/shared/stop-token-policy.mdworkflows/studio.mdworkflows/workspace.mdworkflows/workspace/next-steps.mdworkflows/workspace/phase-1-discover.mdworkflows/workspace/phase-2-configure.mdworkflows/workspace/phase-3-generate.mdworkflows/workspace/phase-4-validate.md
Signed-off-by: ainetx <viator@via-net.org>
Signed-off-by: ainetx <viator@via-net.org>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/studio/agents/cf-generate-author.md (1)
266-267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
contributing_guidetype mismatch inauthor_selectionJSON contract.
contributing_guideis documented as object-or-null, but the example value is a quoted string. This makes the contract ambiguous for consumers.Proposed contract fix
- "contributing_guide": "<object {path, directives}> | null", + "contributing_guide": {"path": "<path>", "directives": ["<directive>", "..."]} | null,🤖 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 `@skills/studio/agents/cf-generate-author.md` around lines 266 - 267, The JSON contract for author_selection currently declares contributing_guide as an object-or-null but the example shows a quoted string; update the contract so contributing_guide's type matches the example by changing its declaration to "<string | null>" (or, if you prefer object semantics, change the example to an actual object/null). Ensure the symbol contributing_guide in the author_selection contract is updated and the accompanying description reflects "string | null" (or the chosen object shape) so consumers have an unambiguous contract.
🤖 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.
Outside diff comments:
In `@skills/studio/agents/cf-generate-author.md`:
- Around line 266-267: The JSON contract for author_selection currently declares
contributing_guide as an object-or-null but the example shows a quoted string;
update the contract so contributing_guide's type matches the example by changing
its declaration to "<string | null>" (or, if you prefer object semantics, change
the example to an actual object/null). Ensure the symbol contributing_guide in
the author_selection contract is updated and the accompanying description
reflects "string | null" (or the chosen object shape) so consumers have an
unambiguous contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ed62312-eac0-4815-8932-973a45e889d8
📒 Files selected for processing (8)
architecture/specs/PDSL.mdrequirements/auto-config.mdskills/studio/agents/cf-brainstorm-panel.mdskills/studio/agents/cf-generate-author.mdskills/studio/agents/cf-migrate-migrator.mdskills/studio/agents/storytelling-export.mdskills/studio/agents/storytelling-preflight.mdtests/test_pdsl_keywords.py
🚧 Files skipped from review as they are similar to previous changes (7)
- skills/studio/agents/cf-brainstorm-panel.md
- requirements/auto-config.md
- skills/studio/agents/storytelling-preflight.md
- skills/studio/agents/cf-migrate-migrator.md
- architecture/specs/PDSL.md
- tests/test_pdsl_keywords.py
- skills/studio/agents/storytelling-export.md
This was referenced Jun 2, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Documentation
Tests
Chores