Add prompt caching to OpenAI-compatible custom model info #1562
Conversation
🦋 Changeset detectedLatest commit: 4e6655b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dosubot
bot
added
size:L
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| style={{ fontSize: "12px" }} | ||
| /> | ||
| </div> | ||
| <div className="text-sm text-vscode-descriptionForeground [pt"> |
There was a problem hiding this comment.
Typo detected in the className: "[pt" appears to be incomplete. Please correct it to the intended class (e.g., 'pt-...').
| <div className="text-sm text-vscode-descriptionForeground [pt"> | |
| <div className="text-sm text-vscode-descriptionForeground pt-2"> |
| } | ||
| type="text" | ||
| style={{ | ||
| borderColor: (() => { |
There was a problem hiding this comment.
Consider extracting the repeated inline borderColor styling logic into a helper function to improve readability and maintainability.
There was a problem hiding this comment.
@ellipsis-dev Show us the equivalent of this logic using cn instead of an inline style object.
There was a problem hiding this comment.
@mrubens - For reference, it (sort of) produced:
className={cn("w-full", {
"border-vscode-input-border": !apiConfiguration?.openAiCustomModelInfo?.cacheWritesPrice && apiConfiguration?.openAiCustomModelInfo?.cacheWritesPrice !== 0,
"border-vscode-charts-green": apiConfiguration?.openAiCustomModelInfo?.cacheWritesPrice >= 0,
"border-vscode-errorForeground": apiConfiguration?.openAiCustomModelInfo?.cacheWritesPrice < 0,
})}This is spot on, but pretty verbose. Might want to extract apiConfiguration?.openAiCustomModelInfo?.cacheWritesPrice into a local var.
|
|
||
| return value >= 0 | ||
| ? "var(--vscode-charts-green)" | ||
| : "var(--vscode-errorForeground)" |
There was a problem hiding this comment.
Is this handling the case when cacheWritesPrice is negative? Do we need to handle this case?
There was a problem hiding this comment.
Yeah... I basically just copied the inputs for the uncached inputs and outputs. Not sure it's necessary in either.
mrubens
force-pushed
the
add_caching_to_open_ai_custom_model_info
branch
from
e72aba6 to
4e6655b
Compare
…compatible custom model info);
| } | ||
| type="text" | ||
| style={{ | ||
| borderColor: (() => { |
There was a problem hiding this comment.
There is repeated logic for computing the borderColor and for parsing numeric values in the cache price fields. Please consider extracting these into a helper function to reduce duplication and improve maintainability.
| : "var(--vscode-errorForeground)" | ||
| })(), | ||
| }} | ||
| onChange={handleInputChange("openAiCustomModelInfo", (e) => { |
There was a problem hiding this comment.
Notice that the cache price text fields are using the onChange event, while other similar text inputs in the form use onInput. For consistency and predictable behavior across the form, please review whether onInput should be used instead.
| onChange={handleInputChange("openAiCustomModelInfo", (e) => { | |
| onInput={handleInputChange("openAiCustomModelInfo", (e) => { |
There was a problem hiding this comment.
Pretty sure there was some reason we don't - based on them truncating decimal points too early or something.
Context
Per #1549, we don't currently have a way for users to configure prompt caching for OpenAI-compatible providers.
Implementation
This PR adds a checkbox to enable prompt caching and input fields for the cache read/write prices.
Screenshots
How to Test
That I'm not sure about... @dleen would you be up for helping me figure out how to test this out?
Important
Adds prompt caching configuration for OpenAI-compatible models in
ApiOptions.tsx, including UI elements for enabling caching and setting cache pricing.ApiOptions.tsx.ApiOptions.tsxto include new UI elements for prompt caching settings.This description was created by
for 4e6655b. It will automatically update as commits are pushed.