feat(agent): reduce orchestrator prompt bloat#3367
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds three specialist agents (scheduler, presentation, desktop control), expands ArchetypeDelegationTool to accept structured handoff fields and render compact child prompts, injects a shared Sub-agent Result Contract into subagent role contracts, updates orchestrator routing and tests, and exposes prompts as MCP resources. ChangesSpecialist agent delegation and orchestrator refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/openhuman/agent_orchestration/tools/archetype_delegation.rs (1)
77-106: ⚡ Quick winAdd debug-level delegation-path logs with stable fields.
Please add
tracing::debug!at structured handoff build/dispatch points (e.g.,tool_name,agent_id, presence/count of structured fields,model_overridepresence) so delegation decisions are grep-friendly during incident/debug flows.As per coding guidelines: “Add substantial debug-level logs while implementing features or fixes in Rust using
log/tracingcrate with stable prefixes and correlation fields (request IDs, method names, entity IDs) for grep-friendly tracing”.🤖 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 `@src/openhuman/agent_orchestration/tools/archetype_delegation.rs` around lines 77 - 106, Add structured debug logging around the handoff build and dispatch in archetype_delegation.rs: after computing raw_prompt/prompt (from render_structured_handoff) and before calling super::dispatch_subagent, emit a tracing::debug! with stable fields: tool_name (self.tool_name), agent_id (self.agent_id), structured_fields_count (e.g., args.len() or count of structured entries), prompt_empty (bool) or prompt_length, and model_override_present (bool) / model_override value if present; ensure the logs use stable keys (e.g., "tool_name", "agent_id", "fields_count", "model_override") so grep-friendly and correlate with dispatch_subagent invocation.
🤖 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 `@src/openhuman/agent_registry/agents/desktop_control_agent/prompt.rs`:
- Around line 10-34: Add tracing/debug instrumentation to the prompt assembly in
build: emit a stable-prefix entry log when entering build (include
ctx.agent_id), call and instrument each render helper (render_user_files,
render_tools, render_workspace) with trace logs showing start, end,
success/error and the trimmed length or empty/skipped decision, and log branch
decisions (e.g., "INCLUDE_SECTION" vs "SKIP_SECTION") with a section name field
and agent_id; also emit an exit log with final output length or error. Use the
project's tracing/log crate conventions and stable grep-friendly prefixes (e.g.,
"PROMPT_BUILD:ENTRY", "PROMPT_BUILD:SECTION", "PROMPT_BUILD:EXIT") and attach
correlation fields like agent_id and section names to each event.
In `@src/openhuman/agent_registry/agents/presentation_agent/prompt.rs`:
- Around line 10-34: Add debug/trace-level logs to the build function to record
entry/exit and branch decisions: at function entry log a stable prefix and the
agent id from PromptContext (e.g., "prompt.build"), then after each call to
render_user_files, render_tools, and render_workspace log whether the section
was appended and its trimmed length (use section.trim().is_empty() and
section.trim_end().len()), and before returning Ok(out) log exit with final
prompt length and which sections were included; use the project's tracing/log
crate at debug or trace level and reference symbols build, PromptContext,
ARCHETYPE, render_user_files, render_tools, and render_workspace in the log
messages.
In `@tests/orchestrator_presentation_wiring.rs`:
- Around line 59-60: Replace the weak substring check
PRESENTATION_AGENT_TOML.contains("delegate_name = \"make_presentation\"") with
the same trimmed-line equality helper used elsewhere in these tests to assert an
exact line match for the delegate_name entry; locate the helper used in this
test file (the trimmed-line equality/assertion function) and call it to compare
a trimmed line exactly to 'delegate_name = "make_presentation"' so incidental
matches are prevented.
---
Nitpick comments:
In `@src/openhuman/agent_orchestration/tools/archetype_delegation.rs`:
- Around line 77-106: Add structured debug logging around the handoff build and
dispatch in archetype_delegation.rs: after computing raw_prompt/prompt (from
render_structured_handoff) and before calling super::dispatch_subagent, emit a
tracing::debug! with stable fields: tool_name (self.tool_name), agent_id
(self.agent_id), structured_fields_count (e.g., args.len() or count of
structured entries), prompt_empty (bool) or prompt_length, and
model_override_present (bool) / model_override value if present; ensure the logs
use stable keys (e.g., "tool_name", "agent_id", "fields_count",
"model_override") so grep-friendly and correlate with dispatch_subagent
invocation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffdd1afe-7752-4067-9096-2a5df6bfaa92
📒 Files selected for processing (22)
src/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/subagent_runner/ops_tests.rssrc/openhuman/agent_orchestration/tools/archetype_delegation.rssrc/openhuman/agent_registry/agents/desktop_control_agent/agent.tomlsrc/openhuman/agent_registry/agents/desktop_control_agent/mod.rssrc/openhuman/agent_registry/agents/desktop_control_agent/prompt.mdsrc/openhuman/agent_registry/agents/desktop_control_agent/prompt.rssrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/agent_registry/agents/mod.rssrc/openhuman/agent_registry/agents/orchestrator/agent.tomlsrc/openhuman/agent_registry/agents/orchestrator/prompt.mdsrc/openhuman/agent_registry/agents/orchestrator/prompt.rssrc/openhuman/agent_registry/agents/presentation_agent/agent.tomlsrc/openhuman/agent_registry/agents/presentation_agent/mod.rssrc/openhuman/agent_registry/agents/presentation_agent/prompt.mdsrc/openhuman/agent_registry/agents/presentation_agent/prompt.rssrc/openhuman/agent_registry/agents/scheduler_agent/agent.tomlsrc/openhuman/agent_registry/agents/scheduler_agent/mod.rssrc/openhuman/agent_registry/agents/scheduler_agent/prompt.mdsrc/openhuman/agent_registry/agents/scheduler_agent/prompt.rssrc/openhuman/mcp_server/resources.rstests/orchestrator_presentation_wiring.rs
| pub fn build(ctx: &PromptContext<'_>) -> Result<String> { | ||
| let mut out = String::with_capacity(4096); | ||
| out.push_str(ARCHETYPE.trim_end()); | ||
| out.push_str("\n\n"); | ||
|
|
||
| let user_files = render_user_files(ctx)?; | ||
| if !user_files.trim().is_empty() { | ||
| out.push_str(user_files.trim_end()); | ||
| out.push_str("\n\n"); | ||
| } | ||
|
|
||
| let tools = render_tools(ctx)?; | ||
| if !tools.trim().is_empty() { | ||
| out.push_str(tools.trim_end()); | ||
| out.push_str("\n\n"); | ||
| } | ||
|
|
||
| let workspace = render_workspace(ctx)?; | ||
| if !workspace.trim().is_empty() { | ||
| out.push_str(workspace.trim_end()); | ||
| out.push('\n'); | ||
| } | ||
|
|
||
| Ok(out) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add required debug/trace instrumentation in prompt assembly flow.
build performs multiple rendering calls and branch decisions, but currently emits no diagnostic logs. Please add stable-prefix debug/trace logs for function entry/exit, per-section render outcome, and included/skipped branches (with correlation fields like agent_id and section names).
Proposed patch
use anyhow::Result;
+use tracing::{debug, trace};
const ARCHETYPE: &str = include_str!("prompt.md");
pub fn build(ctx: &PromptContext<'_>) -> Result<String> {
+ debug!(
+ agent_id = %ctx.agent_id,
+ "[desktop_control_agent::prompt] build start"
+ );
let mut out = String::with_capacity(4096);
out.push_str(ARCHETYPE.trim_end());
out.push_str("\n\n");
+ trace!(agent_id = %ctx.agent_id, section = "user_files", "[desktop_control_agent::prompt] rendering section");
let user_files = render_user_files(ctx)?;
if !user_files.trim().is_empty() {
+ debug!(agent_id = %ctx.agent_id, section = "user_files", "[desktop_control_agent::prompt] section included");
out.push_str(user_files.trim_end());
out.push_str("\n\n");
+ } else {
+ trace!(agent_id = %ctx.agent_id, section = "user_files", "[desktop_control_agent::prompt] section skipped_empty");
}
+ trace!(agent_id = %ctx.agent_id, section = "tools", "[desktop_control_agent::prompt] rendering section");
let tools = render_tools(ctx)?;
if !tools.trim().is_empty() {
+ debug!(agent_id = %ctx.agent_id, section = "tools", "[desktop_control_agent::prompt] section included");
out.push_str(tools.trim_end());
out.push_str("\n\n");
+ } else {
+ trace!(agent_id = %ctx.agent_id, section = "tools", "[desktop_control_agent::prompt] section skipped_empty");
}
+ trace!(agent_id = %ctx.agent_id, section = "workspace", "[desktop_control_agent::prompt] rendering section");
let workspace = render_workspace(ctx)?;
if !workspace.trim().is_empty() {
+ debug!(agent_id = %ctx.agent_id, section = "workspace", "[desktop_control_agent::prompt] section included");
out.push_str(workspace.trim_end());
out.push('\n');
+ } else {
+ trace!(agent_id = %ctx.agent_id, section = "workspace", "[desktop_control_agent::prompt] section skipped_empty");
}
+ debug!(
+ agent_id = %ctx.agent_id,
+ final_chars = out.chars().count(),
+ "[desktop_control_agent::prompt] build done"
+ );
Ok(out)
}As per coding guidelines: “Log entry/exit, branches, external calls, retries/timeouts, state transitions, and errors with stable grep-friendly prefixes” and “Add substantial debug-level logs while implementing features or fixes in Rust using log / tracing crate.”
🤖 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 `@src/openhuman/agent_registry/agents/desktop_control_agent/prompt.rs` around
lines 10 - 34, Add tracing/debug instrumentation to the prompt assembly in
build: emit a stable-prefix entry log when entering build (include
ctx.agent_id), call and instrument each render helper (render_user_files,
render_tools, render_workspace) with trace logs showing start, end,
success/error and the trimmed length or empty/skipped decision, and log branch
decisions (e.g., "INCLUDE_SECTION" vs "SKIP_SECTION") with a section name field
and agent_id; also emit an exit log with final output length or error. Use the
project's tracing/log crate conventions and stable grep-friendly prefixes (e.g.,
"PROMPT_BUILD:ENTRY", "PROMPT_BUILD:SECTION", "PROMPT_BUILD:EXIT") and attach
correlation fields like agent_id and section names to each event.
| pub fn build(ctx: &PromptContext<'_>) -> Result<String> { | ||
| let mut out = String::with_capacity(4096); | ||
| out.push_str(ARCHETYPE.trim_end()); | ||
| out.push_str("\n\n"); | ||
|
|
||
| let user_files = render_user_files(ctx)?; | ||
| if !user_files.trim().is_empty() { | ||
| out.push_str(user_files.trim_end()); | ||
| out.push_str("\n\n"); | ||
| } | ||
|
|
||
| let tools = render_tools(ctx)?; | ||
| if !tools.trim().is_empty() { | ||
| out.push_str(tools.trim_end()); | ||
| out.push_str("\n\n"); | ||
| } | ||
|
|
||
| let workspace = render_workspace(ctx)?; | ||
| if !workspace.trim().is_empty() { | ||
| out.push_str(workspace.trim_end()); | ||
| out.push('\n'); | ||
| } | ||
|
|
||
| Ok(out) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add debug-level tracing to the prompt assembly path.
build now owns branching prompt composition, but it emits no debug/trace logs for entry/section inclusion/exit. Please add stable-prefixed tracing with key correlation fields (e.g., agent_id, section lengths, and which sections were appended) so routing/prompt-shape regressions are diagnosable.
Proposed diff
use crate::openhuman::context::prompt::{
render_tools, render_user_files, render_workspace, PromptContext,
};
use anyhow::Result;
@@
pub fn build(ctx: &PromptContext<'_>) -> Result<String> {
+ tracing::debug!(
+ target: "agent_prompt",
+ agent_id = %ctx.agent_id,
+ model_name = %ctx.model_name,
+ "[presentation_agent_prompt] building system prompt"
+ );
let mut out = String::with_capacity(4096);
out.push_str(ARCHETYPE.trim_end());
out.push_str("\n\n");
@@
let user_files = render_user_files(ctx)?;
if !user_files.trim().is_empty() {
+ tracing::debug!(
+ target: "agent_prompt",
+ agent_id = %ctx.agent_id,
+ section = "user_files",
+ chars = user_files.len(),
+ "[presentation_agent_prompt] appending section"
+ );
out.push_str(user_files.trim_end());
out.push_str("\n\n");
}
@@
let tools = render_tools(ctx)?;
if !tools.trim().is_empty() {
+ tracing::debug!(
+ target: "agent_prompt",
+ agent_id = %ctx.agent_id,
+ section = "tools",
+ chars = tools.len(),
+ "[presentation_agent_prompt] appending section"
+ );
out.push_str(tools.trim_end());
out.push_str("\n\n");
}
@@
let workspace = render_workspace(ctx)?;
if !workspace.trim().is_empty() {
+ tracing::debug!(
+ target: "agent_prompt",
+ agent_id = %ctx.agent_id,
+ section = "workspace",
+ chars = workspace.len(),
+ "[presentation_agent_prompt] appending section"
+ );
out.push_str(workspace.trim_end());
out.push('\n');
}
+ tracing::debug!(
+ target: "agent_prompt",
+ agent_id = %ctx.agent_id,
+ final_chars = out.len(),
+ "[presentation_agent_prompt] build complete"
+ );
Ok(out)
}🤖 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 `@src/openhuman/agent_registry/agents/presentation_agent/prompt.rs` around
lines 10 - 34, Add debug/trace-level logs to the build function to record
entry/exit and branch decisions: at function entry log a stable prefix and the
agent id from PromptContext (e.g., "prompt.build"), then after each call to
render_user_files, render_tools, and render_workspace log whether the section
was appended and its trimmed length (use section.trim().is_empty() and
section.trim_end().len()), and before returning Ok(out) log exit with final
prompt length and which sections were included; use the project's tracing/log
crate at debug or trace level and reference symbols build, PromptContext,
ARCHETYPE, render_user_files, render_tools, and render_workspace in the log
messages.
| PRESENTATION_AGENT_TOML.contains("delegate_name = \"make_presentation\""), | ||
| "presentation_agent must expose the make_presentation delegate tool" |
There was a problem hiding this comment.
Harden delegate_name assertion to exact-line matching.
PRESENTATION_AGENT_TOML.contains("delegate_name = \"make_presentation\"") is weaker than the exact-line checks used elsewhere and can pass on incidental text. Use a trimmed line-equality helper for this key too.
Suggested test hardening
fn lists_named_tool(toml: &str, name: &str) -> bool {
let bare = format!("\"{name}\"");
let trailing = format!("\"{name}\",");
toml.lines()
.map(str::trim)
.any(|line| line == bare || line == trailing)
}
+
+fn has_delegate_name(toml: &str, delegate_name: &str) -> bool {
+ let expected = format!("delegate_name = \"{delegate_name}\"");
+ toml.lines().map(str::trim).any(|line| line == expected)
+}
@@
- assert!(
- PRESENTATION_AGENT_TOML.contains("delegate_name = \"make_presentation\""),
- "presentation_agent must expose the make_presentation delegate tool"
- );
+ assert!(
+ has_delegate_name(PRESENTATION_AGENT_TOML, "make_presentation"),
+ "presentation_agent must expose the make_presentation delegate tool"
+ );📝 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.
| PRESENTATION_AGENT_TOML.contains("delegate_name = \"make_presentation\""), | |
| "presentation_agent must expose the make_presentation delegate tool" | |
| fn lists_named_tool(toml: &str, name: &str) -> bool { | |
| let bare = format!("\"{name}\""); | |
| let trailing = format!("\"{name}\","); | |
| toml.lines() | |
| .map(str::trim) | |
| .any(|line| line == bare || line == trailing) | |
| } | |
| fn has_delegate_name(toml: &str, delegate_name: &str) -> bool { | |
| let expected = format!("delegate_name = \"{delegate_name}\""); | |
| toml.lines().map(str::trim).any(|line| line == expected) | |
| } | |
| // ... in test function: | |
| assert!( | |
| has_delegate_name(PRESENTATION_AGENT_TOML, "make_presentation"), | |
| "presentation_agent must expose the make_presentation delegate tool" | |
| ); |
🤖 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/orchestrator_presentation_wiring.rs` around lines 59 - 60, Replace the
weak substring check PRESENTATION_AGENT_TOML.contains("delegate_name =
\"make_presentation\"") with the same trimmed-line equality helper used
elsewhere in these tests to assert an exact line match for the delegate_name
entry; locate the helper used in this test file (the trimmed-line
equality/assertion function) and call it to compare a trimmed line exactly to
'delegate_name = "make_presentation"' so incidental matches are prevented.
M3gA-Mind
left a comment
There was a problem hiding this comment.
Well-structured, well-tested refactor that meaningfully shrinks the orchestrator prompt and pins the new routing with good negative assertions (delegate-name parity, exact-line matching, narrow tool scopes all verified). Two substantive items to resolve before merge plus a couple of minor notes inline.
| @@ -108,37 +121,12 @@ hint = "chat" | |||
| # / composio_execute directly. | |||
| named = [ | |||
There was a problem hiding this comment.
Orchestrator silently loses memory-write + skill-run with no replacement route. This block drops memory_store, save_preference, memory_forget, and run_skill, but the decision tree adds no delegation line for "remember this / save my preference / forget that / run a skill," and none of the three new specialists own those tools — the orchestrator keeps only query_memory (read) and memory_tree. So "remember that I prefer dark mode" now has no tool path on the front-line agent. The PR description only documents removing scheduling/presentation/desktop tools, so this looks undocumented. If it's intended (route to delegate_archivist / hand back to Core), please add the decision-tree line + a test; otherwise it's a capability regression.
| @@ -175,42 +132,7 @@ User: what time is it? | |||
|
|
|||
There was a problem hiding this comment.
Stale ## Response Style examples now contradict the refactor. The examples just above this line still ship User: what time is it? → 7:31pm and User: remind me to stretch in 10 min → got it / reminder set for 7:42pm. With current_time and cron_* removed from the orchestrator, these model the agent answering the clock and confirming a reminder directly from priors — fabricating both the time value and the "reminder set" outcome — which is exactly what scheduler_agent and the new evidence-grounding contract exist to prevent. Please update them to show an ack + delegation to schedule_task, or the prompt will actively coach the wrong behavior.
| } | ||
| } | ||
|
|
||
| fn render_structured_handoff(prompt: &str, args: &Value) -> String { |
There was a problem hiding this comment.
Minor / parity note: render_structured_handoff reshapes every archetype delegation, not just the new specialists. ArchetypeDelegationTool backs all delegates (crypto, integrations, etc.), so every child prompt is now wrapped in Task:\n… even when no structured fields are passed. It's additive and well-tested, but the "existing prompt delegation remains compatible" parity claim is a bit stronger than reality — the literal child-prompt string changes for all delegates. Worth a one-line note in the parity section, and consider skipping the Task:\n wrapper when no structured fields are present to keep the legacy path byte-identical.
| @@ -1,14 +1,14 @@ | |||
| # Orchestrator — Staff Engineer | |||
| # Orchestrator - Staff Engineer | |||
There was a problem hiding this comment.
Nit: this changes the H1 from an em-dash (Orchestrator — Staff Engineer) to a hyphen, which is unrelated churn for this PR. Arguably consistent with the prompt's own "avoid em dashes" guidance, but it's a heading (not model output), so either keep it intentional or revert to keep the diff focused on the refactor.
…gent-prompt-bloat-with-specialis # Conflicts: # src/openhuman/agent_registry/agents/orchestrator/agent.toml
Summary
help,scheduler_agent,presentation_agent, anddesktop_control_agentthrough the orchestrator subagent inventory.ArchetypeDelegationToolwhile preserving the existing freeformpromptcontract.Problem
Solution
scheduler_agent,presentation_agent, anddesktop_control_agent.helpworker and new specialists in the orchestrator subagent list, loader builtins, and MCP prompt resource catalog.make_presentation.objective,evidence,constraints,must_not_assume,expected_output, andcitation_requirement.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/pr-ci.yml. Local diff-cover was not run; focused Rust tests cover changed prompt/delegation wiring and CI will enforce the gate.## Related— N/A: no matrix feature ID changed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release-cut smoke surface changed.Closes #NNNin the## RelatedsectionImpact
promptdelegation compatibility.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
issue/3332-reduce-agent-prompt-bloat-with-specialis4bb91a573f29cf5f0a321f9a3620e15732b890c9Validation Run
pnpm --filter openhuman-app format:check— passed via pre-push hook.pnpm typecheck— passed via pre-push hook (pnpm compile).cargo test --manifest-path Cargo.toml orchestrator -- --nocapturecargo test --manifest-path Cargo.toml subagent_role_contract -- --nocapturecargo test --manifest-path Cargo.toml archetype_delegation -- --nocapturecargo test --manifest-path Cargo.toml specialist_agents_are_registered_with_narrow_tools -- --nocapturecargo test --manifest-path Cargo.toml presentation_agent_lists_generate_presentation_and_grounding_tools -- --nocapturecargo test --manifest-path Cargo.toml openhuman::mcp_server::resources::tests -- --nocapturecargo test --manifest-path Cargo.toml openhuman::memory_tree::tree::rpc::tests::pipeline_status_reports_chunk_aggregates_after_ingest -- --nocapturecargo fmt --manifest-path Cargo.toml;cargo check --manifest-path Cargo.toml;cargo test --manifest-path Cargo.tomlran, but full suite has one pre-existing/order-dependent failure listed below.cargo check --manifest-path app/src-tauri/Cargo.toml; also passed via pre-push hookpnpm rust:check.Validation Blocked
command:cargo test --manifest-path Cargo.tomlerror:full suite fails onlyopenhuman::memory_tree::tree::rpc::tests::pipeline_status_reports_chunk_aggregates_after_ingestwithdegraded status should explain semantic recall loss: Some("wiki structure incomplete"); the same test passed in isolation after the full-suite failure.impact:unrelated to this branch's agent registry/prompt/delegation changes; focused tests for the touched surfaces pass.Behavior Changes
Parity Contract
promptinput remains required and compatible; specialist-owned behavior remains available through delegate tools.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Improvements