fix(openai): add tool_call_id request param by doronkopit5 · Pull Request #73 · traceloop/hub

doronkopit5 · 2025-11-26T09:37:41Z

Fixes TLP-1206

https://platform.openai.com/docs/guides/function-calling#handling-function-calls

Important

Add tool_call_id to ChatCompletionMessage and initialize it in various models, with new tests for OpenAI provider.

Models:
- Add tool_call_id field to ChatCompletionMessage in content.rs.
Initialization:
- Initialize tool_call_id to None in OtelTracer::log_chunk() in otel.rs.
- Initialize tool_call_id to None in From<Vec<ContentBlock>> for ChatCompletionMessage in anthropic/models.rs.
- Initialize tool_call_id to None in From<TitanChatCompletionResponse> for ChatCompletion in bedrock/models.rs.
Tests:
- Add test.rs for OpenAI provider to test chat completions with tool calls, ensuring tool_call_id is correctly handled.

^{This description was created by}^{for 257119e. You can customize this summary. It will automatically update as commits are pushed.}

Summary by CodeRabbit

New Features
- Added support for tracking tool call identifiers in chat completions, enabling improved linkage between tool calls and their corresponding responses across all supported providers.
Tests
- Added comprehensive test coverage for chat completions with tool calls, including integration tests and test fixtures to validate tool call handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-26T09:37:50Z

Walkthrough

A new optional tool_call_id field is added to the ChatCompletionMessage struct in the core models. This field is initialized as None across all provider implementations (OpenAI, Anthropic, Bedrock, VertexAI) and the observability pipeline. A new comprehensive test module for the OpenAI provider is introduced, including cassette-based testing for tool call interactions.

Changes

Cohort / File(s)	Summary
Core Model Update `src/models/content.rs`	Added optional `tool_call_id: Option<String>` field to `ChatCompletionMessage` struct with `#[serde(skip_serializing_if = "Option::is_none")]` attribute
Pipeline Integration `src/pipelines/otel.rs`	Updated `OtelTracer::log_chunk` to initialize `tool_call_id: None` when constructing new `ChatCompletionMessage` instances
Provider Model Updates `src/providers/anthropic/models.rs`, `src/providers/bedrock/models.rs`, `src/providers/vertexai/models.rs`	Added `tool_call_id: None` initialization in `ChatCompletionMessage` construction across all provider-specific model conversions
Provider Tests `src/providers/bedrock/test.rs`, `src/providers/vertexai/tests.rs`	Updated test payloads to include `tool_call_id: None` in `ChatCompletionMessage` initializations
OpenAI Provider Test Module `src/providers/openai/mod.rs`, `src/providers/openai/test.rs`	Added new test module with cassette recording support, including `test_chat_completions_with_tool_calls` test that validates tool call interactions with proper ID linkage
Test Cassette Data `tests/cassettes/openai/chat_completions_with_tool_calls.json`	New fixture file containing recorded OpenAI API interactions demonstrating tool call requests and responses with weather function example

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Specific areas requiring attention:
- Verify the #[serde(skip_serializing_if = "Option::is_none")] attribute correctly suppresses serialization when tool_call_id is None across all use cases
- Review src/providers/openai/test.rs cassette recording logic to ensure proper recording/validation mode handling and cassette file management
- Confirm all provider implementations consistently initialize tool_call_id: None (no missed locations)

Suggested reviewers

nirga

Poem

🐰 A carrot field of tool IDs so fine,
Each message now remembers which one to align,
From Bedrock to Vertex, the changes cascade,
With cassettes recording, no feature unplayed,
None for now, but the field's here to stay! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(openai): add tool_call_id request param' accurately describes the main change: adding a tool_call_id field across the codebase to support tool call identification in requests.
Linked Issues check	✅ Passed	The changes add tool_call_id field support across all provider implementations (OpenAI, Anthropic, Bedrock, Vertex AI) and include comprehensive test coverage with cassette recordings validating tool call ID linkage.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing tool_call_id support: the field is added to ChatCompletionMessage struct, initialized in all provider model converters, and validated in tests.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch dk/support_tool_call_id

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ellipsis-dev

Important

Looks good to me! 👍

Reviewed everything up to 257119e in 1 minute and 51 seconds. Click for details.

Reviewed 721 lines of code in 10 files
Skipped 0 files when reviewing.
Skipped posting 4 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/models/content.rs:29

Draft comment:
For consistency with other optional fields, add #[serde(skip_serializing_if = "Option::is_none")] to the new tool_call_id field.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. src/providers/openai/test.rs:287

Draft comment:
Good use of tool_call_id in the second API call. Consider adding an extra test case to explicitly simulate a scenario where tool_call_id is provided (non-None) to further validate its propagation.
Reason this comment was not posted:
Comment looked like it was already resolved.

3. tests/cassettes/openai/chat_completions_with_tool_calls.json:1

Draft comment:
Cassette file looks correctly formatted for testing tool_calls including tool_call_id. Ensure that any future changes to tool_call_id behavior are reflected in updated cassette files.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is purely informative and suggests ensuring future changes are reflected, which violates the rules. It doesn't provide a specific code suggestion or ask for a specific test to be written.

4. General:0

Draft comment:
Across different provider modules (Anthropic, Titan, VertexAI), tool_call_id is consistently set to None. Consider reviewing whether tool_call_id should sometimes carry a non-null value from incoming requests to better support tool execution tracking.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_uBkoViLX6OACEwNK

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/pipelines/otel.rs (1)

