feat(rust): add initial tanstack-ai Rust SDK and rs-chat example#415
Open
swalker326 wants to merge 5 commits intoTanStack:mainfrom
Open
feat(rust): add initial tanstack-ai Rust SDK and rs-chat example#415swalker326 wants to merge 5 commits intoTanStack:mainfrom
swalker326 wants to merge 5 commits intoTanStack:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Rust workspace crate Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatEngine as TextEngine
participant Middleware as MiddlewareRunner
participant Adapter as TextAdapter
participant Tools as ToolExecutor
Client->>ChatEngine: chat(ChatOptions)
activate ChatEngine
ChatEngine->>Middleware: run_on_start()
Middleware-->>ChatEngine: (ack)
loop Agent Iteration
ChatEngine->>Middleware: run_on_iteration()
Middleware-->>ChatEngine: (maybe modify config)
ChatEngine->>Adapter: chat_stream(TextOptions)
activate Adapter
loop Stream Chunks
Adapter-->>ChatEngine: StreamChunk
ChatEngine->>Middleware: run_on_chunk(chunk)
Middleware-->>ChatEngine: Option<StreamChunk>
end
deactivate Adapter
alt tool_calls detected
ChatEngine->>Tools: execute_tool_calls(tool_calls)
Tools-->>ChatEngine: ExecuteToolCallsResult
ChatEngine->>ChatEngine: append tool result messages
else finish
ChatEngine->>ChatEngine: finish loop
end
end
ChatEngine->>Middleware: run_on_finish(FinishInfo)
Middleware-->>ChatEngine: (ack)
ChatEngine-->>Client: ChatResult
deactivate ChatEngine
sequenceDiagram
participant Provider as AI Provider API
participant Adapter as OpenAiTextAdapter
participant Parser as SSE Parser
participant StreamProc as StreamProcessor
participant Client
Client->>Adapter: chat_stream(TextOptions)
Adapter->>Provider: POST /responses (stream=true)
Provider-->>Adapter: SSE byte stream
Adapter->>Parser: parse_openai_sse_stream(bytes)
loop SSE events
Parser-->>Adapter: StreamChunk
end
Adapter-->>Client: Stream<StreamChunk>
loop For Each Chunk
Client->>StreamProc: process_chunk(chunk)
StreamProc-->>Client: Vec<StreamChunk>
end
Client->>Client: assemble final text/tool results
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (11)
packages/rust/tanstack-ai/src/stream/strategies.rs (1)
99-104: Short-circuit evaluation may cause state inconsistency in stateful strategies.The
any()iterator short-circuits on the firsttrue, meaning subsequent strategies won't haveshould_emitcalled. If aBatchStrategyis later in the list, itschunk_countwon't increment when an earlier strategy returnstrue, causing inconsistent emission timing.Consider whether this is intentional OR-semantics, or if all strategies should be evaluated to maintain state:
♻️ Alternative: evaluate all strategies to maintain state
impl ChunkStrategy for CompositeStrategy { fn should_emit(&mut self, chunk: &str, accumulated: &str) -> bool { - self.strategies - .iter_mut() - .any(|s| s.should_emit(chunk, accumulated)) + // Evaluate all strategies to maintain state, then return OR result + self.strategies + .iter_mut() + .fold(false, |acc, s| s.should_emit(chunk, accumulated) || acc) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/stream/strategies.rs` around lines 99 - 104, The current CompositeStrategy::should_emit uses self.strategies.iter_mut().any(...) which short-circuits and prevents later stateful strategies (e.g., BatchStrategy with chunk_count) from observing the chunk; change the implementation to iterate and call should_emit on every strategy (e.g., a for loop or fold over self.strategies.iter_mut()), accumulate an OR result, and return true if any strategy returned true so that all strategies update their internal state while preserving OR-semantics; update references to ChunkStrategy, CompositeStrategy, should_emit, BatchStrategy, chunk_count, and strategies accordingly.packages/rust/tanstack-ai/src/stream/json_parser.rs (1)
97-101: Add test for interleaved bracket/brace nesting.The current tests don't cover cases like
[{"a": 1where brackets and braces are interleaved, which would expose the closing order bug.🧪 Suggested additional test
#[test] fn test_interleaved_nesting() { let result = parse_partial_json(r#"[{"name": "Jo"#).unwrap(); assert_eq!(result, serde_json::json!([{"name": "Jo"}])); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/stream/json_parser.rs` around lines 97 - 101, Add a unit test that covers interleaved bracket/brace nesting to catch the closing-order bug: create a new test function (e.g., test_interleaved_nesting) that calls parse_partial_json with input like r#"[{"name": "Jo"# and asserts the returned value matches the expected partial JSON array/object (e.g., serde_json::json!([{"name":"Jo"}])). Place the test alongside test_nested_partial in json_parser.rs so parse_partial_json is exercised for mixed '[' and '{' nesting ordering.packages/rust/tanstack-ai/src/stream_response.rs (1)
98-109: Silent failure on chunk serialization may mask bugs.Using
unwrap_or_default()for serialization produces an empty string if serialization fails, which would emitdata: \n\n(empty data) rather than surfacing the error. This could mask bugs inStreamChunkserialization.🛡️ Consider logging or handling serialization errors
Ok(chunk) => { - let json = serde_json::to_string(&chunk).unwrap_or_default(); - Ok(Bytes::from(format!("data: {}\n\n", json))) + match serde_json::to_string(&chunk) { + Ok(json) => Ok(Bytes::from(format!("data: {}\n\n", json))), + Err(e) => Ok(Bytes::from(format!( + "data: {}\n\n", + serde_json::json!({"type": "RUN_ERROR", "error": {"message": format!("Serialization error: {}", e)}}) + ))), + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/stream_response.rs` around lines 98 - 109, The serialization currently uses serde_json::to_string(&chunk).unwrap_or_default() which silences serialization failures and emits empty SSE data; change the closure inside stream.map (the map over result) to handle serde_json::to_string errors explicitly: match or map the Result from serde_json::to_string(&chunk) and on Err produce an SSE chunk similar to the existing Err(e) arm (e.g., serde_json::json!({"type":"SERIALIZATION_ERROR","error":{"message": err.to_string()}})) wrapped with Bytes::from(format!("data: {}\n\n", ...)), or propagate the error out so the stream yields an Err; ensure you replace the use of unwrap_or_default with this explicit error handling for the chunk branch to avoid silent failures (refer to the closure handling Ok(chunk) in stream.map and the existing Err(e) branch for formatting).packages/rust/tanstack-ai/src/adapters/gemini.rs (2)
75-86: GeminifunctionResponse.nameusestool_call_idwhich may contain the function name.This assumes
msg.tool_call_idcontains the function name (since Gemini correlates by name, not ID). Iftool_call_idis a UUID-style ID from another provider, this will fail silently. Consider adding a comment clarifying the expected value.Add clarifying comment
if msg.role == MessageRole::Tool { let tool_result_text = match &msg.content { MessageContent::Text(text) => text.clone(), MessageContent::Null | MessageContent::Parts(_) => String::new(), }; + // Gemini correlates tool results by function name, not a separate ID. + // tool_call_id is expected to contain the function name here. contents.push(serde_json::json!({ "role": "user", "parts": [{ "functionResponse": { "name": msg.tool_call_id.as_deref().unwrap_or(""),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/adapters/gemini.rs` around lines 75 - 86, The code sets functionResponse.name using msg.tool_call_id which may be an opaque ID rather than the function name Gemini expects; update the implementation to either use a dedicated function name field (e.g., msg.tool_name or similar) when available or, at minimum, add a clear comment above the block explaining the expectation that msg.tool_call_id must contain the function name (and note potential mismatch if it is a UUID from another provider). Refer to msg.tool_call_id and the functionResponse.name assignment to locate where to add the comment or swap to the correct field.
379-385: Consider using a logging crate instead ofeprintln!for parse errors.Library code should avoid writing directly to stderr. Consider using
tracingorlogcrate for structured logging that consumers can configure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/adapters/gemini.rs` around lines 379 - 385, Replace the direct stderr write in the Err(e) branch with structured logging: add and enable a logging crate (e.g., tracing or log) in the crate, import the macro (e.g., tracing::error or log::error) at the top of the module, and change the eprintln! call in packages/rust/tanstack-ai/src/adapters/gemini.rs (the Err(e) => { ... } block that prints "Failed to parse Gemini event") to use error! with the error and the truncated data_str so consumers can configure log output instead of writing directly to stderr.packages/rust/tanstack-ai/src/adapters/anthropic.rs (1)
640-661: Duplicate handling ofContentBlockStartcreates maintenance risk.
ContentBlockStartis handled inline at lines 441-464 (which correctly populatestool_block_ids), but also inconvert_anthropic_eventat lines 640-661. The inline match takes precedence, so this code is currently unreachable. Consider removing the duplicate to avoid divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/adapters/anthropic.rs` around lines 640 - 661, The ContentBlockStart arm in convert_anthropic_event duplicates logic already handled inline (which populates tool_block_ids) and is therefore unreachable; remove the ContentBlockStart match branch from convert_anthropic_event (the block that constructs StreamChunk::TextMessageStart / StreamChunk::ToolCallStart) so the single inline handler that updates tool_block_ids remains the authoritative implementation, or refactor convert_anthropic_event to call the same helper used by the inline match to avoid diverging logic between AnthropicEvent::ContentBlockStart and the inline handling.packages/rust/tanstack-ai/src/tools/registry.rs (1)
82-125:FrozenToolRegistryprovides thread-safe tool lookup but isn't integrated into the chat engine.The relevant code snippets show
ChatOptionsandTextEngineuseVec<Tool>directly. While this isn't a bug (ownership transfer prevents mutation), consider documenting when to useFrozenToolRegistryvs passingVec<Tool>, or integrating it into the engine for explicit immutability guarantees.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/tools/registry.rs` around lines 82 - 125, The codebase defines FrozenToolRegistry for immutable, thread-safe tool access but ChatOptions and TextEngine still accept Vec<Tool>; either integrate FrozenToolRegistry into those APIs or add docs clarifying when to use each: update ChatOptions and TextEngine signatures/constructors to accept FrozenToolRegistry instead of Vec<Tool> (or provide overloads/builders that convert Vec<Tool> via FrozenToolRegistry::freeze) and update call sites to pass a frozen registry, or add clear documentation/comments near ChatOptions, TextEngine and FrozenToolRegistry explaining ownership vs immutability and when to prefer FrozenToolRegistry for thread-safe sharing.packages/rust/tanstack-ai/src/messages.rs (2)
277-282:normalize_to_model_messagesis a no-op - clarify intent.The function documentation suggests it should handle different input types, but it just clones the input. If this is a placeholder for future polymorphism, consider adding a TODO or implementing the actual conversion logic.
Add clarifying comment or TODO
/// Normalize messages to ModelMessage format. /// If the messages are already ModelMessages, pass through. /// If they are UI messages, convert them. +// TODO: This currently only handles ModelMessage input. +// Future: accept enum of UiMessage | ModelMessage for true normalization. pub fn normalize_to_model_messages(messages: &[ModelMessage]) -> Vec<ModelMessage> { messages.to_vec() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/messages.rs` around lines 277 - 282, The function normalize_to_model_messages currently just clones its ModelMessage slice (a no-op) despite docs promising conversion from UI message types; update the implementation to either (A) implement actual conversion logic from any UI/other message variants into ModelMessage (accepting the appropriate input type/enum and mapping fields into ModelMessage) or (B) if this is a deliberate placeholder, add a clear TODO comment and adjust the docstring to state it's intentionally a passthrough for ModelMessage inputs only; refer to normalize_to_model_messages and ModelMessage to locate where to implement the mapping or insert the TODO and updated docs.
284-324: Test coverage is minimal for a conversion layer.The tests cover basic text scenarios but not:
- Tool calls in assistant messages
- Tool results
- Multimodal content (images, audio)
- Thinking parts
Consider adding tests for these paths to ensure conversion correctness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/messages.rs` around lines 284 - 324, Add tests covering conversion edge cases not currently asserted: create UiMessage and ModelMessage fixtures exercising tool calls (MessagePart::ToolCall / ModelMessage.tool_calls), tool results (MessagePart::ToolResult / MessageContent::ToolResult), multimodal parts (MessagePart::Image, MessagePart::Audio) and thinking parts (MessagePart::Thinking), then assert round-trip correctness via ui_message_to_model_messages and model_message_to_ui_message (check roles, content variants, tool_call ids/names/results, and that image/audio metadata round-trips). Target the existing test module and functions (e.g., add new #[test] functions alongside test_ui_to_model_text and test_model_to_ui_round_trip) to validate each conversion path.packages/rust/tanstack-ai/src/adapter.rs (2)
8-16:TextAdapterConfigfieldstimeout,max_retries,headersare unused.The config struct defines these fields, but
with_configimplementations in the adapters only usebase_url. Consider implementing retry/timeout logic or documenting these as reserved for future use.Add TODO or implement usage
/// Configuration for adapter instances. +/// +/// Note: `timeout`, `max_retries`, and `headers` are reserved for future use. +/// Currently only `base_url` and `api_key` are utilized by adapters. #[derive(Debug, Clone, Default)] pub struct TextAdapterConfig {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/adapter.rs` around lines 8 - 16, TextAdapterConfig declares timeout, max_retries, and headers but they aren't used by the adapters' with_config implementations (which only apply base_url); update the code so these fields are either actually applied when building the HTTP client or clearly marked for future use: either (A) implement use of TextAdapterConfig.timeout, max_retries, and headers inside the adapters' with_config functions (e.g., configure the reqwest::ClientBuilder or HTTP client in the adapter to set timeout, attach headers, and add retry logic using max_retries), referencing TextAdapterConfig and the adapters' with_config methods, or (B) add a concise TODO and documentation on TextAdapterConfig stating these fields are reserved for future use and annotate them to suppress unused warnings (e.g., #[allow(dead_code)] or similar), so the intent is explicit; pick one approach and apply consistently to all adapters that implement with_config.
43-89: Additional adapter traits are defined but not implemented.
ImageAdapter,SummarizeAdapter,TtsAdapter, andTranscriptionAdapterare placeholder traits for future capabilities. Consider adding a tracking issue or documentation indicating these are planned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/tanstack-ai/src/adapter.rs` around lines 43 - 89, The new adapter traits ImageAdapter, SummarizeAdapter, TtsAdapter, and TranscriptionAdapter are declared but not documented or tracked; add a short TODO and a reference to a tracking issue/PR in the file to signal these are planned placeholders. Concretely, update the adapter.rs file to include a one-line comment above each trait (or a single file-level comment) mentioning the trait name (ImageAdapter, SummarizeAdapter, TtsAdapter, TranscriptionAdapter) and a link or identifier for the tracking issue/board where implementation is planned so reviewers and future contributors know these are intentional stubs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rust/tanstack-ai/src/adapters/anthropic.rs`:
- Around line 673-681: convert_anthropic_event must not synthesize tool_call_id
for AnthropicDelta::InputJsonDelta because ToolCallStart emits the real
Anthropic id and ToolCallManager looks up by that id; remove the InputJsonDelta
arm from convert_anthropic_event (or make it accept a tool_block_ids mapping) so
that InputJsonDelta is handled only where tool_block_ids is available (the
inline block that uses tool_block_ids.get(&index)), ensuring ToolCallArgs uses
the actual Anthropic id (the id emitted by ToolCallStart) instead of
format!("block-{}", index); update any callers accordingly so InputJsonDelta
processing remains in the context that has access to tool_block_ids.
In `@packages/rust/tanstack-ai/src/adapters/gemini.rs`:
- Around line 277-292: The structured_output function currently overwrites the
entire "generationConfig" produced by build_request_body, discarding sampling
params; modify structured_output to merge into the existing "generationConfig"
instead of replacing it: get the JSON object from body.as_object_mut(), check if
"generationConfig" exists and is an object (if so, insert or overwrite the keys
"response_mime_type" and "response_schema" into that object), otherwise create a
new object with both the schema fields and any defaults; ensure you preserve
existing fields like temperature/topP/maxOutputTokens present in
generationConfig so callers' sampling parameters remain intact (refer to
structured_output, build_request_body, and the "generationConfig" key and
options.output_schema).
In `@packages/rust/tanstack-ai/src/adapters/openai.rs`:
- Around line 127-180: When serializing messages in the MessageContent::Text and
MessageContent::Parts branches, include any msg.tool_calls alongside the
existing "content" field instead of only handling tool_calls in
MessageContent::Null; update the item construction (the serde_json objects
pushed via items.push in the MessageContent::Text and MessageContent::Parts code
paths) to conditionally add a "tool_calls" key when msg.tool_calls.is_some(),
using the same mapping logic used in the MessageContent::Null branch (iterating
tc -> { "id", "type":"function", "function": { "name", "arguments" } }) so
assistant tool calls are preserved even when content is present.
- Around line 764-791: The ResponseCompleted branch
(OpenAiStreamEvent::ResponseCompleted) currently derives finish_reason only from
response.status; change it to override that mapping and set finish_reason =
Some("tool_calls".to_string()) when the tool_call_manager has accumulated calls
(use tool_call_manager.has_tool_calls() or equivalent) so that
StreamChunk::RunFinished emits finish_reason "tool_calls" when OutputItemAdded
events added function_call outputs; preserve the existing mapping for other
cases and ensure the final finish_reason passed to StreamChunk::RunFinished uses
the tool_call_manager override.
In `@packages/rust/tanstack-ai/src/chat.rs`:
- Around line 451-490: The wrapper execute_tool_calls is incorrectly converting
pending client-side and approval-gated work into synthetic error ToolResult
entries (see execute_tool_calls, approvals, client_results,
result.needs_client_execution, result.needs_approval, all_results); instead of
creating error ToolResult objects, preserve and propagate the original result
object (or extend the return type) so the caller can handle needs_approval and
needs_client_execution (e.g., return result directly or include separate fields
for needs_approval/needs_client_execution), and stop populating all_results with
synthetic errors so client/approval flows can complete correctly.
- Around line 307-369: The loop exits without persisting the final assistant
turn: when breaking out (the "No tool calls..." path) you must append the last
assistant ModelMessage using accumulated_content (or MessageContent::Null if
empty) into self.messages so TextEngine::content() and structured-output
followups see the final assistant response; update the break branch to push
ModelMessage { role: MessageRole::Assistant, content: if
accumulated_content.is_empty() { MessageContent::Null } else {
MessageContent::Text(accumulated_content.clone()) }, name: None, tool_calls:
None, tool_call_id: None } before breaking, and apply the same fix to the other
loop location that handles the final turn (the similar block referenced around
the second region).
In `@packages/rust/tanstack-ai/src/client/connection_adapters.rs`:
- Around line 97-109: Reset the accumulated_content buffer when starting a new
assistant turn: locate the block where a new user UiMessage is pushed and
state.is_loading set (in connection_adapters.rs around the code that constructs
UiMessage and sets state.is_loading/state.error) and set
state.accumulated_content = None or clear the buffer immediately after pushing
the user message so previous assistant deltas don't carry over; apply the same
reset at the other assistant-turn entry points referenced (the blocks around the
ranges corresponding to lines 244-249 and 262-277) where a new assistant
response sequence begins.
- Around line 196-201: The non-2xx branch in the HTTP streaming path returns an
Err without updating the adapter's observable state, leaving subscribers stuck;
update the branch in the function handling the request (same place as the shown
snippet in connection_adapters.rs / the stream_via_http logic) to mirror the SSE
error-state handling: set is_loading to false and publish the error into the
shared state/error observable (e.g. assign state.error or call the same
publish/update method used by the SSE path) before returning
Err(AiError::Provider(...)) so subscribers receive the error and loading is
cleared.
- Around line 135-138: The write lock obtained via self.state.write().await in
functions like stop must be released before calling await
self.notify_subscribers(), because notify_subscribers() immediately tries to
read() the same RwLock and will self-deadlock; fix each call site (e.g., stop,
and other methods that do let mut state = self.state.write().await ...) by
mutating the state while holding the lock, then drop the lock (e.g., let _ =
state; or explicitly scope the write lock) before invoking
self.notify_subscribers().await so notify_subscribers() can acquire the read
lock successfully.
- Around line 134-139: The stop() method only flips state.is_loading and doesn't
cancel an in-flight send()/process_stream(); add a cancellation mechanism: add a
CancellationToken or AbortHandle field to the shared state (e.g.,
state.abort_token or state.abort_handle), set/replace it when starting
generation in send()/process_stream(), and have process_stream() use that token
(or wrap the stream/futures with futures::future::Abortable or
tokio_util::sync::CancellationToken) to abort iteration on cancellation; then
update stop() to call the token.abort()/handle.abort() before setting is_loading
= false and calling notify_subscribers() so in-flight tasks are actually
terminated.
- Around line 53-60: ChatClient holds a long-lived processor (processor:
Arc<Mutex<StreamProcessor>>) so StreamProcessor state (message/chunk strategy)
leaks across requests; instead make StreamProcessor request-scoped by removing
the persistent processor field from ChatClient and instantiating a new
StreamProcessor inside the send/stream handling code paths (where send() /
process_stream logic currently uses processor). Update all places that reference
ChatClient::processor (including the areas that construct ChatClient and the
stream handling code that send() calls) to create a new StreamProcessor per
request, pass it into the existing processing functions (or wrap it locally in a
Mutex/Arc if needed for internal async tasks), and ensure no global/shared
StreamProcessor remains in the ChatClient struct or its constructors.
In `@packages/rust/tanstack-ai/src/lib.rs`:
- Around line 123-133: The RIFF/WEBP detection in detect_image_mime_type
incorrectly uses data.len() > 12, which rejects a minimal 12-byte RIFF+WEBP
header; change the check to require data.len() >= 12 so the slice &data[8..12]
is valid for a 12-byte buffer and WEBP files are correctly detected.
In `@packages/rust/tanstack-ai/src/messages.rs`:
- Around line 253-267: The code is creating duplicate content for tool messages
by adding a MessagePart::Text (from MessageContent::Text) and then also adding
MessagePart::ToolResult for MessageRole::Tool; update the conversion logic in
messages.rs so that when msg.role == MessageRole::Tool (or
msg.tool_call_id.is_some()) you skip pushing the MessagePart::Text in the
earlier text-handling branch (the code around the existing MessageContent::Text
handling at lines ~194-199) and only emit the MessagePart::ToolResult (the block
using MessageRole::Tool and MessagePart::ToolResult). Ensure the condition
prevents pushing the text part for tool messages while leaving non-tool text
messages unchanged.
In `@packages/rust/tanstack-ai/src/middleware.rs`:
- Around line 199-203: The trait defines on_tool_phase_complete() and on_usage()
but MiddlewareRunner never dispatches them, so these hooks are dead; add
corresponding dispatch methods to MiddlewareRunner (e.g.,
MiddlewareRunner::dispatch_on_tool_phase_complete(&self, ctx:
&ChatMiddlewareContext, info: &ToolPhaseCompleteInfo) and
MiddlewareRunner::dispatch_on_usage(&self, ctx: &ChatMiddlewareContext, usage:
&UsageInfo)) that iterate the registered middlewares and call the trait methods
in order, then call these dispatchers at the appropriate spots in the chat flow
(where tool phase completion is finalized and when usage data is produced),
ensuring the ctx and info objects are passed through unchanged so
on_tool_phase_complete and on_usage receive the data.
In `@packages/rust/tanstack-ai/src/stream/json_parser.rs`:
- Around line 58-66: The code currently appends all closing brackets then all
closing braces (using bracket_depth and brace_depth) which breaks interleaved
nesting (e.g., `[{"a":1`), so replace that logic with a stack-based tracker:
while scanning the original input build a Vec<char> stack that pushes '[' or '{'
for each opener found and when you need to auto-close, pop the stack and append
the matching closer (']' for '[' and '}' for '{') to attempt before calling
serde_json::from_str(&attempt). Update the code that currently uses
bracket_depth/brace_depth and the loops that push closers to instead use this
opener stack to preserve correct closing order.
In `@packages/rust/tanstack-ai/src/stream/processor.rs`:
- Around line 102-116: On StreamChunk::TextMessageEnd, the code updates
current_message.last_emitted_text but never emits a final TextMessageContent
chunk for the buffered current_segment_text, so downstream chunk consumers miss
the last segment; modify the TextMessageEnd arm in processor.rs to, before
setting is_complete and taking current_message, check if current_segment_text is
non-empty and create/emit a final StreamChunk::TextMessageContent (using the
message id/metadata and the remaining text), update last_emitted_text to
total_text_content, clear current_segment_text, then proceed to mark
is_complete, take/push the message, and reset chunk_strategy; ensure the emitted
chunk matches other TextMessageContent chunks so consumers receive it (or
alternatively document that result() must be called if you choose not to emit).
In `@packages/rust/tanstack-ai/src/tools/definition.rs`:
- Around line 102-105: The current json_schema function swallows deserialization
errors by returning a default JsonSchema; change it to a fallible API that
surfaces errors: replace pub fn json_schema(value: serde_json::Value) ->
JsonSchema with a fallible signature (e.g., pub fn try_json_schema(value:
serde_json::Value) -> Result<JsonSchema, serde_json::Error> {
serde_json::from_value(value) }) or change json_schema to return
Result<JsonSchema, serde_json::Error> and propagate the error from
serde_json::from_value; update all call sites (or add a thin wrapper that calls
.expect with a clear message only where panicking is acceptable) so callers
handle the Result instead of silently accepting an empty schema.
In `@packages/rust/tanstack-ai/src/tools/tool_calls.rs`:
- Around line 139-145: The event channel type passed into execute_tool_calls
(event_tx: Option<mpsc::UnboundedSender<ToolCustomEvent>>) doesn't match what
ToolExecutionContext expects (mpsc::UnboundedSender<CustomEventData>), so custom
events emitted via ctx.emit_custom_event() never reach the caller; fix by
aligning types — either change execute_tool_calls' parameter to
Option<mpsc::UnboundedSender<CustomEventData>> or wrap/convert the sender when
constructing ToolExecutionContext (convert ToolCustomEvent <-> CustomEventData
in a single place), and ensure all uses (including the other occurrences around
lines ~304-316) pass the same unified sender type to ToolExecutionContext so
ctx.emit_custom_event() forwards to the caller.
- Around line 57-59: The ToolCallManager currently stores calls in a HashMap
causing nondeterministic iteration; change its internal storage to preserve
insertion order (e.g., Vec<ToolCall> or an insertion-ordered map) and update
tool_calls() to return the calls in the original emission order rather than
cloning HashMap::values(); update any helpers that insert or lookup (where you
currently push into the HashMap) to maintain the ordered collection and keep
lookups working (e.g., keep an index map for name-to-position if needed) so the
sequence emitted by the model is executed deterministically.
In `@packages/rust/tanstack-ai/src/types.rs`:
- Around line 59-61: The fields r#ref and defs in the JsonSchema type are
serializing to "ref" and "defs" (because of rename_all = "camelCase") but JSON
Schema requires "$ref" and "$defs"; update the struct field attributes for r#ref
and defs to include explicit serde(rename = "$ref") and serde(rename = "$defs")
respectively (preserving existing serde(skip_serializing_if = "Option::is_none")
on defs and any other attributes) so they serialize to the correct JSON Schema
property names.
---
Nitpick comments:
In `@packages/rust/tanstack-ai/src/adapter.rs`:
- Around line 8-16: TextAdapterConfig declares timeout, max_retries, and headers
but they aren't used by the adapters' with_config implementations (which only
apply base_url); update the code so these fields are either actually applied
when building the HTTP client or clearly marked for future use: either (A)
implement use of TextAdapterConfig.timeout, max_retries, and headers inside the
adapters' with_config functions (e.g., configure the reqwest::ClientBuilder or
HTTP client in the adapter to set timeout, attach headers, and add retry logic
using max_retries), referencing TextAdapterConfig and the adapters' with_config
methods, or (B) add a concise TODO and documentation on TextAdapterConfig
stating these fields are reserved for future use and annotate them to suppress
unused warnings (e.g., #[allow(dead_code)] or similar), so the intent is
explicit; pick one approach and apply consistently to all adapters that
implement with_config.
- Around line 43-89: The new adapter traits ImageAdapter, SummarizeAdapter,
TtsAdapter, and TranscriptionAdapter are declared but not documented or tracked;
add a short TODO and a reference to a tracking issue/PR in the file to signal
these are planned placeholders. Concretely, update the adapter.rs file to
include a one-line comment above each trait (or a single file-level comment)
mentioning the trait name (ImageAdapter, SummarizeAdapter, TtsAdapter,
TranscriptionAdapter) and a link or identifier for the tracking issue/board
where implementation is planned so reviewers and future contributors know these
are intentional stubs.
In `@packages/rust/tanstack-ai/src/adapters/anthropic.rs`:
- Around line 640-661: The ContentBlockStart arm in convert_anthropic_event
duplicates logic already handled inline (which populates tool_block_ids) and is
therefore unreachable; remove the ContentBlockStart match branch from
convert_anthropic_event (the block that constructs StreamChunk::TextMessageStart
/ StreamChunk::ToolCallStart) so the single inline handler that updates
tool_block_ids remains the authoritative implementation, or refactor
convert_anthropic_event to call the same helper used by the inline match to
avoid diverging logic between AnthropicEvent::ContentBlockStart and the inline
handling.
In `@packages/rust/tanstack-ai/src/adapters/gemini.rs`:
- Around line 75-86: The code sets functionResponse.name using msg.tool_call_id
which may be an opaque ID rather than the function name Gemini expects; update
the implementation to either use a dedicated function name field (e.g.,
msg.tool_name or similar) when available or, at minimum, add a clear comment
above the block explaining the expectation that msg.tool_call_id must contain
the function name (and note potential mismatch if it is a UUID from another
provider). Refer to msg.tool_call_id and the functionResponse.name assignment to
locate where to add the comment or swap to the correct field.
- Around line 379-385: Replace the direct stderr write in the Err(e) branch with
structured logging: add and enable a logging crate (e.g., tracing or log) in the
crate, import the macro (e.g., tracing::error or log::error) at the top of the
module, and change the eprintln! call in
packages/rust/tanstack-ai/src/adapters/gemini.rs (the Err(e) => { ... } block
that prints "Failed to parse Gemini event") to use error! with the error and the
truncated data_str so consumers can configure log output instead of writing
directly to stderr.
In `@packages/rust/tanstack-ai/src/messages.rs`:
- Around line 277-282: The function normalize_to_model_messages currently just
clones its ModelMessage slice (a no-op) despite docs promising conversion from
UI message types; update the implementation to either (A) implement actual
conversion logic from any UI/other message variants into ModelMessage (accepting
the appropriate input type/enum and mapping fields into ModelMessage) or (B) if
this is a deliberate placeholder, add a clear TODO comment and adjust the
docstring to state it's intentionally a passthrough for ModelMessage inputs
only; refer to normalize_to_model_messages and ModelMessage to locate where to
implement the mapping or insert the TODO and updated docs.
- Around line 284-324: Add tests covering conversion edge cases not currently
asserted: create UiMessage and ModelMessage fixtures exercising tool calls
(MessagePart::ToolCall / ModelMessage.tool_calls), tool results
(MessagePart::ToolResult / MessageContent::ToolResult), multimodal parts
(MessagePart::Image, MessagePart::Audio) and thinking parts
(MessagePart::Thinking), then assert round-trip correctness via
ui_message_to_model_messages and model_message_to_ui_message (check roles,
content variants, tool_call ids/names/results, and that image/audio metadata
round-trips). Target the existing test module and functions (e.g., add new
#[test] functions alongside test_ui_to_model_text and
test_model_to_ui_round_trip) to validate each conversion path.
In `@packages/rust/tanstack-ai/src/stream_response.rs`:
- Around line 98-109: The serialization currently uses
serde_json::to_string(&chunk).unwrap_or_default() which silences serialization
failures and emits empty SSE data; change the closure inside stream.map (the map
over result) to handle serde_json::to_string errors explicitly: match or map the
Result from serde_json::to_string(&chunk) and on Err produce an SSE chunk
similar to the existing Err(e) arm (e.g.,
serde_json::json!({"type":"SERIALIZATION_ERROR","error":{"message":
err.to_string()}})) wrapped with Bytes::from(format!("data: {}\n\n", ...)), or
propagate the error out so the stream yields an Err; ensure you replace the use
of unwrap_or_default with this explicit error handling for the chunk branch to
avoid silent failures (refer to the closure handling Ok(chunk) in stream.map and
the existing Err(e) branch for formatting).
In `@packages/rust/tanstack-ai/src/stream/json_parser.rs`:
- Around line 97-101: Add a unit test that covers interleaved bracket/brace
nesting to catch the closing-order bug: create a new test function (e.g.,
test_interleaved_nesting) that calls parse_partial_json with input like
r#"[{"name": "Jo"# and asserts the returned value matches the expected partial
JSON array/object (e.g., serde_json::json!([{"name":"Jo"}])). Place the test
alongside test_nested_partial in json_parser.rs so parse_partial_json is
exercised for mixed '[' and '{' nesting ordering.
In `@packages/rust/tanstack-ai/src/stream/strategies.rs`:
- Around line 99-104: The current CompositeStrategy::should_emit uses
self.strategies.iter_mut().any(...) which short-circuits and prevents later
stateful strategies (e.g., BatchStrategy with chunk_count) from observing the
chunk; change the implementation to iterate and call should_emit on every
strategy (e.g., a for loop or fold over self.strategies.iter_mut()), accumulate
an OR result, and return true if any strategy returned true so that all
strategies update their internal state while preserving OR-semantics; update
references to ChunkStrategy, CompositeStrategy, should_emit, BatchStrategy,
chunk_count, and strategies accordingly.
In `@packages/rust/tanstack-ai/src/tools/registry.rs`:
- Around line 82-125: The codebase defines FrozenToolRegistry for immutable,
thread-safe tool access but ChatOptions and TextEngine still accept Vec<Tool>;
either integrate FrozenToolRegistry into those APIs or add docs clarifying when
to use each: update ChatOptions and TextEngine signatures/constructors to accept
FrozenToolRegistry instead of Vec<Tool> (or provide overloads/builders that
convert Vec<Tool> via FrozenToolRegistry::freeze) and update call sites to pass
a frozen registry, or add clear documentation/comments near ChatOptions,
TextEngine and FrozenToolRegistry explaining ownership vs immutability and when
to prefer FrozenToolRegistry for thread-safe sharing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e5b6270-f7b2-4aac-aae9-16bc8c7a3c07
⛔ Files ignored due to path filters (2)
examples/rs-chat/Cargo.lockis excluded by!**/*.lockpackages/rust/tanstack-ai/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.gitignoreexamples/rs-chat/Cargo.tomlexamples/rs-chat/src/main.rspackages/rust/tanstack-ai/Cargo.tomlpackages/rust/tanstack-ai/src/adapter.rspackages/rust/tanstack-ai/src/adapters/anthropic.rspackages/rust/tanstack-ai/src/adapters/gemini.rspackages/rust/tanstack-ai/src/adapters/mod.rspackages/rust/tanstack-ai/src/adapters/openai.rspackages/rust/tanstack-ai/src/chat.rspackages/rust/tanstack-ai/src/client/connection_adapters.rspackages/rust/tanstack-ai/src/client/mod.rspackages/rust/tanstack-ai/src/error.rspackages/rust/tanstack-ai/src/lib.rspackages/rust/tanstack-ai/src/messages.rspackages/rust/tanstack-ai/src/middleware.rspackages/rust/tanstack-ai/src/stream/json_parser.rspackages/rust/tanstack-ai/src/stream/mod.rspackages/rust/tanstack-ai/src/stream/processor.rspackages/rust/tanstack-ai/src/stream/strategies.rspackages/rust/tanstack-ai/src/stream/types.rspackages/rust/tanstack-ai/src/stream_response.rspackages/rust/tanstack-ai/src/tools/definition.rspackages/rust/tanstack-ai/src/tools/mod.rspackages/rust/tanstack-ai/src/tools/registry.rspackages/rust/tanstack-ai/src/tools/tool_calls.rspackages/rust/tanstack-ai/src/types.rspackages/rust/tanstack-ai/tests/chat_tests.rspackages/rust/tanstack-ai/tests/stream_processor_tests.rspackages/rust/tanstack-ai/tests/test_utils.rspackages/rust/tanstack-ai/tests/tool_call_manager_tests.rspackages/rust/tanstack-ai/tests/unit_tests.rs
Contributor
|
how would we publish this/new versions? |
Author
We would have to setup proper github actions for publishing a crate. Very similar to npm publishing |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (12)
crates/tanstack-ai/src/chat.rs (2)
118-140:⚠️ Potential issue | 🟠 MajorPersist the final assistant turn before breaking out of the loop.
Only the tool-call path appends an assistant
ModelMessage. On a normal stop this just breaks, socontent()and the structured-output follow-up both read staleself.messages.Also applies to: 323-334, 368-369, 519-526
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/chat.rs` around lines 118 - 140, The final assistant turn isn't being pushed into engine.messages before exiting the normal-stop path, so downstream reads (e.g., when building StructuredOutputOptions/TextOptions and calling adapter.structured_output) use stale self.messages; update the code paths that handle a non-tool stop to append the assistant ModelMessage (same shape used in the tool-call path) into engine.messages (and ensure engine.system_prompts stays aligned) before constructing StructuredOutputOptions and invoking adapter.structured_output; apply the same fix in the other analogous locations handling final turns (the other places building StructuredOutputOptions/TextOptions and calling adapter.structured_output).
465-488:⚠️ Potential issue | 🟠 MajorDon’t convert pending tool work into synthetic failures.
execute_tool_calls()already separatesneeds_client_executionandneeds_approval, but this wrapper immediately turns both into errorToolResults. Approval-gated and client-side tools can never complete successfully from the agent loop with this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/chat.rs` around lines 465 - 488, The wrapper is turning needs_client_execution and needs_approval into synthetic Error ToolResult entries, preventing those tools from ever completing; instead preserve their pending status by not converting them to errors—either stop pushing them into all_results and propagate the original needs_client_execution and needs_approval vectors back to the caller, or if a ToolResult must be created use a non-error state (e.g., ToolResultOutputState::Pending) with a null/empty result and duration_ms: None; update the code around execute_tool_calls, the loops handling result.needs_client_execution and result.needs_approval, and the construction of ToolResult so approvals and client-side tool calls remain as pending work rather than failures.crates/tanstack-ai/src/stream/processor.rs (1)
102-115:⚠️ Potential issue | 🟡 MinorEmit the buffered tail before
TextMessageEnd.With batching strategies,
current_segment_textcan still hold the last segment here. Clearing it without emitting a finalTextMessageContentmeans chunk consumers never receive the trailing text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream/processor.rs` around lines 102 - 115, When handling StreamChunk::TextMessageEnd in processor.rs, you're clearing current_segment_text without emitting its buffered tail, so consumers miss the final text; before clearing current_segment_text and marking msg.is_complete, construct and push a final TextMessageContent chunk (or call the existing emission helper used elsewhere) using msg.current_segment_text (and update msg.last_emitted_text/total_text_content accordingly), then clear current_segment_text, set is_complete, take and push current_message into messages, and finally call chunk_strategy.reset() as currently done.crates/tanstack-ai/src/adapters/gemini.rs (1)
283-291:⚠️ Potential issue | 🟠 MajorDon’t clobber
generationConfigin structured output.Gemini’s generation config uses
responseMimeType/responseSchema, and those settings live alongside sampling knobs liketemperature,topP, andmaxOutputTokens. This branch replaces the wholegenerationConfigobject and sends snake_case keys, so structured output drops caller sampling params and does not match the documented request shape. (ai.google.dev)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/gemini.rs` around lines 283 - 291, The current code replaces the entire "generationConfig" object with snake_case keys, clobbering existing sampling params; change the logic in the body.as_object_mut() branch to merge into an existing generationConfig object instead of inserting a new value: if body contains "generationConfig" and it's an object, set/overwrite the camelCase fields "responseMimeType" and "responseSchema" (use options.output_schema) on that object, otherwise create a new object with those camelCase keys; keep all other generationConfig fields (e.g., temperature, topP, maxOutputTokens) intact and do not replace the whole "generationConfig" value.crates/tanstack-ai/src/client/connection_adapters.rs (4)
135-138:⚠️ Potential issue | 🔴 CriticalDrop the state write lock before awaiting
notify_subscribers().
notify_subscribers()immediately readsself.state, so every one of these paths self-deadlocks by awaiting it while the write guard is still alive.Also applies to: 143-147, 167-170, 224-227, 233-235, 241-335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/client/connection_adapters.rs` around lines 135 - 138, The write guard obtained via self.state.write().await is held across an await, causing self-deadlock when notify_subscribers() reads self.state; in functions like stop() adjust the scope so the write lock is dropped before awaiting notify_subscribers() — e.g., perform mutations on the state (using the write guard from self.state.write().await and setting state.is_loading = false), then explicitly drop the guard (or let it go out of scope) before calling self.notify_subscribers().await; apply the same pattern wherever you call self.state.write().await, mutate fields, and then await notify_subscribers() to ensure the write lock is released before the await.
95-109:⚠️ Potential issue | 🟠 MajorReset
accumulated_contentwhen a new response starts.
accumulated_contentsurvives across sends and assistant turns. If the next response begins with deltas, Lines 246-249 append onto the previous reply instead of starting fresh.Also applies to: 244-249, 262-277
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/client/connection_adapters.rs` around lines 95 - 109, The bug is that accumulated_content is not cleared when a new assistant response begins, so delta appends (from the streams handling code) continue from the previous reply; locate where a new user message is added (the block that pushes UiMessage using generate_message_id, UiMessageRole::User and MessagePart::Text) and reset the adapter's accumulated_content (or equivalent field) to an empty string/None there and also at the start of the assistant-response handling path (where assistant deltas are processed) so each assistant turn starts fresh before appending delta text.
196-200:⚠️ Potential issue | 🟠 MajorMirror the SSE error-state update here.
The non-2xx HTTP path returns
Errwithout clearingis_loadingor publishingstate.error, so subscribers can remain stuck in a loading state with no visible failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/client/connection_adapters.rs` around lines 196 - 200, The non-2xx HTTP branch returns Err without updating the adapter state; set state.is_loading = false and populate state.error with the HTTP error message before returning, then publish the state update so subscribers see the failure (e.g., in the same place you update SSE state). Locate the non-success block around the response handling (the code using response.status(), response.text()) and, before returning Err(AiError::Provider(...)), assign the error text to state.error, clear state.is_loading, and call the existing state-publish helper (e.g., publish_state()/dispatch_state()/self.publish_state_update()) used elsewhere in connection_adapters.rs.
134-139:⚠️ Potential issue | 🟠 Major
stop()still doesn’t stop the stream.This only flips
is_loading. The in-flight request keeps running and can continue mutating state after the caller thinks generation was cancelled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/client/connection_adapters.rs` around lines 134 - 139, The stop() method only flips state.is_loading and doesn’t cancel the underlying in-flight stream/request; update stop() to abort the active request/stream and ensure no further mutations occur. Specifically, locate the active request handle used by the adapter (e.g., an AbortHandle/AbortRegistration, Stream/JoinHandle, or channel sender stored on the adapter struct) and call its abort/close method from stop() before or while setting state.is_loading = false and calling notify_subscribers(); if such a handle doesn't exist, add one (e.g., store an AbortHandle or JoinHandle when starting the stream in the code that spawns the request) and use it here; also ensure the stream-processing code checks the adapter state (is_loading or a cancelled flag) before mutating state so aborted requests cannot race and apply updates after stop().crates/tanstack-ai/src/messages.rs (1)
192-199:⚠️ Potential issue | 🟡 MinorAvoid rendering tool payloads twice.
Tool messages already surface their payload as
MessagePart::ToolResultat Lines 260-265. Keeping theMessagePart::Textfrom Lines 194-198 shows the same content twice in the UI.Also applies to: 253-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/messages.rs` around lines 192 - 199, The Text branch currently pushes a MessagePart::Text for all messages, which causes tool outputs to be duplicated because tool payloads are also emitted as MessagePart::ToolResult; update the logic around the match on &msg.content so that MessageContent::Text only pushes MessagePart::Text when the message is not a tool result (e.g., check msg.role != Role::Tool or guard against messages that will produce MessagePart::ToolResult) and skip adding the Text for tool messages; reference the match on MessageContent::Text and the MessagePart::ToolResult handling to ensure you only surface the payload once.crates/tanstack-ai/src/tools/tool_calls.rs (2)
57-59:⚠️ Potential issue | 🟠 MajorPreserve the model-emitted tool-call order.
ToolCallManagerstores calls in aHashMap,tool_calls()iteratesvalues(), and the streamedindexis ignored. Multi-call batches can execute in a different order than the model emitted, which is risky for side-effecting tools. Keep ordered state and sort byindexwhen it is present.Also applies to: 69-75, 121-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/tools/tool_calls.rs` around lines 57 - 59, ToolCallManager currently uses a HashMap (tool_calls) and tool_calls() iterates values() ignoring the streamed index, so multi-call batches can be executed out-of-order; change the internal storage from an unordered HashMap to an ordered collection (e.g., Vec or IndexMap) that preserves insertion and/or supports stable ordering, update ToolCallManager::tool_calls() to return calls sorted by the emitted index when present (fallback to insertion order if index is missing), and adjust any insert/update logic that references tool_calls to maintain the chosen ordering; ensure the sort-by-index behavior is applied in the other affected areas you flagged (around lines referenced for the same struct and iterations).
139-145:⚠️ Potential issue | 🟠 MajorCustom events never leave the tool execution context.
The public API takes
UnboundedSender<ToolCustomEvent>, butToolExecutionContextexpectsUnboundedSender<CustomEventData>, socustom_event_txis forced toNonehere. That makesctx.emit_custom_event()a no-op for every server tool until the channel types are unified or adapted in one place.Also applies to: 308-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/tools/tool_calls.rs` around lines 139 - 145, The public API execute_tool_calls accepts an UnboundedSender<ToolCustomEvent> but ToolExecutionContext expects UnboundedSender<CustomEventData>, so the custom_event_tx is dropped and ctx.emit_custom_event() becomes a no-op; fix by adapting the sender instead of passing None: either change ToolExecutionContext (or its constructor) to accept UnboundedSender<ToolCustomEvent> directly, or create an adapter that takes the provided UnboundedSender<ToolCustomEvent> and produces an UnboundedSender<CustomEventData> (or vice‑versa) by spawning a short-lived task that maps ToolCustomEvent <-> CustomEventData and forwards messages, then pass that adapted sender into ToolExecutionContext (referencing execute_tool_calls, ToolExecutionContext, custom_event_tx, ToolCustomEvent and CustomEventData to locate where to implement the conversion).crates/tanstack-ai/src/types.rs (1)
42-43:⚠️ Potential issue | 🟠 MajorUse the JSON Schema keyword names here.
rename_all = "camelCase"turns these intorefanddefs, but schema references require the literal keys$refand$defs.Suggested fix
- #[serde(skip_serializing_if = "Option::is_none")] - pub r#ref: Option<String>, - #[serde(skip_serializing_if = "Option::is_none")] - pub defs: Option<HashMap<String, JsonSchema>>, + #[serde(rename = "$ref", skip_serializing_if = "Option::is_none")] + pub r#ref: Option<String>, + #[serde(rename = "$defs", skip_serializing_if = "Option::is_none")] + pub defs: Option<HashMap<String, JsonSchema>>,Also applies to: 58-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/types.rs` around lines 42 - 43, The struct-level serde rename_all should not convert JSON Schema keywords to camelCase; update the JsonSchema definition to stop using #[serde(rename_all = "camelCase")] and instead explicitly rename the schema keyword fields so they serialize to the literal JSON Schema keys (e.g. annotate the field representing $ref with #[serde(rename = "$ref")] and the field representing $defs with #[serde(rename = "$defs")], using raw identifiers like r#ref if needed), and apply the same explicit-field-rename fix to the other struct(s) referenced around lines 58-61 so their $ref/$defs fields keep the exact JSON Schema names.
🧹 Nitpick comments (5)
crates/tanstack-ai/src/stream/json_parser.rs (1)
97-101: Add a test case for interleaved array/object nesting.The existing
test_nested_partialonly tests nested objects ({"user": {"name": "Jo"). Consider adding a test for[{"a": 1to verify correct closing order for mixed structures once the parser is fixed.📝 Suggested test case
#[test] fn test_interleaved_array_object() { let result = parse_partial_json(r#"[{"a": 1"#).unwrap(); assert_eq!(result, serde_json::json!([{"a": 1}])); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream/json_parser.rs` around lines 97 - 101, Add a new unit test named test_interleaved_array_object that calls parse_partial_json with the input r#"[{"a": 1"# and asserts the returned value equals serde_json::json!([{"a": 1}]); this verifies the parser handles interleaved array/object nesting and proper closing order for mixed structures—if the test fails, fix the parse_partial_json implementation to correctly track and close nested arrays and objects in mixed contexts (ensure stack/state used by parse_partial_json accounts for entering an array then an object and pops in the correct order).crates/tanstack-ai/src/middleware.rs (1)
226-238: Silent skip when middleware returnsNonefromon_config.If a middleware returns
Nonefromon_config, the config transformation is silently skipped, and the previous config is retained. Consider whether returningNoneshould instead signal an abort or error, rather than being treated as a no-op pass-through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/middleware.rs` around lines 226 - 238, The current run_on_config silently treats a middleware returning None from on_config as a no-op; change run_on_config to surface this as an error by returning a Result<ChatMiddlewareConfig, MiddlewareError> (or similar) instead of ChatMiddlewareConfig, iterate over self.middlewares calling mw.on_config(ctx, ¤t) and if it returns Some(update) continue, but if it returns None return Err(MiddlewareError::ConfigAborted { middleware: mw.identifier_or_index(), context: ctx.clone() }) so callers can handle aborts; update the run_on_config signature and all call sites, and add a small MiddlewareError type and an identifier accessor (or use index) to help trace which middleware returned None (refer to run_on_config and the on_config method on middleware traits/types).crates/tanstack-ai/src/error.rs (1)
50-59:From<String>andFrom<&str>conversions route all strings toOther.This is convenient but can obscure the actual error category. Consider whether callers should explicitly construct the appropriate variant (e.g.,
AiError::Validation(msg)) for better error categorization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/error.rs` around lines 50 - 59, The blanket impls From<String> and From<&str> collapse all string errors into AiError::Other which hides error categories; remove these two impl blocks (the From<String> for AiError and From<&str> for AiError implementations) so callers must construct explicit variants (e.g., AiError::Validation(...), AiError::Other(...)), and optionally add small constructor helpers (e.g., AiError::validation(msg: impl Into<String>) -> Self) or document preferred variant construction to make error categorization explicit.crates/tanstack-ai/tests/tool_call_manager_tests.rs (1)
6-8: Consider extracting thenow()helper to shared test utilities.This helper is duplicated in
stream_processor_tests.rs. Extracting it to a commontest_utils.rsmodule would reduce duplication across test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/tests/tool_call_manager_tests.rs` around lines 6 - 8, Extract the duplicated now() helper into a shared test utilities module (e.g., a test_utils module exposing pub fn now() -> f64) and replace the local now() implementations in tool_call_manager_tests.rs and stream_processor_tests.rs with calls to that shared function; update each test file to import or reference test_utils::now so both tests use the single shared implementation.crates/tanstack-ai/tests/stream_processor_tests.rs (1)
7-9: Duplicatenow()helper.Same helper exists in
tool_call_manager_tests.rs. Consider sharing via a common test utilities module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/tests/stream_processor_tests.rs` around lines 7 - 9, Extract the duplicate now() helper into a shared test utilities module and have both tests import it: create a new test utils module (e.g. tests/utils.rs or tests/common/mod.rs) with a pub fn now() -> f64 implementation, remove the local now() definitions from stream_processor_tests.rs and tool_call_manager_tests.rs, and update those files to call the shared utils::now() (or common::now()) so both tests reuse the single implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tanstack-ai/src/adapters/anthropic.rs`:
- Around line 219-227: The ContentBlockStop enum variant (ContentBlockStop {
index: usize }) is parsed but not handled — ensure the stream handling code
emits a TextMessageEnd (or equivalent) when encountering ContentBlockStop so
streamed text blocks are closed and StreamProcessor can flush/reset chunk
strategy; locate the match/handling of Anthropic stream events (cases handling
MessageDelta and MessageStop) and add a branch for ContentBlockStop that maps it
to a TextMessageEnd emission (and perform any needed index-based bookkeeping),
similarly updating the logic around MessageDelta/MessageStop to maintain
consistent open/close state.
- Around line 629-637: The code emits StreamChunk::RunStarted with message.id in
AnthropicEvent::MessageStart but later creates RunFinished using a fabricated
"anthropic-<timestamp>" id in the MessageDelta handling, breaking start/finish
correlation; update the MessageDelta handling (and the other similar block at
685-708) to use the original message.id from the event/state instead of
generating a new id so RunFinished.run_id matches the RunStarted.run_id (locate
the logic in the AnthropicEvent::MessageDelta match arms and replace the
fabricated run id with the message.id field or stored run id used by
StreamChunk::RunStarted).
- Around line 88-159: The Text and Parts branches currently only serialize the
message content and ignore msg.tool_calls; update the handlers inside the match
on msg.content (the MessageContent::Text and MessageContent::Parts arms) to
detect if msg.tool_calls.is_some(), convert existing content into a
Vec<serde_json::Value> (if Text, start with a single {"type":"text","text":...}
element; if Parts, use the existing content vec), then append tool-use objects
for each tool call matching the structure used in the Null branch
({"type":"tool_use","id": tc.id, "name": tc.function.name, "input": parsed
arguments or {} }), and finally insert that combined array as the "content"
value into m instead of the current content-only value.
In `@crates/tanstack-ai/src/adapters/gemini.rs`:
- Around line 396-403: The EOF branch currently returns None and discards any
buffered SSE line in line_buf; modify the None arm so that if line_buf is
non-empty you parse/emit that buffered line as a final SSE event (map it into
the same item type used by the stream and return
Some((futures_util::stream::iter(vec![Ok(parsed_event)]), (stream,
String::new()))) otherwise return None; reference the variables/values in this
scope (line_buf, stream, the stream item type used elsewhere and AiError
variants) so the final 'data:' chunk is not silently dropped.
- Around line 65-131: The serializer skips assistant-sourced tool calls
(msg.tool_calls) so the request history lacks the function-call turn; modify the
loop over options.messages (inside the for msg in &options.messages block) to
detect when msg.role == MessageRole::Assistant and msg.tool_calls is present,
and emit a JSON content entry representing the function-call turn (using the
tool call name, arguments, etc. from msg.tool_calls) into the contents vector
before you emit the corresponding functionResponse entry built from
msg.tool_call_id/msg.content; ensure the new entry uses the same role mapping
logic (role for assistant -> "model") and is inserted prior to pushing the
functionResponse so the function-call turn precedes the function-response turn.
- Around line 75-83: Update the Gemini adapter to use the API's camelCase field
names and preserve real function-call IDs: add #[serde(rename = "...")]
attributes to all response struct fields that are currently snake_case (e.g.,
usage_metadata → "usageMetadata", finish_reason → "finishReason",
prompt_token_count → "promptTokenCount", etc.) so deserialization matches
Gemini's JSON; change the outgoing request key "system_instruction" →
"systemInstruction" in the request body; extend the GeminiFunctionCall struct to
include the API-provided id field and ensure you read and store that id from
Gemini responses (replace any synthetic ids produced by
format!("gemini-tool-{}", i)) and use that real id when constructing
functionResponse entries (the code building the echo at the former
functionResponse block) so tool responses match their originating functionCall.
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 33-42: with_config currently only applies
TextAdapterConfig::base_url and silently drops timeout, headers, and
max_retries; update OpenAI's with_config (and mirror the same change in
AnthropicTextAdapter::with_config) to read and apply all fields from
TextAdapterConfig by: setting the adapter's timeout (e.g., adapter.timeout or
equivalent client timeout) when config.timeout is Some(...), merging or
replacing adapter.headers with config.headers when present, and configuring the
adapter's retry/max_retries behavior (e.g., adapter.max_retries or its retry
policy) from config.max_retries so callers can tune transport behavior through
with_config.
- Around line 134-163: The MessageContent::Parts branch is building payloads
with Chat Completions types; update it to produce OpenAI Responses API types by
changing ContentPart::Text handling to emit {"type":"input_text","text": ...}
and ContentPart::Image handling to emit
{"type":"input_image","image_url":{"url": ...},"detail":"auto"} (or include a
configurable detail) instead of "text" and "image_url"; adjust the JSON
construction where items.push(...) uses msg.role and content so the content
array contains these new type keys and values (refer to MessageContent::Parts,
ContentPart::Text, ContentPart::Image, ContentPartSource, and the items.push
call).
- Around line 113-123: Replace the Chat Completions-style tool message payload
with a Responses API function_call_output item: in the MessageRole::Tool branch
(where msg.tool_call_id and msg.content are read and items.push is called),
build and push an object with "type": "function_call_output", "call_id":
msg.tool_call_id, and "output": <the content string derived from msg.content>
instead of using "role":"tool" and "tool_call_id"; ensure the same content
extraction logic for MessageContent::Text/Null/Parts is reused and that
items.push uses this new shape so tool results follow the Responses API
contract.
- Around line 193-256: The OpenAiStreamEvent enum is missing variants for the
"*.done" events and OutputTextDone is currently treated as content rather than a
terminal event; add enum variants for "response.content_part.done" (e.g.,
ContentPartDone with item_id, output_index, content_index, maybe text/part
fields as needed) and "response.output_item.done" (e.g., OutputItemDone with
output_index/item_id as appropriate), and update the code that maps
OpenAiStreamEvent::OutputTextDone to emit a TextMessageEnd chunk (instead of
TextMessageContent) so StreamProcessor can finalize messages; locate uses of
OpenAiStreamEvent and the mapping logic that handles
OutputTextDelta/OutputTextDone to implement these changes.
In `@crates/tanstack-ai/src/chat.rs`:
- Around line 70-82: The reducer is accumulating assistant text across
agent-loop iterations because the accumulator variable content (and
accumulated_content_from_chunks used elsewhere) is never reset per assistant
turn; modify the loop handling StreamChunk::TextMessageContent so you detect the
start of a new assistant message/turn (e.g., a chunk that marks a new assistant
role/message) and reset content.clear() (or reassign String::new()) before
appending or replacing text, and apply the same reset behavior to
accumulated_content_from_chunks used by onFinish middleware to prevent merging
pre-tool planning and final answers.
In `@crates/tanstack-ai/src/client/connection_adapters.rs`:
- Around line 279-291: The ToolCallStart handling in StreamChunk::ToolCallStart
currently assumes the last message is an assistant and drops the tool-call when
it's actually a user turn; update the ChatClient code that handles
StreamChunk::ToolCallStart (and the similar block at 294-323) to mirror
StreamProcessor: if state.messages.last_mut() is None or last.role !=
UiMessageRole::Assistant, first push a new assistant message (e.g., create a
UiMessage with role Assistant and empty parts) into state.messages, then push
the MessagePart::ToolCall (with id/tool_name/arguments/state as before) onto the
assistant message’s parts so subsequent ToolCallArg/ToolCallEnd events attach
correctly.
In `@crates/tanstack-ai/src/stream_response.rs`:
- Around line 41-84: The EOF branch currently returns None and drops any
buffered SSE data; update the None arm of the match in the async loop that reads
from lines (the block handling lines.next().await) to flush data_buf: if
data_buf is non-empty, treat it like a terminated event (handle the "[DONE]"
sentinel, attempt serde_json::from_str::<serde_json::Value>(&data_buf) and
return Some with Ok(json) or Err(AiError::Stream(...)) as appropriate), clearing
or consuming data_buf, otherwise return None to indicate stream end. Use the
same error handling pattern already used in the empty-line branch and reuse
symbols lines, data_buf, parse_sse_line, AiError, and serde_json::Value.
In `@crates/tanstack-ai/src/stream/processor.rs`:
- Around line 194-195: The ProcessorResult currently sets finish_reason to None
even when the StreamChunk is StreamChunk::RunFinished or StreamChunk::RunError;
update the logic in the result() assembly (and the related branches around lines
203-240) to extract and persist the finish reason from the chunk into
ProcessorResult.finish_reason instead of hard-coding None; specifically, when
matching StreamChunk::RunFinished { finish_reason, .. } or StreamChunk::RunError
{ finish_reason, .. } propagate that finish_reason (or an appropriate mapped
value) into the ProcessorResult returned by result(), and mirror this change in
all analogous match arms so callers can inspect why the stream ended.
In `@crates/tanstack-ai/src/stream/strategies.rs`:
- Around line 99-104: CompositeStrategy's should_emit uses .any() which
short-circuits and prevents later strategies (e.g., stateful BatchStrategy) from
observing the chunk and updating internal state; change should_emit to call
should_emit on every strategy in self.strategies (iter_mut) but still return
true if any strategy returned true: iterate through all strategies, track a
boolean flag (emit) that becomes true when a strategy returns true, continue
calling remaining strategies to let them update state, and finally return the
flag; update the impl ChunkStrategy for CompositeStrategy's should_emit
accordingly.
In `@crates/tanstack-ai/src/types.rs`:
- Line 666: TextOptions cloning currently drops any custom agent_loop_strategy
because AgentLoopStrategy is a boxed Fn (pub type AgentLoopStrategy = Box<dyn
Fn(&AgentLoopState) -> bool + Send + Sync>), so update the type to use Arc
(e.g., Arc<dyn Fn(&AgentLoopState) -> bool + Send + Sync>) so clones preserve
the strategy, and adjust places that construct or store it (notably where
TextOptions::clone() sets agent_loop_strategy and where StructuredOutputOptions
derives Clone) to use the Arc-wrapped type; alternatively, if preserving clone
isn't desired, make the fields/types non-Clone, but prefer the Arc approach for
parity with ToolExecuteFn to ensure cloned configs keep the custom strategy.
- Around line 119-121: The four structs ImagePart, AudioPart, VideoPart, and
DocumentPart currently derive Deserialize but define part_type: &'static str
which cannot be deserialized from runtime JSON; fix by either (a) making the
field owned (e.g., change part_type to String) so serde can deserialize it, (b)
removing the Deserialize derive from these structs if they are unused, or (c)
delete the structs entirely if ContentPart/MessagePart cover the API—update the
declarations for ImagePart/AudioPart/VideoPart/DocumentPart and their part_type
field accordingly to match the chosen approach and keep consistency with
ContentPart and MessagePart.
---
Duplicate comments:
In `@crates/tanstack-ai/src/adapters/gemini.rs`:
- Around line 283-291: The current code replaces the entire "generationConfig"
object with snake_case keys, clobbering existing sampling params; change the
logic in the body.as_object_mut() branch to merge into an existing
generationConfig object instead of inserting a new value: if body contains
"generationConfig" and it's an object, set/overwrite the camelCase fields
"responseMimeType" and "responseSchema" (use options.output_schema) on that
object, otherwise create a new object with those camelCase keys; keep all other
generationConfig fields (e.g., temperature, topP, maxOutputTokens) intact and do
not replace the whole "generationConfig" value.
In `@crates/tanstack-ai/src/chat.rs`:
- Around line 118-140: The final assistant turn isn't being pushed into
engine.messages before exiting the normal-stop path, so downstream reads (e.g.,
when building StructuredOutputOptions/TextOptions and calling
adapter.structured_output) use stale self.messages; update the code paths that
handle a non-tool stop to append the assistant ModelMessage (same shape used in
the tool-call path) into engine.messages (and ensure engine.system_prompts stays
aligned) before constructing StructuredOutputOptions and invoking
adapter.structured_output; apply the same fix in the other analogous locations
handling final turns (the other places building
StructuredOutputOptions/TextOptions and calling adapter.structured_output).
- Around line 465-488: The wrapper is turning needs_client_execution and
needs_approval into synthetic Error ToolResult entries, preventing those tools
from ever completing; instead preserve their pending status by not converting
them to errors—either stop pushing them into all_results and propagate the
original needs_client_execution and needs_approval vectors back to the caller,
or if a ToolResult must be created use a non-error state (e.g.,
ToolResultOutputState::Pending) with a null/empty result and duration_ms: None;
update the code around execute_tool_calls, the loops handling
result.needs_client_execution and result.needs_approval, and the construction of
ToolResult so approvals and client-side tool calls remain as pending work rather
than failures.
In `@crates/tanstack-ai/src/client/connection_adapters.rs`:
- Around line 135-138: The write guard obtained via self.state.write().await is
held across an await, causing self-deadlock when notify_subscribers() reads
self.state; in functions like stop() adjust the scope so the write lock is
dropped before awaiting notify_subscribers() — e.g., perform mutations on the
state (using the write guard from self.state.write().await and setting
state.is_loading = false), then explicitly drop the guard (or let it go out of
scope) before calling self.notify_subscribers().await; apply the same pattern
wherever you call self.state.write().await, mutate fields, and then await
notify_subscribers() to ensure the write lock is released before the await.
- Around line 95-109: The bug is that accumulated_content is not cleared when a
new assistant response begins, so delta appends (from the streams handling code)
continue from the previous reply; locate where a new user message is added (the
block that pushes UiMessage using generate_message_id, UiMessageRole::User and
MessagePart::Text) and reset the adapter's accumulated_content (or equivalent
field) to an empty string/None there and also at the start of the
assistant-response handling path (where assistant deltas are processed) so each
assistant turn starts fresh before appending delta text.
- Around line 196-200: The non-2xx HTTP branch returns Err without updating the
adapter state; set state.is_loading = false and populate state.error with the
HTTP error message before returning, then publish the state update so
subscribers see the failure (e.g., in the same place you update SSE state).
Locate the non-success block around the response handling (the code using
response.status(), response.text()) and, before returning
Err(AiError::Provider(...)), assign the error text to state.error, clear
state.is_loading, and call the existing state-publish helper (e.g.,
publish_state()/dispatch_state()/self.publish_state_update()) used elsewhere in
connection_adapters.rs.
- Around line 134-139: The stop() method only flips state.is_loading and doesn’t
cancel the underlying in-flight stream/request; update stop() to abort the
active request/stream and ensure no further mutations occur. Specifically,
locate the active request handle used by the adapter (e.g., an
AbortHandle/AbortRegistration, Stream/JoinHandle, or channel sender stored on
the adapter struct) and call its abort/close method from stop() before or while
setting state.is_loading = false and calling notify_subscribers(); if such a
handle doesn't exist, add one (e.g., store an AbortHandle or JoinHandle when
starting the stream in the code that spawns the request) and use it here; also
ensure the stream-processing code checks the adapter state (is_loading or a
cancelled flag) before mutating state so aborted requests cannot race and apply
updates after stop().
In `@crates/tanstack-ai/src/messages.rs`:
- Around line 192-199: The Text branch currently pushes a MessagePart::Text for
all messages, which causes tool outputs to be duplicated because tool payloads
are also emitted as MessagePart::ToolResult; update the logic around the match
on &msg.content so that MessageContent::Text only pushes MessagePart::Text when
the message is not a tool result (e.g., check msg.role != Role::Tool or guard
against messages that will produce MessagePart::ToolResult) and skip adding the
Text for tool messages; reference the match on MessageContent::Text and the
MessagePart::ToolResult handling to ensure you only surface the payload once.
In `@crates/tanstack-ai/src/stream/processor.rs`:
- Around line 102-115: When handling StreamChunk::TextMessageEnd in
processor.rs, you're clearing current_segment_text without emitting its buffered
tail, so consumers miss the final text; before clearing current_segment_text and
marking msg.is_complete, construct and push a final TextMessageContent chunk (or
call the existing emission helper used elsewhere) using msg.current_segment_text
(and update msg.last_emitted_text/total_text_content accordingly), then clear
current_segment_text, set is_complete, take and push current_message into
messages, and finally call chunk_strategy.reset() as currently done.
In `@crates/tanstack-ai/src/tools/tool_calls.rs`:
- Around line 57-59: ToolCallManager currently uses a HashMap (tool_calls) and
tool_calls() iterates values() ignoring the streamed index, so multi-call
batches can be executed out-of-order; change the internal storage from an
unordered HashMap to an ordered collection (e.g., Vec or IndexMap) that
preserves insertion and/or supports stable ordering, update
ToolCallManager::tool_calls() to return calls sorted by the emitted index when
present (fallback to insertion order if index is missing), and adjust any
insert/update logic that references tool_calls to maintain the chosen ordering;
ensure the sort-by-index behavior is applied in the other affected areas you
flagged (around lines referenced for the same struct and iterations).
- Around line 139-145: The public API execute_tool_calls accepts an
UnboundedSender<ToolCustomEvent> but ToolExecutionContext expects
UnboundedSender<CustomEventData>, so the custom_event_tx is dropped and
ctx.emit_custom_event() becomes a no-op; fix by adapting the sender instead of
passing None: either change ToolExecutionContext (or its constructor) to accept
UnboundedSender<ToolCustomEvent> directly, or create an adapter that takes the
provided UnboundedSender<ToolCustomEvent> and produces an
UnboundedSender<CustomEventData> (or vice‑versa) by spawning a short-lived task
that maps ToolCustomEvent <-> CustomEventData and forwards messages, then pass
that adapted sender into ToolExecutionContext (referencing execute_tool_calls,
ToolExecutionContext, custom_event_tx, ToolCustomEvent and CustomEventData to
locate where to implement the conversion).
In `@crates/tanstack-ai/src/types.rs`:
- Around line 42-43: The struct-level serde rename_all should not convert JSON
Schema keywords to camelCase; update the JsonSchema definition to stop using
#[serde(rename_all = "camelCase")] and instead explicitly rename the schema
keyword fields so they serialize to the literal JSON Schema keys (e.g. annotate
the field representing $ref with #[serde(rename = "$ref")] and the field
representing $defs with #[serde(rename = "$defs")], using raw identifiers like
r#ref if needed), and apply the same explicit-field-rename fix to the other
struct(s) referenced around lines 58-61 so their $ref/$defs fields keep the
exact JSON Schema names.
---
Nitpick comments:
In `@crates/tanstack-ai/src/error.rs`:
- Around line 50-59: The blanket impls From<String> and From<&str> collapse all
string errors into AiError::Other which hides error categories; remove these two
impl blocks (the From<String> for AiError and From<&str> for AiError
implementations) so callers must construct explicit variants (e.g.,
AiError::Validation(...), AiError::Other(...)), and optionally add small
constructor helpers (e.g., AiError::validation(msg: impl Into<String>) -> Self)
or document preferred variant construction to make error categorization
explicit.
In `@crates/tanstack-ai/src/middleware.rs`:
- Around line 226-238: The current run_on_config silently treats a middleware
returning None from on_config as a no-op; change run_on_config to surface this
as an error by returning a Result<ChatMiddlewareConfig, MiddlewareError> (or
similar) instead of ChatMiddlewareConfig, iterate over self.middlewares calling
mw.on_config(ctx, ¤t) and if it returns Some(update) continue, but if it
returns None return Err(MiddlewareError::ConfigAborted { middleware:
mw.identifier_or_index(), context: ctx.clone() }) so callers can handle aborts;
update the run_on_config signature and all call sites, and add a small
MiddlewareError type and an identifier accessor (or use index) to help trace
which middleware returned None (refer to run_on_config and the on_config method
on middleware traits/types).
In `@crates/tanstack-ai/src/stream/json_parser.rs`:
- Around line 97-101: Add a new unit test named test_interleaved_array_object
that calls parse_partial_json with the input r#"[{"a": 1"# and asserts the
returned value equals serde_json::json!([{"a": 1}]); this verifies the parser
handles interleaved array/object nesting and proper closing order for mixed
structures—if the test fails, fix the parse_partial_json implementation to
correctly track and close nested arrays and objects in mixed contexts (ensure
stack/state used by parse_partial_json accounts for entering an array then an
object and pops in the correct order).
In `@crates/tanstack-ai/tests/stream_processor_tests.rs`:
- Around line 7-9: Extract the duplicate now() helper into a shared test
utilities module and have both tests import it: create a new test utils module
(e.g. tests/utils.rs or tests/common/mod.rs) with a pub fn now() -> f64
implementation, remove the local now() definitions from
stream_processor_tests.rs and tool_call_manager_tests.rs, and update those files
to call the shared utils::now() (or common::now()) so both tests reuse the
single implementation.
In `@crates/tanstack-ai/tests/tool_call_manager_tests.rs`:
- Around line 6-8: Extract the duplicated now() helper into a shared test
utilities module (e.g., a test_utils module exposing pub fn now() -> f64) and
replace the local now() implementations in tool_call_manager_tests.rs and
stream_processor_tests.rs with calls to that shared function; update each test
file to import or reference test_utils::now so both tests use the single shared
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 893b509a-ec9e-4a70-877d-cb6ff9e234cb
⛔ Files ignored due to path filters (1)
crates/tanstack-ai/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
crates/tanstack-ai/Cargo.tomlcrates/tanstack-ai/release.mdcrates/tanstack-ai/src/adapter.rscrates/tanstack-ai/src/adapters/anthropic.rscrates/tanstack-ai/src/adapters/gemini.rscrates/tanstack-ai/src/adapters/mod.rscrates/tanstack-ai/src/adapters/openai.rscrates/tanstack-ai/src/chat.rscrates/tanstack-ai/src/client/connection_adapters.rscrates/tanstack-ai/src/client/mod.rscrates/tanstack-ai/src/error.rscrates/tanstack-ai/src/lib.rscrates/tanstack-ai/src/messages.rscrates/tanstack-ai/src/middleware.rscrates/tanstack-ai/src/stream/json_parser.rscrates/tanstack-ai/src/stream/mod.rscrates/tanstack-ai/src/stream/processor.rscrates/tanstack-ai/src/stream/strategies.rscrates/tanstack-ai/src/stream/types.rscrates/tanstack-ai/src/stream_response.rscrates/tanstack-ai/src/tools/definition.rscrates/tanstack-ai/src/tools/mod.rscrates/tanstack-ai/src/tools/registry.rscrates/tanstack-ai/src/tools/tool_calls.rscrates/tanstack-ai/src/types.rscrates/tanstack-ai/tests/chat_tests.rscrates/tanstack-ai/tests/stream_processor_tests.rscrates/tanstack-ai/tests/test_utils.rscrates/tanstack-ai/tests/tool_call_manager_tests.rscrates/tanstack-ai/tests/unit_tests.rsexamples/rs-chat/Cargo.toml
✅ Files skipped from review due to trivial changes (5)
- examples/rs-chat/Cargo.toml
- crates/tanstack-ai/release.md
- crates/tanstack-ai/src/tools/mod.rs
- crates/tanstack-ai/Cargo.toml
- crates/tanstack-ai/src/adapters/mod.rs
Comment on lines
+194
to
+195
| StreamChunk::RunFinished { .. } => Some(chunk), | ||
| StreamChunk::RunError { .. } => Some(chunk), |
Contributor
There was a problem hiding this comment.
Persist the finish reason into ProcessorResult.
RunFinished is passed through unchanged, but result() hard-codes finish_reason: None, so callers can never inspect why the stream ended.
Also applies to: 203-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/stream/processor.rs` around lines 194 - 195, The
ProcessorResult currently sets finish_reason to None even when the StreamChunk
is StreamChunk::RunFinished or StreamChunk::RunError; update the logic in the
result() assembly (and the related branches around lines 203-240) to extract and
persist the finish reason from the chunk into ProcessorResult.finish_reason
instead of hard-coding None; specifically, when matching
StreamChunk::RunFinished { finish_reason, .. } or StreamChunk::RunError {
finish_reason, .. } propagate that finish_reason (or an appropriate mapped
value) into the ProcessorResult returned by result(), and mirror this change in
all analogous match arms so callers can inspect why the stream ended.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
crates/tanstack-ai/src/adapters/openai.rs (1)
33-42:⚠️ Potential issue | 🟠 Major
with_config()still leaves most ofTextAdapterConfigunused.Only
base_urlis applied here.timeout,headers, andmax_retriesare still silent no-ops, so callers cannot actually tune transport behavior through this constructor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 33 - 42, with_config currently only applies TextAdapterConfig.base_url and ignores timeout, headers, and max_retries; update with_config (the constructor for the OpenAI/Text adapter) to apply all config fields to the adapter instance: set adapter.timeout from config.timeout (if present), merge or extend adapter.headers with config.headers (preserving existing defaults), and set adapter.max_retries from config.max_retries (if present); ensure you reference the existing adapter fields (adapter.base_url, adapter.timeout, adapter.headers, adapter.max_retries) and handle Option values appropriately so callers can tune transport behavior via TextAdapterConfig.crates/tanstack-ai/src/chat.rs (1)
324-356:⚠️ Potential issue | 🟠 MajorPending tool requests still disappear from the public result.
ExecuteToolCallsResultpreservesneeds_approvalandneeds_client_execution, but this block only feeds them tophase_info/middleware and then breaks. Since neitherStreamChunknorChatResultcarries equivalent data, callers without an out-of-band middleware channel cannot approve or complete the next tool step.Also applies to: 374-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/chat.rs` around lines 324 - 356, The ExecuteToolCallsResult's needs_approval and needs_client_execution are being converted into ToolPhaseCompleteInfo and sent only to middleware via self.middleware_runner.run_on_tool_phase_complete(&middleware_ctx, &phase_info), but they are not propagated into the emitted results so callers without middleware cannot act on pending tool requests; modify the emission path (where ToolPhaseCompleteInfo is built and where the code produces StreamChunk/ChatResult) to add fields for needs_approval and needs_client_execution into the public result types (extend StreamChunk/ChatResult variants to carry these lists) and populate them from execution.needs_approval and execution.needs_client_execution when handling tool-phase completion so both middleware and callers receive the pending tool requests; ensure updates touch the construction site using ToolPhaseCompleteInfo, run_on_tool_phase_complete, and the code paths that create StreamChunk/ChatResult for tool-phase completion.
🧹 Nitpick comments (5)
crates/tanstack-ai/src/adapters/anthropic.rs (1)
595-601: Parse errors are logged to stderr but not propagated to the stream.When an Anthropic event fails to parse (line 595-601), the error is logged via
eprintln!but the stream continues silently. This could mask protocol issues in production. Consider emitting aStreamChunk::RunErroror at minimum using a proper logging framework.Option: Emit parse errors as stream errors
Err(e) => { - eprintln!( - "Failed to parse Anthropic event: {} - {}", - e, - &data_str[..data_str.len().min(200)] - ); + chunks.push(Err(AiError::Stream(format!( + "Failed to parse Anthropic event: {}", + e + )))); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/anthropic.rs` around lines 595 - 601, The parse failure branch currently prints to stderr with eprintln! and swallows the error; change it to emit a stream error instead by creating and sending a StreamChunk::RunError (or otherwise propagating a RunError variant) from the same error-handling block where the parse fails (the Err(e) branch that references data_str). Ensure you use the same channel/sender used for other chunks in this function (the sender or stream push used elsewhere in this event loop) and include the parse error message and a snippet of data_str in the RunError payload so consumers see the protocol/parse failure instead of silent logging.crates/tanstack-ai/src/stream_response.rs (1)
139-166: Consider propagating the error code fromRunError.
stream_to_textconvertsRunErrortoAiError::Providerbut only useserror.message, discarding any error code. If downstream consumers need the error code for categorization or retry logic, this information is lost.Optional: include error code in the error message
StreamChunk::RunError { error, .. } => { - return Err(AiError::Provider(error.message)); + return Err(AiError::Provider( + if let Some(code) = &error.code { + format!("{}: {}", code, error.message) + } else { + error.message + } + )); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream_response.rs` around lines 139 - 166, stream_to_text currently maps StreamChunk::RunError to AiError::Provider using only error.message, losing the error code; update the RunError handling in stream_to_text to include error.code (e.g., format!("{} (code: {})", error.message, error.code.as_deref().unwrap_or("unknown"))) so the AiError::Provider carries both message and code, or alternatively extend AiError::Provider to accept a code field and populate it from RunError.code; adjust the match arm for StreamChunk::RunError accordingly in stream_to_text to propagate the code.crates/tanstack-ai/src/stream/processor.rs (1)
105-119: Buffered text is flushed internally but no finalTextMessageContentchunk is emitted.When using a batching strategy,
current_segment_textmay contain un-emitted text atTextMessageEnd. The code updateslast_emitted_textbut doesn't emit a finalTextMessageContentchunk. Consumers relying solely on emitted chunks (not callingresult()) may miss the final text segment.This appears to be intentional design—callers should use
result()for complete text. Consider adding a doc comment clarifying this behavior.Optional: Document the expected usage pattern
/// Process a single stream chunk, updating internal state. /// /// Returns the processed chunk if it should be emitted to the UI. + /// Note: When using batching strategies, call `result()` after stream completion + /// to retrieve the full accumulated text, as some content may be buffered. pub fn process_chunk(&mut self, chunk: StreamChunk) -> Option<StreamChunk> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream/processor.rs` around lines 105 - 119, The TextMessageEnd branch currently flushes buffered text into message state (manipulating current_message, current_segment_text and last_emitted_text) but does not emit a final StreamChunk::TextMessageContent, so consumers that only read emitted chunks can miss the last segment; either emit a final TextMessageContent chunk before taking self.current_message or explicitly document this behavior so callers know to call result() to obtain buffered text—update the TextMessageEnd handling (around StreamChunk::TextMessageEnd) to push a final TextMessageContent when current_segment_text is non-empty, or add a clear doc comment on the processor/stream API (mentioning current_message, current_segment_text, last_emitted_text and result()) explaining that the final buffered segment is returned only via result() and not emitted as a chunk.crates/tanstack-ai/src/lib.rs (1)
58-69: Consider more selective re-exports to avoid potential name collisions.The glob re-exports (
pub use module::*) from all submodules could lead to name collisions if modules happen to export identically-named items. This is a common SDK pattern but worth documenting in the crate-level docs which modules take precedence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/lib.rs` around lines 58 - 69, The crate currently does broad glob re-exports (pub use adapter::*; pub use adapters::*; pub use chat::*; pub use client::*; pub use error::*; pub use messages::*; pub use middleware::*; pub use stream::*; pub use stream_response::*; pub use tools::*; pub use types::*), which risks public name collisions; replace these with selective, explicit re-exports (e.g., pub use client::Client; pub use error::{Error, Result}; pub use messages::Message; etc.) or create a documented public prelude module that re-exports a curated set, and update crate-level docs to state which module provides each symbol to avoid ambiguity — change the glob lines to explicit item lists or a prelude module and ensure exported names are unique and documented.crates/tanstack-ai/src/client/connection_adapters.rs (1)
493-512: Subscriber cleanup has a potential race condition.
notify_subscriberscollects failed subscriber IDs while holding a read lock, then drops it to acquire a write lock for removal. Between these operations, another task could add or remove subscribers, though the impact is minor (worst case: a subscriber that just closed is not removed until next notification).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/client/connection_adapters.rs` around lines 493 - 512, notify_subscribers currently reads state and checks subscribers under a read lock then drops it to acquire a write lock, allowing a race where subscribers change between checks and removals; fix by cloning the state up front (read().await and state.clone()), then acquire a write lock on self.subscribers (write().await) and iterate directly, attempting tx.send(cloned_state) and removing any entries where send() returns Err, so failed subscribers are removed immediately while holding the write lock (change logic in notify_subscribers and use the cloned state variable when sending).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tanstack-ai/src/chat.rs`:
- Around line 103-128: The code calls adapter.structured_output()
unconditionally after engine.run(), which can run against an unfinished agent
state and omits metadata/model options/conversation_id; change the flow to
inspect engine.run() result or engine state (e.g., check a terminal flag or
returned status from engine.run()) and only call adapter.structured_output()
when the agent reached a terminal state, and when building
StructuredOutputOptions/TextOptions include engine.metadata,
engine.model_options, and engine.conversation_id (or copy any provider-specific
settings) so the structured pass uses the same context and provider settings as
the completed run.
- Around line 318-320: The code is bypassing middleware because it takes
tool_calls from self.tool_call_manager.tool_calls() straight into
execute_tool_calls(); instead iterate the tool_calls and call
self.on_before_tool_call(tool_call).await (or the appropriate
on_before_tool_call method) for each item, apply middleware decisions: if it
returns TransformArgs use the transformed args when building the execution list,
if it returns Skip omit that call, and if it returns Abort propagate/return the
abort error/result immediately; only pass the final filtered/transformed set
into execute_tool_calls(), keeping references to tool_call_manager.tool_calls(),
execute_tool_calls(), on_before_tool_call(), and the middleware variants
TransformArgs/Skip/Abort to locate the logic.
---
Duplicate comments:
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 33-42: with_config currently only applies
TextAdapterConfig.base_url and ignores timeout, headers, and max_retries; update
with_config (the constructor for the OpenAI/Text adapter) to apply all config
fields to the adapter instance: set adapter.timeout from config.timeout (if
present), merge or extend adapter.headers with config.headers (preserving
existing defaults), and set adapter.max_retries from config.max_retries (if
present); ensure you reference the existing adapter fields (adapter.base_url,
adapter.timeout, adapter.headers, adapter.max_retries) and handle Option values
appropriately so callers can tune transport behavior via TextAdapterConfig.
In `@crates/tanstack-ai/src/chat.rs`:
- Around line 324-356: The ExecuteToolCallsResult's needs_approval and
needs_client_execution are being converted into ToolPhaseCompleteInfo and sent
only to middleware via
self.middleware_runner.run_on_tool_phase_complete(&middleware_ctx, &phase_info),
but they are not propagated into the emitted results so callers without
middleware cannot act on pending tool requests; modify the emission path (where
ToolPhaseCompleteInfo is built and where the code produces
StreamChunk/ChatResult) to add fields for needs_approval and
needs_client_execution into the public result types (extend
StreamChunk/ChatResult variants to carry these lists) and populate them from
execution.needs_approval and execution.needs_client_execution when handling
tool-phase completion so both middleware and callers receive the pending tool
requests; ensure updates touch the construction site using
ToolPhaseCompleteInfo, run_on_tool_phase_complete, and the code paths that
create StreamChunk/ChatResult for tool-phase completion.
---
Nitpick comments:
In `@crates/tanstack-ai/src/adapters/anthropic.rs`:
- Around line 595-601: The parse failure branch currently prints to stderr with
eprintln! and swallows the error; change it to emit a stream error instead by
creating and sending a StreamChunk::RunError (or otherwise propagating a
RunError variant) from the same error-handling block where the parse fails (the
Err(e) branch that references data_str). Ensure you use the same channel/sender
used for other chunks in this function (the sender or stream push used elsewhere
in this event loop) and include the parse error message and a snippet of
data_str in the RunError payload so consumers see the protocol/parse failure
instead of silent logging.
In `@crates/tanstack-ai/src/client/connection_adapters.rs`:
- Around line 493-512: notify_subscribers currently reads state and checks
subscribers under a read lock then drops it to acquire a write lock, allowing a
race where subscribers change between checks and removals; fix by cloning the
state up front (read().await and state.clone()), then acquire a write lock on
self.subscribers (write().await) and iterate directly, attempting
tx.send(cloned_state) and removing any entries where send() returns Err, so
failed subscribers are removed immediately while holding the write lock (change
logic in notify_subscribers and use the cloned state variable when sending).
In `@crates/tanstack-ai/src/lib.rs`:
- Around line 58-69: The crate currently does broad glob re-exports (pub use
adapter::*; pub use adapters::*; pub use chat::*; pub use client::*; pub use
error::*; pub use messages::*; pub use middleware::*; pub use stream::*; pub use
stream_response::*; pub use tools::*; pub use types::*), which risks public name
collisions; replace these with selective, explicit re-exports (e.g., pub use
client::Client; pub use error::{Error, Result}; pub use messages::Message; etc.)
or create a documented public prelude module that re-exports a curated set, and
update crate-level docs to state which module provides each symbol to avoid
ambiguity — change the glob lines to explicit item lists or a prelude module and
ensure exported names are unique and documented.
In `@crates/tanstack-ai/src/stream_response.rs`:
- Around line 139-166: stream_to_text currently maps StreamChunk::RunError to
AiError::Provider using only error.message, losing the error code; update the
RunError handling in stream_to_text to include error.code (e.g., format!("{}
(code: {})", error.message, error.code.as_deref().unwrap_or("unknown"))) so the
AiError::Provider carries both message and code, or alternatively extend
AiError::Provider to accept a code field and populate it from RunError.code;
adjust the match arm for StreamChunk::RunError accordingly in stream_to_text to
propagate the code.
In `@crates/tanstack-ai/src/stream/processor.rs`:
- Around line 105-119: The TextMessageEnd branch currently flushes buffered text
into message state (manipulating current_message, current_segment_text and
last_emitted_text) but does not emit a final StreamChunk::TextMessageContent, so
consumers that only read emitted chunks can miss the last segment; either emit a
final TextMessageContent chunk before taking self.current_message or explicitly
document this behavior so callers know to call result() to obtain buffered
text—update the TextMessageEnd handling (around StreamChunk::TextMessageEnd) to
push a final TextMessageContent when current_segment_text is non-empty, or add a
clear doc comment on the processor/stream API (mentioning current_message,
current_segment_text, last_emitted_text and result()) explaining that the final
buffered segment is returned only via result() and not emitted as a chunk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4ccd160-011f-44f6-9ccc-4b134fb88027
📒 Files selected for processing (19)
crates/tanstack-ai/src/adapters/anthropic.rscrates/tanstack-ai/src/adapters/gemini.rscrates/tanstack-ai/src/adapters/openai.rscrates/tanstack-ai/src/chat.rscrates/tanstack-ai/src/client/connection_adapters.rscrates/tanstack-ai/src/lib.rscrates/tanstack-ai/src/messages.rscrates/tanstack-ai/src/middleware.rscrates/tanstack-ai/src/stream/json_parser.rscrates/tanstack-ai/src/stream/mod.rscrates/tanstack-ai/src/stream/processor.rscrates/tanstack-ai/src/stream/strategies.rscrates/tanstack-ai/src/stream_response.rscrates/tanstack-ai/src/tools/tool_calls.rscrates/tanstack-ai/src/types.rscrates/tanstack-ai/tests/chat_tests.rscrates/tanstack-ai/tests/stream_processor_tests.rscrates/tanstack-ai/tests/test_utils.rscrates/tanstack-ai/tests/unit_tests.rs
✅ Files skipped from review due to trivial changes (1)
- crates/tanstack-ai/src/stream/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/tanstack-ai/src/stream/json_parser.rs
- crates/tanstack-ai/src/stream/strategies.rs
- crates/tanstack-ai/tests/chat_tests.rs
- crates/tanstack-ai/src/adapters/gemini.rs
- crates/tanstack-ai/src/types.rs
crates/tanstack-ai/src/chat.rs
Outdated
Comment on lines
+318
to
+320
| let tool_calls = self.tool_call_manager.tool_calls(); | ||
| let execution = self.execute_tool_calls(&tool_calls).await?; | ||
| let results = execution.results; |
Contributor
There was a problem hiding this comment.
on_before_tool_call() is never used.
tool_calls go straight into execute_tool_calls(), so middleware decisions like TransformArgs, Skip, and Abort in crates/tanstack-ai/src/middleware.rs never affect execution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/chat.rs` around lines 318 - 320, The code is bypassing
middleware because it takes tool_calls from self.tool_call_manager.tool_calls()
straight into execute_tool_calls(); instead iterate the tool_calls and call
self.on_before_tool_call(tool_call).await (or the appropriate
on_before_tool_call method) for each item, apply middleware decisions: if it
returns TransformArgs use the transformed args when building the execution list,
if it returns Skip omit that call, and if it returns Abort propagate/return the
abort error/result immediately; only pass the final filtered/transformed set
into execute_tool_calls(), keeping references to tool_call_manager.tool_calls(),
execute_tool_calls(), on_before_tool_call(), and the middleware variants
TransformArgs/Skip/Abort to locate the logic.
swalker326
force-pushed
the
feat/rust-sdk-initial
branch
from
91181bd to
9c7d4d7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (9)
crates/tanstack-ai/src/adapters/anthropic.rs (2)
95-161:⚠️ Potential issue | 🟠 MajorText and Parts content branches don't serialize tool_calls.
When an assistant message has both
content(Text or Parts) ANDtool_calls, only the content is serialized. The tool calls are dropped, breaking multi-modal assistant responses that include tool invocations.The
Nullbranch (lines 132-160) correctly handlestool_calls, butText(lines 96-98) andParts(lines 99-131) do not check formsg.tool_calls.Suggested fix for Text branch
MessageContent::Text(text) => { - m.insert("content".to_string(), serde_json::json!(text)); + if let Some(tool_calls) = &msg.tool_calls { + let mut content = vec![serde_json::json!({"type": "text", "text": text})]; + for tc in tool_calls { + content.push(serde_json::json!({ + "type": "tool_use", + "id": tc.id, + "name": tc.function.name, + "input": serde_json::from_str::<serde_json::Value>( + &tc.function.arguments + ).unwrap_or(serde_json::json!({})) + })); + } + m.insert("content".to_string(), serde_json::json!(content)); + } else { + m.insert("content".to_string(), serde_json::json!(text)); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/anthropic.rs` around lines 95 - 161, The Text and Parts branches in the message serialization (match on MessageContent in anthropic.rs) currently insert only "content" and drop msg.tool_calls; update both MessageContent::Text and MessageContent::Parts handling to also detect and serialize msg.tool_calls similarly to the MessageContent::Null branch: when msg.tool_calls.is_some() produce a "content" array that includes the existing text/parts representation plus serialized tool_call entries (type "tool_use", id, name, parsed input), and when the message role is Tool include the "tool_result" item with tool_call_id; ensure you reuse the same field names and JSON shapes used in the Null branch so Text, Parts, and Null produce consistent output.
670-676:⚠️ Potential issue | 🟠 Major
RunFinisheduses fabricated ID, breaking start/finish correlation.
MessageStartemitsmessage.idforRunStarted(line 633), butMessageDeltagenerates a new ID usingformat!("anthropic-{}", chrono::Utc::now().timestamp_millis())forRunFinished. Consumers keying byrun_idwill fail to correlate start and finish events.Suggested approach
Track the
message.idfromMessageStartin the stream state (similar totool_block_ids) and use it when emittingRunFinished:fn parse_anthropic_sse_stream<S>( stream: S, model: String, ) -> impl Stream<Item = AiResult<StreamChunk>> where S: Stream<Item = Result<bytes::Bytes, reqwest::Error>> + Send, { use futures_util::StreamExt; let stream = Box::pin(stream); futures_util::stream::unfold( - (stream, String::new(), HashMap::<usize, String>::new()), - move |(mut stream, mut line_buf, mut tool_block_ids)| { + (stream, String::new(), HashMap::<usize, String>::new(), None::<String>), + move |(mut stream, mut line_buf, mut tool_block_ids, mut run_id)| {Then capture
message.idinMessageStartand use it inMessageDelta.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/anthropic.rs` around lines 670 - 676, The RunFinished chunk is using a fabricated run_id (format!("anthropic-{}", chrono::Utc::now().timestamp_millis())) which breaks correlation with the RunStarted emitted from MessageStart that uses message.id; fix by persisting message.id from MessageStart into the stream state (analogous to tool_block_ids) when you handle MessageStart, then read that stored id in the MessageDelta/finish path and use it for the RunFinished.run_id instead of generating a new one; update the code around MessageStart, the stream state struct, and the chunks.push for RunFinished in anthropic.rs to reference the saved message.id.crates/tanstack-ai/src/stream_response.rs (1)
79-90:⚠️ Potential issue | 🟠 MajorBuffered SSE data may be dropped on EOF.
When the stream ends (line 83), the code returns
Nonewithout flushing any remaining data indata_buf. If the upstream closes without a trailing blank line, the last buffered event is lost.Suggested fix
Some(Err(e)) => { return Some((Err(AiError::Stream(e.to_string())), (lines, data_buf))); } - None => return None, // Stream ended + None => { + // Flush any remaining buffered data on EOF + if data_buf.is_empty() { + return None; + } + let data = std::mem::take(&mut data_buf); + if data.trim() == "[DONE]" { + return None; + } + match serde_json::from_str::<serde_json::Value>(&data) { + Ok(json) => return Some((Ok(json), (lines, data_buf))), + Err(e) => { + return Some(( + Err(AiError::Stream(format!("Failed to parse SSE data: {}", e))), + (lines, data_buf), + )); + } + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream_response.rs` around lines 79 - 90, The None branch that handles stream EOF currently returns immediately and drops any buffered SSE data in data_buf; update the match arm for None to first flush any remaining data_buf into lines (as a final event), yield that last event (i.e., produce the same tuple structure used for events), and only then return None; look for the match handling the stream items and the variables data_buf, lines, and the stream/Box::pin to implement this flush-before-return behavior.crates/tanstack-ai/src/stream/processor.rs (1)
248-253:⚠️ Potential issue | 🟡 Minor
result()hard-codesfinish_reason: None, discarding completion status.When
RunFinishedchunks are processed, the finish reason is not captured anywhere inStreamProcessorstate. Theresult()method always returnsfinish_reason: None, so callers inspecting the result cannot determine why the stream ended (stop, tool_calls, error, etc.).Consider adding a
last_finish_reason: Option<String>field toStreamProcessorthat gets set when processingRunFinishedorRunErrorchunks, then use it inresult().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/stream/processor.rs` around lines 248 - 253, The result() method currently returns ProcessorResult with finish_reason: None, losing completion status; add a new field on StreamProcessor named last_finish_reason: Option<String> (or Option<FinishReason> if a type exists), update the chunk-processing logic that handles RunFinished and RunError to set self.last_finish_reason = Some(reason_string) (derive the string from the chunk or error), and change result() to return finish_reason: self.last_finish_reason.clone() so callers can observe why the stream ended; reference StreamProcessor, result(), RunFinished and RunError when making these changes.crates/tanstack-ai/src/adapters/gemini.rs (4)
58-67:⚠️ Potential issue | 🟠 MajorRequest uses
"system_instruction"but Gemini API requires"systemInstruction".The Gemini REST API expects camelCase field names. The current code inserts
"system_instruction"(snake_case) which will be ignored by the API.🔧 Suggested fix
if !options.system_prompts.is_empty() { body.insert( - "system_instruction".to_string(), + "systemInstruction".to_string(), serde_json::json!({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/gemini.rs` around lines 58 - 67, The Gemini request body is using the snake_case key "system_instruction" which the Gemini API will ignore; update the code that builds the request body (around the block that checks options.system_prompts and inserts into body) to insert the camelCase key "systemInstruction" instead of "system_instruction" so the system prompts are accepted by the API (keep the same JSON value structure and the surrounding logic in the function that constructs the request body in adapters/gemini.rs).
416-424:⚠️ Potential issue | 🟠 MajorEOF branch discards buffered SSE data without processing.
When the stream ends (
None), the code returnsNoneimmediately without checking ifline_bufcontains a final incomplete SSE event. If the lastdata:line arrives without a trailing newline, it will be silently dropped.🔧 Suggested fix
None => { + // Process any remaining buffered data + let line = line_buf.trim(); + if !line.is_empty() { + if let Some(data_str) = line.strip_prefix("data: ") { + if let Ok(response) = serde_json::from_str::<GeminiResponse>(data_str) { + let chunks: Vec<AiResult<StreamChunk>> = convert_gemini_response(response, &model) + .into_iter() + .map(Ok) + .collect(); + if !chunks.is_empty() { + return Some(( + futures_util::stream::iter(chunks), + (stream, String::new()), + )); + } + } + } + } return None; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/gemini.rs` around lines 416 - 424, When handling the SSE stream EOF in the branch that currently returns None, ensure any buffered data in line_buf is processed instead of discarded: if line_buf is non-empty, push its contents as a final SSE event into the returned stream (e.g., via futures_util::stream::iter with an Ok variant) and then return the (stream, line_buf) state; only return None when line_buf is empty. Update the None branch that currently returns None to check line_buf and emit the final buffered event before closing, using the existing stream/line_buf variables and the same result types (matching the Some(Err(e)) path that returns futures_util::stream::iter and (stream, line_buf)).
188-245:⚠️ Potential issue | 🔴 CriticalResponse structs use snake_case fields but Gemini API returns camelCase.
The
GeminiResponse,GeminiCandidate,GeminiUsagestructs have fields likeusage_metadata,finish_reason,prompt_token_countin snake_case, but the Gemini REST API returns camelCase (usageMetadata,finishReason,promptTokenCount). Without#[serde(rename_all = "camelCase")]or individual#[serde(rename = "...")]attributes, deserialization will fail to populate these fields.🔧 Suggested fix
#[allow(dead_code)] #[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] struct GeminiResponse { candidates: Option<Vec<GeminiCandidate>>, usage_metadata: Option<GeminiUsage>, } #[allow(dead_code)] #[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] struct GeminiCandidate { content: Option<GeminiContent>, finish_reason: Option<String>, } // ... similar for GeminiUsage🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/gemini.rs` around lines 188 - 245, The structs GeminiResponse, GeminiCandidate, GeminiUsage (and any other structs with mismatch like GeminiPart/GeminiFunctionCall/GeminiFunctionResponse/GeminiContent) use Rust snake_case field names but the Gemini API returns camelCase; add serde field renaming so deserialization works — e.g. add #[serde(rename_all = "camelCase")] above the struct definitions (or add individual #[serde(rename = "...")] on specific fields such as usage_metadata, finish_reason, prompt_token_count, candidates_token_count, total_token_count) to align the struct fields with the API JSON names.
451-472:⚠️ Potential issue | 🟠 MajorTool call IDs are synthetic and don't capture the API-provided ID.
The code generates synthetic IDs (
format!("gemini-tool-{}", i)) using the candidate index, but Gemini's API returns a uniqueidfield in eachfunctionCall. TheGeminiFunctionCallstruct doesn't have anidfield to capture this. Without the real ID, tool responses cannot be properly correlated with their originating calls in multi-turn conversations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/gemini.rs` around lines 451 - 472, The code uses synthetic tool_call_id (format!("gemini-tool-{}", i)) in GeminiPart::FunctionCall handling which loses the API-provided ID; add an id: Option<String> field to the GeminiFunctionCall struct (and update its deserialization) to capture the functionCall.id returned by Gemini, then change the StreamChunk::ToolCallStart and ToolCallEnd constructions to use that id when present (falling back to the existing synthetic format if id is None) so tool responses can be correlated to the real API call ID.crates/tanstack-ai/src/adapters/openai.rs (1)
812-820:⚠️ Potential issue | 🟠 Major
OutputTextDoneemitsTextMessageContentinstead ofTextMessageEnd.When
OutputTextDoneis received, it should emitTextMessageEndto properly finalize the message. Currently it emits anotherTextMessageContentwith an empty delta, which meansStreamProcessornever receives the end signal to finalize message state and flush buffered text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 812 - 820, OpenAiStreamEvent::OutputTextDone is currently building a StreamChunk::TextMessageContent with an empty delta which prevents StreamProcessor from finalizing buffered text; replace that construction so OutputTextDone emits StreamChunk::TextMessageEnd (not TextMessageContent) and populate the end variant with the same timestamp (now), message_id (item_id), final content (text) and model (model_str.clone()) so the StreamProcessor receives the proper end signal to flush and finalize the message.
🧹 Nitpick comments (4)
crates/tanstack-ai/src/types.rs (1)
843-843: Consider moving the import to the top of the file.The
use crate::error::AiResultstatement at line 843 is placed at the bottom of the file, which is unconventional. While valid Rust, imports are typically grouped at the top for discoverability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/types.rs` at line 843, Move the standalone import statement `use crate::error::AiResult` from near the bottom of types.rs up into the module-level imports grouping at the top of the file so all `use` declarations are discoverable and consistent; locate the existing `use` block(s) (e.g., other `use crate::...` lines) and add `use crate::error::AiResult` there, removing the bottom occurrence to avoid duplication.crates/tanstack-ai/tests/test_utils.rs (2)
187-206: MockAdapter returns raw chunks without ensuring complete event sequences.The
chat_streamimplementation returns whatever chunks are initerations[idx]directly. If tests don't explicitly includeRunStartedandRunFinishedchunks in their iterations, the chat engine's agent loop may not function correctly since it relies onRunFinishedto setlast_finish_reasonfor loop termination and tool execution decisions.Consider documenting this requirement or providing a helper that wraps chunks with lifecycle events.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/tests/test_utils.rs` around lines 187 - 206, The mock chat_stream currently returns iterations[*idx] verbatim which can omit lifecycle events; update chat_stream (and/or provide a helper) to ensure each returned chunk sequence begins with a RunStarted event and ends with a RunFinished event (or at minimum set last_finish_reason-equivalent behavior) when iterations[*idx] does not already contain them: inspect iterations.get(*idx) in chat_stream, wrap the Vec<Chunk> with synthetic RunStarted and RunFinished chunks as needed before creating the stream so the agent loop that relies on RunFinished and last_finish_reason will always see a complete event sequence.
261-270: Mutex lock in async closure may block the executor.The
server_toolhelper uses a synchronousMutexinside an async closure. While this works for simple test scenarios, usingtokio::sync::Mutexwould be more appropriate for async contexts to avoid potential blocking.However, since this is test utility code and the lock is held briefly, this is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/tests/test_utils.rs` around lines 261 - 270, The async closure in server_tool uses a synchronous Mutex which can block the executor; replace any std::sync::Mutex usage with tokio::sync::Mutex (wrap in Arc if shared) and change synchronous locking to await the async lock (e.g., mutex.lock().await) inside the closure; update imports and ensure Tool::new(...).with_execute closure uses the async lock and clones the Arc<tokio::sync::Mutex<...>> as needed so no blocking sync mutex is held in the async path.crates/tanstack-ai/src/lib.rs (1)
58-69: Glob re-exports may cause name collisions and increase compile times.Using
pub use module::*for all modules can lead to ambiguous imports when multiple modules export items with the same name, and increases incremental compilation times. Consider using explicit re-exports for the public API surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/lib.rs` around lines 58 - 69, The current lib.rs uses broad glob re-exports (adapter::*, adapters::*, chat::*, client::*, error::*, messages::*, middleware::*, stream::*, stream_response::*, tools::*, types::*) which can cause name collisions and slow compilation; replace each `pub use <module>::*;` with explicit `pub use <module>::{TypeA, fn_b, CONST_C, ...};` exports listing only the public types/functions/constants intended for the crate API (e.g., pick the actual structs, enums, traits and functions from adapter, adapters, chat, client, error, messages, middleware, stream, stream_response, tools, and types) so collisions are avoided and the public surface is clear. Ensure you update export lists in lib.rs to reference the precise symbols (function/struct/trait names) from each module and run cargo check to validate missing or unused exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 135-146: convert_messages() currently emits tool messages in Chat
Completions format; update the MessageRole::Tool branch so it builds a Responses
API-style object: replace the JSON with {"type": "function_call_output",
"call_id": msg.tool_call_id, "output": <content>} (where <content> is the same
extracted string from MessageContent) instead of {"role":"tool",...}; keep the
same content extraction logic and push that new object into items so
MessageRole::Tool conforms to the Responses API.
- Around line 162-188: In the MessageContent::Parts branch update the serialized
part type strings to match the OpenAI Responses API: change the
ContentPart::Text serialization from "type": "text" to "type": "input_text" and
change the ContentPart::Image serialization from "type": "image_url" to "type":
"input_image" (keep the rest of the payload structure, e.g., the image_url
object with url). This affects the code that constructs the JSON for
ContentPart::Text and ContentPart::Image in the adapter (the
MessageContent::Parts match arm).
In `@crates/tanstack-ai/src/tools/tool_calls.rs`:
- Around line 113-118: The current code silently replaces tc.function.arguments
with an empty string when serde_json::to_string(final_input) fails; instead,
detect the serialization error and avoid corrupting arguments—replace the
unwrap_or_default() usage with explicit error handling: if
to_string(final_input) returns Err, log an error (including tool_call_id and the
serialization error and optionally final_input) using your project's logger
(e.g., tracing::error! or log::error!) and do not overwrite
tc.function.arguments; only assign the serialized string on Ok. This keeps
previous arguments intact and surfaces failures for debugging while still
updating tc.function.arguments on success.
---
Duplicate comments:
In `@crates/tanstack-ai/src/adapters/anthropic.rs`:
- Around line 95-161: The Text and Parts branches in the message serialization
(match on MessageContent in anthropic.rs) currently insert only "content" and
drop msg.tool_calls; update both MessageContent::Text and MessageContent::Parts
handling to also detect and serialize msg.tool_calls similarly to the
MessageContent::Null branch: when msg.tool_calls.is_some() produce a "content"
array that includes the existing text/parts representation plus serialized
tool_call entries (type "tool_use", id, name, parsed input), and when the
message role is Tool include the "tool_result" item with tool_call_id; ensure
you reuse the same field names and JSON shapes used in the Null branch so Text,
Parts, and Null produce consistent output.
- Around line 670-676: The RunFinished chunk is using a fabricated run_id
(format!("anthropic-{}", chrono::Utc::now().timestamp_millis())) which breaks
correlation with the RunStarted emitted from MessageStart that uses message.id;
fix by persisting message.id from MessageStart into the stream state (analogous
to tool_block_ids) when you handle MessageStart, then read that stored id in the
MessageDelta/finish path and use it for the RunFinished.run_id instead of
generating a new one; update the code around MessageStart, the stream state
struct, and the chunks.push for RunFinished in anthropic.rs to reference the
saved message.id.
In `@crates/tanstack-ai/src/adapters/gemini.rs`:
- Around line 58-67: The Gemini request body is using the snake_case key
"system_instruction" which the Gemini API will ignore; update the code that
builds the request body (around the block that checks options.system_prompts and
inserts into body) to insert the camelCase key "systemInstruction" instead of
"system_instruction" so the system prompts are accepted by the API (keep the
same JSON value structure and the surrounding logic in the function that
constructs the request body in adapters/gemini.rs).
- Around line 416-424: When handling the SSE stream EOF in the branch that
currently returns None, ensure any buffered data in line_buf is processed
instead of discarded: if line_buf is non-empty, push its contents as a final SSE
event into the returned stream (e.g., via futures_util::stream::iter with an Ok
variant) and then return the (stream, line_buf) state; only return None when
line_buf is empty. Update the None branch that currently returns None to check
line_buf and emit the final buffered event before closing, using the existing
stream/line_buf variables and the same result types (matching the Some(Err(e))
path that returns futures_util::stream::iter and (stream, line_buf)).
- Around line 188-245: The structs GeminiResponse, GeminiCandidate, GeminiUsage
(and any other structs with mismatch like
GeminiPart/GeminiFunctionCall/GeminiFunctionResponse/GeminiContent) use Rust
snake_case field names but the Gemini API returns camelCase; add serde field
renaming so deserialization works — e.g. add #[serde(rename_all = "camelCase")]
above the struct definitions (or add individual #[serde(rename = "...")] on
specific fields such as usage_metadata, finish_reason, prompt_token_count,
candidates_token_count, total_token_count) to align the struct fields with the
API JSON names.
- Around line 451-472: The code uses synthetic tool_call_id
(format!("gemini-tool-{}", i)) in GeminiPart::FunctionCall handling which loses
the API-provided ID; add an id: Option<String> field to the GeminiFunctionCall
struct (and update its deserialization) to capture the functionCall.id returned
by Gemini, then change the StreamChunk::ToolCallStart and ToolCallEnd
constructions to use that id when present (falling back to the existing
synthetic format if id is None) so tool responses can be correlated to the real
API call ID.
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 812-820: OpenAiStreamEvent::OutputTextDone is currently building a
StreamChunk::TextMessageContent with an empty delta which prevents
StreamProcessor from finalizing buffered text; replace that construction so
OutputTextDone emits StreamChunk::TextMessageEnd (not TextMessageContent) and
populate the end variant with the same timestamp (now), message_id (item_id),
final content (text) and model (model_str.clone()) so the StreamProcessor
receives the proper end signal to flush and finalize the message.
In `@crates/tanstack-ai/src/stream_response.rs`:
- Around line 79-90: The None branch that handles stream EOF currently returns
immediately and drops any buffered SSE data in data_buf; update the match arm
for None to first flush any remaining data_buf into lines (as a final event),
yield that last event (i.e., produce the same tuple structure used for events),
and only then return None; look for the match handling the stream items and the
variables data_buf, lines, and the stream/Box::pin to implement this
flush-before-return behavior.
In `@crates/tanstack-ai/src/stream/processor.rs`:
- Around line 248-253: The result() method currently returns ProcessorResult
with finish_reason: None, losing completion status; add a new field on
StreamProcessor named last_finish_reason: Option<String> (or
Option<FinishReason> if a type exists), update the chunk-processing logic that
handles RunFinished and RunError to set self.last_finish_reason =
Some(reason_string) (derive the string from the chunk or error), and change
result() to return finish_reason: self.last_finish_reason.clone() so callers can
observe why the stream ended; reference StreamProcessor, result(), RunFinished
and RunError when making these changes.
---
Nitpick comments:
In `@crates/tanstack-ai/src/lib.rs`:
- Around line 58-69: The current lib.rs uses broad glob re-exports (adapter::*,
adapters::*, chat::*, client::*, error::*, messages::*, middleware::*,
stream::*, stream_response::*, tools::*, types::*) which can cause name
collisions and slow compilation; replace each `pub use <module>::*;` with
explicit `pub use <module>::{TypeA, fn_b, CONST_C, ...};` exports listing only
the public types/functions/constants intended for the crate API (e.g., pick the
actual structs, enums, traits and functions from adapter, adapters, chat,
client, error, messages, middleware, stream, stream_response, tools, and types)
so collisions are avoided and the public surface is clear. Ensure you update
export lists in lib.rs to reference the precise symbols (function/struct/trait
names) from each module and run cargo check to validate missing or unused
exports.
In `@crates/tanstack-ai/src/types.rs`:
- Line 843: Move the standalone import statement `use crate::error::AiResult`
from near the bottom of types.rs up into the module-level imports grouping at
the top of the file so all `use` declarations are discoverable and consistent;
locate the existing `use` block(s) (e.g., other `use crate::...` lines) and add
`use crate::error::AiResult` there, removing the bottom occurrence to avoid
duplication.
In `@crates/tanstack-ai/tests/test_utils.rs`:
- Around line 187-206: The mock chat_stream currently returns iterations[*idx]
verbatim which can omit lifecycle events; update chat_stream (and/or provide a
helper) to ensure each returned chunk sequence begins with a RunStarted event
and ends with a RunFinished event (or at minimum set
last_finish_reason-equivalent behavior) when iterations[*idx] does not already
contain them: inspect iterations.get(*idx) in chat_stream, wrap the Vec<Chunk>
with synthetic RunStarted and RunFinished chunks as needed before creating the
stream so the agent loop that relies on RunFinished and last_finish_reason will
always see a complete event sequence.
- Around line 261-270: The async closure in server_tool uses a synchronous Mutex
which can block the executor; replace any std::sync::Mutex usage with
tokio::sync::Mutex (wrap in Arc if shared) and change synchronous locking to
await the async lock (e.g., mutex.lock().await) inside the closure; update
imports and ensure Tool::new(...).with_execute closure uses the async lock and
clones the Arc<tokio::sync::Mutex<...>> as needed so no blocking sync mutex is
held in the async path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63af5fc2-490d-443f-9c82-63d9dc382228
⛔ Files ignored due to path filters (2)
crates/tanstack-ai/Cargo.lockis excluded by!**/*.lockexamples/rs-chat/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
crates/tanstack-ai/Cargo.tomlcrates/tanstack-ai/src/adapter.rscrates/tanstack-ai/src/adapters/anthropic.rscrates/tanstack-ai/src/adapters/gemini.rscrates/tanstack-ai/src/adapters/openai.rscrates/tanstack-ai/src/chat.rscrates/tanstack-ai/src/client/connection_adapters.rscrates/tanstack-ai/src/lib.rscrates/tanstack-ai/src/messages.rscrates/tanstack-ai/src/middleware.rscrates/tanstack-ai/src/stream/json_parser.rscrates/tanstack-ai/src/stream/mod.rscrates/tanstack-ai/src/stream/processor.rscrates/tanstack-ai/src/stream/strategies.rscrates/tanstack-ai/src/stream_response.rscrates/tanstack-ai/src/tools/definition.rscrates/tanstack-ai/src/tools/registry.rscrates/tanstack-ai/src/tools/tool_calls.rscrates/tanstack-ai/src/types.rscrates/tanstack-ai/tests/chat_tests.rscrates/tanstack-ai/tests/stream_processor_tests.rscrates/tanstack-ai/tests/test_utils.rscrates/tanstack-ai/tests/tool_call_manager_tests.rscrates/tanstack-ai/tests/unit_tests.rsexamples/rs-chat/src/main.rs
✅ Files skipped from review due to trivial changes (5)
- crates/tanstack-ai/Cargo.toml
- crates/tanstack-ai/tests/tool_call_manager_tests.rs
- crates/tanstack-ai/src/stream/mod.rs
- crates/tanstack-ai/tests/chat_tests.rs
- crates/tanstack-ai/src/middleware.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- crates/tanstack-ai/src/stream/json_parser.rs
- crates/tanstack-ai/src/stream/strategies.rs
- crates/tanstack-ai/src/tools/registry.rs
- crates/tanstack-ai/src/adapter.rs
- crates/tanstack-ai/src/client/connection_adapters.rs
- crates/tanstack-ai/src/tools/definition.rs
- crates/tanstack-ai/src/chat.rs
Comment on lines
+162
to
+188
| MessageContent::Parts(parts) => { | ||
| let mut content = Vec::new(); | ||
| for part in parts { | ||
| match part { | ||
| ContentPart::Text { content: text } => { | ||
| content.push(serde_json::json!({ | ||
| "type": "text", | ||
| "text": text | ||
| })); | ||
| } | ||
| ContentPart::Image { source } => { | ||
| let url = match source { | ||
| ContentPartSource::Url { value, .. } => value.clone(), | ||
| ContentPartSource::Data { | ||
| value, mime_type, .. | ||
| } => { | ||
| format!("data:{};base64,{}", mime_type, value) | ||
| } | ||
| }; | ||
| content.push(serde_json::json!({ | ||
| "type": "image_url", | ||
| "image_url": { "url": url } | ||
| })); | ||
| } | ||
| _ => {} | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the content part types used in the OpenAI adapter
rg -n '"type".*"text"|"type".*"image_url"|input_text|input_image' crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 171
🏁 Script executed:
# Check for API version indicators or documentation in the openai adapter
head -50 crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 1563
🏁 Script executed:
# Search for "Responses API" or "Response API" references in the codebase
rg -i "responses?\s+api" crates/tanstack-ai/src/Repository: TanStack/ai
Length of output: 562
🏁 Script executed:
# Check for any OpenAI documentation links or API version markers
rg -i "openai.*api|api.*version|responses|chat.*completion" crates/tanstack-ai/src/adapters/openai.rs | head -20Repository: TanStack/ai
Length of output: 630
🌐 Web query:
OpenAI Responses API format for message content parts input_text input_image
💡 Result:
In the Responses API, a message’s content can be an array of “content parts”. For the two you asked about:
- Text part (
input_text):{ "type": "input_text", "text": "..." } - Image part (
input_image):{ "type": "input_image", "image_url": "...", "detail": "auto" }image_urlcan be a normal URL or a base64 data URL (e.g.,data:image/png;base64,...).- You can also provide an uploaded image via
file_idinstead ofimage_url. detailis optional:"low" | "high" | "auto"(default"auto"). [1]
Example request (text + image in one user message):
{
"model": "gpt-4.1",
"input": [
{
"role": "user",
"content": [
{ "type": "input_text", "text": "what is in this image?" },
{ "type": "input_image", "image_url": "https://example.com/image.jpg" }
]
}
]
}[1] [2]
Multimodal content uses Chat Completions API format instead of Responses API format.
The adapter explicitly targets the OpenAI Responses API, but the MessageContent::Parts branch serializes content parts with "type": "text" and "type": "image_url" (lines 168, 182). The Responses API requires "type": "input_text" and "type": "input_image" respectively. Update both content part type declarations to match the Responses API specification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 162 - 188, In the
MessageContent::Parts branch update the serialized part type strings to match
the OpenAI Responses API: change the ContentPart::Text serialization from
"type": "text" to "type": "input_text" and change the ContentPart::Image
serialization from "type": "image_url" to "type": "input_image" (keep the rest
of the payload structure, e.g., the image_url object with url). This affects the
code that constructs the JSON for ContentPart::Text and ContentPart::Image in
the adapter (the MessageContent::Parts match arm).
Comment on lines
+113
to
+118
| if let Some(tc) = self.tool_calls.get_mut(tool_call_id) { | ||
| if let Some(final_input) = input { | ||
| tc.function.arguments = serde_json::to_string(final_input).unwrap_or_default(); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Silent failure on JSON serialization error.
If serde_json::to_string(final_input) fails, unwrap_or_default() silently returns an empty string, potentially corrupting the tool call arguments without any indication of the failure.
Suggested fix
if let Some(tc) = self.tool_calls.get_mut(tool_call_id) {
if let Some(final_input) = input {
- tc.function.arguments = serde_json::to_string(final_input).unwrap_or_default();
+ tc.function.arguments = match serde_json::to_string(final_input) {
+ Ok(s) => s,
+ Err(e) => {
+ tracing::warn!(error = %e, "failed to serialize tool call input");
+ "{}".to_string()
+ }
+ };
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if let Some(tc) = self.tool_calls.get_mut(tool_call_id) { | |
| if let Some(final_input) = input { | |
| tc.function.arguments = serde_json::to_string(final_input).unwrap_or_default(); | |
| } | |
| } | |
| } | |
| if let Some(tc) = self.tool_calls.get_mut(tool_call_id) { | |
| if let Some(final_input) = input { | |
| tc.function.arguments = match serde_json::to_string(final_input) { | |
| Ok(s) => s, | |
| Err(e) => { | |
| tracing::warn!(error = %e, "failed to serialize tool call input"); | |
| "{}".to_string() | |
| } | |
| }; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/tools/tool_calls.rs` around lines 113 - 118, The
current code silently replaces tc.function.arguments with an empty string when
serde_json::to_string(final_input) fails; instead, detect the serialization
error and avoid corrupting arguments—replace the unwrap_or_default() usage with
explicit error handling: if to_string(final_input) returns Err, log an error
(including tool_call_id and the serialization error and optionally final_input)
using your project's logger (e.g., tracing::error! or log::error!) and do not
overwrite tc.function.arguments; only assign the serialized string on Ok. This
keeps previous arguments intact and surfaces failures for debugging while still
updating tc.function.arguments on success.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
crates/tanstack-ai/src/adapters/openai.rs (2)
223-283:⚠️ Potential issue | 🟠 MajorEmit a terminal message event on OpenAI's done boundary.
response.output_text.doneis currently mapped to anotherTextMessageContent, andresponse.content_part.done/response.output_item.doneare not modeled here, so the adapter never emitsTextMessageEnd. OpenAI's streaming docs treat those events as the completion boundary for the generated text and output item. (platform.openai.com)Also applies to: 812-819
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 223 - 283, The streaming enum OpenAiStreamEvent currently doesn't model the done boundaries so the adapter never emits a terminal TextMessageEnd; update the enum OpenAiStreamEvent to include variants for the done events (add serde variants for "response.content_part.done" and "response.output_item.done") and change/ensure the existing OutputTextDone (serde rename "response.output_text.done") is treated as a terminal event; then update the adapter dispatch logic that matches on OpenAiStreamEvent to map OutputTextDone, ResponseContentPartDone, and ResponseOutputItemDone (use the exact variant names you add) to emit the terminal TextMessageEnd event for the corresponding item_id/output_index/content_index so the stream completes properly.
166-184:⚠️ Potential issue | 🟠 MajorUse Responses API multimodal part types here.
This branch still emits
type: "text"andtype: "image_url". The Responses API examples useinput_textandinput_imageinsideinput[].content, so multimodal requests built here won't match the expected input format. (platform.openai.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 166 - 184, The code that serializes ContentPart into the Responses API payload currently emits legacy types ("text" and "image_url"); update the ContentPart::Text and ContentPart::Image branches in the adapter (the JSON built from ContentPart::Text { content: text } and ContentPart::Image { source }) to emit the Responses API multimodal part types: use "type": "input_text" for text parts and "type": "input_image" for image parts and nest the image URL appropriately (build the same data URL from ContentPartSource::Url and ::Data as before but place it under the expected image field for "input_image"). Keep using the ContentPartSource matching (ContentPartSource::Url { .. } and ContentPartSource::Data { .. }) to construct the URL, but change the JSON objects to match the Responses API naming ("input_text" / "input_image") and expected field names used inside input[].content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 355-362: The POST to /responses is being replayed on retries;
generate a single idempotency key (e.g., a UUID) per logical request and add it
as an "Idempotency-Key" header on the request you build before calling
send_with_retries so the same key is sent across all retry attempts; apply the
same pattern for the other POST calls referenced (around the 406-413 area) and
ensure the idempotency key is created once and reused for that request so the
server can deduplicate retries instead of replaying side effects.
- Around line 468-470: The code decodes each chunk with
String::from_utf8_lossy(&bytes) before reassembly (see bytes variable and
line_buf push in openai.rs), which can insert replacement characters if a
multibyte UTF-8 sequence is split across chunks; fix by buffering raw bytes
(e.g., a Vec<u8> or BytesMut) and appending each chunk's bytes to that buffer,
only performing UTF-8 decoding on the assembled byte buffer when emitting text
(or when you can safely decode a validated prefix), and apply the same change in
the analogous handlers in anthropic.rs and gemini.rs where chunks are decoded
per-chunk.
---
Duplicate comments:
In `@crates/tanstack-ai/src/adapters/openai.rs`:
- Around line 223-283: The streaming enum OpenAiStreamEvent currently doesn't
model the done boundaries so the adapter never emits a terminal TextMessageEnd;
update the enum OpenAiStreamEvent to include variants for the done events (add
serde variants for "response.content_part.done" and "response.output_item.done")
and change/ensure the existing OutputTextDone (serde rename
"response.output_text.done") is treated as a terminal event; then update the
adapter dispatch logic that matches on OpenAiStreamEvent to map OutputTextDone,
ResponseContentPartDone, and ResponseOutputItemDone (use the exact variant names
you add) to emit the terminal TextMessageEnd event for the corresponding
item_id/output_index/content_index so the stream completes properly.
- Around line 166-184: The code that serializes ContentPart into the Responses
API payload currently emits legacy types ("text" and "image_url"); update the
ContentPart::Text and ContentPart::Image branches in the adapter (the JSON built
from ContentPart::Text { content: text } and ContentPart::Image { source }) to
emit the Responses API multimodal part types: use "type": "input_text" for text
parts and "type": "input_image" for image parts and nest the image URL
appropriately (build the same data URL from ContentPartSource::Url and ::Data as
before but place it under the expected image field for "input_image"). Keep
using the ContentPartSource matching (ContentPartSource::Url { .. } and
ContentPartSource::Data { .. }) to construct the URL, but change the JSON
objects to match the Responses API naming ("input_text" / "input_image") and
expected field names used inside input[].content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7eb1cdb-d872-4e3d-a550-eca126c89be5
📒 Files selected for processing (1)
crates/tanstack-ai/src/adapters/openai.rs
Comment on lines
+355
to
+362
| let response = send_with_retries( | ||
| self.client | ||
| .post(&url) | ||
| .header("Authorization", format!("Bearer {}", self.api_key)) | ||
| .header("Content-Type", "application/json") | ||
| .json(&body), | ||
| self.max_retries, | ||
| ) |
Contributor
There was a problem hiding this comment.
Avoid replaying /responses POSTs on retry.
As wired here, any non-zero max_retries will clone and resend the full request via send_with_retries(). A timeout or connection reset after the server has already accepted the request can replay the prompt and duplicate downstream work or tool side effects.
Also applies to: 406-413
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 355 - 362, The POST
to /responses is being replayed on retries; generate a single idempotency key
(e.g., a UUID) per logical request and add it as an "Idempotency-Key" header on
the request you build before calling send_with_retries so the same key is sent
across all retry attempts; apply the same pattern for the other POST calls
referenced (around the 406-413 area) and ensure the idempotency key is created
once and reused for that request so the server can deduplicate retries instead
of replaying side effects.
Comment on lines
+468
to
+470
| Some(Ok(bytes)) => { | ||
| let text = String::from_utf8_lossy(&bytes); | ||
| line_buf.push_str(&text); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
chunks = [b'data: {"delta":"\xe2', b'\x82\xac"}\n']
print("whole :", b"".join(chunks).decode("utf-8"))
print("chunk-lossy :", "".join(c.decode("utf-8", "replace") for c in chunks))
PYRepository: TanStack/ai
Length of output: 126
🏁 Script executed:
# First, let's look at the file size and examine the problematic lines
wc -l crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 101
🏁 Script executed:
# Read the code around lines 468-470
sed -n '460,480p' crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 992
🏁 Script executed:
# Read the code around lines 600-601
sed -n '590,610p' crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 1092
🏁 Script executed:
# Search for all uses of String::from_utf8_lossy in streaming/chunking context
rg "String::from_utf8_lossy|from_utf8_lossy" crates/tanstack-ai/src/adapters/openai.rs -B 3 -A 3Repository: TanStack/ai
Length of output: 382
🏁 Script executed:
# Check lines 600-601 for similar pattern
sed -n '595,605p' crates/tanstack-ai/src/adapters/openai.rsRepository: TanStack/ai
Length of output: 518
🏁 Script executed:
# Search for all occurrences of from_utf8_lossy in the entire openai.rs file
rg "from_utf8_lossy" crates/tanstack-ai/src/adapters/openai.rs -nRepository: TanStack/ai
Length of output: 130
🏁 Script executed:
# Check what's happening around the broader context of the stream handler
# to understand the "Also applies to: 600-601" reference
sed -n '450,620p' crates/tanstack-ai/src/adapters/openai.rs | cat -nRepository: TanStack/ai
Length of output: 11358
🏁 Script executed:
# Let's search the entire file for all similar patterns
# Look for any string concatenation with lossy decoding or related patterns
rg "String::from_utf8_lossy|from_utf8" crates/tanstack-ai/src/adapters/openai.rs -nRepository: TanStack/ai
Length of output: 130
🏁 Script executed:
# Let's also check if there are multiple stream handlers in the file
# that might have similar issues
rg "bytes_stream\|from_utf8" crates/tanstack-ai/src/adapters/openai.rs -B 2 -A 2Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
# Count total lines to understand what 600-601 reference might be about
tail -50 crates/tanstack-ai/src/adapters/openai.rs | cat -nRepository: TanStack/ai
Length of output: 1992
🏁 Script executed:
# Check if there are other adapter files with similar streaming patterns
fd "adapter" crates/tanstack-ai/src/ --type fRepository: TanStack/ai
Length of output: 141
🏁 Script executed:
# Search across all adapter files for similar from_utf8_lossy patterns
rg "from_utf8_lossy" crates/tanstack-ai/src/adapters/ -nRepository: TanStack/ai
Length of output: 411
🏁 Script executed:
# Let me check if there's another stream handler or function around line 600
# by looking at the actual function definitions
rg "fn " crates/tanstack-ai/src/adapters/openai.rs -n | head -20Repository: TanStack/ai
Length of output: 913
🏁 Script executed:
# Let me check the context around line 442 (parse_openai_sse_stream function)
# to see if there are multiple places within that function where this happens
sed -n '442,700p' crates/tanstack-ai/src/adapters/openai.rs | grep -n "from_utf8_lossy\|line_buf\|String::from"Repository: TanStack/ai
Length of output: 745
🏁 Script executed:
# Let's look at the function signature and structure of parse_openai_sse_stream more carefully
sed -n '442,760p' crates/tanstack-ai/src/adapters/openai.rs | head -100 | tail -50Repository: TanStack/ai
Length of output: 3588
🏁 Script executed:
# Let's verify the anthropic.rs location to see if it has the same issue
sed -n '410,430p' crates/tanstack-ai/src/adapters/anthropic.rsRepository: TanStack/ai
Length of output: 1015
🏁 Script executed:
# And check gemini.rs
sed -n '364,384p' crates/tanstack-ai/src/adapters/gemini.rsRepository: TanStack/ai
Length of output: 962
🏁 Script executed:
# Let me verify if there are any other stream patterns I might have missed
rg "line_buf.push_str" crates/tanstack-ai/src/adapters/ -nRepository: TanStack/ai
Length of output: 357
Buffer raw bytes before UTF-8 decoding.
bytes_stream() can split a multibyte code point across chunks. Decoding each chunk with String::from_utf8_lossy() before reassembly inserts replacement characters permanently, which can corrupt streamed text and JSON tool arguments.
This same issue exists in anthropic.rs (420-421) and gemini.rs (374-375).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tanstack-ai/src/adapters/openai.rs` around lines 468 - 470, The code
decodes each chunk with String::from_utf8_lossy(&bytes) before reassembly (see
bytes variable and line_buf push in openai.rs), which can insert replacement
characters if a multibyte UTF-8 sequence is split across chunks; fix by
buffering raw bytes (e.g., a Vec<u8> or BytesMut) and appending each chunk's
bytes to that buffer, only performing UTF-8 decoding on the assembled byte
buffer when emitting text (or when you can safely decode a validated prefix),
and apply the same change in the analogous handlers in anthropic.rs and
gemini.rs where chunks are decoded per-chunk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/rust/tanstack-aicrate that ports core TanStack AI chat, streaming, tools, middleware, and provider adapters (OpenAI/Anthropic/Gemini) to Rust.examples/rs-chatas a runnable OpenAI smoke-test app, and ignore Rust build artifacts via.gitignore.Validation
cargo testinpackages/rust/tanstack-aicargo checkinexamples/rs-chatSummary by CodeRabbit