Add Python / TypeScript code-style tool dispatchers - #184
Conversation
Bump the vendored tinytools submodule to commit 705677c, incorporating upstream fixes and improvements. Auto-committed-on: macbook
Add opt-in `Python` and `Typescript` tool dispatcher variants that render tool schemas as code-style signatures in the system prompt, sharing P-Format's positional argument registry. Also skip appending the protocol block when the host has already rendered one, avoiding duplicate prompt content and token waste. Auto-committed-on: macbook
Added tests covering Python and TypeScript code dialects, verifying that schemas are stripped from the wire and signatures are rendered into the system prompt. Also added a test ensuring a host-rendered protocol block is not duplicated when the dialect rewrite runs. Auto-committed-on: macbook
Reformatted multi-line assert macros and the invoke_default call to comply with rustfmt line-width rules, wrapping expressions that exceeded the configured maximum line length. No behavior or test logic was changed. Auto-committed-on: macbook
The tool dialect documentation now covers the newly added `CodeDialect` for Python and TypeScript call syntax, alongside the existing XML, P-Format, and native dialects. It also explains how the agent loop selects a dialect per run via `RunPolicy::tool_dialect`, including the automatic fallback to XML when native is unsupported, and clarifies that P-Format and code dialects are opt-in while the loop strips pre-composed schemas without duplicating the tool protocol block. Auto-committed-on: macbook
|
Warning Review limit reached
This review includes 11 billable files and costs up to $2.75. Or wait 9 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
Comment |
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. |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 5 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before mergeNone. How this fits togetherflowchart LR
n0["RunDialect<br/>changed"]:::changed
n1["tool_dispatcher_round_trips_as_snake_case<br/>changed"]:::changed
n2["ToolDispatcher<br/>changed"]:::changed
n3["...d_pformat_dialect_parses_positional_calls<br/>changed"]:::changed
n4["invoke_default"]:::impacted
n5["...her_gates_on_the_post_middleware_tool_set"]:::impacted
n6["set_default_model"]:::impacted
n7["find"]:::impacted
n8["run_loop_body"]:::impacted
n1 -->|uses| n2
n3 -->|calls| n4
n3 -->|tests| n4
n3 -->|calls| n6
n3 -->|tests| n6
n3 -->|calls| n7
n3 -->|tests| n7
n5 -->|uses| n2
n5 -->|calls| n4
n5 -->|tests| n4
n5 -->|calls| n6
n5 -->|tests| n6
n8 -->|uses| n0
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
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![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: 68b0eb0abc
ℹ️ 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".
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0274 · 538,980 in / 16,809 out · 74,279 cached (14%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 849 embedded
critique: $0.0146 · 278,045 in / 10,015 out · 30,283 cached (11%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0114 · 226,124 in / 4,813 out · 21,086 cached (9%) · gpt-5.6-luna
tests: $0.0004 · 17,996 in / 139 out · 14,028 cached (78%) · deepseek/deepseek-v4-flash
description: $0.0003 · 8,954 in / 83 out · 4,989 cached (56%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p1
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update the vendored tinytools subproject to a newer commit, incorporating upstream changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The previous logic skipped appending the protocol block when the system prompt contained a matching heading, but this could suppress the canonical block when the heading was merely mentioned in user-controlled text. The loop now always renders its authoritative block from the final post-middleware tool set, ensuring the correct dialect and catalogue are presented regardless of prompt content. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The host prompt string was split across two lines, which is unnecessary and makes the test fixture harder to read. This change joins the string onto a single line without altering its content, keeping the test setup concise. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4d1cf23b6
ℹ️ 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".
The ToolDispatcher enum now includes `Python` and `Typescript` as additional tool-call encoding strategies, alongside the existing `Auto`, `Native`, `Xml`, and `Pformat` variants. The documentation has been updated to reflect these new forced text dialects, which are used when parsing tool-call markup from assistant text in the runtime and when normalizing tool results in the harness. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # vendor/tinytools
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38470d98b
ℹ️ 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".
| prompt_tools::with_tool_instructions(&messages, &tools, &request.tool_choice) | ||
| } | ||
| Self::PFormat(_) => { | ||
| Self::PFormat(_) | Self::Code(..) => { |
There was a problem hiding this comment.
When either new code dispatcher completes a tool call and enters another model iteration, coalesce_tool_results has already serialized the recovered structured call back into the generic JSON-in-tag form before this Self::Code arm appends Python or TypeScript instructions. The next request therefore demonstrates one call syntax in its history while requiring another in its system catalogue, undermining the selected dialect precisely during multi-tool loops; make transcript coalescing dialect-aware or replay these calls through CodeDialect. This also conflicts with the updated module documentation's definition of a dialect as including its history replay shape.
AGENTS.md reference: AGENTS.md:L78-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The previously-blocking findings are resolved. Clearing the changes request.
$0.0072 · 116,512 in / 11,454 out · 25,639 cached (22%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash, deepseek-v4-flash · 939 embedded
critique: $0.0059 · 80,835 in / 7,702 out · 8,066 cached (10%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
tests: $0.0005 · 18,299 in / 191 out · 12,360 cached (68%) · deepseek/deepseek-v4-flash
description: $0.0002 · 9,222 in / 2,426 out · 1,024 cached (11%) · deepseek-v4-flash
| `Native`, `Xml`, `Pformat`); modelled as an enum rather than a free-form | ||
| string so an unrecognised mode is a mapping error at the boundary, not a | ||
| silent fallthrough in the turn loop. | ||
| `Native`, `Xml`, `Pformat`, `Python`, `Typescript`); modelled as an enum |
There was a problem hiding this comment.
Preserve compatibility for exhaustive ToolDispatcher matches
Adding Python and Typescript variants to the public ToolDispatcher enum makes existing downstream match expressions that exhaustively handle the previous variants fail to compile. Listing the variants here confirms that the breaking public surface remains present; either provide a compatibility strategy (such as a non-exhaustive enum and migration guidance) or explicitly handle this as a versioned breaking change before merging.
[RULE] public-api-compatibility ·
tinysweeper
Bot
added
priority: p2
1 participant
Summary
Wires the new
tinytools-agentcode-call dialect into the agent loop:ToolDispatcher::{Python, Typescript}are opt-in text dialects, never selected byAuto.RunDialect::Coderenders Python or TypeScript signatures, strips schemas from the wire, and shares the P-Format registry for batch and streaming recovery.API or behavior changes
config::ToolDispatchergainsPythonandTypescriptwith serde spellingspythonandtypescript.ToolDispatcheris a public exhaustive enum, downstream exhaustive matches must add arms for both variants. This intentional public API expansion must be accounted for in release versioning.Auto,Native,Xml, andPformatselection behavior is unchanged.Tests
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo build --workspace --all-targetscargo test -p tinyagents-harness --lib(1,230 passed)cargo test -p tinyagents-integration-tests(all non-network tests passed)cargo test --workspace --exclude tinyagents-harness --exclude tinyagents-integration-tests(all non-network tests passed)New coverage includes Python and TypeScript dispatch, forced P-Format tool choices, structured-output recovery, and a regression proving that an untrusted protocol heading cannot suppress the current catalogue.