Skip to content

fix(sse): give extended-thinking targets the reasoning readiness budget - #11959

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
abhisheksharma2411:fix/thinking-readiness-11922
Aug 30, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
abhisheksharma2411:fix/thinking-readiness-11922

Conversation

@abhisheksharma2411

Copy link
Copy Markdown
Contributor

Summary

kiro/claude-sonnet-5-thinking fails with [504] Stream produced no non-ping SSE event within 125000ms when the upstream stays ping-only through a long reasoning turn.

125000 is not a hardcoded Kiro timeout — it is STREAM_READINESS_TIMEOUT_MS (80s) plus the 45s very-large-history/payload bump, capped there because nothing in resolveStreamReadinessTimeout recognised the request as a reasoning target.

The policy already grants a 30s warm-up allowance to codex high-effort models and to Claude-format replicas, but it keys the latter on the provider registry's format field. Kiro serves Anthropic thinking models through its own CodeWhisperer translator, so format: "kiro" excludes it — and the -thinking alias was never a reasoning signal the way -high is. devin, antigravity and chatgpt-web publish -thinking ids from outside claude format too, so they had the same gap.

This detects the -thinking alias suffix directly and applies the same 30s allowance, so kiro/claude-sonnet-5-thinking gets 155s instead of 125s. The three reasoning bumps are mutually exclusive — they model the same one-off warm-up, so a single request cannot stack them into a multi-minute readiness window.

Scoped to the alias suffix (-thinking, -thinking-1m) rather than a substring, so an unrelated id that merely contains the word does not qualify.

Related Issues

Validation

  • Change type: routing (SSE stream readiness)
  • Focused tests and category gates from the golden path
  • npm run lint — clean on both changed files
  • Reconciled with the current active release base (release/v3.8.51); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Reproduced first: the new #11922 test failed at exactly 125000 !== 155000, matching the reporter's number before any fix was written.

Focused suites, all green after the change:

tests/unit/stream-readiness-policy.test.ts
tests/unit/stream-readiness.test.ts
tests/unit/combo-stream-readiness-fallback.test.ts
tests/unit/runtime-timeouts.test.ts
tests/unit/lmarena-stream-readiness-repro-9306.test.ts
tests/unit/stream-early-eof-breaker.test.ts
→ 76 tests, 76 pass, 0 fail

npx tsc --noEmit reports the same 4782 pre-existing errors on release/v3.8.51 with and without this change — it adds none.

Each new guard was mutation-tested; every mutation killed exactly one test:

mutation result
drop && !extendedThinking from the claude-format bump 1 fail
drop && !codexHighReasoning from the thinking bump 1 fail
loosen `/-thinking(?:- $)/to/thinking/`

Tests Added Or Updated

Coverage Notes

open-sse/utils/streamReadinessPolicy.ts is the only production file changed. Every added branch is covered by the tests above: the bump firing, both exclusivity guards, and the suffix-boundary rejection. Coverage does not move down in any touched file.

Reviewer Notes

  • Behavioural change is bounded: a -thinking request gets at most +30s of readiness budget, still clamped by maxTimeoutMs, and nothing else in the request path changes.
  • Worth a second opinion on scope: this treats -thinking as a reasoning signal for every provider. That is deliberate — the alias means the client asked for extended thinking regardless of who serves it — but if you would rather gate it to a provider allowlist, say so and I will narrow it.
  • I did not touch the Kiro executor or add a per-provider timeout control, which is the other option feat(providers): Kiro provider: 125000ms non-ping SSE idle timeout not configurable anywhere #11922 raises. The env vars (STREAM_READINESS_TIMEOUT_MS, STREAM_READINESS_MAX_TIMEOUT_MS) already exist and do govern this path; the reporter's real problem was the default being too low for thinking targets, not the absence of a knob. Happy to add the dashboard control as a follow-up if you want it.

kiro/claude-sonnet-5-thinking 504s with "Stream produced no non-ping SSE
event within 125000ms" — the 80s base plus the 45s large-history bump,
capped there because nothing recognised the request as a reasoning target.

The readiness policy already grants a 30s warm-up allowance to codex
high-effort models and to claude-format replicas, but it keys the latter on
the provider registry's format field. Kiro serves Anthropic thinking models
through its own CodeWhisperer translator, so format: "kiro" excludes it,
and the -thinking alias was never a reasoning signal the way -high is. Devin,
antigravity and chatgpt-web publish -thinking ids from outside claude format
too, so they had the same gap.

Detect the -thinking alias suffix directly and apply the same 30s allowance,
exclusive with the two existing reasoning bumps so a single request cannot
stack them into a multi-minute readiness window.

Closes diegosouzapw#11922
@abhisheksharma2411

Copy link
Copy Markdown
Contributor Author

CI note, so nobody has to dig: the three red checks here are all base-branch state, not this change.

No new ESLint warnings — I ran the job's exact command on both refs. release/v3.8.51 alone:

$ npm run lint:json -- --max-warnings 0
exit=2
There are suppressions left that do not occur anymore. Consider re-running the command with `--prune-suppressions`.

Identical on this branch — same exit code, same message, byte-identical output apart from a [BABEL] size note about a cached dist/ artifact. The exit 2 comes from stale suppression entries, which is exactly what #11924 measured in a clean-room clone.

Build (advisory) and dast-smoke are the hosted-runner OOM in #11946.

All 4 unit-test shards, Vitest, semgrep, docs gates and merge integrity are green, which is the part this PR actually touches.

Bring in diegosouzapw#11940/diegosouzapw#11955 ESLint suppressions re-freeze and later
release/v3.8.51 commits so inherited stale-suppression reds can
clear without widening the repo baseline.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Owner

Babysit summary (#11959)

Verdict: green for required checks. Not merging (human merge).

Contributor credit preserved: original commit 2ad1f3cc remains authored by @abhisheksharma2411.

Before (head 2ad1f3cc)

What we did

PR was 37 commits behind origin/release/v3.8.51. Merged current base (includes #11940 / #11955 ESLint suppressions re-freeze, #11975) and pushed to the fork (maintainerCanModify: true).

Pushed SHA: 8220a44b334b2a8a226e062e541a0c9055f82172
(2ad1f3cc + merge origin/release/v3.8.51)

No production-file edits beyond the merge. ESLint gate not widened.

After (head 8220a44b)

  • No new ESLint warnings — PASS
  • Fast Quality Gates — PASS
  • Unit Tests fast-path 1/4–4/4 — PASS
  • Vitest, Docs Gates, Merge integrity, semgrep — PASS
  • dast-smoke — not scheduled on this run (no longer on the PR rail after fix(ci): take the two hosted-runner builds off the PR rail (#11946, option 3) #11962)
  • Build (advisory) — FAIL (npm run build cancelled on ubuntu-latest mid-compile; same advisory hosted-runner OOM/shutdown as before — ignored; Mergify already allows this single failure)

Ready for owner merge when the ⭐ gate is applied.

@diegosouzapw
diegosouzapw merged commit 4c8074b into diegosouzapw:release/v3.8.51 Aug 30, 2026
14 of 15 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…et (diegosouzapw#11959)

Dá aos alvos de extended-thinking o orçamento de prontidão de reasoning, com cobertura de teste ampliada em `stream-readiness-policy.test.ts`. Validado no worktree combinado. Obrigado!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(providers): Kiro provider: 125000ms non-ping SSE idle timeout not configurable anywhere

2 participants