-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(codex): preserve sequential reasoning summaries #5233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -691,6 +691,8 @@ class CodexStreamRuntime { | |
| nativeOutputItems: Array<Record<string, unknown>> = []; | ||
| /** Sequential-cutoff summary sections/emitted text, global to the response (indices span reasoning items). */ | ||
| cutoffSummaries: SequentialCutoffSummaryState = createSequentialCutoffSummaryState(); | ||
| /** Summary deltas buffered while waiting to see whether atomic `.done` events arrive. */ | ||
| pendingSummaryDeltas = new Map<CodexOpenItem, string[]>(); | ||
| websocketStreamRetries = 0; | ||
| providerRetryAttempt = 0; | ||
| sawTerminalEvent = false; | ||
|
|
@@ -723,6 +725,7 @@ class CodexStreamRuntime { | |
| this.currentItem = null; | ||
| this.currentBlock = null; | ||
| this.nativeOutputItems.length = 0; | ||
| this.pendingSummaryDeltas.clear(); | ||
| this.cutoffSummaries = createSequentialCutoffSummaryState(); | ||
| } | ||
|
|
||
|
|
@@ -744,6 +747,19 @@ class CodexStreamRuntime { | |
| if (outputIndex !== undefined) return this.openItemsByOutputIndex.get(outputIndex) ?? null; | ||
| return this.currentEntry; | ||
| } | ||
| queueSummaryDelta(entry: CodexOpenItem | null | undefined, delta: string): void { | ||
| if (entry?.block?.type !== "thinking" || delta.length === 0) return; | ||
| const pending = this.pendingSummaryDeltas.get(entry) ?? []; | ||
| pending.push(delta); | ||
| this.pendingSummaryDeltas.set(entry, pending); | ||
| } | ||
|
|
||
| takeSummaryDeltas(entry: CodexOpenItem | null | undefined): string[] { | ||
| if (!entry) return []; | ||
| const pending = this.pendingSummaryDeltas.get(entry) ?? []; | ||
| this.pendingSummaryDeltas.delete(entry); | ||
| return pending; | ||
| } | ||
|
|
||
| closeOpenItem(entry: CodexOpenItem | null | undefined): void { | ||
| if (!entry) return; | ||
|
|
@@ -1693,10 +1709,9 @@ class CodexStreamProcessor { | |
|
|
||
| /** | ||
| * Whether the request actually sent (post-`onPayload`) opted into | ||
| * sequential-cutoff summary delivery: summaries then arrive as atomic | ||
| * `response.reasoning_summary_text.done` events and incremental | ||
| * `.delta`/`.part.*` events are ignored (mirrors codex-rs | ||
| * `uses_sequential_cutoff_reasoning_summaries`). | ||
| * sequential-cutoff summary delivery. Atomic `.done` events are preferred; | ||
| * incremental deltas remain buffered as a compatibility fallback when a | ||
| * transport emits them instead. | ||
| */ | ||
| get #sequentialCutoffSummaries(): boolean { | ||
| return this.runtime.requestBodyForState.stream_options?.reasoning_summary_delivery === "sequential_cutoff"; | ||
|
|
@@ -1772,18 +1787,19 @@ class CodexStreamProcessor { | |
| } | ||
| return firstTokenTime; | ||
| } | ||
|
|
||
| if (eventType === "response.reasoning_summary_text.delta") { | ||
| if (this.#sequentialCutoffSummaries) return firstTokenTime; | ||
| if (this.runtime.currentItem?.type === "reasoning" && this.runtime.currentBlock?.type === "thinking") { | ||
| appendReasoningSummaryTextDelta( | ||
| this.runtime.currentItem, | ||
| this.runtime.currentBlock, | ||
| (rawEvent as { delta?: string }).delta || "", | ||
| stream, | ||
| output, | ||
| output.content.length - 1, | ||
| ); | ||
| const entry = this.runtime.openItemForEvent(rawEvent); | ||
| const delta = typeof rawEvent.delta === "string" ? rawEvent.delta : ""; | ||
| if (this.#sequentialCutoffSummaries) { | ||
| // Some Codex transports still emit incremental summary deltas even | ||
| // after opting into sequential-cutoff delivery. Buffer them until | ||
| // output_item.done so atomic `.done` events can supersede them | ||
| // without duplicate UI output. | ||
| this.runtime.queueSummaryDelta(entry, delta); | ||
| return firstTokenTime; | ||
| } | ||
| if (entry?.item.type === "reasoning" && entry.block?.type === "thinking") { | ||
| appendReasoningSummaryTextDelta(entry.item, entry.block, delta, stream, output, entry.contentIndex); | ||
| } | ||
| return firstTokenTime; | ||
| } | ||
|
|
@@ -1793,6 +1809,7 @@ class CodexStreamProcessor { | |
| if (!this.#sequentialCutoffSummaries) return firstTokenTime; | ||
| const entry = this.runtime.openItemForEvent(rawEvent); | ||
| if (entry?.item.type === "reasoning" && entry.block?.type === "thinking") { | ||
| this.runtime.takeSummaryDeltas(entry); | ||
| if (!firstTokenTime) firstTokenTime = performance.now(); | ||
| const summaryIndex = | ||
| typeof rawEvent.summary_index === "number" && Number.isFinite(rawEvent.summary_index) | ||
|
|
@@ -1827,15 +1844,13 @@ class CodexStreamProcessor { | |
| } | ||
|
|
||
| if (eventType === "response.reasoning_summary_part.done") { | ||
| if (this.#sequentialCutoffSummaries) return firstTokenTime; | ||
| if (this.runtime.currentItem?.type === "reasoning" && this.runtime.currentBlock?.type === "thinking") { | ||
| appendReasoningSummaryPartDone( | ||
| this.runtime.currentItem, | ||
| this.runtime.currentBlock, | ||
| stream, | ||
| output, | ||
| output.content.length - 1, | ||
| ); | ||
| const entry = this.runtime.openItemForEvent(rawEvent); | ||
| if (this.#sequentialCutoffSummaries) { | ||
| this.runtime.queueSummaryDelta(entry, "\n\n"); | ||
|
Comment on lines
1846
to
+1849
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking: |
||
| return firstTokenTime; | ||
| } | ||
| if (entry?.item.type === "reasoning" && entry.block?.type === "thinking") { | ||
| appendReasoningSummaryPartDone(entry.item, entry.block, stream, output, entry.contentIndex); | ||
| } | ||
| return firstTokenTime; | ||
| } | ||
|
|
@@ -1930,6 +1945,18 @@ class CodexStreamProcessor { | |
| return firstTokenTime; | ||
| } | ||
|
|
||
| #flushSummaryDeltas(entry: CodexOpenItem | null): void { | ||
| if (entry?.block?.type !== "thinking") return; | ||
| for (const delta of this.runtime.takeSummaryDeltas(entry)) { | ||
|
Comment on lines
+1948
to
+1950
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking: This flush updates only the current block; it does not advance the response-global |
||
| entry.block.thinking += delta; | ||
| this.stream.push({ | ||
| type: "thinking_delta", | ||
| contentIndex: entry.contentIndex, | ||
| delta, | ||
| partial: this.output, | ||
| }); | ||
| } | ||
| } | ||
| #handleOutputItemDone(rawEvent: Record<string, unknown>): void { | ||
| const { runtime, output, stream } = this; | ||
| const rawItem = rawEvent.item; | ||
|
|
@@ -1948,6 +1975,7 @@ class CodexStreamProcessor { | |
| const contentIndex = entry?.contentIndex ?? output.content.length - 1; | ||
|
|
||
| if (item.type === "reasoning" && block?.type === "thinking") { | ||
| this.#flushSummaryDeltas(entry); | ||
| block.thinking = finalizeReasoningThinking( | ||
| item, | ||
| block.thinking, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should-fix: This changes observable Codex stream assembly in
packages/ai, but the two-file PR omits the requiredpackages/ai/CHANGELOG.mdentry under## [Unreleased]→### Fixed. Please record the user-visible fix.