feat(composio): tell the agent what a toolkit's actions hand back - #5322
feat(composio): tell the agent what a toolkit's actions hand back#5322yh928 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds optional toolkit result notes to ChangesToolkit result guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ComposioIntegrationBuilder
participant ProviderNotes
participant ConnectedIntegration
participant IntegrationsAgentPrompt
ComposioIntegrationBuilder->>ProviderNotes: lookup toolkit_result_notes(slug)
ProviderNotes-->>ComposioIntegrationBuilder: curated notes or None
ComposioIntegrationBuilder->>ConnectedIntegration: store result_notes
IntegrationsAgentPrompt->>ConnectedIntegration: read result_notes
IntegrationsAgentPrompt-->>IntegrationsAgentPrompt: render Results: when notes are non-empty
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e991847d5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| src/openhuman/memory/sync/composio/providers/descriptions.rs | New toolkit_result_notes function with gmail/slack entries; docstring encodes the ground-truth rule; three well-targeted tests including a slug-validity guard that already caught one real error during development. |
| src/openhuman/agent/registry/agents/integrations_agent/prompt.rs | Renders result_notes as a Results: … line per connected toolkit; trim-and-empty guard prevents blank lines; two new tests cover present and absent cases. |
| src/openhuman/agent/prompts/types.rs | Adds result_notes: Option<String> field to ConnectedIntegration with a clear doc comment explaining the separation from description. |
| src/openhuman/integrations/composio/connected_integrations.rs | Populates result_notes via toolkit_result_notes(slug) in the fetch path; comment correctly explains why there is no catalog counterpart. |
| src/openhuman/agent/harness/subagent_runner/ops/runner.rs | Sets result_notes: None on the locally-reconstructed integration used only for tool filtering/registration; the prompt uses narrowed_integrations cloned from live_integrations which carry the live value — no functional regression. |
| src/openhuman/memory/sync/composio/providers/mod.rs | Re-exports toolkit_result_notes alongside toolkit_description; one-line change. |
Sequence Diagram
sequenceDiagram
participant Cache as Integrations Cache
participant CI as fetch_connected_integrations_uncached
participant RN as toolkit_result_notes(slug)
participant IA as integrations_agent prompt.rs
participant LM as Language Model
CI->>RN: slug (e.g. "gmail")
RN-->>CI: Some("GMAIL_LIST_THREADS answers with thread ids…")
CI->>Cache: "ConnectedIntegration { result_notes: Some(…), … }"
Cache-->>IA: narrowed_integrations (cloned, result_notes preserved)
IA->>IA: render_connected_integrations()
Note over IA: if result_notes.is_some() and non-empty
IA->>LM: System prompt with Results line
LM->>LM: Knows to pass thread id to GMAIL_FETCH_MESSAGE_BY_THREAD_ID
Reviews (3): Last reviewed commit: "feat(composio): tell the agent what a to..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/composio/connected_integrations.rs (1)
958-974: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd coverage for
result_notesin connected integration construction.
toolkit_result_noteshas unit tests, but the connected integration construction path infetch_connected_integrations_uncacheddoes not assert thatgmail/slackentries receive non-emptyresult_noteswhile unestablished toolkits receiveNone. Add a test for this returned payload field.🤖 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/composio/connected_integrations.rs` around lines 958 - 974, Add coverage in fetch_connected_integrations_uncached for the constructed integrations payload, asserting gmail and slack entries contain non-empty result_notes while an unestablished toolkit has result_notes set to None. Reuse the existing test setup and lookup patterns, and verify the returned field rather than only testing toolkit_result_notes directly.Source: Path instructions
🤖 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/prompts/types.rs`:
- Around line 136-146: Update every ConnectedIntegration struct literal to
initialize the new result_notes field, using the relevant notes where available
or result_notes: None otherwise; preserve existing values and behavior in all
other fields.
---
Outside diff comments:
In `@src/openhuman/composio/connected_integrations.rs`:
- Around line 958-974: Add coverage in fetch_connected_integrations_uncached for
the constructed integrations payload, asserting gmail and slack entries contain
non-empty result_notes while an unestablished toolkit has result_notes set to
None. Reuse the existing test setup and lookup patterns, and verify the returned
field rather than only testing toolkit_result_notes directly.
🪄 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 Plus
Run ID: a6a6820f-0543-4895-a078-a03f66ae44b8
📒 Files selected for processing (5)
src/openhuman/agent/prompts/types.rssrc/openhuman/agent_registry/agents/integrations_agent/prompt.rssrc/openhuman/composio/connected_integrations.rssrc/openhuman/memory_sync/composio/providers/descriptions.rssrc/openhuman/memory_sync/composio/providers/mod.rs
2e99184 to
a984972
Compare
Everything the agent reads before calling is input-side: the tool catalogue, the parameter schema, the toolkit description, the contract the gate delivers. Nothing states what comes back. So when a list action returns records keyed by id, the model has no statement that the id is the handle for the detail it actually wanted — and it re-issues the same list call. Observed live against Gmail. `toolkit_result_notes(slug)` adds that one missing sentence, carried on `ConnectedIntegration::result_notes` and rendered per connected toolkit in the integrations-agent prompt. Kept separate from `description` because only the agent that calls the actions needs it. Two entries, gmail and slack, and the rule for adding a third is in the function doc: state only what this repository establishes — which curated action carries which identifier, and which action that identifier is the argument for. A toolkit we pass through unreshaped has no such ground truth and gets no entry, because a guess about a response is worse here than silence. The doc also forbids reciting field-by-field record shapes, with the reason: both Composio dispatch routes prefer the backend's rendered `markdownFormatted` body and fall back to the JSON envelope only when it is absent, so the reshapes in `providers/*/post_process.rs` describe just one of two possible renderings. An earlier draft of this function recited their keys and thereby told the model that every Gmail read action answers with a markdown body, when only `GMAIL_FETCH_EMAILS` carries one. The gmail entry leads with the distinction that failure turned on: `GMAIL_LIST_THREADS` answers with ids and a snippet, never a body, so a thread whose snippet looks right still has to be read. A test pins that every action slug these notes name is one the toolkit actually exposes — a note pointing at a renamed or dropped action is worse than no note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
a984972 to
672a479
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/integrations/composio/connected_integrations.rs`:
- Around line 962-965: Split the oversized Rust files into focused modules while
preserving behavior: in
src/openhuman/integrations/composio/connected_integrations.rs:962-965 move
connected-integration construction helpers; in
src/openhuman/agent/profiles/paths.rs:661-665 move unrelated test groups; in
src/openhuman/agent/registry/agents/orchestrator/prompt.rs:636-647, 688-697, and
721-750 move delegation-guide tests, and at 826-845 move visibility tests and at
883-893 move disconnected-integration tests; in
src/openhuman/flows/tinyflows/caps/ops.rs:1178-1192 split tests by capability
area; and in src/openhuman/integrations/composio/ops_tests.rs:1449-1467 split
integration-operation tests by behavior. Keep each Rust file at or below 500
lines and update module declarations/imports as needed.
In `@src/openhuman/memory/sync/composio/providers/descriptions.rs`:
- Around line 91-120: Add debug logging to toolkit_result_notes at lookup entry
and before returning both the resolved and absent branches, using a stable
“[domain]” prefix and the toolkit = %slug field. Log only the lookup outcome,
not the result-note text, while preserving the existing match behavior and
return values.
- Around line 132-145: Update the validation loop around toolkit_result_notes so
each slug is checked against its own catalog: use GMAIL_CURATED when slug is
"gmail" and SLACK_CURATED when slug is "slack", rather than combining both into
one curated collection. Keep the existing uppercase-token validation and
assertion behavior unchanged.
🪄 Autofix
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 Plus
Run ID: 6cdce5ad-5c78-4bf1-b72f-76f734b9b3e8
📒 Files selected for processing (20)
src/openhuman/agent/harness/session/tests.rssrc/openhuman/agent/harness/subagent_runner/ops/runner.rssrc/openhuman/agent/orchestration/tools/tools_e2e_tests.rssrc/openhuman/agent/profiles/paths.rssrc/openhuman/agent/prompts/types.rssrc/openhuman/agent/registry/agents/context_scout/prompt.rssrc/openhuman/agent/registry/agents/integrations_agent/prompt.rssrc/openhuman/agent/registry/agents/orchestrator/prompt.rssrc/openhuman/channels/runtime/dispatch/routing.rssrc/openhuman/flows/tinyflows/caps/ops.rssrc/openhuman/integrations/composio/connected_integrations.rssrc/openhuman/integrations/composio/ops_tests.rssrc/openhuman/memory/sync/composio/providers/descriptions.rssrc/openhuman/memory/sync/composio/providers/mod.rssrc/openhuman/tools/orchestrator_tools.rstests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rstests/raw_coverage/agent_large_round25_raw_coverage_e2e.rstests/raw_coverage/composio_raw_coverage_e2e.rstests/raw_coverage/inference_agent_raw_coverage_e2e.rstests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs
- src/openhuman/agent/harness/subagent_runner/ops/runner.rs
- tests/raw_coverage/inference_agent_raw_coverage_e2e.rs
- tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs
- src/openhuman/agent/prompts/types.rs
- tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs
- src/openhuman/agent/harness/session/tests.rs
- src/openhuman/channels/runtime/dispatch/routing.rs
- src/openhuman/tools/orchestrator_tools.rs
- tests/raw_coverage/composio_raw_coverage_e2e.rs
The slug check pooled `GMAIL_CURATED` and `SLACK_CURATED`, so a Gmail note could name a Slack-only action and still pass — which is the mistake most likely to be made when editing prose that mentions both toolkits, and the one the test exists to catch. Each toolkit is now checked against its own list, and the failure message says whose list it missed. Adds the debug event for the lookup: toolkit and whether an entry exists. The notes themselves stay out of the log — they are prose bound for the prompt, and repeating a paragraph per lookup is noise. providers::descriptions 3 pass. Reported by CodeRabbit on tinyhumansai#5322. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
toolkit_result_notes(slug)— one short statement per toolkit of what its actions hand back and which field feeds which follow-up action.ConnectedIntegration::result_notesand rendered per connected toolkit in the integrations-agent prompt.Problem
Everything the agent reads before calling is input-side: the tool catalogue, the parameter schema, the toolkit description, the contract the gate delivers. Nothing states what comes back.
So when a list action returns records keyed by id, the model has no statement that the id is the handle for the detail it actually wanted, and it re-issues the same list call. Observed live against Gmail, where a worse run reported that mail which does exist could not be found.
Solution
result_notesis kept separate fromdescriptionbecause only the agent that calls the actions needs it — the description answers "what can this service do", which every caller wants.The grounding rule is written into the function doc, because the note is only as good as it:
It also forbids reciting field-by-field record shapes, with the reason. Both Composio dispatch routes prefer the backend's rendered
markdownFormattedbody and fall back to the JSON envelope only when it is absent, so the reshapes inproviders/*/post_process.rsdescribe just one of two possible renderings. An earlier draft of this function recited their keys and thereby told the model that every Gmail read action answers with a markdown body, when onlyGMAIL_FETCH_EMAILScarries one. That draft is the reason the prohibition is in the doc rather than in a reviewer's head.The gmail entry leads with the distinction the live failure turned on:
GMAIL_LIST_THREADSanswers with ids and a snippet, never a body, so a thread whose snippet looks right still has to be read.Submission Checklist
Closes #NNNin the## RelatedsectionTesting
result_notes_only_name_curated_action_slugs— every all-caps slug in the prose must be a curated action. A note pointing at a renamed or dropped action is worse than no note: it sends the model after a tool that is not in its list. This test already caught one error during development (SLACK_SEARCH_MESSAGESis reshaped but not curated).result_notes_absent_for_unestablished_toolkits— the silence rule.gmail_notes_separate_finding_a_thread_from_reading_it— pins both halves of the failure this entry exists for.Impact
Related
Closes #5318
Summary by CodeRabbit
New Features
Tests