feat(classification): add ToolExposure so a tool can declare it is not advertised - #3
feat(classification): add ToolExposure so a tool can declare it is not advertised#3senamakel wants to merge 1 commit into
Conversation
…t advertised Tool schemas are a fixed per-turn cost. Measured on OpenHuman's orchestrator they were 45 KB against 34 KB of system prompt, and most of that is tools the model reaches for on a handful of turns a week. `ToolExposure` lets a tool say so: `Direct` (the default, so nothing changes for a tool that has not considered the question), `Deferred` (registered, schema off the wire, found through the host's tool search), and `Hidden` (dispatchable, never shown). Exposure is a property of the tool rather than of a config posture because the answer rarely varies by deployment — a market-data tool is niche on a general assistant whoever is running it. A host that offers no search facility must treat `Deferred` as `Direct`: a capability the model can neither see nor look up is gone, which is worse than the tokens it saves. Modelled on Codex's ToolExposure (codex-rs/tools/src/tool_executor.rs), minus its Code Mode surface, which has no equivalent here yet. Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesTool exposure API
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This adds opt-in tool exposure metadata while existing tools retain direct advertisement by default. The API wiring and compatibility behavior are complete, with no current merge-blocking risk identified. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0118 · 113,548 in / 1,807 out · 9,438 cached (8%) · deepseek/deepseek-v4-flash, openrouter/openai/text-embedding-3-small, z-ai/glm-5.2 · 266 embedded
critique: $0.0042 · 46,607 in / 260 out · 0 cached (0%) · deepseek/deepseek-v4-flash
security: $0.0056 · 45,815 in / 753 out · 9,438 cached (21%) · z-ai/glm-5.2, deepseek/deepseek-v4-flash
tests: $0.0011 · 12,746 in / 102 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0004 · 4,627 in / 81 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| mod types; | ||
|
|
||
| pub use types::{ToolCategory, ToolScope}; | ||
| pub use types::{ToolCategory, ToolExposure, ToolScope}; |
There was a problem hiding this comment.
Document the new public type ToolExposure
ToolExposure is now publicly exported from the module but does not yet appear to have a rustdoc comment on its definition (the diff only changes the re-export line). The repository rules require every public item to get a rustdoc comment with missing_docs treated as an error. Add /// documentation explaining what ToolExposure represents, how it is used (including its relation to ToolCategory and ToolScope), and any wire-format or default behaviour.
[RULE] missing-docs ·
How this change flows0 changed behaviours across 9 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 33 further behaviours left out to keep the diagram readable. flowchart LR
n0["execute_with_context"]:::impacted
n1["execute_with_options"]:::impacted
n2["execute"]:::impacted
n3["Isolated"]:::impacted
n4["...workspace_root_through_the_erased_context"]:::impacted
n5["...text_overloads_default_through_to_execute"]:::impacted
n1 -->|calls| n2
n4 -->|calls| n0
n4 -->|tests| n0
n4 -->|calls| n3
n4 -->|tests| n3
n5 -->|calls| n0
n5 -->|tests| n0
n5 -->|calls| n1
n5 -->|tests| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 095aff787e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum ToolExposure { |
There was a problem hiding this comment.
No test references ToolExposure, leaving its Direct default, predicates, and the new "direct"/"deferred"/"hidden" wire values unprotected against accidental changes. In particular, the repository requires every serializable payload type to pin its serde representation because mismatches otherwise surface as runtime decode failures.
AGENTS.md reference: AGENTS.md:L170-L172
Useful? React with 👍 / 👎.
|
|
||
| pub use call::{ToolCallOptions, ToolTimeout}; | ||
| pub use classification::{ToolCategory, ToolScope}; | ||
| pub use classification::{ToolCategory, ToolExposure, ToolScope}; |
There was a problem hiding this comment.
This makes ToolExposure part of the crate's public surface, but the package README's “What is here” table still describes classification as containing only ToolScope and ToolCategory. Users reading the published crate README therefore receive an incomplete account of the API; update it in this commit as required for behavior and documentation changes.
AGENTS.md reference: AGENTS.md:L204-L205
Useful? React with 👍 / 👎.
| /// Register the tool and keep its schema off the wire, reachable through | ||
| /// the host's tool-search facility. | ||
| /// | ||
| /// A host that offers no such facility must treat this as [`Self::Direct`] | ||
| /// rather than hiding the tool — a capability the model cannot see *and* | ||
| /// cannot look up is simply gone, which is a bigger regression than the | ||
| /// tokens it saves. | ||
| Deferred, |
There was a problem hiding this comment.
This introduces host-facing behavior and an important constraint that hosts without search must reinterpret Deferred as Direct, but no document under docs/specs/ defines that accepted contract and no linked implementation plan exists. Add the specification and plan so downstream host implementations have a stable behavioral source rather than relying only on enum rustdoc.
AGENTS.md reference: AGENTS.md:L206-L208
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
What
Adds
ToolExposuretotinytools::classification— three states a tool can declare about how it reaches the model:ToolExposureDirect(default)DeferredHiddenDirectis the default, so every existingToolimpl compiles unchanged and behaves exactly as before.Why
A tool that is registered is not necessarily a tool that should be advertised, and until now the vocabulary had no way to say so. Two real cases in OpenHuman need it:
Hiddenthe host ships both surfaces and the collapse saves nothing.Deferred.Both are properties of the tool, which is why they belong in the shared vocabulary rather than in one host's registry:
tinyagentsruns the loop over this same trait, so a host and the harness must agree on what a tool declares.Modelled on Codex's
ToolExposure(codex-rs/tools/src/tool_executor.rs).Note on the second commit
fix: prefer unwrap_or_default over unwrap_or("")is an unrelated one-line doc-example tidy that was already sitting on this branch.Testing
cargo testin this crate. Downstream, the enum is exercised by OpenHuman's delegation collapse (tinyhumansai/openhuman) whereHiddentakes 16 duplicate delegation schemas off the wire.Summary by CodeRabbit