Skip to content

fix(sse): record unknown previous response rejections in the request journal - #16193

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
maxmad64bis:fix/previous-response-not-found-call-log
Oct 15, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
maxmad64bis:fix/previous-response-not-found-call-log

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15306

Summary

When a request references an unknown previous response, it gets a 400 with no row in the request journal, so a client retrying that refusal stays invisible. That 400 is built by hand and returns without calling the rejection logger the file's other rejections share. Now the reply is byte-identical and a row with its status and code is written without delaying it.

Related Issues

  • No linked issue — found while reading the handler on the release base

Target Branch

  • Base is the active release/v3.8.x branch (the highest open release/v*), not main
  • New feature? From 3.8.55 on it is held with the v4-feature label for the v4 channel

Validation

  • Change type: routing
  • Focused tests and category gates from the golden path — inherited: 2 red test file(s) among those touched also fail on the release base (see the banner above)
  • 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/handler-rejection-call-log.test.ts — three new cases: an unknown id writes one 400 row, a known id writes no rejection row, and the 400 body stays byte-identical.

Coverage Notes

  • src/sse/handlers/chat.ts — covered by the three new cases in tests/unit/handler-rejection-call-log.test.ts.
  • src/sse/handlers/admissionRejectionLog.ts — covered by the three new cases in tests/unit/handler-rejection-call-log.test.ts (shared rejection logger used by the new envelope).

Reviewer Notes

@maxmad64bis
maxmad64bis force-pushed the fix/previous-response-not-found-call-log branch 2 times, most recently from 680130d to 2ffe147 Compare October 10, 2026 12:33
@maxmad64bis
maxmad64bis marked this pull request as ready for review October 10, 2026 12:53
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @maxmad64bis — closing this pull request to clear the open queue. The branch fix/previous-response-not-found-call-log is kept, so the commits stay. Comment here if it should be reopened.

@maxmad64bis

Copy link
Copy Markdown
Contributor Author

Hi @diegosouzapw — following up on your note: please reopen this one when you get a chance. The branch is still there on my side, happy to rebase or rework it after the release if needed. Thanks!

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @maxmad64bis — reopening this pull request. The close was a mistake, and it is back in the merge queue. The branch fix/previous-response-not-found-call-log was kept.

@diegosouzapw diegosouzapw reopened this Oct 10, 2026
…journal

The previous_response_not_found 400 returns untouched but now routes through logHandlerRejection; the unknown-id case fails without the change and the other two guard the reply.
@maxmad64bis
maxmad64bis marked this pull request as draft October 11, 2026 12:38
@maxmad64bis
maxmad64bis force-pushed the fix/previous-response-not-found-call-log branch from 2ffe147 to 4ff2777 Compare October 11, 2026 12:38
@maxmad64bis
maxmad64bis marked this pull request as ready for review October 11, 2026 14:36
# Conflicts:
#	src/sse/handlers/admissionRejectionLog.ts
#	src/sse/handlers/chat.ts
@diegosouzapw
diegosouzapw merged commit 01039ae into cheaper-inference:release/v3.8.52 Oct 15, 2026
8 of 9 checks passed
@maxmad64bis
maxmad64bis deleted the fix/previous-response-not-found-call-log branch October 18, 2026 22:37
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