Skip to content

Conversation

@joemanley201
Copy link
Contributor

@joemanley201 joemanley201 commented Feb 8, 2025

Description

  • Add ability to set temperatures for models at the configuration level
  • Add tests
  • Move some magic numbers to constants and reuse them
Screen.Recording.2025-02-09.at.8.21.55.PM.mov

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Locally
  • Automated tests

Checklist:

  • My code follows the patterns of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Additional context

Related Issues

Reviewers


Important

Adds per-configuration model temperature settings with UI support and tests.

  • Behavior:
    • Adds modelTemperature option to ApiHandlerOptions in api.ts for configuring model temperatures.
    • Updates handlers like AnthropicHandler, AwsBedrockHandler, GeminiHandler, etc., to use modelTemperature with a default fallback.
    • Introduces TemperatureControl component in TemperatureControl.tsx for UI temperature configuration.
  • UI:
    • Integrates TemperatureControl in ApiOptions.tsx to allow users to set model temperature.
  • Tests:
    • Adds tests for TemperatureControl in TemperatureControl.test.tsx to verify UI behavior.
  • Misc:
    • Moves magic numbers to constants for better reuse and clarity.

This description was created by Ellipsis for d4b7a60. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Feb 8, 2025

⚠️ No Changeset found

Latest commit: bcd54d5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mrubens mrubens force-pushed the temperature_control branch from 3011dff to 5a24542 Compare February 8, 2025 05:13
@mrubens mrubens force-pushed the temperature_control branch from 5a24542 to bc9773f Compare February 8, 2025 05:21
@joemanley201 joemanley201 marked this pull request as ready for review February 8, 2025 07:20
}}>
<VSCodeCheckbox
checked={isCustomTemperature}
onChange={(e: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

When the checkbox is toggled on and the current temperature value is undefined, nothing is set. Consider providing a default value (e.g., 0.5) so that enabling custom temperature immediately shows a sensible slider value.

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 the behavior we want - mainly because of how we're relying on the value to toggle checkbox on or off

@joemanley201 joemanley201 changed the title [WIP] Configure per-configuration temperature Configure per-configuration temperature Feb 8, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@cte
Copy link
Collaborator

cte commented Feb 8, 2025

Nice!

@nissa-seru
Copy link
Contributor

Awesome! Test-driving this locally - when I check the box and try to drag the slider around to configure the temperature, I notice it is very laggy (to the extent that it takes some time to settle after I finish dragging it.) Might be worth a quick check to see if you can repro this since it's a bit unexpected re UI behavior

@mrubens
Copy link
Collaborator

mrubens commented Feb 8, 2025

Awesome! Test-driving this locally - when I check the box and try to drag the slider around to configure the temperature, I notice it is very laggy (to the extent that it takes some time to settle after I finish dragging it.) Might be worth a quick check to see if you can repro this since it's a bit unexpected re UI behavior

Maybe we need a debounce in these slider callbacks?

@joemanley201
Copy link
Contributor Author

Awesome! Test-driving this locally - when I check the box and try to drag the slider around to configure the temperature, I notice it is very laggy (to the extent that it takes some time to settle after I finish dragging it.) Might be worth a quick check to see if you can repro this since it's a bit unexpected re UI behavior

ooh thanks for trying this out. Do you see this for other sliders we have in our settings as well? or just this one?

@nissa-seru
Copy link
Contributor

nissa-seru commented Feb 8, 2025 via email

@hannesrudolph
Copy link
Collaborator

@joemanley201
image
image

I have sonnet selected through OR and I am able to select a temp of 2 but sonnet only allows for 0 < x < 1.

@joemanley201
Copy link
Contributor Author

sonnet

Good catch - will take a look at this now.

@joemanley201
Copy link
Contributor Author

#912
Tested this after and it seems to be good. When I intentionally try to click on done immediately after changing the value, I can see the change not getting picked up (both onBlur and onChange). I think it's very rare for the end user to run into this.

As a follow up, I think we should audit our input boxes - or just add a tiny bit of more delay and introduce a loader.

@mrubens mrubens merged commit ceec089 into main Feb 10, 2025
6 checks passed
@mrubens mrubens deleted the temperature_control branch February 10, 2025 15:48
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.

6 participants