fix(responses): backfill missing status and created_at for strict decoders - #2639
fix(responses): backfill missing status and created_at for strict decoders#2639bet4it wants to merge 1 commit into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe 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 ChangesResponse field backfill
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to 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: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
9fdd5af to
bbc0ee1
Compare
리뷰 · 우선순위 64 / 80설명 이 보정기는 실제 통과 경로에도 이미 알맞게 연결되어 있습니다. 스트리밍 응답은
하지만 또한 PR은
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
bbc0ee1 to
ba19022
Compare
…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.
ba19022 to
86b95b6
Compare
Summary
Follow-up to #2142. That PR backfilled the required
idfield 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 omitstatuson message output items.OutputMessage.statusis a requiredOutputStatus(no#[serde(default)]) in the async-openai fork that grok-build pins (rev95b52eb).serialization error: missing field created_at— some upstream relays omitcreated_aton theResponseobject inresponse.created/response.completedevents.Response.created_atis a requiredu64(no#[serde(default)]).Both are the same class of problem as
annotations(#1941) andid(#2142): a required field with no serde default, omitted by a relay that theopenai-responsespassthrough adapter forwards verbatim. The translation path (bridge.ts) is unaffected —closeCurrentMessagealready emitsstatus: "completed", andresponseSnapshotalready emitscreated_at.What changed
backfillItemStatus(responses-field-backfill.ts): addsstatusto output items whentype === "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.added→in_progress(the item is still being generated)output_item.done→completedresponse.created,response.completed, etc.) → derived from the responsestatusfieldExisting values are never overwritten.
created_atbackfill (inbackfillResponseOutput): addscreated_atto 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 inclaude-messages.tsandfetch-helpers.tsare unchanged onorigin/dev).Checklist
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
created_atis missing.Tests