Repository navigation
fix: stabilize reasoning streams and request logs - #3879
diegosouzapw merged 17 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces exact pending request tracking and finalization using a unique request ID, preventing mismatches when multiple requests for the same model overlap. It also updates the call logs API to include completed in-memory fallback rows, extends the completed details TTL to 120 seconds, and ensures proper reserialization when splitting mixed reasoning and content deltas in SSE streams. Feedback on the changes highlights a potential TypeError in open-sse/utils/requestLogger.ts due to inconsistent optional chaining on entry when accessing entry.provider directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds stronger in-memory request lifecycle tracking so stream chunks and “just completed” requests reliably show up in usage logs and can be finalized by exact request id.
Changes:
- Introduces
finalizePendingRequestByIdand extends the completed-details cache TTL to support slower UI polling. - Updates request logging to bind
streamChunksto the exact pending request viarequestId(with safe fallback matching). - Extends
/api/usage/call-logsto include “completed in-memory” rows and adds test coverage around the new behaviors.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/stream-utils.test.ts | Adds a regression test ensuring passthrough SSE output is consumable by SillyTavern-style reasoning parsing. |
| tests/unit/request-logger-endpoints.test.ts | Adds tests for completed in-memory rows and requestId-based stream chunk binding. |
| tests/unit/active-request-stream-chunks-lifecycle.test.ts | Updates lifecycle coverage for overlap finalization and longer completed-details visibility. |
| src/lib/usage/usageHistory.ts | Adds completed-details TTL, exact-id finalization, and refactors finalization/cleanup logic. |
| src/app/api/usage/call-logs/route.ts | Prepends pending + completed in-memory rows to persisted call logs with deduping. |
| open-sse/utils/stream.ts | Ensures mixed reasoning/content split triggers reserialization reliably. |
| open-sse/utils/requestLogger.ts | Adds requestId option and tightens matching to avoid cross-connection streamChunk attachment. |
| open-sse/handlers/chatCore.ts | Passes pendingRequestId into logger and finalizes by id first, falling back to legacy behavior. |
…am-log-root-cause-main # Conflicts: # src/app/(dashboard)/dashboard/providers/[id]/components/modals/AddApiKeyModal.tsx # src/app/(dashboard)/dashboard/providers/[id]/components/modals/EditConnectionModal.tsx
|
Thanks, @rdself! 🙏 Three real, observed fixes in one well-tested PR: (1) pending-request lifecycle — keeping the request pending until I reconciled the modal conflict with #3921 (just merged): your |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Integrated into release/v3.8.26 — mid-stream failure persistence (follow-up to #3879). Validated locally: typecheck:core clean, 75/75 stream/usage tests, eslint 0 errors, file-size + any-budget OK.
…unner collects them The 3 React-component tests landed at top-level tests/unit/*.test.tsx, which no runner collects: test:unit only globs tests/unit/*.test.ts, and test:vitest:ui filters to tests/unit/ui. check:test-discovery flagged them as NEW orphans (never run). Moved them under tests/unit/ui/ (collected by vitest.config.ts + the ui path filter) and bumped the relative import one level (../../ -> ../../../). Also bumped the file-size baseline for the two connection modals (cohesive free-models toggle, mirroring diegosouzapw#3879/diegosouzapw#2997). 12/12 vitest + 16/16 node tests pass; test-discovery clean. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…4176) Integrated into release/v3.8.29 — import-only-free-models connection option + free/paid list filters and free-first sort. Thanks @felipesartori! On review we relocated the 3 React-component tests from tests/unit/ to tests/unit/ui/ so a runner actually collects them (check:test-discovery had flagged them as orphans), bumped the file-size baseline for the two connection modals (cohesive toggle, mirroring #3879/#2997), and synced the branch with the current release. 12/12 vitest + 16/16 node tests green.
…iegosouzapw#4176) Integrated into release/v3.8.29 — import-only-free-models connection option + free/paid list filters and free-first sort. Thanks @felipesartori! On review we relocated the 3 React-component tests from tests/unit/ to tests/unit/ui/ so a runner actually collects them (check:test-discovery had flagged them as orphans), bumped the file-size baseline for the two connection modals (cohesive toggle, mirroring diegosouzapw#3879/diegosouzapw#2997), and synced the branch with the current release. 12/12 vitest + 16/16 node tests green.
Integrated into release/v3.8.26 (reconciled cc-defaults UI with diegosouzapw#3921)
Integrated into release/v3.8.26 — mid-stream failure persistence (follow-up to diegosouzapw#3879). Validated locally: typecheck:core clean, 75/75 stream/usage tests, eslint 0 errors, file-size + any-budget OK.
Summary
reasoning_contentpluscontentdeltas, so downstream clients receive a reasoning-only event followed by a content-only event.redact-thinking-2026-02-12by default on Claude Code-compatible relays; expose it as a per-connection CC Compatible toggle for upstreams that require redacted thinking streams.release/v3.8.26and align the quality ratchet baselines to values measured on that release base, keeping this PR neutral against the target branch.Root cause
There were three independent streaming/logging issues that could combine into the same user-visible failure.
First,
createSSEStream()split mixed reasoning/content OpenAI-compatible deltas, but the passthrough branch could still reuse the original serialized SSE payload unless a later mutation forced reserialization. Clients that process reasoning before content, including SillyTavern-style OpenAI-compatible parsers, could then receive a mixed event and treat later reasoning/content boundaries incorrectly.Second, the successful stream
flush()path cleared the pending request beforeonCompleteran. The upper layer now finalizes by exact pending request id, but the request had already been removed from the pending map, so the in-memory completion fallback could be lost before the durablecall_logsrow was visible. That matched the observed behavior where the activity row spun for a while and then disappeared even though the upstream provider completed and billed normally.Third,
anthropic-compatible-cc-*relays sentredact-thinking-2026-02-12unconditionally. With extended thinking enabled, Claude can spend most or all of the output budget on thinking. If the upstream redacts that thinking, OmniRoute receives little or no visible stream content even though the provider reports a successful request.Changes
onCompletewhen completion finalization is available; only the legacy no-callback success path clears pending requests directly.chatCorestream finalization before usage/cost/persistence side effects, so UI lifecycle state is stabilized even if later accounting work fails.onCompletefinalizes the exact request id.providerSpecificData.requestDefaults.redactThinkingand Dashboard add/edit toggles.Validation
node --import tsx/esm --test tests/unit/provider-request-failure-pipeline.test.tsnode --import tsx/esm --test tests/unit/claude-code-compatible-helpers.test.ts tests/unit/request-defaults-store-session.test.ts tests/unit/provider-specific-data-schema.test.ts tests/unit/executor-default-base.test.ts tests/unit/provider-page-helpers-3501.test.tsnode --import tsx/esm --test tests/unit/request-logger-endpoints.test.ts tests/unit/stream-utils.test.ts tests/unit/active-request-stream-chunks-lifecycle.test.tsnpx vitest run open-sse/mcp-server/__tests__/glmCodingProviderConfig.test.tsnpx vitest run 'src/app/(dashboard)/dashboard/providers/[id]/components/modals/__tests__/connModals.test.tsx'npm run typecheck:corenpm run check:file-sizenpm run quality:collectnpm run quality:ratchet -- --allow-missingnpm run check:cognitive-complexitynpm run check:type-coveragenpm run check:dead-codegit diff --checkRelease-base quality comparison
origin/release/v3.8.26:npm run quality:collectmeasuredeslintWarnings=3760,eslintErrors=0,openapiCoverage.pct=38,i18nUiCoverage.pct=79.7.npm run quality:collectmeasured the same values.origin/release/v3.8.26: dedicated gates measuredcognitiveComplexity=753anddeadExports=339.Live stream checks
openrouter/deepseek/deepseek-v4-flash: 59.8s stream, 840 SSE events, 379reasoning_contentchunks followed by 456 content chunks, first reasoning at 6.14s, first content at 30.79s,mixedChunks=0, completed with[DONE].anthropic-compatible-cc-sp-anthropic/claude-opus-4-6before disabling default redaction: 46.9s stream, 4 SSE events, 0 reasoning chunks, 0 content chunks,output_tokens=2048,thinking_tokens=2047,stop_reason=max_tokens.anthropic-compatible-cc-sp-anthropic/claude-opus-4-6after disabling default redaction: 38.7s stream, 62 SSE events, 26reasoning_contentchunks, 32 content chunks, first reasoning at 1.73s, first content at 23.86s,mixedChunks=0, completed with[DONE].openrouter/deepseek/deepseek-v4-pro@preset/preferthrough the local production build: 70.6s stream, 6,434 SSE events, first reasoning at 1.59s, first content at 48.2s,mixedChunks=0, completed with[DONE]and durablecall_logs.detail_state=ready.anthropic-compatible-cc-sp-anthropic/claude-opus-4-6through the local production build: 71.8s stream, 107 SSE events, 101reasoning_contentchunks,mixedChunks=0, completed with[DONE]and durablecall_logs.detail_state=ready.