Refactor skill and workflow generation with required bootstrap - #56
Merged
Conversation
…workflows Signed-off-by: ainetx <viator@via-net.org>
…handling Signed-off-by: ainetx <viator@via-net.org>
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughGenerated agent shims now prepend a required bootstrap unit, use ChangesGenerated bootstrap and control protocol
Sequence Diagram(s)sequenceDiagram
participant cfs_generate_agents as cfs generate-agents
participant GeneratedBootstrapUnit
participant RequiredBootstrap
participant GeneratedFollowProtocol
participant skill_workflow_shim as skill/workflow shim
cfs_generate_agents->>GeneratedBootstrapUnit: prepend bootstrap unit
GeneratedBootstrapUnit->>RequiredBootstrap: LOAD and RUN required-bootstrap.md
GeneratedBootstrapUnit->>GeneratedFollowProtocol: LOAD and RUN <target> as controlling protocol
GeneratedFollowProtocol->>skill_workflow_shim: transfer execution to target
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🧹 Nitpick comments (2)
tests/test_agents_coverage.py (1)
4265-4280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd positive coverage for all accepted managed control-target prefixes.
Line 4265-4280 only validates
{cf-studio-path}/and unmanaged relative paths._extract_studio_control_targetalso accepts@/and absolute/; adding those cases will better lock the contract.🤖 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 `@tests/test_agents_coverage.py` around lines 4265 - 4280, Add positive test coverage for the other accepted managed control-target prefixes in TestExtractStudioControlTarget so _extract_studio_control_target is verified for every supported form. Keep the existing checks for "{cf-studio-path}/" and unmanaged relative paths, and add tests that assert it returns the expected target for "`@/`..." and absolute "/..." inputs. This should live alongside the current _extract_studio_control_target tests in the same unittest class.skills/studio/scripts/studio/commands/agents.py (1)
2659-2659: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated required-bootstrap path into a module constant.
The literal
{cf-studio-path}/.core/skills/studio/modules/runtime/required-bootstrap.mdis repeated in three render contexts. A single constant keeps these in sync and matches the SonarCloud finding.♻️ Suggested constant
+_REQUIRED_BOOTSTRAP_PATH = ( + "{cf-studio-path}/.core/skills/studio/modules/runtime/required-bootstrap.md" +)Then reference
_REQUIRED_BOOTSTRAP_PATHat Lines 2659, 2851, and 3966.Also applies to: 2851-2851, 3966-3966
🤖 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/scripts/studio/commands/agents.py` at line 2659, The required-bootstrap path literal is duplicated across multiple render contexts, so extract it into a single module-level constant in agents.py and reuse that constant wherever the path is referenced. Define the shared value once (for example as _REQUIRED_BOOTSTRAP_PATH) and replace the inline string in the render inputs around the agent setup, ensuring the references in the relevant render paths all point to the same constant.Source: Linters/SAST tools
🤖 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/modules/runtime/required-bootstrap.md`:
- Around line 15-18: The bootstrap ordering in the required bootstrap sequence
is wrong because CommandResolution runs before template-variable support is
loaded, so {cfs_cmd} may remain unresolved. Update the bootstrap flow around the
LOAD/RUN steps in the required bootstrap to load template-vars.md before
invoking CommandResolution, keeping context-memory loading in the appropriate
order afterward.
In `@skills/studio/scripts/studio/commands/agents.py`:
- Around line 179-212: The generated-protocol matcher is rebuilding the
bootstrap block with a placeholder instead of the rendered bootstrap path, so
`_pure_generated_stub_matches()` no longer matches the emitted shim from
`_render_template()`. Update `_follow_protocol_lines()` and the matching logic
in `_pure_generated_stub_matches()` to use the same concrete
`required_bootstrap_path` value that `agents.py` renders for
`GeneratedBootstrapUnit`, ensuring the expected multi-line stub matches the
actual generated protocol text and idempotent cleanup works again.
---
Nitpick comments:
In `@skills/studio/scripts/studio/commands/agents.py`:
- Line 2659: The required-bootstrap path literal is duplicated across multiple
render contexts, so extract it into a single module-level constant in agents.py
and reuse that constant wherever the path is referenced. Define the shared value
once (for example as _REQUIRED_BOOTSTRAP_PATH) and replace the inline string in
the render inputs around the agent setup, ensuring the references in the
relevant render paths all point to the same constant.
In `@tests/test_agents_coverage.py`:
- Around line 4265-4280: Add positive test coverage for the other accepted
managed control-target prefixes in TestExtractStudioControlTarget so
_extract_studio_control_target is verified for every supported form. Keep the
existing checks for "{cf-studio-path}/" and unmanaged relative paths, and add
tests that assert it returns the expected target for "`@/`..." and absolute "/..."
inputs. This should live alongside the current _extract_studio_control_target
tests in the same unittest class.
🪄 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: 25f3b9c7-dbc2-41bd-bb35-5bde46ae2c92
📒 Files selected for processing (11)
architecture/features/agent-integration.mdarchitecture/specs/PDSL.mdarchitecture/specs/cli.mdskills/studio/modules/runtime/pdsl-execution-card.mdskills/studio/modules/runtime/required-bootstrap.mdskills/studio/scripts/studio/commands/agents.pytests/test_agents_coverage.pytests/test_cli_integration.pytests/test_ralphex_delegation.pytests/test_subagent_registration.pytests/test_workflow_subagents_dispatch.py
Signed-off-by: ainetx <viator@via-net.org>
|
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.
Implement a required bootstrap unit for generated skills and workflows to ensure proper execution of runtime rules and enhance PDSL handling. This change streamlines the execution process and improves protocol management.
Summary by CodeRabbit
LOAD and RUN ... as controlling protocolflow, with the legacy follow-based format retained as a fallback.