137-155: Tracing now initializes tool_call_id explicitly on streamed choices

Including tool_call_id: None in the constructed ChatCompletionMessage keeps the Otel tracing path consistent with the updated model without changing current behavior; once streaming tool messages start carrying a tool_call_id, this is the right place to propagate it.

src/providers/openai/test.rs (1)

15-38: Cassette append behavior vs. len() == 2 assertion is brittle

save_to_cassette always appends to any existing file, but the validation branch asserts interactions.len() == 2. If you ever re-run the test in RECORD_MODE without deleting the cassette first, you’ll accumulate more than two interactions and the validation will start failing even though the latest pair is fine.

Consider either:

overwriting the cassette in RECORD_MODE (e.g., don’t load/append existing interactions), or

relaxing the validation to >= 2 and only asserting against the first two (or last two) entries.

This would make re-recording less error-prone.

Also applies to: 115-120, 218-255, 316-338

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3cc62a4 and 257119e.

📒 Files selected for processing (10)

src/models/content.rs (1 hunks)
src/pipelines/otel.rs (1 hunks)
src/providers/anthropic/models.rs (1 hunks)
src/providers/bedrock/models.rs (2 hunks)
src/providers/bedrock/test.rs (10 hunks)
src/providers/openai/mod.rs (1 hunks)
src/providers/openai/test.rs (1 hunks)
src/providers/vertexai/models.rs (1 hunks)
src/providers/vertexai/tests.rs (14 hunks)
tests/cassettes/openai/chat_completions_with_tool_calls.json (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/providers/openai/test.rs (1)

src/providers/vertexai/tests.rs (5)

serde_json (62-62)

serde_json (113-113)

serde_json (586-586)

save_to_cassette (105-132)

create_test_provider (134-161)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: Analyze (rust)

🔇 Additional comments (10)

src/providers/openai/mod.rs (1)

3-4: LGTM!

Standard Rust pattern for conditionally including a test module. The #[cfg(test)] attribute correctly ensures the test module is only compiled during test runs.

src/providers/vertexai/models.rs (1)

536-556: LGTM!

The tool_call_id: None initialization is correctly added to the ChatCompletionMessage construction. This field is appropriately None for assistant responses since tool_call_id is only relevant for tool result messages sent back to the model.

src/providers/bedrock/models.rs (2)

100-116: LGTM!

The tool_call_id: None initialization is correctly added for Titan response conversion. This is appropriate since tool_call_id applies to tool result messages, not assistant responses.

307-314: LGTM!

Consistent with the Titan conversion above, tool_call_id: None is correctly initialized for Ai21 responses.

src/providers/anthropic/models.rs (1)

195-206: LGTM!

The tool_call_id: None initialization is correctly added to the Anthropic content block conversion, maintaining consistency with other provider implementations.

src/models/content.rs (1)

29-30: LGTM!

The new tool_call_id field is correctly defined with Option<String> and the appropriate skip_serializing_if serde attribute. This matches the OpenAI API specification where tool_call_id is required for tool result messages to link responses back to the original tool calls.

tests/cassettes/openai/chat_completions_with_tool_calls.json (1)

1-75: Cassette structure matches expected ChatCompletion/tool-calls shape

The two recorded interactions (assistant with tool_calls, followed by assistant with content) align with the new OpenAI test expectations and the ChatCompletion schema; nothing problematic here.

src/providers/vertexai/tests.rs (1)

257-287: VertexAI tests correctly updated for new tool_call_id field

All ChatCompletionMessage literals now set tool_call_id: None, which is appropriate for these non-tool messages and keeps the tests aligned with the updated struct without changing behavior.

Also applies to: 486-497, 648-678, 778-787, 814-823, 871-880, 955-964, 1047-1056, 1150-1167, 1201-1219, 1431-1439, 1491-1499, 1535-1546, 1670-1681

src/providers/openai/test.rs (1)

15-91: Tool-call test wiring and tool_call_id usage look correct

The helpers and the two-call flow (first call producing tool_calls, second call posting a role: "tool" message with tool_call_id equal to tool_calls[0].id) match the OpenAI Chat Completions contract and exercise the new tool_call_id field properly. The split between RECORD vs. validation mode and the weather tool schema also look sound.

Also applies to: 185-309

src/providers/bedrock/test.rs (1)

98-109: Bedrock tests correctly aligned with new tool_call_id field

The added tool_call_id: None initializations on Bedrock ChatCompletionMessage payloads keep the tests in sync with the shared message model and are appropriate for these non-tool messages; no behavioral changes introduced.

Also applies to: 214-225, 341-352, 417-428, 469-481, 521-532, 573-581, 624-632, 755-763, 821-830

fix(openai): add tool_call_id request param

257119e

ellipsis-dev bot reviewed Nov 26, 2025

View reviewed changes

coderabbitai bot reviewed Nov 26, 2025

View reviewed changes

galkleinman approved these changes Nov 26, 2025

View reviewed changes

doronkopit5 merged commit e7094bc into main Nov 26, 2025
6 checks passed

doronkopit5 deleted the dk/support_tool_call_id branch November 26, 2025 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(openai): add tool_call_id request param#73

fix(openai): add tool_call_id request param#73
doronkopit5 merged 1 commit intomainfrom
dk/support_tool_call_id

doronkopit5 commented Nov 26, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 26, 2025 •

edited

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

doronkopit5 commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

doronkopit5 commented Nov 26, 2025 •

edited

Loading

coderabbitai bot commented Nov 26, 2025 •

edited

Loading