Skip to content
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

fix: ignore top_p if extended thinking is enabled for playground #6720

Merged
merged 4 commits into from
Mar 6, 2025

Conversation

RogerHYang
Copy link
Contributor

resolves #6711

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Mar 6, 2025
and "type" in invocation_parameters["thinking"]
and invocation_parameters["thinking"]["type"] == "enabled"
):
invocation_parameters.pop("top_p", None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this bubble up an error to the UI somehow? It feels like something a user shouldn't be able to input

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just a stopgap for now

Copy link
Contributor

@cephalization cephalization Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the invocation parameter system does not have enough levers to allow us to change the presence of a field based on another in the UI, so we need to process these kinds of things somewhere. If the server is going to be invoking the anthropic client, who declared this validation rule, then the server might as well be responsible for stripping the extra key imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the other hand, this is changing the user's input silently, instead of rejecting it, since if this is rejected, user has no other way to change their input, so it's not really validation.

@RogerHYang RogerHYang merged commit 7549f11 into main Mar 6, 2025
54 checks passed
@RogerHYang RogerHYang deleted the remove-top-p-when-thinking branch March 6, 2025 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S This PR changes 10-29 lines, ignoring generated files.
Projects
Status: Done
3 participants