Skip to content

fix(openai): stop counting a relocated tool output as a duplicate - #1988

Open
adminwat wants to merge 1 commit into
1jehuang:masterfrom
adminwat:fix/openai-dropped-duplicate-count
Open

adminwat wants to merge 1 commit into
1jehuang:masterfrom
adminwat:fix/openai-dropped-duplicate-count

Conversation

@adminwat

@adminwat adminwat commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

build_responses_input's re-ordering pass emits each function_call_output next to its call from output_map, then meets the original item later in the stream and counts it as a duplicate of itself. Every ordinary request therefore logs [openai] Dropped N duplicate tool output(s) during re-ordering with N = number of tool rounds.

Measured

One gpt-6-astra session, 2026-10-26, ~/.jcode/logs: 1617 of 1740 requests logged exactly N = assistant tool-round count from the preceding messages_for_provider line, climbing 1,2,3,… and resetting after every native compaction. The line never pointed at a real duplicate.

Change

Track which call ids had their output relocated; the first later arrival for such an id is the moved original and is not counted. A second arrival still is. Output items and their order are unchanged.

Tests

  • build_responses_input_does_not_report_relocated_outputs_as_duplicates: 3 rounds, one output per call directly after its call, no duplicate log line. Reverting to the old counting turns it RED (verified).
  • build_responses_input_filters_a_real_duplicate_result_in_the_first_pass: the same ToolResult persisted twice is dropped by the first pass and reported as a filtered orphan, so the two log lines stay distinguishable.

cargo test -p jcode-provider-openai --lib: 57 passed. clippy -D warnings and fmt clean.

Closes #1990

The re-ordering pass in build_responses_input emits each function_call_output
next to its call from output_map, then meets the original item later in the
stream and counted it as a duplicate of itself. So every ordinary request
logged "[openai] Dropped N duplicate tool output(s) during re-ordering" with
N equal to the number of tool rounds in the request.

Measured 2026-10-26 on one gpt-6-astra session (~/.jcode/logs): 1617 of 1740
requests logged exactly N = assistant tool-round count from
messages_for_provider, climbing 1,2,3,... and resetting after every native
compaction. The line never pointed at a real duplicate, and a real one could
not be told apart from the noise.

Only a second arrival for a call id whose relocated original has already
passed is counted now. Output items and their order are unchanged.

Tests: new does_not_report_relocated_outputs_as_duplicates (3 rounds, one
output per call, each directly after its call, no duplicate line) goes RED
on the old counting; new filters_a_real_duplicate_result_in_the_first_pass
pins that the same ToolResult persisted twice is already handled by the
first pass as a filtered orphan, so the two log lines stay distinguishable.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T20:00:15.924394Z 82c349a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82c349adf6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if let Some(call_id) = call_id {
if let Some(output_item) = output_map.get(&call_id) {
ordered.push(output_item.clone());
relocated_outputs.insert(call_id.clone());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not re-arm relocation exemptions for replayed calls

When a history contains the same ToolUse ID twice without a result, the first pass emits call x, synthetic output x, call x, synthetic output x, precisely the replay shape this change says the reordering counter should expose. Processing the first original output removes x, but this unconditional insert re-arms the exemption for the second call, so the second synthetic output is also treated as merely relocated and dropped_duplicate_outputs stays zero. Only grant the relocation exemption the first time an ID is used so replayed outputs still produce the diagnostic.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Low impact] The change stays within OpenAI request building and duplicate logging.

Safe to merge with one non-blocking logging concern; the reproduced replayed-call case does not change the returned request contents.

Findings

  1. P2 Replayed calls hide duplicate warnings ▶
Fix with agent prompt
### Issue 1
crates/jcode-provider-openai/src/request.rs:601
A repeated `ToolUse` resets the exemption for the same `call_id`. With a call, its result, then a replay of that call without another result, the first pass adds a synthetic result. The replay re-adds the ID to `relocated_outputs`, so that distinct synthetic result is dropped without increasing the duplicate count. No duplicate-output diagnostic is emitted for this sequence.

This is a non-blocking logging issue: request contents remain unchanged, but people investigating replayed calls miss the dropped result in the duplicate diagnostic. Allow only one relocation exemption per ID, and add a regression test for this sequence that expects one duplicate to be reported.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Stops ordinary tool results from being counted as duplicates when they are moved next to their calls.

  • OpenAI requests now count only extra tool outputs as duplicates.

Reviews (1) · Last reviewed commit: "fix(openai): stop counting a relocated t..." · Reviewed by Greptile

if let Some(call_id) = call_id {
if let Some(output_item) = output_map.get(&call_id) {
ordered.push(output_item.clone());
relocated_outputs.insert(call_id.clone());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Replayed calls hide duplicate warnings

A repeated ToolUse resets the exemption for the same call_id. With a call, its result, then a replay of that call without another result, the first pass adds a synthetic result. The replay re-adds the ID to relocated_outputs, so that distinct synthetic result is dropped without increasing the duplicate count. No duplicate-output diagnostic is emitted for this sequence.

This is a non-blocking logging issue: request contents remain unchanged, but people investigating replayed calls miss the dropped result in the duplicate diagnostic. Allow only one relocation exemption per ID, and add a regression test for this sequence that expects one duplicate to be reported.

Artifacts

Executed Rust test for a replayed tool call

  • The focused test invokes the real request builders, captures logger levels and messages, and asserts exact returned items, separating payload preservation from diagnostic suppression.

Executed runner for base and head comparison

  • The runner installs the identical test against each exact revision, executes Cargo, captures source and output, and restores production source, making the comparison reproducible.

Base behavior for a replayed tool call

  • The executed base test captures the exact four returned items and both Info messages, showing that the real output survives and a duplicate-drop diagnostic is emitted.

Head behavior for a replayed tool call

  • The executed head test captures the same four returned items but only the synthetic-injection Info message, confirming diagnostic suppression without a payload change.

Production source restoration and artifact checksums

  • The final Git check shows no production-source or manifest differences from the requested head and records evidence-file checksums, confirming temporary source edits were removed.

View artifacts

TREX Ran code and verified through TREX

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-openai/src/request.rs
Line: 601

Comment:
**Replayed calls hide duplicate warnings**

A repeated `ToolUse` resets the exemption for the same `call_id`. With a call, its result, then a replay of that call without another result, the first pass adds a synthetic result. The replay re-adds the ID to `relocated_outputs`, so that distinct synthetic result is dropped without increasing the duplicate count. No duplicate-output diagnostic is emitted for this sequence.

This is a non-blocking logging issue: request contents remain unchanged, but people investigating replayed calls miss the dropped result in the duplicate diagnostic. Allow only one relocation exemption per ID, and add a regression test for this sequence that expects one duplicate to be reported.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAI: Dropped N duplicate tool output(s) logs N = tool-round count on every request

1 participant