diff --git a/src/openhuman/agent/dispatcher.rs b/src/openhuman/agent/dispatcher.rs
index 8aba3e8459..9b5415b58e 100644
--- a/src/openhuman/agent/dispatcher.rs
+++ b/src/openhuman/agent/dispatcher.rs
@@ -334,14 +334,17 @@ impl ToolDispatcher for PFormatToolDispatcher {
than JSON.\n\n",
);
instructions
- .push_str("```\n\nget_weather[London|metric]\n\n```\n\n");
+ .push_str("```\n\nget_weather[0|London|1|metric]\n\n```\n\n");
instructions.push_str(
"**Rules:**\n\
- - Form: `name[arg1|arg2|...|argN]`. Arguments are positional and must match the \
- order shown in each tool's `Call as:` signature in the `## Tools` section above \
- (alphabetical by parameter name).\n\
+ - Form: `name[index|value|index|value|...]`. A `Call as:` signature numbers its \
+ slots and shows each as a `` placeholder; replace each one with a value, \
+ keeping its number. `get_weather[0||1|]` is called as \
+ `get_weather[0|London|1|metric]`.\n\
+ - Send only the arguments you are actually passing, each with its own number. \
+ `get_weather[1|metric]` sends unit and no location. The numbers do the \
+ skipping, so there are no empty slots to count.\n\
- Empty calls: `name[]` for zero-arg tools.\n\
- - Empty argument: `name[||value]` is three positional values, the first two empty.\n\
- Escapes inside argument values: `\\|` → `|`, `\\]` → `]`, `\\\\` → `\\`.\n\
- You may emit multiple `` blocks in a single response. Each tag holds \
exactly one call.\n\
diff --git a/src/openhuman/agent/dispatcher_tests.rs b/src/openhuman/agent/dispatcher_tests.rs
index ffaaad1e83..d55a6c12c1 100644
--- a/src/openhuman/agent/dispatcher_tests.rs
+++ b/src/openhuman/agent/dispatcher_tests.rs
@@ -129,7 +129,8 @@ fn pformat_dispatcher_parses_tool_call_tag() {
let dispatcher = PFormatToolDispatcher::new(registry);
let response = ChatResponse {
text: Some(
- "Let me check the weather.\nget_weather[London|metric]".into(),
+ "Let me check the weather.\nget_weather[0|London|1|metric]"
+ .into(),
),
tool_calls: vec![],
usage: None,
@@ -181,7 +182,7 @@ fn pformat_dispatcher_handles_multiple_tags() {
let dispatcher = PFormatToolDispatcher::new(registry);
let response = ChatResponse {
text: Some(
- "Step 1.\nshell[ls]\nStep 2.\nshell[pwd]"
+ "Step 1.\nshell[0|ls]\nStep 2.\nshell[0|pwd]"
.into(),
),
tool_calls: vec![],
diff --git a/src/openhuman/agent/harness/subagent_runner/tool_prep.rs b/src/openhuman/agent/harness/subagent_runner/tool_prep.rs
index 1279d9bef7..8c0c242137 100644
--- a/src/openhuman/agent/harness/subagent_runner/tool_prep.rs
+++ b/src/openhuman/agent/harness/subagent_runner/tool_prep.rs
@@ -87,12 +87,16 @@ pub(crate) fn build_text_mode_tool_instructions() -> String {
"Tool calls use **P-Format** (Parameter-Format): compact, positional, \
pipe-delimited syntax wrapped in `` tags.\n\n",
);
- out.push_str("```\n\nGMAIL_FETCH_EMAILS[ca_123||10]\n\n```\n\n");
+ out.push_str("```\n\nGMAIL_FETCH_EMAILS[0|ca_123|2|10]\n\n```\n\n");
out.push_str(
"**Rules:**\n\
- - Form: `name[arg1|arg2|...|argN]`. Arguments are positional and must match the \
- order shown in each tool's `Call as:` signature in the `## Tools` section \
- (alphabetical by parameter name). Leave a slot empty to omit that argument.\n\
+ - Form: `name[index|value|index|value|...]`. A `Call as:` signature numbers its \
+ slots and shows each as a `` placeholder; replace each one with a value, \
+ keeping its number. `get_weather[0||1|]` is called as \
+ `get_weather[0|London|1|metric]`.\n\
+ - Send only the arguments you are actually passing, each with its own number. \
+ `get_weather[1|metric]` sends unit and no location. The numbers do the skipping, \
+ so there are no empty slots to count.\n\
- Empty calls: `name[]` for zero-arg tools.\n\
- Escapes inside argument values: `\\|` for a literal `|`, `\\]` for `]`, `\\\\` for `\\`.\n\
- Do not nest tags. Emit one tag per call; you can emit multiple tags in the same \
diff --git a/src/openhuman/agent/pformat.rs b/src/openhuman/agent/pformat.rs
index b8d350664d..0bef6ea353 100644
--- a/src/openhuman/agent/pformat.rs
+++ b/src/openhuman/agent/pformat.rs
@@ -23,22 +23,50 @@
//! # Spec
//!
//! - One call per `...` tag body.
-//! - Form: `name[arg1|arg2|...|argN]`.
+//! - Form: `name[index|value|index|value|...]` — each argument carries the
+//! slot index it belongs to, so **only the arguments actually being sent
+//! appear**.
//! - `name` is the tool's registered name (alphanumerics + `_`).
-//! - Arguments are **positional**, with the order pinned to the
-//! **alphabetical** sort of the JSON-schema property names. The
-//! project's `serde_json` build does not enable `preserve_order`, so
-//! `Map` iterates as a `BTreeMap` — alphabetical iteration is the
-//! only order we can produce deterministically without flipping a
-//! crate-wide feature flag, and it is stable across rebuilds and
-//! workspaces.
-//! - The renderer always exposes the order in the tool catalogue
-//! (e.g. `get_weather[location|unit]`, `math[verbose|x|y]`), so the
-//! model never has to guess which slot maps to which parameter — it
-//! reads the signature line and copies that order verbatim.
-//! - Empty calls: `tool_name[]` for zero-arg tools.
-//! - Empty arguments: `tool_name[||value]` is three args, the first two
-//! being empty strings.
+//! - Slot indices number the parameters **required first** (in the order the
+//! schema declares them), then the optional ones alphabetically. Both halves
+//! are deterministic across rebuilds and workspaces: a JSON array preserves
+//! order, and `Map` iterates as a `BTreeMap` because this build does not
+//! enable `preserve_order`.
+//! - The renderer exposes the numbering in the tool catalogue, each slot marked
+//! as a placeholder to fill:
+//! `get_weather[0||1|]`, `math[0||1||2|]`.
+//! The brackets matter: rendered as bare names the signature reads as a call
+//! to copy, and a live model duly sent the parameter names as the argument
+//! values.
+//! - Empty calls: `tool_name[]` for zero-arg tools, and for a call that sends
+//! no arguments at all.
+//!
+//! ## Why indices, rather than counting empty slots
+//!
+//! The form used to be bare positional — `name[arg1|arg2|...]` — with skipped
+//! arguments written as empty slots (`name[||value]`). That made the *count
+//! of leading delimiters* load-bearing, and it is the single thing models get
+//! wrong most:
+//!
+//! - `GMAIL_LIST_THREADS[||50|]` failed schema validation **12 times in
+//! one turn** before the turn was cut short.
+//! - A live `GMAIL_LIST_THREADS` call wrote four leading empties where three
+//! were needed, so `query` and `user_id` each landed one slot late, in
+//! `user_id` and `verbose`. The call ran with the search text as the
+//! account id.
+//!
+//! Both are off-by-one on a delimiter, and both bound arguments to the wrong
+//! parameter **silently** — the tool ran, with the wrong values. Indices
+//! remove the counting: a sparse call names its slots, and there is nothing
+//! to miscount. An index that is missing, non-numeric, or out of range is
+//! **rejected** rather than guessed at, so the failure mode moves from a
+//! wrong call that succeeds to a malformed call the model is told about.
+//!
+//! Required-first ordering is kept. It is why the natural minimal call — the
+//! one required value — is `name[0|value]` rather than an arbitrary index. An
+//! alphabetical layout put the optional parameters first for most tools, and
+//! a live model wrote `memory_recall[Colorado]` six times in one turn against
+//! `[limit|namespace|query]` and never got a tool to run.
//! - Escapes: `\|` → `|`, `\]` → `]`, `\\` → `\`. Other backslashes
//! pass through verbatim so URLs and Windows paths remain readable.
//! - Type coercion: schema property `type: integer | number | boolean`
@@ -114,11 +142,10 @@ impl PFormatToolParams {
/// shell-style tools) return an empty list — the renderer falls
/// back to `name[]`.
///
- /// Iteration order is alphabetical because `serde_json::Map` is
- /// a `BTreeMap` in this build (no `preserve_order` feature). The
- /// renderer always shows the resulting order in the tool catalogue
- /// so the model — and the parser — agree on the layout. See the
- /// module-level docs for the rationale.
+ /// Order is required-first, then optional alphabetically. The renderer
+ /// always shows the resulting order in the tool catalogue so the model — and
+ /// the parser — agree on the layout; both read it from here, so they cannot
+ /// disagree. See the module-level docs for why required comes first.
pub fn from_schema(schema: &Value) -> Self {
let Some(props) = schema.get("properties").and_then(|p| p.as_object()) else {
return Self {
@@ -126,11 +153,35 @@ impl PFormatToolParams {
types: Vec::new(),
};
};
- let mut names = Vec::with_capacity(props.len());
- let mut types = Vec::with_capacity(props.len());
- for (name, def) in props {
- names.push(name.clone());
- types.push(PFormatParamType::from_schema_type(def.get("type")));
+ // Required parameters first, in the order the schema declares them, then
+ // the optional ones alphabetically. Both halves are deterministic (a JSON
+ // array preserves order; `Map` is a `BTreeMap` in this build), which is the
+ // property the layout actually needs.
+ let required: Vec<&str> = schema
+ .get("required")
+ .and_then(|r| r.as_array())
+ .map(|a| a.iter().filter_map(Value::as_str).collect())
+ .unwrap_or_default();
+
+ let mut ordered: Vec<&String> = Vec::with_capacity(props.len());
+ for name in &required {
+ if let Some((key, _)) = props.get_key_value(*name) {
+ if !ordered.contains(&key) {
+ ordered.push(key);
+ }
+ }
+ }
+ for key in props.keys() {
+ if !ordered.contains(&key) {
+ ordered.push(key);
+ }
+ }
+
+ let mut names = Vec::with_capacity(ordered.len());
+ let mut types = Vec::with_capacity(ordered.len());
+ for key in ordered {
+ names.push(key.clone());
+ types.push(PFormatParamType::from_schema_type(props[key].get("type")));
}
Self { names, types }
}
@@ -161,7 +212,7 @@ pub fn build_registry(tools: &[Box]) -> PFormatRegistry {
.collect()
}
-/// Render a single tool's p-format signature, e.g. `get_weather[location|unit]`.
+/// Render a single tool's p-format signature, e.g. `get_weather[|]`.
///
/// This signature is included in the tool catalogue within the system prompt
/// to tell the LLM exactly how to order positional arguments for a tool.
@@ -169,7 +220,20 @@ pub fn render_signature(name: &str, params: &PFormatToolParams) -> String {
if params.names.is_empty() {
format!("{name}[]")
} else {
- format!("{name}[{}]", params.names.join("|"))
+ // Each slot is wrapped in angle brackets so it reads as a placeholder to
+ // fill, not as a call to copy. Bare names do get copied: live on flo the
+ // model answered `memory_recall[limit|namespace|query]` — the signature
+ // verbatim, the parameter names sent as the argument *values*. Backticking
+ // the whole signature does not help either; that made it copy the backticks
+ // (`` C `memory_recall[…]` ``) instead. `<…>` marks the slot without
+ // decorating the form.
+ let slots: Vec = params
+ .names
+ .iter()
+ .enumerate()
+ .map(|(i, n)| format!("{i}|<{n}>"))
+ .collect();
+ format!("{name}[{}]", slots.join("|"))
}
}
@@ -212,27 +276,80 @@ pub fn parse_call(body: &str, registry: &PFormatRegistry) -> Option<(String, Val
// values back to named JSON keys with the correct types.
let params = registry.get(name)?;
- let raw_values = split_pipes(inner);
- let mut args = Map::with_capacity(params.names.len());
- for (i, raw) in raw_values.iter().enumerate() {
- let Some(param_name) = params.names.get(i) else {
- // Excess values: drop silently. The schema is the source
- // of truth for argument count.
+ // The correlation field for every rejection below. `parse_call` is pure and
+ // takes no context, but it runs inside the turn's task, so the ambient chat
+ // thread id is readable without threading an argument through a parser that
+ // has no other use for one. `None` outside a turn (unit tests, CLI probes).
+ let session = crate::openhuman::agent::tinyagents::thread_context::current_thread_id();
+ let session_id = session.as_deref().unwrap_or("");
+
+ let tokens = split_pipes(inner);
+ // Index/value pairs, so an odd token count means the model dropped or added
+ // a delimiter. Reject rather than guess: the whole point of the indices is
+ // that a miscounted delimiter can no longer bind a value to the wrong
+ // parameter, and silently keeping the pairs that happen to line up would
+ // put that failure right back.
+ if !tokens.len().is_multiple_of(2) {
+ tracing::debug!(
+ tool = name,
+ session_id,
+ tokens = tokens.len(),
+ "[pformat] odd token count — not index/value pairs, refusing to parse"
+ );
+ return None;
+ }
+
+ let mut args = Map::with_capacity(tokens.len() / 2);
+ for pair in tokens.chunks_exact(2) {
+ let (raw_index, raw) = (pair[0].trim(), &pair[1]);
+ let Ok(slot) = raw_index.parse::() else {
+ // A non-numeric index is a call in the old bare-positional form (or
+ // simply malformed). Refusing is deliberate: parsing it positionally
+ // would silently resurrect the off-by-one this format exists to end.
tracing::debug!(
tool = name,
- index = i,
- "[pformat] dropping excess positional argument"
+ session_id,
+ index = %raw_index,
+ "[pformat] slot index is not a number — refusing to parse"
);
- continue;
+ return None;
+ };
+ let Some(param_name) = params.names.get(slot) else {
+ tracing::debug!(
+ tool = name,
+ session_id,
+ slot,
+ slots = params.names.len(),
+ "[pformat] slot index out of range — refusing to parse"
+ );
+ return None;
};
+ // An empty value is an argument the model did not send, so the key is
+ // left out entirely rather than set to `""`. Inserting `""` makes every
+ // non-string parameter fail schema validation — a typed `max_results`
+ // arriving as `""` means the tool never runs, and the error names a
+ // field the model deliberately left blank, which it cannot satisfy.
+ if raw.trim().is_empty() {
+ tracing::debug!(
+ tool = name,
+ session_id,
+ slot,
+ param = %param_name,
+ "[pformat] empty value for a named slot — argument omitted"
+ );
+ continue;
+ }
let coerced = coerce_value(
raw,
params
.types
- .get(i)
+ .get(slot)
.copied()
.unwrap_or(PFormatParamType::String),
);
+ // Last write wins on a repeated slot. Rare enough not to be worth
+ // rejecting the whole call over, and the later value is the model's
+ // latest intent.
args.insert(param_name.clone(), coerced);
}
@@ -367,14 +484,92 @@ mod tests {
let reg = make_registry();
assert_eq!(
render_signature("get_weather", ®["get_weather"]),
- "get_weather[location|unit]"
+ "get_weather[0||1|]"
);
}
+ /// Required parameters take the leading slots, so the shortest useful call —
+ /// the required values and nothing else — parses correctly. Under plain
+ /// alphabetical order it did not: `memory_recall` advertises
+ /// `required: ["query"]` with optional `limit`/`namespace`, so alphabetical put
+ /// `limit` first and a live model's `memory_recall[Colorado]` set `limit` to a
+ /// string, failing schema validation on all six of its attempts in one turn.
+ #[test]
+ fn required_parameters_take_the_leading_slots() {
+ let params = PFormatToolParams::from_schema(&json!({
+ "type": "object",
+ "properties": {
+ "limit": { "type": "integer" },
+ "namespace": { "type": "string" },
+ "query": { "type": "string" }
+ },
+ "required": ["query"]
+ }));
+ assert_eq!(params.names, vec!["query", "limit", "namespace"]);
+ // Types stay aligned with the reordered names, or coercion would apply the
+ // wrong rule to each slot.
+ assert_eq!(
+ params.types,
+ vec![
+ PFormatParamType::String,
+ PFormatParamType::Integer,
+ PFormatParamType::String
+ ]
+ );
+ assert_eq!(
+ render_signature("memory_recall", ¶ms),
+ "memory_recall[0||1||2|]"
+ );
+
+ // The minimal call is `[0|value]` — required-first is what makes the one
+ // value the model has to send slot 0, rather than an arbitrary number it
+ // has to look up.
+ let mut reg = PFormatRegistry::new();
+ reg.insert("memory_recall".to_string(), params);
+ let (name, args) = parse_call("memory_recall[0|Colorado]", ®).unwrap();
+ assert_eq!(name, "memory_recall");
+ assert_eq!(args, json!({"query": "Colorado"}));
+
+ // Several required parameters keep the schema's declared order, not
+ // alphabetical, so the layout matches how the tool documents itself.
+ let multi = PFormatToolParams::from_schema(&json!({
+ "type": "object",
+ "properties": {
+ "alpha": { "type": "string" },
+ "rule": { "type": "string" },
+ "tool_name": { "type": "string" }
+ },
+ "required": ["tool_name", "rule"]
+ }));
+ assert_eq!(multi.names, vec!["tool_name", "rule", "alpha"]);
+ }
+
+ /// Slots are rendered as `` placeholders, never bare names. A bare
+ /// signature reads as a call to copy: live on flo the model replied
+ /// `memory_recall[limit|namespace|query]`, sending the parameter names as the
+ /// argument values, and every one failed schema validation.
+ #[test]
+ fn signature_slots_are_marked_as_placeholders() {
+ let reg = make_registry();
+ let sig = render_signature("get_weather", ®["get_weather"]);
+ for name in ®["get_weather"].names {
+ assert!(
+ sig.contains(&format!("<{name}>")),
+ "slot {name} must be a placeholder in {sig}"
+ );
+ assert!(
+ !sig.contains(&format!("[{name}|")) && !sig.contains(&format!("|{name}]")),
+ "slot {name} must not appear bare in {sig}"
+ );
+ }
+ // A zero-arg tool has no slots to mark.
+ assert_eq!(render_signature("ping", ®["ping"]), "ping[]");
+ }
+
#[test]
fn parses_simple_call() {
let reg = make_registry();
- let (name, args) = parse_call("get_weather[London|metric]", ®).unwrap();
+ let (name, args) = parse_call("get_weather[0|London|1|metric]", ®).unwrap();
assert_eq!(name, "get_weather");
assert_eq!(args, json!({"location": "London", "unit": "metric"}));
}
@@ -390,7 +585,7 @@ mod tests {
#[test]
fn parses_single_arg_with_spaces() {
let reg = make_registry();
- let (name, args) = parse_call("shell[ls -la /tmp]", ®).unwrap();
+ let (name, args) = parse_call("shell[0|ls -la /tmp]", ®).unwrap();
assert_eq!(name, "shell");
assert_eq!(args, json!({"command": "ls -la /tmp"}));
}
@@ -398,38 +593,38 @@ mod tests {
#[test]
fn handles_pipe_escape() {
let reg = make_registry();
- let (_, args) = parse_call(r"shell[cat foo \| grep bar]", ®).unwrap();
+ let (_, args) = parse_call(r"shell[0|cat foo \| grep bar]", ®).unwrap();
assert_eq!(args, json!({"command": "cat foo | grep bar"}));
}
#[test]
fn handles_bracket_escape() {
let reg = make_registry();
- let (_, args) = parse_call(r"shell[echo \]done\]]", ®).unwrap();
+ let (_, args) = parse_call(r"shell[0|echo \]done\]]", ®).unwrap();
assert_eq!(args, json!({"command": "echo ]done]"}));
}
#[test]
fn handles_backslash_escape() {
let reg = make_registry();
- let (_, args) = parse_call(r"shell[C:\\Users\\bob]", ®).unwrap();
+ let (_, args) = parse_call(r"shell[0|C:\\Users\\bob]", ®).unwrap();
assert_eq!(args, json!({"command": r"C:\Users\bob"}));
}
#[test]
fn coerces_typed_arguments() {
let reg = make_registry();
- // Alphabetical order: verbose, x, y. The signature the model
- // sees in the catalogue is `math[verbose|x|y]` so this is the
- // order it would emit.
- let (_, args) = parse_call("math[true|42|3.14]", ®).unwrap();
+ // Alphabetical order: verbose, x, y. The signature the model sees in
+ // the catalogue is `math[0||1||2|]`, so this is the call
+ // it would write.
+ let (_, args) = parse_call("math[0|true|1|42|2|3.14]", ®).unwrap();
assert_eq!(args, json!({"verbose": true, "x": 42, "y": 3.14}));
}
#[test]
fn coercion_falls_back_to_string_on_failure() {
let reg = make_registry();
- let (_, args) = parse_call("math[maybe|notanumber|alsonotanumber]", ®).unwrap();
+ let (_, args) = parse_call("math[0|maybe|1|notanumber|2|alsonotanumber]", ®).unwrap();
assert_eq!(
args,
json!({
@@ -445,13 +640,16 @@ mod tests {
let reg = make_registry();
// `math` has properties (in source) {x, y, verbose} but
// BTreeMap iteration sorts to {verbose, x, y}.
- assert_eq!(render_signature("math", ®["math"]), "math[verbose|x|y]");
+ assert_eq!(
+ render_signature("math", ®["math"]),
+ "math[0||1||2|]"
+ );
}
#[test]
fn rejects_unknown_tool() {
let reg = make_registry();
- assert!(parse_call("nope[arg]", ®).is_none());
+ assert!(parse_call("nope[0|arg]", ®).is_none());
}
#[test]
@@ -468,30 +666,112 @@ mod tests {
assert!(parse_call("get_weather[London|metric] // comment", ®).is_none());
}
+ /// A slot number the schema has no parameter for is refused outright.
+ /// Silently dropping it is what the bare-positional form did with an excess
+ /// value, and dropping is only safe when the remaining values are still in
+ /// the right slots — which is exactly the assumption indices exist to stop
+ /// relying on.
#[test]
- fn drops_excess_positional_arguments() {
+ fn an_out_of_range_slot_is_refused() {
let reg = make_registry();
- // get_weather only has 2 schema params; the third value is dropped.
- let (_, args) = parse_call("get_weather[London|metric|extra]", ®).unwrap();
- assert_eq!(args, json!({"location": "London", "unit": "metric"}));
+ assert!(parse_call("get_weather[0|London|1|metric|2|extra]", ®).is_none());
+ }
+
+ /// An odd token count means a delimiter was dropped or added, so the pairs
+ /// no longer say what the model meant. Refuse rather than keep the prefix
+ /// that happens to line up.
+ #[test]
+ fn an_odd_token_count_is_refused() {
+ let reg = make_registry();
+ assert!(parse_call("get_weather[0|London|1]", ®).is_none());
+ assert!(parse_call("get_weather[London]", ®).is_none());
+ }
+
+ /// The old bare-positional form must not parse. This is the guarantee that
+ /// replaces the old failure mode: a leftover positional call is rejected and
+ /// reported, instead of binding its values to whichever slots they land in.
+ #[test]
+ fn a_bare_positional_call_is_refused_not_reinterpreted() {
+ let reg = make_registry();
+ // Two values, no indices — the pre-index form.
+ assert!(parse_call("get_weather[London|metric]", ®).is_none());
+ // And the shape that actually misfired live: leading empties standing in
+ // for skipped arguments.
+ assert!(parse_call("get_weather[||metric]", ®).is_none());
+ }
+
+ /// A named slot with nothing after it is an argument the model chose not to
+ /// send, so it does not appear in the object at all. Sending `""` instead is
+ /// what made a skipped integer slot fail validation with an error the model
+ /// could not act on.
+ #[test]
+ fn an_empty_value_is_an_omitted_argument() {
+ let reg = make_registry();
+ let (_, args) = parse_call("get_weather[0||1|]", ®).unwrap();
+ assert_eq!(args, json!({}));
+
+ // Whitespace is nothing written, too.
+ let (_, args) = parse_call("get_weather[0| |1|metric]", ®).unwrap();
+ assert_eq!(args, json!({"unit": "metric"}));
+ }
+
+ /// Sending only the arguments you mean to send is the whole point: no
+ /// leading empties, so no delimiters to miscount.
+ #[test]
+ fn a_sparse_call_names_only_the_slots_it_fills() {
+ let reg = make_registry();
+ let (_, args) = parse_call("get_weather[1|metric]", ®).unwrap();
+ assert_eq!(args, json!({"unit": "metric"}));
+ }
+
+ /// The shape that hard-looped a live turn twelve times, and the one that
+ /// later ran with the search text as the account id. Written with indices,
+ /// both are unambiguous.
+ #[test]
+ fn the_live_gmail_call_binds_correctly_with_indices() {
+ let mut reg = PFormatRegistry::new();
+ reg.insert(
+ "list_threads".to_string(),
+ PFormatToolParams::from_schema(&json!({
+ "type": "object",
+ "properties": {
+ "connection_id": { "type": "string" },
+ "max_results": { "type": "integer" },
+ "query": { "type": "string" },
+ "user_id": { "type": "string" },
+ },
+ })),
+ );
+
+ let (_, args) = parse_call("list_threads[2|Colorado|3|me]", ®).unwrap();
+ assert_eq!(args, json!({"query": "Colorado", "user_id": "me"}));
+ assert!(
+ args.get("max_results").is_none(),
+ "a slot that was never named must be absent, not an empty string: {args}"
+ );
+
+ // One extra leading delimiter used to shift every value one slot late.
+ // It cannot now: the slot is named, so an accidental delimiter makes the
+ // count odd and the call is refused instead of misbound.
+ assert!(parse_call("list_threads[|2|Colorado|3|me]", ®).is_none());
}
+ /// A repeated slot takes the later value — the model's latest intent — and
+ /// does not fail the call.
#[test]
- fn empty_body_pipes_produce_empty_strings() {
+ fn a_repeated_slot_takes_the_last_value() {
let reg = make_registry();
- let (_, args) = parse_call("get_weather[||]", ®).unwrap();
- // 3 raw values: "", "", "". get_weather has 2 params, third is dropped.
- assert_eq!(args, json!({"location": "", "unit": ""}));
+ let (_, args) = parse_call("get_weather[0|London|0|Berlin]", ®).unwrap();
+ assert_eq!(args, json!({"location": "Berlin"}));
}
#[test]
fn signature_round_trips_with_parser() {
let reg = make_registry();
let sig = render_signature("get_weather", ®["get_weather"]);
- // Render uses the same identifier the parser expects.
- assert!(sig.starts_with("get_weather["));
- let synthesised = "get_weather[Berlin|imperial]";
- let (name, args) = parse_call(synthesised, ®).unwrap();
+ assert_eq!(sig, "get_weather[0||1|]");
+ // The numbering the signature shows is the numbering the parser reads.
+ let (name, args) = parse_call("get_weather[0|Berlin|1|imperial]", ®).unwrap();
assert_eq!(name, "get_weather");
assert_eq!(args["location"], json!("Berlin"));
assert_eq!(args["unit"], json!("imperial"));
diff --git a/src/openhuman/agent/prompts/mod_tests.rs b/src/openhuman/agent/prompts/mod_tests.rs
index ffeb89bb0c..34f1066c44 100644
--- a/src/openhuman/agent/prompts/mod_tests.rs
+++ b/src/openhuman/agent/prompts/mod_tests.rs
@@ -783,6 +783,50 @@ fn subagent_render_options_invert_definition_flags() {
assert!(!narrow.include_memory_md);
}
+/// The sub-agent block must teach every escape `split_escaped` honours. It
+/// already drifted once on the slot numbering — a sub-agent wrote the old
+/// `name[arg|arg]` grammar and `parse_pformat_call` dropped the call whole — so
+/// an escape documented in one prompt path and not the other is the same defect
+/// in a quieter form: a value carrying a backslash is encoded on a guess.
+#[test]
+fn the_subagent_pformat_block_documents_every_escape_the_parser_honours() {
+ let workspace = std::env::temp_dir().join(format!(
+ "openhuman_pformat_escapes_{}",
+ uuid::Uuid::new_v4()
+ ));
+ std::fs::create_dir_all(&workspace).unwrap();
+
+ let tools: Vec> = vec![Box::new(TestTool)];
+ let rendered = render_subagent_system_prompt_with_format(
+ &workspace,
+ "reasoning-v1",
+ &[0],
+ &tools,
+ &[],
+ "You are a specialist.",
+ SubagentRenderOptions::default(),
+ ToolCallFormat::PFormat,
+ &[],
+ None,
+ None,
+ );
+
+ for (escape, what) in [
+ (r"`\|`", "a literal pipe"),
+ (r"`\]`", "a literal closing bracket"),
+ (r"`\\`", "a literal backslash"),
+ ] {
+ assert!(
+ rendered.contains(escape),
+ "the sub-agent P-Format block must document {escape} for {what}"
+ );
+ }
+
+ // The escape set is the parser's, not this block's: `pformat::split_pipes`
+ // decodes exactly these three (`handles_backslash_escape` and its
+ // neighbours pin the decoding itself).
+}
+
#[test]
fn render_subagent_system_prompt_honors_identity_safety_and_skills_flags() {
let workspace =
diff --git a/src/openhuman/agent/prompts/render_helpers.rs b/src/openhuman/agent/prompts/render_helpers.rs
index 91f2f81f5b..514292e976 100644
--- a/src/openhuman/agent/prompts/render_helpers.rs
+++ b/src/openhuman/agent/prompts/render_helpers.rs
@@ -434,10 +434,11 @@ pub fn render_subagent_system_prompt_with_format(
"## Tool Use Protocol\n\n\
Tool calls use **P-Format**: compact, positional, pipe-delimited syntax \
wrapped in `` tags.\n\n\
- ```\n\ntool_name[arg1|arg2]\n\n```\n\n\
- Arguments are positional — match the order shown in each tool's `Call as:` \
- signature above (alphabetical by parameter name). \
- Escape `|` as `\\|`, `]` as `\\]` inside values. \
+ ```\n\nget_weather[0|London|1|metric]\n\n```\n\n\
+ A `Call as:` signature numbers its slots and shows each as a `` \
+ placeholder; replace each one with a value, keeping its number, and send \
+ only the arguments you are actually passing. \
+ Escape `|` as `\\|`, `]` as `\\]`, and `\\` as `\\\\` inside values. \
You may emit multiple `` blocks per response.\n\n\
Use the provided tools to accomplish the task. Reply with a concise, dense \
final answer when you have one — the parent agent will weave it back into the \
diff --git a/src/openhuman/agent/prompts/types.rs b/src/openhuman/agent/prompts/types.rs
index d0a4237da4..5b8776650a 100644
--- a/src/openhuman/agent/prompts/types.rs
+++ b/src/openhuman/agent/prompts/types.rs
@@ -273,7 +273,7 @@ impl<'a> PromptTool<'a> {
/// historic format; P-Format is the new default text protocol.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
pub enum ToolCallFormat {
- /// `tool_name[arg1|arg2|...]` — compact, positional. Default.
+ /// `tool_name[index|value|...]` — compact, slot-numbered. Default.
#[default]
PFormat,
/// Legacy JSON-in-tag rendering with full schemas.
diff --git a/src/openhuman/agent/tinyagents/model.rs b/src/openhuman/agent/tinyagents/model.rs
index 1b42f4c4ef..285d73f29d 100644
--- a/src/openhuman/agent/tinyagents/model.rs
+++ b/src/openhuman/agent/tinyagents/model.rs
@@ -751,7 +751,7 @@ mod g1_usage_tests {
#[test]
fn prompt_guided_response_keeps_legacy_pformat_fallback() {
let response = prompt_guided_text_response(
- "lookup[7|needle]".to_string(),
+ "lookup[0|7|1|needle]".to_string(),
&tool_request(),
);