Unify prompt assembly and align tool routing with sub-agent delegation - #346
Merged
Merged
Conversation
Introduce a unified assembly seam for MainChat and sub-agent contexts, centralize tool visibility filtering, and remove legacy LlmSession prompt APIs.
Put when-to-use routing in tool descriptions and add routing lint tests; keep detailed usage in tool prompts and leave the Agent tool prompt empty.
Add English explore/plan/general-purpose system prompts with read-only and search strategy guidance, and tighten Agent prompt schema for scope and thoroughness.
Convert runtime embedded templates to English, drop unused template keys, remove stale aish-shell prompt markdown copies, and english SSH error context.
…aries Point oracle tool choice at per-tool descriptions only, and keep enter_plan_mode routing in description while limiting its prompt appendix to post-entry usage.
Contributor
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
Contributor
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
github-actions
Bot
added
docs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces prompt assembly and visibility controls, wires them into session and sub-agent execution paths, expands tool routing guidance, and converts embedded prompt templates to English while removing unused prompt files. ChangesPrompt assembly and routing refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/aish-llm/src/prompt/visibility.rs (1)
40-43: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant
session.tool_specs()call.
tool_specsis already computed at Line 40; Line 43 recomputes it viasession.tool_specs()again just to pass intoparent_has_skill_tool, cloning the full tool spec list twice.♻️ Proposed fix
- let tool_specs = session.tool_specs(); - let registered: Vec<String> = tool_specs.into_iter().map(|s| s.function.name).collect(); - let registered_refs: Vec<&str> = registered.iter().map(|s| s.as_str()).collect(); - let parent_has_skill = parent_has_skill_tool(&session.tool_specs()); + let tool_specs = session.tool_specs(); + let parent_has_skill = parent_has_skill_tool(&tool_specs); + let registered: Vec<String> = tool_specs.into_iter().map(|s| s.function.name).collect(); + let registered_refs: Vec<&str> = registered.iter().map(|s| s.as_str()).collect();🤖 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 `@crates/aish-llm/src/prompt/visibility.rs` around lines 40 - 43, The visibility logic is recomputing the tool spec list unnecessarily by calling session.tool_specs() twice. Reuse the existing tool_specs binding in the prompt/visibility flow, and pass that same value into parent_has_skill_tool instead of cloning the session data again; this keeps the registered tool collection and the skill check aligned without the extra allocation.crates/aish-llm/src/lib.rs (1)
62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SUBAGENT_GLOBAL_DENYnot re-exported at crate root.
prompt::mod.rsre-exportsSUBAGENT_GLOBAL_DENYfrom thepromptmodule, but this crate-root re-export list omits it. Only relevant if external crates need this constant; internal usage viacrate::prompt::SUBAGENT_GLOBAL_DENYin session.rs is unaffected.🤖 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 `@crates/aish-llm/src/lib.rs` at line 62, The crate-root re-export list in aish-llm::lib currently omits SUBAGENT_GLOBAL_DENY, so add that symbol alongside the existing prompt exports. Update the pub use statement in the lib.rs re-export block to include SUBAGENT_GLOBAL_DENY from prompt::mod.rs so external crates can access it through the crate root, while keeping the existing PromptAssembly, PromptBundle, PromptContext, and ToolVisibilityPolicy exports unchanged.
🤖 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.
Nitpick comments:
In `@crates/aish-llm/src/lib.rs`:
- Line 62: The crate-root re-export list in aish-llm::lib currently omits
SUBAGENT_GLOBAL_DENY, so add that symbol alongside the existing prompt exports.
Update the pub use statement in the lib.rs re-export block to include
SUBAGENT_GLOBAL_DENY from prompt::mod.rs so external crates can access it
through the crate root, while keeping the existing PromptAssembly, PromptBundle,
PromptContext, and ToolVisibilityPolicy exports unchanged.
In `@crates/aish-llm/src/prompt/visibility.rs`:
- Around line 40-43: The visibility logic is recomputing the tool spec list
unnecessarily by calling session.tool_specs() twice. Reuse the existing
tool_specs binding in the prompt/visibility flow, and pass that same value into
parent_has_skill_tool instead of cloning the session data again; this keeps the
registered tool collection and the skill check aligned without the extra
allocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a006ba8e-2811-4162-b2ab-0be4ac1d2831
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
CHANGELOG.mdcrates/aish-llm/Cargo.tomlcrates/aish-llm/src/agent.rscrates/aish-llm/src/agents/builtin_prompts.rscrates/aish-llm/src/agents/mod.rscrates/aish-llm/src/agents/registry.rscrates/aish-llm/src/agents/spawn.rscrates/aish-llm/src/agents/tool_loop.rscrates/aish-llm/src/lib.rscrates/aish-llm/src/prompt/assembly.rscrates/aish-llm/src/prompt/context.rscrates/aish-llm/src/prompt/mod.rscrates/aish-llm/src/prompt/visibility.rscrates/aish-llm/src/session.rscrates/aish-llm/src/types.rscrates/aish-llm/tests/prompt_assembly_contract.rscrates/aish-prompts/src/manager.rscrates/aish-shell/prompts/cmd_error.mdcrates/aish-shell/prompts/error_detect.mdcrates/aish-shell/prompts/failure_diagnose.mdcrates/aish-shell/prompts/guess_command.mdcrates/aish-shell/prompts/oracle.mdcrates/aish-shell/prompts/role.mdcrates/aish-shell/prompts/skill.mdcrates/aish-shell/prompts/system_diagnose.mdcrates/aish-shell/src/app.rscrates/aish-tools/src/agent_tool/agent_tool.rscrates/aish-tools/src/agent_tool/prompt.rscrates/aish-tools/src/bash/prompt.rscrates/aish-tools/src/edit_file/prompt.rscrates/aish-tools/src/glob_tool/prompt.rscrates/aish-tools/src/grep_tool/prompt.rscrates/aish-tools/src/plan_tool/enter_plan_mode.rscrates/aish-tools/src/plan_tool/prompt.rscrates/aish-tools/src/python/prompt.rscrates/aish-tools/src/read_file/prompt.rscrates/aish-tools/src/skill_tool/prompt.rscrates/aish-tools/src/write_file/prompt.rscrates/aish-tools/tests/tool_routing_lint.rs
💤 Files with no reviewable changes (8)
- crates/aish-shell/prompts/role.md
- crates/aish-shell/prompts/skill.md
- crates/aish-shell/prompts/cmd_error.md
- crates/aish-shell/prompts/failure_diagnose.md
- crates/aish-shell/prompts/system_diagnose.md
- crates/aish-shell/prompts/oracle.md
- crates/aish-shell/prompts/error_detect.md
- crates/aish-shell/prompts/guess_command.md
Resolve mod.rs conflict by keeping both builtin_prompts and event_metadata after upstream sub-agent TUI progress landed on main.
Merged
3 tasks
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
PromptAssemblyto unify MainChat and sub-agent system prompt + tool spec assembly, replacing legacyLlmSessionprompt filtering APIs.enter_plan_moderouting vs usage boundaries.Test plan
make format-check && make lintcargo test -p aish-llm --test prompt_assembly_contractcargo test -p aish-tools --test tool_routing_lintcargo test -p aish-promptsAgentappropriately.aish/plans/artifact routes toenter_plan_modeSummary by CodeRabbit
New Features
Bug Fixes
Documentation