Echo reasoning_content exactly as the provider sent it #817

Merged

coilysiren merged 1 commit from aos/claude/xh55-reasoning-roundtrip into main

2026-08-15 16:44:56 +00:00

coilyco-ops commented

2026-08-15 16:41:02 +00:00

Member

Closes #717. ward exec gate PASS on all six steps.

The blocking question is no longer blocking

#717 was parked on one live request: does DeepSeek accept "reasoning_content": ""? Two agents named that gap and stopped, and it has sat for two days.

It does not need answering first, because the change cannot be worse than what it replaces. The failing case currently sends no key, and no key is precisely the shape that earns the 400. Sending the empty string the model actually returned either fixes those turns or reproduces the error they already get. There is no third outcome. The next eval-deep run answers it as a side effect, and the answer belongs here.

The proposed fix had a trap in it

#717 said: drop omitempty and set the field unconditionally at both build sites, "mechanical, no judgement."

It is neither. chatMessage is one struct for every role:

type chatMessage struct {
    Role             string
    Content          any
    ReasoningContent string `json:"reasoning_content,omitempty"`
    ...
}

Dropping omitempty stamps "reasoning_content": "" onto system, user, and tool messages, on every request, on every route. #678 established the defect lives on one evaluation lane and that sirens-echo/deepseek and sirens-echo/default had zero instances in 7 days. Rewriting every request on the healthy lanes to repair a broken one is a blast radius, not a repair.

What it does instead

Both fields become *string, so presence survives the round trip:

Provider named the field, even as "" — echoed, so thinking mode gets what it demands.
Provider never named it — nothing echoed, so a non-thinking model sees a byte-identical request to before.

An explicit null reads as absent, which is what the old encoding did with it too, so no lane changes shape on that account.

This is @Quail's own diagnosis taken at its word: "Under omitempty an empty string and an absent key are the same bytes." The fix is to stop erasing the difference, not to pick one side of it. Same shape as docs/sirens-echo-indistinguishable-values.md, living in an encoding rather than a log line.

Tests

The characterization test is inverted rather than deleted, as it asked, and renamed to match what it now asserts. Two joined it:

TestAnUnnamedReasoningFieldIsNotInvented — the other half of the round trip, and the guard on the healthy lanes.
TestOnlyAssistantMessagesCarryReasoning — the trap above, pinned so a future omitempty drop fails here.

Checked by mutation, not by passing. Reinstating the omitempty erasure fails exactly TestAnEmptyReasoningContentStillSendsTheKey and no other test in the package.

What I did not establish

Whether the provider accepts the empty string. If it is refused, what remains is putting something in a field the model did not fill, which is a decision rather than a repair — and that part stays exactly where #717 left it.

Closes #717. `ward exec gate` PASS on all six steps. ## The blocking question is no longer blocking #717 was parked on one live request: does DeepSeek accept `"reasoning_content": ""`? Two agents named that gap and stopped, and it has sat for two days. It does not need answering first, because **the change cannot be worse than what it replaces.** The failing case currently sends *no key*, and no key is precisely the shape that earns the 400. Sending the empty string the model actually returned either fixes those turns or reproduces the error they already get. There is no third outcome. The next `eval-deep` run answers it as a side effect, and the answer belongs here. ## The proposed fix had a trap in it #717 said: drop `omitempty` and set the field unconditionally at both build sites, "mechanical, no judgement." It is neither. `chatMessage` is **one struct for every role**: ```go type chatMessage struct { Role string Content any ReasoningContent string `json:"reasoning_content,omitempty"` ... } ``` Dropping `omitempty` stamps `"reasoning_content": ""` onto **system, user, and tool** messages, on every request, on every route. #678 established the defect lives on one evaluation lane and that `sirens-echo/deepseek` and `sirens-echo/default` had zero instances in 7 days. Rewriting every request on the healthy lanes to repair a broken one is a blast radius, not a repair. ## What it does instead Both fields become `*string`, so **presence survives the round trip**: * Provider named the field, even as `""` — echoed, so thinking mode gets what it demands. * Provider never named it — nothing echoed, so a non-thinking model sees a byte-identical request to before. An explicit `null` reads as absent, which is what the old encoding did with it too, so no lane changes shape on that account. This is @Quail's own diagnosis taken at its word: *"Under omitempty an empty string and an absent key are the same bytes."* The fix is to stop erasing the difference, not to pick one side of it. Same shape as `docs/sirens-echo-indistinguishable-values.md`, living in an encoding rather than a log line. ## Tests The characterization test is **inverted rather than deleted**, as it asked, and renamed to match what it now asserts. Two joined it: * `TestAnUnnamedReasoningFieldIsNotInvented` — the other half of the round trip, and the guard on the healthy lanes. * `TestOnlyAssistantMessagesCarryReasoning` — the trap above, pinned so a future `omitempty` drop fails here. **Checked by mutation, not by passing.** Reinstating the omitempty erasure fails exactly `TestAnEmptyReasoningContentStillSendsTheKey` and no other test in the package. ## What I did not establish Whether the provider accepts the empty string. If it is refused, what remains is putting something in a field the model did not fill, which is a decision rather than a repair — and that part stays exactly where #717 left it.

coilyco-ops added 1 commit

2026-08-15 16:41:02 +00:00

fix(proxy): echo reasoning_content exactly as the provider sent it

ci / image-build (pull_request) Successful in 30s

Details

ci / test (pull_request) Successful in 49s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

86e2a8ade9

DeepSeek in thinking mode rejects an assistant message with no
reasoning_content: "The `reasoning_content` in the thinking mode must be passed
back to the API." Both assistant build sites copied the field, so the harness
looked faithful. The encoding was not.

    ReasoningContent string `json:"reasoning_content,omitempty"`

Under omitempty a model that returned an empty reasoning string and one that
never mentioned reasoning produce the same bytes: no key. The harness sent the
second shape for both and the provider refused the array. #678 measured 24
litellm 400s in 24h on that signature.

NOT A BARE DROP OF OMITEMPTY

That was the fix #717 proposed, calling it mechanical and judgement-free. It is
neither. chatMessage is one struct for every role, so dropping omitempty stamps
"reasoning_content": "" onto system, user, and tool messages, on every request,
on every route. The defect lives on one evaluation lane; sirens-echo/deepseek
and sirens-echo/default had no instance of it in 7 days. Rewriting every
request on the healthy lanes to repair a broken one is a blast radius, not a
repair.

Both fields are *string instead, so presence survives the round trip. A
provider that named the field, even as "", gets it echoed. A provider that
never named it gets a byte-identical request to before. An explicit null reads
as absent, which is what the old encoding did with it too.

This is the docs/sirens-echo-indistinguishable-values.md shape, living in an
encoding rather than a log line.

ON THE QUESTION THIS WAS BLOCKED ON

Whether the provider accepts "reasoning_content": "" is still unestablished,
and it no longer blocks. The change cannot be worse than what it replaces: the
failing case now sends the field where it sent nothing, and nothing is already
the shape that earns the 400. It either fixes those turns or reproduces the
error they already get. The next evaluation run answers it, and the answer
belongs on #717.

If the empty string is refused, what remains is putting something in a field
the model did not fill, which is a decision rather than a repair.

The characterization test is inverted rather than deleted, as it asked, and
joined by one asserting the other half of the round trip and one asserting no
non-assistant role ever carries the key. Checked by mutation: reinstating the
omitempty erasure fails exactly the empty-string test and no other.

Closes #717

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Angie <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>