-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Fix is_api_message to correctly exclude reasoning messages #4303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix is_api_message to correctly exclude reasoning messages #4303
Conversation
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
1f548e2
to
ea17182
Compare
recheck |
@codex review |
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. |
Hi @jif-oai, could you please take a look at this PR when you have time? |
0a219fe
to
94a763b
Compare
rebased onto main |
94a763b
to
0b9e780
Compare
Problem
The
is_api_message
function inconversation_history.rs
had a misalignment between its documentation and implementation:true
forResponseItem::Reasoning
, meaning reasoning messages were incorrectly treated as API messagesThis inconsistency could lead to reasoning messages being persisted in conversation history when they should be filtered out.
Root Cause
Investigation revealed that reasoning messages are explicitly excluded throughout the codebase:
Chat completions API (lines 267-272 in
chat_completions.rs
) omits reasoning from conversation history:Existing tests like
drops_reasoning_when_last_role_is_user
andignores_reasoning_before_last_user
validate that reasoning should be excluded from API payloadsSolution
Fixed the
is_api_message
function to align with its documentation and the rest of the codebase:Testing
This ensures reasoning messages are consistently excluded from API message processing across the entire codebase.