Skip to content

fix(usage): remove finished pending request by id instead of oldest-first - #14797

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/pending-request-by-id
Sep 25, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/pending-request-by-id

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #14547

Summary

The in-progress request list removes the oldest entry for an account and model when any request for that pair finishes, so a still-running request can vanish from the list while finished ones stay. This change removes the entry that actually finished, by its tracking id, on both the disconnect and the normal streaming-completion paths, so the dashboard keeps showing the request that's still running. An unknown id removes nothing instead of taking another entry.

Related Issues

Related to #12650 (pending-tracking id shared across combo target retries, which this change preserves) and #12910 (finalize-by-exact-id precedent in the same family).

Validation

  • Change type: other
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/usage-pending-removal-by-id.test.ts (new, 5 cases: out-of-order finish keeps the survivor, id-less finish keeps oldest-first fallback, unknown id removes nothing, controller-disconnect by id, lone-request double-removal no-op with counter guard — 5/5 green on the head).
  • Neighbors re-run one by one, all green: sweep 16/16, non-streaming-finalization 5/5, usagehistory-helpers 30/30, call-log-in-memory 5/5, stream-chunks-lifecycle 14/14, body-timeout 8/8.

Coverage Notes

  • New removal path covered by the new test plus the neighbor suites above; no coverage gate movement expected.

Reviewer Notes

  • chatCore.ts streaming paths pass the finishing id (onStreamComplete → finalizeStreamRequestLog by id); callers without an id in scope keep oldest-first behavior.
  • No rebaseline: stream.ts stays within its frozen cap via the extracted 21-line helper.

@maxmad64bis
maxmad64bis force-pushed the fix/pending-request-by-id branch from 641fdf5 to 3326bea Compare September 24, 2026 18:58
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 24, 2026 19:05
@maxmad64bis
maxmad64bis force-pushed the fix/pending-request-by-id branch from 3326bea to c79bb23 Compare September 24, 2026 19:35
@maxmad64bis
maxmad64bis marked this pull request as draft September 24, 2026 23:42
@maxmad64bis
maxmad64bis force-pushed the fix/pending-request-by-id branch from c79bb23 to e0b5b16 Compare September 25, 2026 00:23
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 25, 2026 01:01
@diegosouzapw
diegosouzapw merged commit dda0448 into diegosouzapw:release/v3.8.51 Sep 25, 2026
19 of 25 checks passed
fouadSalkini added a commit to fouadSalkini/OmniRoute that referenced this pull request Sep 26, 2026
Slice 2/3 rewrote open-sse/handlers/chatCore.ts from an older snapshot,
silently undoing four merged fixes. Rebuild the file as the base version
plus only this PR's own hunks: stripNonStreamingForwardedHeaders on the
non-streaming path and apiKeyInfo on the streaming headers meta.

Restored:
- handleChatCore -> withResilienceActionsContext -> handleChatCoreInner
  wrapper, previousResponseResumed handling, notePreviousResponseResumed
  and the three noteBufferedVerdictOutcome calls (diegosouzapw#14810)
- pendingRequestId in every trackPendingRequest call, the pipeline
  options and finalizeToolLoopError (diegosouzapw#14797)
- the full-UUID traceId and its collision comment (diegosouzapw#14474)
- correlationId on buildContinuationLogHooks (diegosouzapw#14793)
@maxmad64bis
maxmad64bis deleted the fix/pending-request-by-id branch September 30, 2026 00:22
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.

2 participants