Skip to content

fix(responses): backfill missing status and created_at for strict decoders - #2639

Open
bet4it wants to merge 1 commit into
lidge-jun:devfrom
bet4it:fix/responses-backfill-status-and-created-at
Open

fix(responses): backfill missing status and created_at for strict decoders#2639
bet4it wants to merge 1 commit into
lidge-jun:devfrom
bet4it:fix/responses-backfill-status-and-created-at

Conversation

@bet4it

@bet4it bet4it commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2142. That PR backfilled the required id field on output items. After it merged, grok-build still fails with two more strict-decoder errors on the Responses passthrough path:

  • serialization error: missing field status — some upstream relays omit status on message output items. OutputMessage.status is a required OutputStatus (no #[serde(default)]) in the async-openai fork that grok-build pins (rev 95b52eb).

  • serialization error: missing field created_at — some upstream relays omit created_at on the Response object in response.created / response.completed events. Response.created_at is a required u64 (no #[serde(default)]).

Both are the same class of problem as annotations (#1941) and id (#2142): a required field with no serde default, omitted by a relay that the openai-responses passthrough adapter forwards verbatim. The translation path (bridge.ts) is unaffected — closeCurrentMessage already emits status: "completed", and responseSnapshot already emits created_at.

What changed

backfillItemStatus (responses-field-backfill.ts): adds status to output items when type === "message" and the field is absent. Only message items carry this field in the Responses schema; reasoning, function_call, and other item types do not. The value is inferred from the event context:

  • output_item.addedin_progress (the item is still being generated)
  • output_item.donecompleted
  • Response-level events (response.created, response.completed, etc.) → derived from the response status field

Existing values are never overwritten.

created_at backfill (in backfillResponseOutput): adds created_at to the response object when absent. The timestamp is captured once per rewrite factory (SSE path) or once per call (JSON path), so every event in the same stream carries the same value, even if the stream spans a second boundary.

Both backfills are wired through backfillOutputItem / backfillResponseOutput, so they cover the SSE block rewrite path (createResponsesFieldBackfillBlockRewrite) and the bounded-JSON passthrough path (backfillResponsesFieldsJson) simultaneously.

Verification

  • bun test tests/responses-field-backfill.test.ts — 34 pass, 0 fail (28 existing + 6 new: backfill in_progress on output_item.added, backfill in_progress on response.created, backfill incomplete on response.incomplete, created_at consistency across events in the same stream, plus the existing status/created_at tests).
  • bun x tsc --noEmit — no new errors (3 pre-existing errors in claude-messages.ts and fetch-helpers.ts are unchanged on origin/dev).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (No user-facing behavior change beyond fixing the crash; the backfill module is internal.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Adds protocol-required fields with safe defaults only; no credential, auth, or user content is read or logged.)

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Responses now consistently include missing message status values based on event progress.
    • Responses now include a Unix timestamp when created_at is missing.
    • Existing status and timestamp values remain unchanged.
    • Timestamps remain consistent across events within the same response stream.
  • Tests

    • Added coverage for streaming and JSON response backfilling, including incomplete events and non-message items.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 25, 2026
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4dd48837-1e12-4563-97f0-f3cbee4d246b

📥 Commits

Reviewing files that changed from the base of the PR and between 9fdd5af and 86b95b6.

📒 Files selected for processing (2)
  • src/server/responses/responses-field-backfill.ts
  • tests/responses-field-backfill.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The response field backfill now adds missing message statuses and response creation timestamps. The behavior applies to streamed and JSON responses, preserves existing values, excludes non-message items, and supports responses without an output array.

Changes

Response field backfill

Layer / File(s) Summary
Backfill response fields
src/server/responses/responses-field-backfill.ts
Missing message-item status values are inferred from event or response status. Missing response created_at values use a Unix-second timestamp. Existing values and non-message items remain unchanged.
Apply fields across response flows
src/server/responses/responses-field-backfill.ts
SSE rewrites reuse one timestamp for all events in a stream. JSON rewrites use one terminal timestamp per call.
Validate streamed and JSON responses
tests/responses-field-backfill.test.ts
Tests cover status inference, timestamp generation and preservation, non-message items, incomplete responses, and stream-level timestamp consistency.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 86b95

This localized change backfills required response fields without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review.

Suggested reviewers: ingwannu, lidge-j

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: backfilling missing status and created_at fields in the Responses path to support strict decoders.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@bet4it
bet4it force-pushed the fix/responses-backfill-status-and-created-at branch from 9fdd5af to bbc0ee1 Compare August 25, 2026 23:40
@github-actions
github-actions Bot marked this pull request as draft August 25, 2026 23:41
@github-actions
github-actions Bot marked this pull request as ready for review August 25, 2026 23:42
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 64 / 80

설명
이 PR은 Responses 통과 경로에서 상류 서버가 꼭 필요한 값을 빼먹었을 때, OpenCodex가 그 값을 채워 주려는 수정입니다. 지금 devsrc/server/responses/responses-field-backfill.tsannotations와 출력 항목 id만 고칩니다. 출력 항목을 고치는 중심은 128140줄의 backfillOutputItem이고, 응답 전체를 걷는 곳은 147158줄의 backfillResponseOutput입니다. PR은 여기에 메시지의 status와 응답의 created_at을 더합니다. 엄격한 디코더는 이 두 값이 없으면 응답 내용을 읽기 전에 실패하므로, 문제를 고치는 방향과 파일 위치는 맞습니다.

이 보정기는 실제 통과 경로에도 이미 알맞게 연결되어 있습니다. 스트리밍 응답은 src/server/responses/core.ts 3905줄에서 createResponsesFieldBackfillBlockRewrite()를 항상 사용하고, JSON 응답은 같은 파일 4113~4114줄에서 backfillResponsesFieldsJson()을 사용합니다. 따라서 이 한 모듈을 고치면 SSE와 JSON을 함께 고칠 수 있습니다. 번역 경로를 따로 건드리지 않은 것도 범위를 작게 지키는 좋은 선택입니다.

created_at을 응답 최상위에 넣고, 이미 값이 있으면 그대로 두는 원칙도 좋습니다. status도 메시지 항목에만 넣고 함수 호출이나 추론 항목에는 넣지 않으므로 스키마 범위를 잘 지켰습니다. 새 테스트는 값이 없을 때 채우기, 기존 값 보존, 메시지가 아닌 항목 보존, SSE와 JSON 두 경로를 확인합니다. CI도 현재 통과했습니다.

하지만 status를 언제나 completed로 넣으면 스트림 초반의 뜻이 틀립니다. 현재 devrewriteEvent는 170186줄에서 response.output_item.addedresponse.output_item.done을 같은 backfillOutputItem으로 보냅니다. PR의 새 함수는 두 이벤트를 구분하지 않으므로, 아직 내용 델타가 이어질 output_item.added 메시지도 이미 끝난 메시지로 바꿉니다. 198205줄의 응답 스냅샷 처리도 response.createdresponse.completed를 구분하지 않으므로 같은 문제가 생깁니다. 새 테스트는 donecompleted만 검사해서 이 잘못된 초반 상태를 잡지 못합니다.

또한 PR은 created_at이 빠진 SSE 이벤트를 만날 때마다 그 순간의 Date.now()를 새로 부릅니다. 한 응답이 1초를 넘기면 response.createdresponse.completed에 서로 다른 생성 시각이 들어갈 수 있습니다. 이 보정기 팩토리는 요청마다 src/server/responses/core.ts 3905줄에서 한 번 만들어지므로, 팩토리를 만들 때 시각을 한 번 잡아 같은 스트림의 모든 이벤트에 재사용할 수 있습니다. JSON 경로는 함수 호출 한 번 안에서 시각을 한 번 잡으면 됩니다.

src/server/responses/responses-field-backfill.ts/backfillItemStatus - output_item.addedresponse.created에도 무조건 completed를 넣어 진행 중인 메시지를 끝난 메시지로 잘못 표시합니다.
src/server/responses/responses-field-backfill.ts/backfillResponseOutput - 이벤트마다 현재 시각을 다시 계산해 같은 응답의 created_at이 스트림 도중 달라질 수 있습니다.
tests/responses-field-backfill.test.ts - output_item.added/response.created의 상태와 한 스트림 안 created_at 일관성을 검사하는 회귀 테스트가 없습니다.

메인테이너의 판단이 필요한 지점

  • 빠진 메시지 상태를 이벤트 종류에 맞춰 in_progress/completed/incomplete로 추론할지, 관찰된 오류가 난 종료 이벤트에서만 보정할지 정해야 합니다.
  • 상류가 생성 시각을 주지 않았을 때 요청 단위의 보정 시각을 쓰는 것을 호환 정책으로 받아들일지 정해야 합니다.

너의 추천
지금 바로 합치지 말고 변경을 요청하는 것을 추천합니다. status를 이벤트 단계에 맞게 넣고, created_at은 보정기 생성 시 한 번 계산해 같은 응답에서 고정하세요. output_item.added, response.created, 1초가 지나도 같은 created_at을 쓰는 테스트를 더한 뒤 합치면 됩니다.

이 댓글은 grok-bot이 작성했습니다

@bet4it
bet4it force-pushed the fix/responses-backfill-status-and-created-at branch from bbc0ee1 to ba19022 Compare August 25, 2026 23:43
@github-actions
github-actions Bot marked this pull request as draft August 25, 2026 23:44
…oders

Follow-up to lidge-jun#2142. Two more required fields cause the same strict-decoder
crash on the Responses passthrough path when an upstream relay omits them:
OutputMessage.status (no #[serde(default)] in the async-openai fork) and
Response.created_at (u64, no default).

Status is inferred from event context: output_item.added gets in_progress,
output_item.done gets completed, and response-level events derive it from
the response status field. created_at is captured once per rewrite factory
so every event in the same stream agrees, even across a second boundary.

Both backfills are wired through backfillOutputItem / backfillResponseOutput,
covering SSE and bounded-JSON passthrough. Existing values are never
overwritten. The translation path (bridge.ts) already emits both fields.
@bet4it
bet4it force-pushed the fix/responses-backfill-status-and-created-at branch from ba19022 to 86b95b6 Compare August 25, 2026 23:47
@github-actions
github-actions Bot marked this pull request as ready for review August 25, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants