Repository navigation
fix(server): send response feedback for translated non-streaming messages - #1358
Conversation
…ages /v1/messages requests translated to a chat provider never notified ResponseFeedbackObservers on the non-streaming path. Chat completions, Responses, native messages and every streaming path already do. An extension tracking prompt-cache usage per session saw nothing for Anthropic-format clients served by a non-Anthropic model.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
github-actions
Bot
added
the
size:XS
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more
View guided diff | Turn on auto-fix | Re-trigger cubic
 [Medium impact] This PR appears safe to merge. Reviews (1) · Last reviewed commit: "fix(server): send response feedback for ..." · Reviewed by Greptile |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Before this change, a non-streaming
/v1/messagesrequest translated to a chat provider (an Anthropic-format client served by a non-Anthropic model) never notifiedResponseFeedbackObservers. Chat completions, Responses, native messages and every streaming path already do.So an extension that tracks prompt-cache usage per session got nothing for these requests. GoModel Pro's
courtesy_replyguardrail (ENTERPILOT/GoModel-pro#58) found this in its e2e tests: such conversations never get past cache stateunknown.Change
messages_handler.go: callnotifyChatResponseFeedbackafter the translated non-streaming response, the same way the chat completions handler does.TestMessages_TranslatedNonStreamingNotifiesFeedbackObservers: the observer is called once, with input and cached tokens. It fails without the fix (expected: 1, actual: 0).Tests
go test ./internal/server/ -run "Messages|Feedback|Rewrite"passes.make test-race, the hot-path performance guard andmake lint.TestAdmin*dashboard tests ininternal/serverfail locally, because the dashboard assets aren't built in this worktree. The change doesn't touch them.Written with Claude Code (Claude Opus 5.5).