Repository navigation
🔐 fix: Skip Reasoning Labels for Encrypted Reasoning - #17100
danny-avila wants to merge 9 commits into
Conversation
OpenAI models on Bedrock (and Claude's redacted thinking on Bedrock) return
reasoning as encrypted `redactedContent` bytes. The current agents SDK
base64-encodes those bytes into the reasoning stream, so they arrive as THINK
text: the reasoning-label wiring summarized the blob ("Inspecting an encoded
data snapshot"), activity and phase labels quoted it as reasoning evidence, and
the UI rendered it as thinking.
Add a shared `stripOpaqueReasoning` / `isOpaqueReasoning` predicate in
data-provider that recognizes long unbroken base64/base64url runs (Bedrock
redactedContent, Anthropic redacted_thinking data, OpenAI encrypted_content,
Gemini thought signatures) and keep them out of every label path:
- reasoning labels drop encoded deltas, ignore persisted payloads on resume,
and re-check the accumulated text before a billed label call
- activity-label and phase-summary evidence carries readable excerpts only;
phase anchoring still uses the raw excerpt
- the THINK renderer, grouped reasoning rows and the live header show a
neutral "Reasoning (encrypted)" marker instead of the payload, and ignore
labels previously generated from it
Readable reasoning on every provider, including readable text mixed with an
encoded payload in the same step, is still labeled.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c26b00325
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function stripOpaqueReasoning(text: string): string { | ||
| return text.replace(OPAQUE_REASONING_RUN, ''); | ||
| } |
There was a problem hiding this comment.
For restored messages, persisted THINK content is wrapped in <think>...</think> (see splitThinkTags in packages/data-provider/src/parts.ts), so stripping the encoded run leaves <think></think> and isOpaqueReasoning returns false. The client consequently renders the original ciphertext instead of the encrypted marker, while backend consumers treat the leftover tags as readable evidence. Strip the wrapper before testing whether any readable content remains so stored encrypted reasoning is handled like the live unwrapped form.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb8ec47. stripOpaqueReasoning now returns '' when only the <think> wrapper would remain, and isOpaqueReasoning ignores the wrapper when it checks for readable content. A persisted <think>{payload}</think> part now renders the encrypted marker and adds nothing to label or phase evidence. Covered in reasoning.spec.ts and Part.test.tsx.
| state.text = appendBoundedReasoning(state.text, text); | ||
| state.totalChars += text.length; |
There was a problem hiding this comment.
When a step already contains readable reasoning and an encrypted payload is split into deltas shorter than 128 characters, every fragment survives the per-delta strip here and increments totalChars. The accumulated gate later removes the joined ciphertext from visibleReasoning, but shouldGenerate still uses the inflated totalChars, so each updateChars worth of ciphertext can trigger another billed label request with unchanged readable text until maxPerRun is exhausted. Track the stripped/visible character count used by the prompt rather than the raw fragment count.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb8ec47. Deltas now go through advanceOpaqueReasoning, a streaming cursor that holds a trailing payload-alphabet run until it settles. The run either ends at a non-payload character and becomes a word, or reaches 128 characters and becomes a payload, which is dropped along with every payload-only delta after it. totalChars counts only settled readable text plus a word-sized (≤32 char) pending tail, so ciphertext fragments never advance minChars or updateChars. The new spec "never spends label revisions on a payload split across small deltas" streams a payload in 100-char deltas with updateChars: 50 and asserts that only the initial label call is made.
- isOpaqueReasoning / stripOpaqueReasoning ignore the persisted `<think>` wrapper, so stored encrypted reasoning renders as the encrypted marker and never reaches label evidence as tags-only "text" - add advanceOpaqueReasoning, a streaming cursor that holds a trailing payload-alphabet run until it settles as a word or grows into a payload, so a payload split across sub-threshold deltas is dropped as it streams - reasoning labels count only settled readable characters plus a word-sized pending tail, so ciphertext fragments never advance the minChars or updateChars gates or trigger extra billed label calls
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb8ec47fdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isOpaqueReasoning(reasoning)) { | ||
| return <ReasoningMarker encrypted />; | ||
| } |
There was a problem hiding this comment.
When a THINK part contains readable prose alongside an encrypted provider payload, isOpaqueReasoning intentionally returns false because some readable content remains, so the original unfiltered string is passed to Reasoning and the ciphertext is displayed and offered by the thought copy control; the grouped renderer has the same all-or-nothing check. Fresh evidence after cb8ec47 is that the new stripOpaqueReasoning tests explicitly support readable text surrounding an encoded payload. Render the stripped readable value, reserving the marker for parts with no readable remainder.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5da1460. Reasoning and ReasoningCompact now render and copy stripOpaqueReasoning(...), the readable text with any encoded payload removed. The live header previews from the same stripped text, and the conversation export (hooks/Conversations/format.ts) also drops payloads. The encrypted marker is still reserved for parts with no readable remainder. Covered by ReasoningCompact.test.tsx, LiveParity.test.tsx and format.test.ts.
| return tail.length <= MAX_PENDING_WORD_CHARS ? tail : ''; | ||
| } | ||
|
|
||
| function visibleChars(state: Pick<ReasoningStepState, 'totalChars' | 'cursor'>): number { |
There was a problem hiding this comment.
With reasoningLabelMinChars or reasoningLabelUpdateChars configured at 32 or below, an encrypted payload streamed in small chunks can trigger a label request before its run reaches the 128-character classification threshold: pendingWord exposes the undecided base64 prefix to both visibleChars and the prompt. Fresh evidence after cb8ec47 is that this cursor now exposes up to 32 undecided characters while the config schema accepts either threshold down to 1, so providers emitting token-sized base64 deltas still cause a billed, payload-derived label before the cursor turns opaque.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5da1460. The undecided tail no longer counts toward any gate and is never sent in the prompt. totalChars and visibleReasoning use only settled readable text, so neither reasoningLabelMinChars nor reasoningLabelUpdateChars can be reached by a payload prefix at any configured value. A short (≤32 char) final word settles when the step closes. New specs cover a payload streamed in 10-char deltas with minChars: 5 / updateChars: 4, which makes no attempt, and an unpunctuated final word that is labeled on close.
…counting - the reasoning disclosures, grouped rows, live header and conversation export render and copy the readable text of a THINK part with encoded payloads removed; the encrypted marker stays for parts with nothing readable - reasoning labels count and send only settled readable text: an undecided trailing run never advances minChars/updateChars or reaches the prompt, whatever the configured thresholds, and a short final word settles when the step closes
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5da14607f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const combined = `${cursor.opaque ? '' : cursor.tail}${delta}`; | ||
| let end = combined.length; | ||
| while (end > 0 && isPayloadChar(combined.charCodeAt(end - 1))) { |
There was a problem hiding this comment.
When a previously classified payload ends in a delta that also contains a delimiter and readable reasoning, this bypasses the payload-only fast path and reparses the entire delta without preserving the opaque state. If the remaining ciphertext prefix is shorter than 128 characters, the regex does not remove it, so it is counted as readable and can be sent to generateLabel; this can occur because deltaText joins all THINK parts in an array-valued event. While cursor.opaque is true, discard the leading payload-alphabet run through its first delimiter before processing the readable remainder.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f53984b. While the cursor is opaque, advanceOpaqueReasoning now discards the leading payload-alphabet run through its first delimiter before it processes the rest of the delta, whether or not that delta also carries readable text. The tradeoff: a word glued directly onto the end of a payload, with no separator, is dropped with it, which is preferable to releasing ciphertext. Covered by "never releases the end of a payload that shares a delta with readable text".
| const readable = thinkingExcerpts | ||
| .map((excerpt) => stripOpaqueReasoning(excerpt).trim()) | ||
| .filter((excerpt) => excerpt.length > 0); |
There was a problem hiding this comment.
When an encrypted run is followed by enough readable reasoning, the activity-phase tracker first retains only the last 600 raw characters; that window can begin within the final 1–127 ciphertext characters. Filtering the already-truncated excerpt here cannot recognize that short fragment, so generatePhase receives part of the encrypted provider payload. Classify or strip the stream before applying the rolling excerpt bound, while retaining separate raw anchor data if positional lookup still requires it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f53984b. The rolling window is now bounded with sliceReasoningTail. When the 600-char cut lands inside an unbroken run, the window skips to the end of that run, so a payload cut by the window cannot leave a sub-128 fragment. The same bound is used for the snapshot and restore of pendingReasoning and for the retained excerpts. The window is still a contiguous substring of the raw THINK text, so findReasoningStart anchoring is unaffected and no separate anchor field is needed. Covered by "never starts the rolling reasoning excerpt inside an encrypted payload".
| text: appendBoundedReasoning('', existingText), | ||
| totalChars: existingText.trim().length, | ||
| cursor: OPAQUE_REASONING_START, |
There was a problem hiding this comment.
When a resumable/HITL snapshot is taken during the first 1–127 characters of an encrypted run, stripOpaqueReasoning treats that prefix as readable, stores it permanently in state.text, and initializes the streaming cursor empty. Later ciphertext can be classified and dropped, but the retained prefix is still counted and included in the next generateLabel prompt (or can trigger immediately with a low configured threshold). Initialize both the readable text and cursor by feeding the existing raw THINK value through the streaming classifier so a continuation can reclassify the undecided prefix.
AGENTS.md reference: AGENTS.md:L45-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f53984b. startStep now replays the persisted THINK text (with <think> tags removed) through advanceOpaqueReasoning and seeds both text and cursor from the result. An undecided payload prefix at the snapshot stays in the cursor, so it is dropped once the continuation classifies it instead of settling as readable text. Covered by "drops a payload prefix that was still undecided when a resumed step continues".
- once a run is classified as a payload, it continues through the next non-payload character even when that arrives in the same delta as readable text, so no tail fragment of the payload is released as readable - add sliceReasoningTail: the activity-phase rolling reasoning window (and its snapshot/restore bounds) never starts inside an unbroken run, so a payload cut by the window cannot leave a fragment too short to classify; the window stays a substring of the raw THINK text, so phase anchoring is unchanged - a resumed reasoning step replays its persisted text through the stream filter and keeps the resulting cursor, so a payload prefix that was still undecided at the snapshot is dropped once the continuation classifies it
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f53984b1b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reasoning.text = sliceReasoningTail( | ||
| `${reasoning.text}${deltaText(data, 'think')}`, | ||
| MAX_EXCERPT_CHARS, | ||
| ); |
There was a problem hiding this comment.
When a payload-only stream grows beyond the 600-character window, sliceReasoningTail can return an empty string after skipping the run, but the next delta then starts a new unclassified run. If the payload ends with a 1–127 character remainder before readable reasoning arrives, stripOpaqueReasoning later treats that remainder as prose and forwards it in thinkingExcerpts to generatePhase. Fresh evidence after f53984b is that this rolling state still retains only text, not an opacity cursor; preserve the classified state across deltas or filter before bounding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79a194d by filtering before bounding. Each pending reasoning lane now feeds every delta through advanceOpaqueReasoning, carrying its opacity cursor across deltas, and keeps a readable rolling window next to the raw one. Tracked activities store that window as readableExcerpts, and that is the only prompt evidence generatePhase receives. thinkingExcerpts stay raw because findReasoningStart matches them against THINK parts. The cursor and readable window are persisted in the snapshot. Older snapshots without them fall back to stripping the raw excerpt. sliceReasoningTail is gone. Covered by "classifies streamed reasoning before bounding its prompt evidence" (a 1,400-char payload in 100-char deltas whose last fragment shares a delta with prose) and a readableExcerpts case in the summarizer-payload spec.
| if (isOpaqueReasoning(reasoning)) { | ||
| return <ReasoningMarker encrypted />; |
There was a problem hiding this comment.
When encrypted reasoning is streamed in small deltas, isOpaqueReasoning remains false until the accumulated unbroken run reaches 128 characters, so the first 1–127 ciphertext characters are passed to Reasoning, rendered in the live thought panel, and exposed to its copy control before being replaced by the encrypted marker. Defer the undecided payload-alphabet prefix during streaming, rather than rendering it while classification is pending.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79a194d. A new readableReasoning(text, streaming) runs the streaming classifier over the text. While a part is streaming, it withholds a trailing payload-alphabet run longer than a word (32 chars) until the run is classified. Reasoning (when submitting and last), ReasoningCompact (isStreaming) and the live header use it, so the first characters of a streaming payload are never rendered or copyable. A pure prefix renders nothing until it becomes the encrypted marker. Covered in ReasoningCompact.test.tsx, LiveParity.test.tsx and reasoning.spec.ts.
|
|
||
| // Strip <think> tags from the reasoning content (modern format) | ||
| const reasoningText = useMemo(() => stripThinkTags(reasoning), [reasoning]); | ||
| const reasoningText = useMemo(() => readableReasoning(reasoning), [reasoning]); |
There was a problem hiding this comment.
When a completed assistant message enters edit mode, this sanitization is bypassed: isEditablePart still accepts every THINK part and EditContentParts initializes its textarea from the raw part.think value. Consequently, both payload-only reasoning shown here as an encrypted marker and payloads removed from mixed reasoning reappear in full in the editor and can be modified or saved. Apply the same opaque-span handling in the edit flow while preserving the original hidden payload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79a194d. isEditablePart returns false for payload-only THINK parts, which render as the marker, so the editor offers no field for them. Mixed parts are edited through getEditableText, which offers only the readable text. EditContentParts writes only parts whose draft differs from that readable original, so an unchanged part is never saved and its stored payload stays intact. Saving an edited mixed part stores the edited readable text. Covered in editableParts.spec.ts and EditContentParts.spec.tsx.
… pending - activity phases classify each reasoning delta with the stream cursor and keep a readable rolling window beside the raw anchor window; tracked activities carry readableExcerpts as prompt evidence, so a window cut into a payload can never forward a fragment (older snapshots fall back to stripping); the cursor-free sliceReasoningTail is removed - add readableReasoning / flushOpaqueReasoning: while reasoning streams, the disclosures and live header hold back a trailing run that could still grow into a payload instead of showing its first characters - the message editor offers no field for payload-only reasoning and shows only the readable text of mixed reasoning; an unchanged part is never written, so its stored payload stays intact
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79a194d88a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| readable: readable ?? stripOpaqueReasoning(boundedText), | ||
| cursor: cursor ?? OPAQUE_REASONING_START, |
There was a problem hiding this comment.
When a run created by the previous version resumes after snapshotting the first 1–127 characters of an encrypted payload, stripOpaqueReasoning(boundedText) treats that short prefix as readable while the fallback cursor starts empty. Subsequent deltas can classify and discard the remainder, but the persisted prefix stays in reasoning.readable and is later sent to generatePhase. Seed both fallback fields by replaying boundedText through advanceOpaqueReasoning, as the reasoning-label resume path does, so mixed-version resumptions cannot disclose payload prefixes.
AGENTS.md reference: AGENTS.md:L45-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b2d7c79. A pending lane restored without readable/cursor is now replayed through advanceOpaqueReasoning, the same way the reasoning-label resume path handles it, so an undecided payload prefix stays in the cursor and is dropped once the continuation classifies it. Covered by "replays a legacy pending-reasoning snapshot through the stream filter".
A pending-reasoning lane restored from a snapshot without a readable window or cursor is replayed through advanceOpaqueReasoning instead of stripped, so a payload prefix that was still undecided at the snapshot stays in the cursor and is dropped once the continuation classifies it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2d7c79b02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const classified = | ||
| readable != null && cursor != null | ||
| ? { readable, cursor } | ||
| : advanceOpaqueReasoning(OPAQUE_REASONING_START, boundedText); |
There was a problem hiding this comment.
When resuming a legacy snapshot whose 600-character rolling window begins inside a previously classified payload and contains only 1–127 payload characters before readable prose, replaying boundedText from a fresh cursor settles that leading fragment as an ordinary word. It is retained in classified.readable and later forwarded to generatePhase. Fresh evidence on b2d7c79 is that the new replay handles payload suffixes that were undecided at snapshot time, but cannot recognize a short prefix created by the old rolling-window truncation; seed from the matching full THINK part when available, or conservatively discard a leading payload-alphabet run when a legacy window may have been cut.
AGENTS.md reference: AGENTS.md:L45-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e74afd6, using the conservative option. If a legacy window filled the 600-char bound, it may have been cut, so its leading payload-alphabet run is dropped via trimTruncatedReasoning before the replay. A shorter window is the whole lane, so it is replayed as is. Seeding from the matching THINK part wasn't viable: a legacy lane's part is not reliably identifiable, since an ambiguous anchor is deliberately treated as no anchor. The cost is a partial first word on a truncated readable window. Covered by "drops the cut leading run of a full legacy pending-reasoning window".
A legacy pending-reasoning window that filled the 600-char bound may have been cut inside a payload, so its leading payload-alphabet run is dropped before the replay; a shorter window is the whole lane and is replayed as is.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e74afd66f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| const original = getPartText(part); | ||
| const original = getEditableText(part); |
There was a problem hiding this comment.
When a THINK part contains readable prose plus an encrypted run, any actual edit starts from this sanitized original and sends only the visible draft. Fresh evidence on e74afd6 is that api/server/routes/messages.js:713-718 replaces the entire persisted think value with that submitted text, so editing one prose character silently deletes the encrypted span from storage and local state. Merge the edited prose back into the raw original, or make mixed reasoning read-only if the opaque span cannot be positioned safely.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a334a99 by making it read-only. The edit endpoint replaces the whole think value, and an encrypted span cannot be positioned safely in an arbitrarily edited draft, so isEditablePart now returns false for any THINK part that carries a payload. Payload-only parts were already excluded. Such a part renders through the read-only path, and the sanitized-draft helper is removed, so the editor never writes a part with a payload. Covered in editableParts.spec.ts and EditContentParts.spec.tsx.
| textValue(part?.think).replace(THINK_TAGS, ''), | ||
| ) | ||
| : { readable: '', cursor: OPAQUE_REASONING_START }; | ||
| const existingText = resumed.readable; |
There was a problem hiding this comment.
When a step resumes from a payload-only THINK part created by the old behavior, replay correctly makes existingText empty, but the existing label is still copied into state at lines 595-598. If the resumed step then adds fewer than 120 readable characters, the final-update gate does not regenerate the label and markComplete preserves the old payload-derived title; because the part is now mixed rather than wholly opaque, Part.tsx renders that stale title again. Drop the existing label when replay finds no readable reasoning so a short readable continuation cannot revive it.
AGENTS.md reference: AGENTS.md:L45-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a334a99. When replay finds no readable reasoning in a resumed step, startStep treats the existing title as stale. Such a title could only have described the payload. It is not loaded into state, and reasoning_label/reasoning_label_status are removed from the part, while the revision and attempt counters are kept so later revisions stay monotonic. A short readable continuation therefore leaves the step untitled until it earns a fresh label. Covered by "does not revive a payload-derived title when a resumed step continues briefly".
…d titles on resume - reasoning that carries an encoded payload is not offered in the message editor: the edit endpoint replaces the whole value and the payload cannot be positioned in an edited draft, so editing would silently delete it - a resumed reasoning step whose persisted text has no readable reasoning drops its existing title (which could only describe the payload) instead of carrying it forward where a short readable continuation would revive it
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a334a99c7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if (staleLabel) { | ||
| delete part.reasoning_label; | ||
| delete part.reasoning_label_status; |
There was a problem hiding this comment.
On HITL resume, when the same reasoning step is recognized as payload-only, deleting these fields only changes the server-side part; the client still holds the labeled snapshot. synthesizeReasoningLabelGapEvents emits a reset only when the step ID changes and skips same-step parts without a fresh label, so if a short readable continuation makes the part mixed, the old payload-derived title remains visible until a full reload. Fresh evidence after a334a99 is that the new deletion has no corresponding same-step reset event; emit one so restored clients clear the title too.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d68368e. When startStep drops a stale title, it now emits a same-step on_reasoning_label reset ({ index, stepId, previousStepId: stepId, attempts, reset: true }) through a new optional emitResetEvent dependency, which the host wires to ON_REASONING_LABEL. The client's applyReasoningLabel already applies reset events: it matches previousStepId against the part's current step id, so a same-step reset clears the title the resumed client still holds. Covered in runtime.spec.ts and host.spec.ts.
| /** A title on a step with no readable reasoning can only describe an | ||
| * encoded payload (labeled before payloads were skipped), so it is not | ||
| * carried forward where a short readable continuation would revive it. */ | ||
| const staleLabel = ownsExistingStep && existingText.trim().length === 0; |
There was a problem hiding this comment.
On a mixed-version resume with a low reasoningLabelMinChars, a previously labeled short, unbroken readable thought such as Investigating is returned entirely in resumed.cursor.tail, so existingText is empty even though the text is not opaque. This condition consequently classifies the valid stored label as payload-derived and deletes it. Do not equate an empty settled prefix with confirmed opacity; defer cleanup until the cursor is classified or otherwise distinguish a readable final token from an encoded run.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d68368e. The stale-title condition is now isOpaqueReasoning(persisted think), which means the text has content, contains a classified (128+ char) payload run, and nothing readable remains. An empty settled prefix no longer counts. A short readable token such as Investigating, still undecided in the cursor, keeps its title, and no reset is emitted. Covered by "keeps the title of a resumed step whose reasoning is a short readable token".
…yload-only steps - a resumed step drops its title only when its persisted reasoning is entirely an encoded payload, so a short readable token (still undecided for the stream cursor) keeps a valid title - dropping it emits a same-step on_reasoning_label reset, which the client already applies, so a resumed client clears the snapshot title too
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
1 participant
Pull Request
Summary
OpenAI models served through Bedrock (for example
global.openai.gpt-6.1-solandglobal.openai.gpt-6-sol) return their reasoning encrypted. Bedrock's Converse stream delivers it asreasoningContent.redactedContentbytes, which is also how Claude's redacted thinking arrives on Bedrock. The current@librechat/agentsrelease base64-encodes those bytes intoadditional_kwargs.reasoning_content, and the stream handler reads that field as reasoning text. Every encrypted block therefore reaches LibreChat as an ordinary THINK delta whose text is a ~1.4 KB base64 blob.Several things then treat the blob as readable thinking. The reasoning-label wiring sends it to the label model, so steps get labels like "Inspecting an encoded data snapshot" (confirmed in Langfuse traces of
gpt-6.1-solon demo). Activity labels and phase summaries quote it as reasoning evidence. The THINK renderer, grouped reasoning rows, the live activity header and conversation export show, preview or export the blob as if it were thinking.This PR adds one shared predicate in data-provider,
stripOpaqueReasoning/isOpaqueReasoning. It treats an unbroken run of 128 or more base64/base64url characters as an encoded payload. That covers BedrockredactedContent, Anthropicredacted_thinking.data, OpenAIencrypted_contentand Gemini thought signatures. Natural-language reasoning breaks on whitespace or punctuation long before 128 characters. Every label path now uses the predicate. In the UI, a THINK part that is entirely encrypted shows a neutral "Reasoning (encrypted)" marker, persisted<think>wrapper included. A part that mixes readable text with a payload shows and copies only the readable text. Readable reasoning on every provider is labeled as before, including readable text mixed into the same step as an encoded payload.The root cause is in the SDK, and LibreChat-AI/agents will get a companion fix: keep
redactedContenton the content block, where replay needs it, but stop copying it intoreasoning_content. This PR does not depend on that fix. It works with the current@librechat/agents@4.0.4, which still streams the blob. It also covers messages already stored with the blob.How it works
Where the blob comes from in the current SDK, and where this PR stops it:
The predicate:
Streamed reasoning goes through
advanceOpaqueReasoning. It holds a trailing payload-alphabet run until a later character settles it: the run either ends and becomes a word, or reaches 128 characters and becomes a payload. After that, the payload continues through its next non-payload character, across deltas, so no tail fragment is released. The label gates (minChars,updateChars) count only settled readable characters, and the label prompt receives only settled readable text, so a payload never triggers or shapes a billed label call. This holds whether the payload arrives as one delta (as Bedrock sends it) or split into small ones, and at any configured threshold. A short final word settles when the step closes. A resumed step replays its persisted text through the same filter, so a payload prefix that was still undecided at the snapshot is dropped once the continuation classifies it. A step with no readable text closes without a provider call. If a resumed step has no readable text, any title it already carries (which could only describe the payload) is dropped.Type of change
Testing
Confirmed the failure from Langfuse traces of
global.openai.gpt-6.1-solon demo. The main generation's output is[{ type: 'reasoning_content', redactedContent: 'cnNuX3Jy…' }, { type: 'text', … }], and theReasoningLabelgeneration in the same trace received that base64 as its "Visible reasoning snapshot" and returned "Inspecting an encoded data snapshot". A realhandleConverseStreamContentBlockDeltachunk withredactedContent, driven through the SDK'sChatModelStreamHandler, dispatches the base64 as THINK text on the current SDK.Tested environments/configuration:
global.openai.gpt-6.1-sol/global.openai.gpt-6-sol(payload shape taken from demo traces)reasoningLabelenabledAutomated tests:
packages/data-provider/src/reasoning.spec.ts: BedrockredactedContent, Anthropicredacted_thinking, OpenAIencrypted_content, Gemini signatures and concatenated deltas are classified as opaque. Prose, code, a long URL and short tokens stay readable, and readable text that quotes a token is not hidden.reasoningLabels/runtime.spec.ts: an encoded step is never labeled (no attempt, no label event). A payload split into 100- or 10-character deltas is never labeled, even withminChars: 5/updateChars: 4, and never spends a revision after a readable label. Mixed reasoning is labeled from its readable text only. Readable reasoning of the same length is still labeled. A resumed step ignores a persisted payload, and an unpunctuated final word settles on close. Existing specs now end their synthetic deltas with a separator, because the filter holds an unfinished word until the next one.activityLabels/__tests__/wiring.spec.tsandactivityPhases/runtime.spec.ts: encoded excerpts are dropped from label and phase evidence, and readable excerpts are kept.Part.test.tsx,ToolCallGroup.test.tsx,ReasoningMarker.test.tsx,ReasoningCompact.test.tsx,LiveParity.test.tsxandformat.test.tscover the encrypted marker (including the persisted<think>wrapper), delegation out of grouped rows, ignoring stale blob-derived labels, readable-only rendering of mixed parts, the live header, and export.LiveParity's long-run preview fixture now uses a non-ASCII run, because a 1,199-character unbroken ASCII alphanumeric run is classified as a payload.npx tsc --noEmitinpackages/data-providerandpackages/api, plusnpm run typecheckinclient, all pass. ESLint passes on the touched files.Screenshots / recordings
The encrypted marker reuses the existing
ThinkingLabelrow, the same header row the background-agent "reasoning unavailable" marker already renders, with the new label "Reasoning (encrypted)" and a tooltip. No new styles or theme roles were added. I have not attached screenshots: reproducing this in the running app needs a Bedrock OpenAI reasoning model.Risk / compatibility
The heuristic only acts on an unbroken 128+ character base64/base64url run. Such runs are removed from displayed, exported and label text, and a THINK part becomes the marker only when nothing readable remains. While reasoning streams, the label wiring counts the last unfinished word only after the next separator arrives. This delays label gating by at most one token. A word glued directly onto the end of a payload, with no separator between them, is dropped along with the payload. While reasoning streams, the UI likewise holds back a trailing run longer than 32 characters until it is classified. Reasoning that carries a payload cannot be edited in the message editor, because the edit endpoint replaces the whole value and would delete the payload. No schema or configuration changes.
Checklist