fix: generate Mistral-compatible 9-char alphanumeric tool call IDs#1242
fix: generate Mistral-compatible 9-char alphanumeric tool call IDs#1242noverby wants to merge 19 commits intonearai:stagingfrom
Conversation
Mistral's API requires tool call IDs to match [a-zA-Z0-9]{9} exactly.
Previously, IDs like 'turn1_0', 'recovered_0', 'call_<uuid>', and
'generated_tool_call_N' were generated, which Mistral rejects with
HTTP 400.
Add generate_tool_call_id() that produces deterministic 9-char base-36
IDs from two seed values, and use it at all tool call ID generation
sites.
Fixes nearai#1241
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a compatibility problem with the Mistral API by standardizing the format of tool call IDs. It introduces a new, deterministic utility function to generate IDs that meet Mistral's specific 9-character alphanumeric requirement, thereby preventing HTTP 400 errors and ensuring robust tool call handling across various LLM providers. The change centralizes ID generation, making the system more resilient and consistent. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the previous tool call ID generation with a new deterministic method compatible with Mistral's API. The new generate_tool_call_id function is well-implemented and used consistently across the codebase. I have a minor suggestion for improvement regarding performance.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a provider-compatible, deterministic tool-call ID generator (notably to satisfy Mistral’s strict 9-character alphanumeric requirement) and replaces several ad-hoc tool-call ID formats across the LLM and agent layers.
Changes:
- Added
generate_tool_call_id(seed_a, seed_b)to standardize tool-call IDs across providers. - Replaced multiple string-formatted tool-call IDs (e.g.,
generated_tool_call_{seed},recovered_{n},turn{n}_{i}, UUID-based IDs in tests) with the new generator. - Re-exported the generator from
crate::llmfor broader use.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Adds generate_tool_call_id helper producing deterministic 9-char base-36 IDs. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the LLM module. |
src/llm/rig_adapter.rs |
Uses the generator when normalizing missing/blank tool-call IDs. |
src/llm/reasoning.rs |
Uses the generator for recovered tool calls parsed from content tags. |
src/agent/session.rs |
Switches per-turn tool-call IDs to generator-based IDs. |
src/agent/dispatcher.rs |
Updates tests to use the generator instead of UUIDs for tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call ID generation across the LLM stack to ensure provider compatibility (notably Mistral’s strict 9-character alphanumeric requirement) and to better preserve tool call identity across persisted/replayed histories.
Changes:
- Added a shared
generate_tool_call_id(seed_a, seed_b)helper that produces deterministic 9-char base-36 IDs. - Replaced ad-hoc
format!(...)tool-call IDs in the rig adapter and tool-call recovery logic with the shared generator. - Updated agent code/tests to use persisted tool call IDs / the shared generator instead of UUID/constructed strings.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Adds generate_tool_call_id helper to produce provider-compatible deterministic tool-call IDs. |
src/llm/rig_adapter.rs |
Uses shared generator when a tool-call ID is missing/empty during message conversion. |
src/llm/reasoning.rs |
Uses shared generator for IDs when recovering tool calls from unstructured content. |
src/llm/mod.rs |
Re-exports generate_tool_call_id for broader crate use. |
src/agent/session.rs |
Switches thread message reconstruction to use “persisted” tool call IDs (but currently references a non-existent field). |
src/agent/dispatcher.rs |
Updates tests to use generate_tool_call_id instead of UUID-based IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Introduces a deterministic, provider-compatible tool-call ID generator (notably to satisfy Mistral’s strict [a-zA-Z0-9]{9} requirement) and starts migrating call sites away from ad-hoc IDs.
Changes:
- Added
generate_tool_call_id(seed_a, seed_b)insrc/llm/provider.rsand re-exported it fromsrc/llm/mod.rs. - Updated multiple tool-call ID creation paths (rig adapter + tool-call recovery + some tests) to use the new generator.
- Refactored
Thread::messages()tool-call/result correlation by generating per-turn synthetic IDs (currently not provider-compatible).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/llm/provider.rs | Adds generate_tool_call_id helper intended to satisfy all providers’ tool-call ID constraints. |
| src/llm/mod.rs | Re-exports generate_tool_call_id for crate-level access. |
| src/llm/rig_adapter.rs | Uses generated IDs when tool-call IDs are missing/empty during Rig message conversion. |
| src/llm/reasoning.rs | Switches recovered tool-call IDs from recovered_{n} to generated provider-compatible IDs. |
| src/agent/session.rs | Refactors tool-call/result ID assignment within Thread::messages() (but currently generates non-compliant IDs). |
| src/agent/dispatcher.rs | Updates test providers to use the new generator for tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call ID generation across the LLM stack to ensure provider compatibility (notably Mistral’s strict 9-character alphanumeric requirement) while keeping IDs deterministic for replay/history scenarios.
Changes:
- Introduces
generate_tool_call_id(seed_a, seed_b)insrc/llm/provider.rsto produce deterministic 9-char base-36 IDs. - Updates tool-call ID creation in the rig adapter and tool-call recovery logic to use the shared generator.
- Re-exports the generator from
crate::llmand updates internal usages/tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/llm/provider.rs | Adds the shared deterministic, provider-compatible tool-call ID generator. |
| src/llm/mod.rs | Re-exports generate_tool_call_id from the llm module surface. |
| src/llm/rig_adapter.rs | Uses the shared generator when a tool call ID is missing/empty. |
| src/llm/reasoning.rs | Uses the shared generator for recovered tool calls from content. |
| src/agent/session.rs | Attempts to switch synthetic IDs to the shared generator for turn tool calls/results (currently with a signature mismatch). |
| src/agent/dispatcher.rs | Updates test providers to use the shared generator for tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call IDs across the LLM stack to be deterministic and compatible with stricter providers (notably Mistral’s 9-character alphanumeric requirement).
Changes:
- Added a shared
generate_tool_call_id(seed_a, seed_b)helper and re-exported it fromcrate::llm. - Switched multiple call sites (rig adapter normalization, reasoning tool-call recovery, session history tool-call IDs, dispatcher tests) to use the new generator.
- Updated session message construction to ensure tool call declarations and tool results share consistent synthetic IDs within a turn.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Introduces the shared tool-call ID generator intended to satisfy provider constraints. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module. |
src/llm/rig_adapter.rs |
Uses the shared generator when a tool-call ID is missing during message conversion. |
src/llm/reasoning.rs |
Uses the shared generator for recovered tool calls parsed from content tags. |
src/agent/session.rs |
Generates provider-compatible synthetic tool-call IDs from (turn, tool) indices for message history building. |
src/agent/dispatcher.rs |
Updates tests to use the shared generator for tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call IDs across the LLM stack to satisfy stricter provider constraints (notably Mistral’s requirement of exactly 9 alphanumeric characters), ensuring tool call declarations and tool results can be reliably correlated.
Changes:
- Added a provider-level
generate_tool_call_id(seed_a, seed_b)helper and tests for format/determinism. - Updated tool-call ID generation/normalization paths (rig adapter + reasoning recovery + session message building) to produce provider-compatible IDs.
- Adjusted a couple of agent tests to use the new deterministic ID generator.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/rig_adapter.rs |
Normalizes tool-call IDs to provider constraints; hashes non-compliant raw IDs into compliant IDs. |
src/llm/reasoning.rs |
Uses provider-compatible IDs when recovering tool calls from unstructured content. |
src/llm/provider.rs |
Introduces generate_tool_call_id plus unit tests to validate constraints and determinism. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module API surface. |
src/agent/session.rs |
Generates provider-compatible synthetic IDs for per-turn tool calls/results in conversation history. |
src/agent/dispatcher.rs |
Updates test fixtures to use generate_tool_call_id instead of random UUID-based IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call IDs across the LLM stack to satisfy stricter provider constraints (notably Mistral’s [a-zA-Z0-9]{9} requirement) by introducing a shared ID generator and normalizing/deriving IDs where needed.
Changes:
- Added
generate_tool_call_id(seed_a, seed_b)to produce deterministic provider-compatible 9-char base62 IDs. - Updated tool-call recovery and session message construction to use provider-compatible IDs.
- Updated rig adapter tool-call ID normalization to pass through compliant IDs and hash-map non-compliant IDs into compliant ones.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/rig_adapter.rs |
Normalizes tool-call IDs for rig-core providers; hashes non-compliant IDs into provider-compatible IDs. |
src/llm/reasoning.rs |
Uses provider-compatible IDs when recovering tool calls from model output. |
src/llm/provider.rs |
Introduces generate_tool_call_id plus unit tests for format/determinism. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module. |
src/agent/session.rs |
Generates provider-compatible synthetic tool-call IDs when rebuilding message history. |
src/agent/dispatcher.rs |
Updates test providers to emit provider-compatible tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call IDs across the LLM stack to satisfy stricter provider constraints (notably Mistral’s [a-zA-Z0-9]{9} requirement) by introducing a deterministic, provider-compatible ID generator and using it in multiple tool-call construction paths.
Changes:
- Added
generate_tool_call_id(seed_a, seed_b)to produce deterministic 9-char base62 tool-call IDs. - Updated tool-call ID generation/normalization in the rig adapter and tool-call recovery logic to ensure provider compatibility.
- Updated agent session/test code paths to use the new generator for synthetic IDs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/llm/rig_adapter.rs | Normalizes tool-call IDs to provider-compliant 9-char alphanumeric IDs (pass-through or hash→generate). |
| src/llm/reasoning.rs | Uses provider-compatible generated IDs for recovered tool calls. |
| src/llm/provider.rs | Introduces generate_tool_call_id plus unit tests for format/determinism. |
| src/llm/mod.rs | Re-exports generate_tool_call_id for external callers. |
| src/agent/session.rs | Generates provider-compatible synthetic tool-call IDs for turn tool calls/results. |
| src/agent/dispatcher.rs | Adjusts test provider tool-call IDs to use the shared generator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call ID generation/normalization across the LLM stack to satisfy stricter providers (notably Mistral’s [a-zA-Z0-9]{9} requirement), while keeping IDs deterministic for replayed history and recovered tool calls.
Changes:
- Added a deterministic, provider-compatible
generate_tool_call_id(seed_a, seed_b)utility and unit tests. - Normalized/rehydrated tool-call IDs in the rig adapter and reasoning recovery logic to ensure provider compatibility.
- Updated session message building and dispatcher tests to use the new tool-call ID generator.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Introduces generate_tool_call_id (9-char base62) and tests validating format/determinism. |
src/llm/rig_adapter.rs |
Normalizes tool-call IDs (pass-through if already compliant; otherwise hash → compliant ID). |
src/llm/reasoning.rs |
Uses provider-compatible IDs when recovering tool calls from content. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module API surface. |
src/agent/session.rs |
Generates consistent provider-compatible call IDs when constructing per-turn message history. |
src/agent/dispatcher.rs |
Updates test providers to emit compliant tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes tool-call ID generation/normalization to satisfy “Responses-style” provider constraints (notably Mistral’s requirement of exactly 9 ASCII-alphanumeric characters), while keeping IDs deterministic so tool-call declarations and tool results can be reliably correlated across message history.
Changes:
- Added a shared
generate_tool_call_id()helper that produces[a-zA-Z0-9]{9}IDs and unit tests for its format/determinism. - Updated tool-call ID normalization in the rig adapter to pass through already-compliant IDs and deterministically map non-compliant IDs into compliant ones.
- Replaced ad-hoc IDs in tool-call recovery and agent session message construction with provider-compliant generated IDs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/rig_adapter.rs |
Normalizes tool-call IDs to provider-compliant [a-zA-Z0-9]{9} using pass-through or deterministic hashing + generator fallback. |
src/llm/reasoning.rs |
Uses the shared generator for recovered tool calls instead of recovered_{n} IDs. |
src/llm/provider.rs |
Introduces generate_tool_call_id() plus unit tests validating format and determinism. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module. |
src/agent/session.rs |
Generates provider-compatible synthetic tool-call IDs per turn/tool index and reuses them for tool results. |
src/agent/dispatcher.rs |
Updates test providers to return provider-compatible tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
zmanian
left a comment
There was a problem hiding this comment.
Review: REQUEST CHANGES
Sound approach -- centralizing tool call ID generation for Mistral compatibility. All four generation sites updated, good test coverage for the new function. But two project policy violations need fixing.
Must fix
1. .unwrap() in production code (src/llm/provider.rs)
String::from(std::str::from_utf8(&buf).unwrap())Per CLAUDE.md: "No .unwrap() or .expect() in production code." The buffer only contains ASCII, so this is technically safe, but the policy is zero-tolerance. Build the string without from_utf8, e.g., buf.iter().map(|&b| b as char).collect::<String>().
2. .expect() in production code (src/llm/rig_adapter.rs)
let bytes: [u8; 8] = digest[0..8].try_into().expect("slice with incorrect length");SHA-256 always produces 32 bytes, but use a match or array indexing instead of .expect().
Should fix
3. Magic constant 99 in reasoning.rs -- used as sentinel seed_b for recovered tool calls. Add a named constant with a comment explaining the collision avoidance strategy.
4. No tests for normalized_tool_call_id in rig_adapter.rs -- this function was significantly refactored (SHA-256 hashing, pass-through logic) but has zero test coverage. Add tests for: conforming ID pass-through, non-conforming ID hashing, empty/whitespace input, determinism.
5. session.rs uses turn_idx instead of turn.turn_number -- subtle semantic change after truncate_turns(). Add a comment noting this is intentional and safe because truncation re-numbers turns.
What's good
- Centralized
generate_tool_call_id()is the right design - Base-62 encoding produces Mistral-compatible
[a-zA-Z0-9]{9}format - Deterministic generation is essential for replayed conversation histories
- Good test coverage for the new function
CI: only classify/scope checks visible -- needs full build/test run.
- Remove .unwrap() in generate_tool_call_id (provider.rs) per zero-tolerance policy - Remove .expect() in normalized_tool_call_id (rig_adapter.rs), use direct array indexing - Replace magic constant 99 with named RECOVERED_TOOL_CALL_SEED in reasoning.rs - Add tests for normalized_tool_call_id: passthrough, hashing, empty/whitespace, determinism - Add comment explaining intentional use of turn_idx vs turn.turn_number in session.rs - Fix duplicate `mod tests` block in provider.rs (pre-existing compile error) - Update stale test assertions expecting old `generated_tool_call_` prefix format [skip-regression-check]
df589ce to
d7f31e4
Compare
There was a problem hiding this comment.
Pull request overview
Fixes tool-call ID generation to satisfy Mistral’s strict requirement of exactly 9 ASCII alphanumeric characters, preventing 400 errors when tool calling is enabled.
Changes:
- Added a shared
generate_tool_call_id(seed_a, seed_b)utility (9-char base-62) and re-exported it. - Normalized/rewrote tool-call IDs in the rig adapter (including hashing non-conforming incoming IDs).
- Updated tool-call ID generation in tool-call recovery and agent session/test paths to use the compliant generator.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Adds generate_tool_call_id + unit tests asserting format/determinism. |
src/llm/mod.rs |
Re-exports generate_tool_call_id for use across the crate. |
src/llm/rig_adapter.rs |
Normalizes tool-call IDs (pass-through if compliant, otherwise hash→generate) and updates tests. |
src/llm/reasoning.rs |
Uses compliant IDs when recovering tool calls from malformed text responses. |
src/agent/session.rs |
Generates provider-compatible tool-call IDs when building message history (assistant tool calls + tool results). |
src/agent/dispatcher.rs |
Updates tests to use compliant tool-call IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses Mistral’s strict tool call ID validation by ensuring all tool call IDs sent through the LLM provider layer can conform to the required 9-character alphanumeric format, avoiding HTTP 400 failures when tool calls are present.
Changes:
- Added a shared
generate_tool_call_id(seed_a, seed_b)utility to produce deterministic 9-character alphanumeric IDs. - Updated
RigAdaptermessage conversion to normalize/pass-through conforming IDs and deterministically map non-conforming IDs to provider-compatible ones. - Updated tool call ID generation in session history building and in the tool-call recovery path.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/llm/provider.rs |
Introduces generate_tool_call_id() and tests validating length/character constraints and determinism. |
src/llm/rig_adapter.rs |
Normalizes tool call IDs to [a-zA-Z0-9]{9} (pass-through when already valid; otherwise deterministic mapping) and updates tests accordingly. |
src/llm/reasoning.rs |
Switches recovered tool call IDs to use generate_tool_call_id() with a dedicated seed. |
src/llm/mod.rs |
Re-exports generate_tool_call_id from the llm module. |
src/agent/session.rs |
Generates provider-compatible synthetic tool call IDs for per-turn tool call/result correlation. |
src/agent/dispatcher.rs |
Updates test tool call IDs to use the new generator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| /// Generate a tool-call ID that satisfies all providers. | ||
| /// | ||
| /// Mistral requires exactly 9 alphanumeric characters (`[a-zA-Z0-9]{9}`). | ||
| /// Other providers accept any non-empty string. By default we produce a | ||
| /// 9-char base-62 string derived from two seed values so the ID is both | ||
| /// deterministic (for replayed history) and provider-compatible. |
Summary
[a-zA-Z0-9]{9}exactlyturn1_0,recovered_0,call_<uuid>, andgenerated_tool_call_Nwere generated, causing HTTP 400 errorsgenerate_tool_call_id()utility that produces deterministic 9-char base-36 IDs from two seed valuessession.rs,rig_adapter.rs,reasoning.rs, anddispatcher.rsDetails
The function uses a simple hash-combine of two seed values (e.g., turn number and tool index) to produce a deterministic, provider-compatible ID. The output is zero-padded base-36 (digits + lowercase letters), which satisfies Mistral's constraint while remaining valid for all other providers.
Fixes #1241