Improve subagent routing and reasoning policies - #37
Merged
Conversation
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-explore,cf-brainstorm
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-analyze
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-analyze
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR refines intent routing across analyze, generate, and plan workflows by adding free-text intent capture, companion multi-skill selection, and "describe intent" flows. Phase agents are split into default (non-isolated) and isolated variants with explicit policy selection. Sub-agent dispatch is restructured with a new SubAgentSelectionRegistry and two-level approval model (native vs inline, with session persistence). Multiple contracts are extracted into dedicated modules (git-commit-mode, template-vars, review gates, creative/language/migration/plan-first gates, next-actions, shutdown). Agents across code generation, PDSL, and phase execution have ChangesIntent Routing, Phase Isolation, and Sub-Agent Dispatch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/SKILL.md`:
- Around line 83-91: The IntentDescribeCapture unit in SKILL.md is intended to
be a generic pattern, but the RULES section (around line 95) contains a
hardcoded reference to AnalyzeRoute, which is specific to the analyze router
defined in workflows/analyze.md. This creates unwanted coupling between a
generic pattern and router-specific logic. To fix this, either (1) remove or
abstract the AnalyzeRoute reference from the RULES of IntentDescribeCapture to
make the pattern truly generic and router-agnostic, or (2) add clear
documentation at the top of IntentDescribeCapture indicating that this is a
template pattern and that actual execution should use router-specific variants
like AnalyzeDescribeIntent and GenerateDescribeIntent defined in their
respective workflow files. Choose the approach that best aligns with your
architecture and document it clearly.
- Around line 146-162: The SubAgentSelectionRegistry unit's DO block describes
the selection procedure (filtering and choosing the cheapest capable candidate)
but lacks an explicit action to set or expose the selected agent for
SubAgentDispatch to consume. Add an explicit DO action (for example, SET
selected_agent = <result of selection logic>) that clearly shows how the
selection result is made available, or add documentation clarifying whether the
selection happens inline during dispatch and what the contract is between
SubAgentSelectionRegistry and SubAgentDispatch. Ensure the output mechanism is
expressed as an executable action in the DO block, not solely as a rule.
In `@workflows/plan.md`:
- Around line 127-128: The markdown file has fence-spacing violations (MD031)
around PDSL code blocks that require fixing. At line 127-128, line 142-143, and
line 154-155, add blank lines before and after the fenced code blocks (the
```pdsl and closing ``` markers) to comply with markdown spacing standards. Each
fenced code block boundary should have a blank line separating it from adjacent
content.
- Around line 132-140: The isolation policy selection condition for
cf-phase-compiler-isolated is overly strict because it requires phase outputs to
already be worktree-visible before selection, but these files don't exist yet on
first compile dispatch. In the RULES section around line 139, modify the ALWAYS
rule for cf-phase-compiler-isolated to remove the requirement that phase outputs
must be tracked or worktree-visible, since those outputs will be generated by
the compiler itself. The condition should only verify that plan.toml, briefs,
and declared output paths are worktree-visible. Apply the same fix to the
corresponding location at lines 221-223 which contains the same overly strict
predicate.
🪄 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: b73893d6-90b4-4ee5-8aa8-7aec23170e58
📒 Files selected for processing (10)
architecture/features/agent-integration.mdarchitecture/features/execution-plans.mdarchitecture/features/subagent-registration.mdskills/studio/SKILL.mdskills/studio/agents.tomltests/test_agents_existing_snapshot.pytests/test_workflow_subagents_dispatch.pyworkflows/analyze.mdworkflows/generate.mdworkflows/plan.md
Comment on lines
127
to
128
| ``` | ||
| ```pdsl |
There was a problem hiding this comment.
Fix MD031 fence-spacing warnings around adjacent PDSL blocks.
Static analysis flags missing blank lines around fenced code block boundaries at Line 127-Line 128, Line 142-Line 143, and Line 154-Line 155.
Also applies to: 142-143, 154-155
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 127-127: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 128-128: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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.md` around lines 127 - 128, The markdown file has
fence-spacing violations (MD031) around PDSL code blocks that require fixing. At
line 127-128, line 142-143, and line 154-155, add blank lines before and after
the fenced code blocks (the ```pdsl and closing ``` markers) to comply with
markdown spacing standards. Each fenced code block boundary should have a blank
line separating it from adjacent content.
Source: Linters/SAST tools
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-analyze
Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-analyze Signed-off-by: ainetx <viator@via-net.org>
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric Studio-Workflows: cf-write-skills,cf-analyze
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/studio/SKILL.md (1)
85-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep free-text intents on the matched-menu path.
Line 96 sends non-empty free text back to
IntentSkillMenuwhen it doesn't span multiple domains, which contradicts the free-text routing rule above and can bounce a valid single-domain intent to the top menu instead of matching it.🛠 Proposed fix
- INVALID -> treat non-empty free text as ORIGINAL_INTENT, load companion-skills module when the text spans domains, run matching, and EMIT_MENU MatchedIntentSkillMenu; otherwise EMIT_MENU IntentSkillMenu + INVALID -> treat non-empty free text as ORIGINAL_INTENT, load companion-skills module when the text spans domains, run matching, and EMIT_MENU MatchedIntentSkillMenu🤖 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/SKILL.md` around lines 85 - 96, The INVALID case in the IntentSkillMenu definition is sending single-domain free text back to IntentSkillMenu instead of matching it. According to the rules in lines 85-89, free text should always be treated as ORIGINAL_INTENT and run matching to emit MatchedIntentSkillMenu. In the INVALID case on line 96, change the logic so that when free text does not span multiple domains (single domain case), it still emits MatchedIntentSkillMenu after running intent matching, rather than reverting to IntentSkillMenu. This ensures valid single-domain intents follow the matched-menu path consistently instead of being bounced back to the top menu.skills/studio/migrate-from-cypilot.md (1)
291-325:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate the canonical footer contract into migrator retries.
E3_RunMigratorforwardsgit_commit_mode,contributing_guide, andgit_constraint, but it omitscommit_footer_contract.GitCommitModeGaterequires that field on every write-capable dispatch payload, so the migrator can enforce the same trailer policy on both the initial run and the E5 retry path.Also applies to: 416-423
🤖 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 291 - 325, The UNIT E3_RunMigrator is missing the commit_footer_contract parameter in its DISPATCH cf-migrate-migrator block. Add commit_footer_contract to the list of parameters passed to the migrator (alongside git_commit_mode, contributing_guide, and git_constraint) and retrieve it from the same source as the other contract values set by GitCommitModeGate. This ensures the migrator can enforce consistent trailer policy on both initial runs and the E5 retry path. Apply the same fix at the other affected location(s) referenced in the comment where similar dispatcher calls exist.
🤖 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 `@architecture/features/agent-integration.md`:
- Around line 128-140: The markdown file contains an orphaned text fragment on
line 140 that reads "approvals, and terminal boundaries -
`inst-companion-multiselect`" which is a dangling duplicate of step 7's
acceptance criteria. This fragment uses an inconsistent spelling of the
instruction ID (multiselect instead of multi-select) and creates conflicting
acceptance criteria. Delete this orphaned tail line entirely, as step 7 (the
checklist item beginning with "If a task maps to multiple domains...") already
contains complete and correct acceptance criteria.
In `@skills/studio/modules/subagents/dispatch.md`:
- Around line 62-68: The SubAgentFallbackRequest menu option for retry lacks a
mechanism to enforce the "at most 2 retries" limit, allowing the failure path to
reopen indefinitely. Add a state variable (e.g., SUB_AGENT_RETRY_COUNT) to track
the number of retry attempts, increment it when the retry option is selected,
and modify the menu logic to only offer the retry option when the count is less
than 2. When the retry limit is reached, the menu should only offer inline or
stop options to prevent infinite retry loops.
---
Outside diff comments:
In `@skills/studio/migrate-from-cypilot.md`:
- Around line 291-325: The UNIT E3_RunMigrator is missing the
commit_footer_contract parameter in its DISPATCH cf-migrate-migrator block. Add
commit_footer_contract to the list of parameters passed to the migrator
(alongside git_commit_mode, contributing_guide, and git_constraint) and retrieve
it from the same source as the other contract values set by GitCommitModeGate.
This ensures the migrator can enforce consistent trailer policy on both initial
runs and the E5 retry path. Apply the same fix at the other affected location(s)
referenced in the comment where similar dispatcher calls exist.
In `@skills/studio/SKILL.md`:
- Around line 85-96: The INVALID case in the IntentSkillMenu definition is
sending single-domain free text back to IntentSkillMenu instead of matching it.
According to the rules in lines 85-89, free text should always be treated as
ORIGINAL_INTENT and run matching to emit MatchedIntentSkillMenu. In the INVALID
case on line 96, change the logic so that when free text does not span multiple
domains (single domain case), it still emits MatchedIntentSkillMenu after
running intent matching, rather than reverting to IntentSkillMenu. This ensures
valid single-domain intents follow the matched-menu path consistently instead of
being bounced back to the top menu.
🪄 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: 784dd8ad-45e4-4ab8-9ad4-1beae1f8ced1
📒 Files selected for processing (24)
architecture/features/agent-integration.mdskills/studio/SKILL.mdskills/studio/agents.tomlskills/studio/agents/cf-generate-author-worker.mdskills/studio/agents/cf-phase-compiler.mdskills/studio/agents/cf-phase-runner.mdskills/studio/agents/cf-semantic-reviewer-consistency.mdskills/studio/agents/cf-semantic-reviewer-prompt.mdskills/studio/migrate-from-cypilot.mdskills/studio/modules/gates/creative-brainstorm-offer.mdskills/studio/modules/gates/language-complexity.mdskills/studio/modules/gates/migrate-from-cypilot-offer.mdskills/studio/modules/gates/plan-first.mdskills/studio/modules/review/finding-contract.mdskills/studio/modules/review/fix-approval.mdskills/studio/modules/routing/companion-skills.mdskills/studio/modules/runtime/template-vars.mdskills/studio/modules/session/shutdown.mdskills/studio/modules/subagents/dispatch.mdskills/studio/modules/subagents/git-commit-mode.mdskills/studio/modules/ui/next-actions.mdtests/test_workflow_parsing.pytests/test_workflow_subagents_dispatch.pyworkflows/plan.md
✅ Files skipped from review due to trivial changes (4)
- skills/studio/modules/review/finding-contract.md
- skills/studio/modules/gates/creative-brainstorm-offer.md
- skills/studio/modules/gates/language-complexity.md
- skills/studio/modules/runtime/template-vars.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/studio/agents.toml
Comment on lines
+128
to
+140
| 7. [ ] - `p1` - If a task maps to multiple domains, agent offers compatible | ||
| companion workflows as a multi-select menu and invokes every selected | ||
| workflow sequentially, preserving each workflow's prerequisites, gates, | ||
| STOP_TURN boundaries, and approval requirements - `inst-companion-multi-select` | ||
| 8. [ ] - `p1` - Root `cf` keeps only always-on bootstrap/routing/memory/command | ||
| rules; every conditional module is loaded through `ConditionalModuleLoading` | ||
| before use, and a rule may be moved out of the root skill only when its | ||
| trigger can be stated as one short stable `BEFORE`/`WHEN` loading rule - | ||
| `inst-cf-conditional-module-loading` | ||
| 9. [ ] - `p1` - During cf load, agent reports both loaded always-on sources and | ||
| the conditional-module trigger table so the user can see which modules will | ||
| load when their conditions fire - `inst-cf-module-load-report` | ||
| approvals, and terminal boundaries - `inst-companion-multiselect` |
There was a problem hiding this comment.
Delete the orphaned checklist tail.
Line 140 is a dangling duplicate of step 7 and even switches the instruction ID spelling, so the acceptance criteria read as two conflicting fragments. Merge it back into step 7 or remove it entirely.
🧹 Proposed fix
- approvals, and terminal boundaries - `inst-companion-multiselect`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 7. [ ] - `p1` - If a task maps to multiple domains, agent offers compatible | |
| companion workflows as a multi-select menu and invokes every selected | |
| workflow sequentially, preserving each workflow's prerequisites, gates, | |
| STOP_TURN boundaries, and approval requirements - `inst-companion-multi-select` | |
| 8. [ ] - `p1` - Root `cf` keeps only always-on bootstrap/routing/memory/command | |
| rules; every conditional module is loaded through `ConditionalModuleLoading` | |
| before use, and a rule may be moved out of the root skill only when its | |
| trigger can be stated as one short stable `BEFORE`/`WHEN` loading rule - | |
| `inst-cf-conditional-module-loading` | |
| 9. [ ] - `p1` - During cf load, agent reports both loaded always-on sources and | |
| the conditional-module trigger table so the user can see which modules will | |
| load when their conditions fire - `inst-cf-module-load-report` | |
| approvals, and terminal boundaries - `inst-companion-multiselect` | |
| 7. [ ] - `p1` - If a task maps to multiple domains, agent offers compatible | |
| companion workflows as a multi-select menu and invokes every selected | |
| workflow sequentially, preserving each workflow's prerequisites, gates, | |
| STOP_TURN boundaries, and approval requirements - `inst-companion-multi-select` | |
| 8. [ ] - `p1` - Root `cf` keeps only always-on bootstrap/routing/memory/command | |
| rules; every conditional module is loaded through `ConditionalModuleLoading` | |
| before use, and a rule may be moved out of the root skill only when its | |
| trigger can be stated as one short stable `BEFORE`/`WHEN` loading rule - | |
| `inst-cf-conditional-module-loading` | |
| 9. [ ] - `p1` - During cf load, agent reports both loaded always-on sources and | |
| the conditional-module trigger table so the user can see which modules will | |
| load when their conditions fire - `inst-cf-module-load-report` |
🤖 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/features/agent-integration.md` around lines 128 - 140, The
markdown file contains an orphaned text fragment on line 140 that reads
"approvals, and terminal boundaries - `inst-companion-multiselect`" which is a
dangling duplicate of step 7's acceptance criteria. This fragment uses an
inconsistent spelling of the instruction ID (multiselect instead of
multi-select) and creates conflicting acceptance criteria. Delete this orphaned
tail line entirely, as step 7 (the checklist item beginning with "If a task maps
to multiple domains...") already contains complete and correct acceptance
criteria.
Signed-off-by: ainetx <viator@via-net.org> Co-authored-by: Constructor Studio <291158726+constructor-studio[bot]@users.noreply.github.com> Studio-Generated-By: Constructor Studio Studio-Source-Repo: https://github.com/constructorfabric/studio Constructor-Fabric: https://github.com/constructorfabric
|
This was referenced Jun 26, 2026
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.
Enhance routing capabilities and refine subagent decision-making processes. Address findings from the review loop and ensure compliance with one-shot dispatch decisions. Adjust reasoning effort for improved performance.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Tests