From e2b91875b7b7a0f199ed621c04f9b5ceab62ba5e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 14 Aug 2026 02:41:25 +0300 Subject: [PATCH] feat(tool_calling): make the module usable by a host Follow-up to #102/#105, found by wiring the first real consumer. The module could not be compiled against without the host keeping private copies of the code it had just stopped owning. Three gaps: - `extract_json_values` was not exported, and it is not a test helper. Pulling the first JSON object out of model prose is how a host validates a required-output contract, which has nothing to do with tool calls - OpenHuman calls it from production code, not just tests. Exported alongside `parse_arguments_value`, `parse_glm_style_tool_calls` and `parse_tool_calls_from_json_value`, which its tests drive directly. - `parse_tool_call_value` was `#[cfg(test)]`, inherited from a host where it happened to be test-only. It is a reasonable public primitive - parse one JSON value as a tool call - so it is un-gated and documented rather than duplicated downstream. Its doc says what licenses the argument-key aliases, so a caller cannot reach for it on arbitrary model output by mistake. - `build_registry` took `&Value`. A host tool trait that RETURNS a schema by value - the common shape, and OpenHuman's - then has to collect into a temporary purely to hand out references. It takes `Borrow` now, so `Value` and `&Value` both work. The alternative was for the host to shadow the parsers it had just moved, or drop the tests covering them. With these, all 62 of OpenHuman's parser tests run against the crate with no coverage lost. No behaviour change: exports and one signature widening. 1823 lib tests pass. Clippy and fmt clean. Co-authored-by: Medulla --- src/harness/tool_calling/mod.rs | 10 +++++++++- src/harness/tool_calling/parse.rs | 8 +++++++- src/harness/tool_calling/pformat.rs | 11 ++++++++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/harness/tool_calling/mod.rs b/src/harness/tool_calling/mod.rs index 70105007d..dcf8f274b 100644 --- a/src/harness/tool_calling/mod.rs +++ b/src/harness/tool_calling/mod.rs @@ -41,7 +41,15 @@ pub(crate) mod parse; pub(crate) mod pformat; -pub use parse::{ParsedToolCall, parse_tool_calls, parse_tool_calls_with_pformat}; +// The two entry points, plus the building blocks a host legitimately reaches +// for on its own. `extract_json_values` in particular is not a test helper: +// pulling the first JSON object out of model prose is how a host checks a +// required-output contract, which has nothing to do with tool calls. +pub use parse::{ + ParsedToolCall, extract_json_values, parse_arguments_value, parse_glm_style_tool_calls, + parse_tool_call_value, parse_tool_calls, parse_tool_calls_from_json_value, + parse_tool_calls_with_pformat, +}; pub use pformat::{ PFormatParamType, PFormatRegistry, PFormatToolParams, build_registry, parse_call, render_signature, render_signature_from_schema, diff --git a/src/harness/tool_calling/parse.rs b/src/harness/tool_calling/parse.rs index a74c8e08b..ca3d7aac2 100644 --- a/src/harness/tool_calling/parse.rs +++ b/src/harness/tool_calling/parse.rs @@ -42,7 +42,13 @@ fn first_args_by_keys(obj: &serde_json::Value) -> serde_json::Value { parse_arguments_value(None) } -#[cfg(test)] +/// Parse a single JSON value as a tool call, honouring the argument-key +/// aliases. +/// +/// The permissive entry point: callers reach a value through an explicit +/// tool-call marker (a `tool_calls` array, a `` tag, a fenced +/// block), which is what licenses the aliases. Do not use it on arbitrary +/// model output — see the module docs. pub fn parse_tool_call_value(value: &serde_json::Value) -> Option { // Default to the permissive (tagged) behaviour: callers that reach a // value through an explicit tool-call marker (`tool_calls` array, diff --git a/src/harness/tool_calling/pformat.rs b/src/harness/tool_calling/pformat.rs index e7ccb5ff4..dbd81fb5f 100644 --- a/src/harness/tool_calling/pformat.rs +++ b/src/harness/tool_calling/pformat.rs @@ -150,14 +150,19 @@ pub type PFormatRegistry = HashMap; /// type is its own vocabulary, and requiring it here would make this module /// depend on the very thing it exists to stay independent of. Hosts keep a /// one-line adapter over their own tool slice. -pub fn build_registry<'a, I, N>(tools: I) -> PFormatRegistry +/// +/// The schema is `Borrow` rather than `&Value` so a host whose tool +/// trait *returns* a schema by value — the common shape — can map straight +/// into this without collecting into a temporary first. +pub fn build_registry(tools: I) -> PFormatRegistry where - I: IntoIterator, + I: IntoIterator, N: Into, + S: std::borrow::Borrow, { tools .into_iter() - .map(|(name, schema)| (name.into(), PFormatToolParams::from_schema(schema))) + .map(|(name, schema)| (name.into(), PFormatToolParams::from_schema(schema.borrow()))) .collect() }