Skip to content

fix: coerce null chat completion content to empty string at the normalization boundary - #1169

Merged
sakibsadmanshajib merged 1 commit into
mainfrom
fix/chat-completions-model-shape
Aug 25, 2026
Merged

sakibsadmanshajib merged 1 commit into
mainfrom
fix/chat-completions-model-shape

Conversation

@sakibsadmanshajib

Copy link
Copy Markdown
Owner

Summary

The deploy of merge commit 2df3afd (PR #1163) failed its SDK replay
against the demo box (run 32879931588):

tests/chat-completions/chat-completions.test.ts:36
"Chat Completions > returns a valid chat completion via SDK"
AssertionError: expected 'object' to be 'string'
Expected: "string"
Received: "object"

Correction to the field named in the incident report. The failing
assertion is typeof response.choices[0].message.content, not
response.model as first reported. Line 36 of the test is:

expect(typeof response.choices[0].message.content).toBe("string");

response.model is asserted separately at line 51 in the very next test
in the same file (model field shows Hive alias not provider handle),
which passed. typeof null === "object" in JavaScript, so "Received:
object" for a content assertion means the wire value was a bare null,
not an actual object.

Root cause

Not PR #1163. That PR touches only apps/edge-api/internal/anthropic/,
adding request-side fields (Thinking, TopK, User,
ParallelToolCalls, ThinkingBlocks, ThinkingConfig) to that package's
own OAIRequest/OAIMessage structs. Those structs are private to the
anthropic package: grep -rn "anthropic\." apps/edge-api/internal/inference/*.go
returns no hits outside a comment. The OpenAI-compatible chat-completions
path (apps/edge-api/internal/inference) has its own separate
ChatCompletionResponse/ChatCompletionMessage types and never touches
the anthropic package's types. No shared struct, no cross-package
marshaling effect.

The real cause is the free-pool router, PR #1115 (hive-free, four
load-balanced provider members: OpenRouter dots-3-note-preview:free,
Google gemini-flash-latest, and two Groq gpt-oss-20b keys) plus its
retry fix PR #1155 (dispatchWithRetry, which lets a request land on any
member on retry). The replay's HIVE_TEST_MODEL is hive-free
(deploy-demo-box.yml), so this plain "Say hello" / max_tokens: 256
request could land on any of the four. The failing test ran for 50.8s
versus 1-7s for its four sibling tests in the same run, consistent with a
reasoning-capable member (Groq's gpt-oss-20b or Gemini's thinking-capable
flash model) spending its entire token budget on hidden reasoning before
hitting the length limit, returning finish_reason: "length" with
message.content omitted from the JSON entirely.

Go's ChatCompletionMessage.Content is *string. Unmarshaling a response
missing that key leaves it nil; normalizeChatCompletion in
apps/edge-api/internal/inference/chat_completions.go re-marshaled that
nil pointer straight back out as JSON null, which every OpenAI SDK
(this test uses the real openai npm package) treats as an unconditional
string.

This exact class of failure was already documented in
deploy-demo-box.yml's own comments for a different alias
(deepseek-v4-flash: "returned message.content as object/null/string
across probes"), which is why HIVE_TOOLS_MODEL was pinned away from it.
hive-free was never given the same treatment because its tool-free
smoke test uses no max_tokens pressure test and had not yet hit a
reasoning-heavy member in CI.

This gateway is provider-blind by design. No pool member's response
shape should leak to a client through the OpenAI-compatible surface.
OpenAI's own contract makes content nullable only alongside
tool_calls/function_call; every other case must be a string. That is
where this fix lives.

Fix

normalizeChatCompletion (apps/edge-api/internal/inference/chat_completions.go)
now coerces a nil, tool-free message.content to an empty string
before marshaling the response, via a new coerceNullContent helper. A
genuine tool-call message with content: null is left untouched, since
that shape is spec-correct per OpenAI and is exercised by the existing
"passes tools through" replay test.

Why no existing test caught this pre-merge

packages/sdk-tests only runs against a live deployed box (sdk-replay
job, post-merge), never in CI unit/integration tests, because it needs a
real LiteLLM + provider round trip. The Go unit-test suite for
normalizeChatCompletion (usage_clamp_test.go) had fixtures for
zero-completion-token clamping but none exercising an upstream response
with content entirely absent from the JSON. Added two new unit tests in
chat_completions_null_content_test.go that assert on the actual
marshaled wire bytes (not just the Go struct), which would have caught
this before any live deploy:

  • TestNormalizeChatCompletion_NullContentCoercedToEmptyString:
    reproduces the missing-content upstream shape and asserts the
    outgoing JSON has "content":"", never null.
  • TestNormalizeChatCompletion_NullContentPreservedWithToolCalls: asserts
    a genuine tool-call message's content: null is left alone.

Buglog entry

To be appended to .wolf/buglog.jsonl on main via a separate
buglog-only PR, per .claude/rules/openwolf.md:

{"date":"2026-08-25","error_message":"AssertionError: expected 'object' to be 'string' at tests/chat-completions/chat-completions.test.ts:36 (response.choices[0].message.content)","root_cause":"hive-free free-pool member (PR #1115/#1155) returned message.content omitted (null) after burning its max_tokens budget on hidden reasoning; normalizeChatCompletion re-marshaled the nil *string as JSON null instead of coercing it, leaking a non-OpenAI-contract shape to every SDK client","fix":"apps/edge-api/internal/inference/chat_completions.go: normalizeChatCompletion coerces nil, tool-free message.content to an empty string; tool-call messages with null content are left untouched per the OpenAI contract","tags":["free-pool","chat-completions","normalization","sdk-replay","hive-free"]}

Test plan

  • go build ./apps/edge-api/...
  • go vet ./apps/edge-api/...
  • New unit tests pass:
    go test ./apps/edge-api/internal/inference/... -run 'NullContent|NormalizeChatCompletion' -v
  • Full apps/edge-api suite green, no regressions:
    go test ./apps/edge-api/... -count=1 -short
  • Next deploy's sdk-replay job green against the live demo box
    (this PR cannot verify that itself; the orchestrator confirms on
    merge)

Reproduction: static, from the actual failing-run log
(gh run view --job 97907530300 --log) plus static code reading. Live
reproduction against the demo box was not attempted directly (no API key
available to this agent; another agent was separately reported to be
investigating an unrelated Cloudflare Tunnel issue on the same box). The
box itself answered a plain reachability probe (401 on /v1/models
with no auth) during this session, so it was not down at the time of
investigation.

…lization boundary

The SDK replay against the demo box (deploy run 32879931588) failed
tests/chat-completions/chat-completions.test.ts:36 with
"expected 'object' to be 'string'" on response.choices[0].message.content
for a plain, tool-free "Say hello" request against the hive-free alias.
JavaScript's typeof null is "object", so the wire value was a bare null.

Root cause is not PR #1163 (the Anthropic /v1/messages translator): that
package has no shared type and no caller relationship with the OpenAI
chat-completions path (apps/edge-api/internal/inference), confirmed by
grep. The actual cause is the free-pool router (PR #1115) plus its retry
fix (PR #1155): hive-free load-balances across four heterogeneous
providers, at least one of which (a reasoning-capable Groq/Gemini member)
can burn its whole max_tokens budget on hidden reasoning and return
finish_reason=length with content omitted, which Go's *string field
unmarshals as nil and then re-marshals as JSON null. This gateway is
provider-blind by design, so no pool member's response shape should leak
to the client. OpenAI's own contract makes content nullable only
alongside tool_calls/function_call; every other case must be a string.

normalizeChatCompletion now coerces a nil, tool-free message.content to
an empty string before marshaling the response. A genuine tool-call
message with null content is left untouched, since that shape is
spec-correct.

Buglog entry (to be appended to main via a separate buglog-only PR, per
.claude/rules/openwolf.md):
{"date":"2026-08-25","error_message":"AssertionError: expected 'object' to be 'string' at tests/chat-completions/chat-completions.test.ts:36 (response.choices[0].message.content)","root_cause":"hive-free free-pool member (PR #1115/#1155) returned message.content omitted (null) after burning its max_tokens budget on hidden reasoning; normalizeChatCompletion re-marshaled the nil *string as JSON null instead of coercing it, leaking a non-OpenAI-contract shape to every SDK client","fix":"apps/edge-api/internal/inference/chat_completions.go: normalizeChatCompletion coerces nil, tool-free message.content to an empty string; tool-call messages with null content are left untouched per the OpenAI contract","tags":["free-pool","chat-completions","normalization","sdk-replay","hive-free"]}
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 56 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6b00eee8-9f77-40b2-983d-b3d8fdd9fd6f

📥 Commits

Reviewing files that changed from the base of the PR and between 2df3afd and f466d03.

📒 Files selected for processing (2)
  • apps/edge-api/internal/inference/chat_completions.go
  • apps/edge-api/internal/inference/chat_completions_null_content_test.go

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sakibsadmanshajib
sakibsadmanshajib merged commit 4a4aa97 into main Aug 25, 2026
25 checks passed
@sakibsadmanshajib
sakibsadmanshajib deleted the fix/chat-completions-model-shape branch August 25, 2026 18:59
sakibsadmanshajib added a commit that referenced this pull request Aug 25, 2026
…1177)

## Summary

Live verification of every capability DEMO.md claims, against the
actually deployed box (chat-hive, console-hive, api-hive, control-hive),
run today after the 2026-08-25 Cloudflare regional-edge false alarm
cleared. Full capability matrix and methodology in
`docs/proof/demo-readiness-verify-2026-08-25/log.md`.

**Confirmed fixed, DEMO.md corrected (was stale):**
- Artifacts (#1110, fixed by PR #1141): `/artifacts` renders a real
empty-state index today, not the "spins forever" DEMO.md described. No
sidebar entry yet (tracked by #943 item 4, not new).
- In-chat credits (#1063, fixed by PR #1119): a "You've used N credits
today, N remaining" strip sits above the composer, matching the console
Billing balance exactly.

**Confirmed still broken, unchanged:**
- Knowledge nav (#1109): clicking it still does nothing (URL unchanged).
Direct `/knowledge` now answers an honest 404 instead of the
originally-reported silent bounce home, a minor symptom shift, not a
fix.

**Verified today's merges, all landed after this session started:**
- Cache-aware billing (#1157) and Anthropic `cache_control` passthrough
(#1152): shipped and tested, but unexercised live. The catalog has no
Anthropic model today, and a direct `usage_events` query shows zero
cache-bearing requests since deploy.
- Free pool failover (#1155) and the null-content coercion fix (#1169):
no regressions in the trailing 24h of live traffic (zero error-status
`usage_events` rows across 356 requests), though neither fix's specific
trigger recurred live to re-test directly.
- External uptime probe (#1166): confirmed running on its 15-minute
schedule, all green. Added a T-1 checklist note pointing at it, since
today's regional Cloudflare maintenance window is exactly the scenario
it exists to catch.

**Corrected a claim broader than the two named stale items:** the "not
demoable: multi user isolation (#947, #948, #949 family)" line was
itself stale. All three were fixed 2026-08-23 (PRs #960, #1067, #1091,
#1096). One residual, #1056 (two Knowledge by-id/files routes still
short-circuit on `role == admin`), is still open, so the line now says
that precisely instead of citing three closed issues.

## New issues filed

None. Every genuinely broken thing found already has an open tracking
issue (#1109, #1056, #943).

## Verification

- `node tools/lint-no-token-in-proof-captures.mjs` passes against the
new proof log.
- Live session obtained via the standard admin one-time-token mint
(`docs/live-test-auth.md`), read-only against the demo fixture account,
no password touched, no message sent, no key minted, no task submitted.
- Screenshots posted separately to the PR via
`scripts/post-pr-visual-proof.sh` (permanent GitHub Release, per
`.wolf/decisions.md` D-042).

## Test plan

- [x] `node tools/lint-no-token-in-proof-captures.mjs`
- [x] Manual read of the rendered DEMO.md for internal consistency

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant