fix(gui): do not drop batched messages after a <think> block - #13163
Open
ScrewTSW wants to merge 10 commits into
Open
fix(gui): do not drop batched messages after a <think> block#13163ScrewTSW wants to merge 10 commits into
ScrewTSW wants to merge 10 commits into
Conversation
Author
|
The failing check here is the JetBrains test job, which is unrelated to this change - tracked in #13164. This PR touches no JetBrains code. |
Author
|
I have read the CLA Document and I hereby sign the CLA |
The JetBrains test job pinned FFmpeg 7.1, but BtbN/FFmpeg-Builds keeps only the most recent release branches under its rolling `latest` tag and has since dropped the 7.1 artifacts, so the asset URL the action builds now 404s. `AnimMouse/setup-ffmpeg` pipes `wget -qO-` straight into `tar -xJ`, so the 404 body reaches tar rather than an archive and the real cause is reported one stage downstream as "xz: (stdin): File format not recognized". The step fails in 240ms, before Gradle, prepackage, the binary build or the tests run, so no JetBrains tests execute at all. Bump the pin to 8.1 and add a preflight check that verifies the artifact exists first, failing with an explicit annotation naming the missing URL so the next branch rotation is self-explaining. The version is a single action input, so the pin lives in one place.
The `build-and-upload-vsix` matrix was capped at 10 minutes, which cold
Windows and macOS builds of this monorepo routinely exceed. Observed job
durations against the cap:
linux/x64 4m23s ok
win32/x64 9m20s ok (40s margin)
darwin/arm64 8m18s ok
win32/x64 10m41s killed
darwin/arm64 10m16s killed
GitHub reports a timeout as `cancelled`, not `failure`, and kills the job
mid-step. Because the cache save in the composite action's post phase is
the last thing running at teardown, it is what surfaces in the UI - which
made this look like a Windows npm caching bug rather than a timeout.
Whichever platform is slowest on a given run is the one that dies, so the
failure moves between win32 and darwin.
Raising the cap only stops the truncation. Warm builds are unaffected; the
underlying cause of slow cold builds is that the three node_modules caches
in .github/actions/build-vscode-extension share a `${{ runner.os }}-node-`
key prefix with no restore-keys, so any lockfile change is a total miss and
forces a full `npm ci`. That is left for a follow-up.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* chore: drop Node v18/v20, raise minimum to lts/v22 * fix: bump sqlite3 to v6 to unblock native build on Node 22 * fix: use documented `macos` platform id for pkg target * fix(vscode): derive sqlite3 prebuild URL from installed version * fix(vscode): validate sqlite3 target, drop shell from download Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…OpenAI SDK (#8) * fix: convert requestOptions.timeout from seconds to milliseconds for OpenAI SDK The OpenAI JS SDK expects timeout in milliseconds, but users configure requestOptions.timeout in seconds. Passing the value directly results in sub-second timeouts (e.g. timeout: 300 becomes 300ms instead of 5 minutes), causing "Connection error" for any model that takes more than a fraction of a second to respond. Fixes continuedev#12450 * fix: preserve an explicit `timeout: 0` for the OpenAI SDK Addresses review feedback from CodeRabbit and Copilot on #8. The truthiness check treated a configured `timeout: 0` as unset and passed `undefined`, so the SDK applied its 10-minute default instead. `timeout` is `z.number().optional()` with no positivity constraint, so 0 is schema-valid and reaches this code. Use a nullish check, matching how both sides already treat 0 as a real value: the SDK resolves its default with `options.timeout ?? DEFAULT_TIMEOUT`, and our own getAgentOptions uses `?? TIMEOUT`. Adds OpenAI.test.ts covering the seconds→ms conversion, the zero case, and the unset case. Verified the zero test fails against the previous code (expected 0, got 600000) rather than merely passing after the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* ci: add OSV-Scanner for PR and main/release vulnerability scanning * ci: drop merge_group trigger, skip SARIF upload on fork PRs Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ease) (#11) * ci: replace upstream release flow with GitHub-only nightly + on-demand release * ci: harden release workflows after review * ci: pin node version in release workflows, document versionCheck removal * ci: paginate nightly prune instead of capping at 200 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2.2.0 is already taken by an empty `v2.2.0-vscode` draft release left behind by the old `auto-release.yml`, which had no build step. Bumping lets `release.yaml` create a clean tag rather than colliding with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(gui): add ui.expandThinkingBlocks setting * test(gui): cover ThinkingBlockPeek expansion state * test(gui): cover disabling expandThinkingBlocks while mounted Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(openai-adapters): don't swallow stream chunks that carry usage * Fixing mocked test values * fix(openai-adapters): classify usage-only chunks across all choices Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The `<think>...</think>` fast-path in the `streamUpdate` reducer ended in `return` rather than `continue`. Since it sits inside a `for (const message of action.payload)` loop, returning exits the reducer entirely and silently discards every remaining message in the batch, not just the one being handled. The sibling early-exit for redacted thinking uses `continue`, and nothing runs after the loop, so `continue` is the intended control flow here. This went unnoticed because every existing `streamUpdate` test dispatches a single-element payload, where `return` and `continue` are indistinguishable. The added test uses a two-message payload and fails on `return`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ScrewTSW
force-pushed
the
fix/gui-think-block-batch-drop
branch
from
August 19, 2026 21:57
5ef36c2 to
5391928
Compare
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
<think>…</think>fast-path in thestreamUpdatereducer ends inreturnrather thancontinue. It sits insidefor (const message of action.payload), so returning exits the reducer entirely and silently discards every remaining message in the batch — not just the one being handled.The sibling early-exit for redacted thinking (a few lines above) uses
continue, and nothing runs after the loop, socontinueis the intended control flow here.Impact is limited to payloads carrying more than one message, where any content following a
<think>block in the same batch is dropped.Checklist
Screen recording or screenshot
Not applicable — a one-keyword control-flow fix in a reducer. The added test demonstrates the dropped message directly.
Tests
Added to
gui/src/redux/slices/sessionSlice.test.ts: a two-message payload where the first carries a<think>block and the second continues the answer.This went unnoticed because all 11 existing
streamUpdatetests dispatch single-element payloads, wherereturnandcontinueare indistinguishable.Mutation-verified: with
returnthe new test fails ('First part.'vs'First part. Second part.'); withcontinueall 10 pass and the other 9 are unaffected.