-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: OpenAI Native service tier support (selection, API fallback, resolved-tier costing, tiered pricing UI) #7643
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,7 +25,14 @@ export const ModelInfoView = ({ | |||||||||||||
| }: ModelInfoViewProps) => { | ||||||||||||||
| const { t } = useAppTranslation() | ||||||||||||||
|
|
||||||||||||||
| const infoItems = [ | ||||||||||||||
| // Show tiered pricing table for OpenAI Native when model supports non-standard tiers | ||||||||||||||
| const allowedTiers = | ||||||||||||||
| (modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") ?? [] | ||||||||||||||
|
Comment on lines
+29
to
+30
|
||||||||||||||
| const allowedTiers = | |
| (modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") ?? [] | |
| (modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nullish coalescing operator (??) is redundant here since the logical OR (||) already provides an empty array as fallback.
| (modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") ?? [] | |
| const allowedTiers = | |
| (modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical issue: The properties allowedServiceTiers and serviceTierPricing don't exist in the ModelInfo type. This will cause TypeScript compilation errors. The type definitions need to be added to packages/types/src/model.ts first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User‐facing text in the tiered pricing table header and column labels is hard-coded. Consider internationalizing these strings using t() for consistency.
| Pricing by service tier (price per 1M tokens) | |
| {t('settings:modelInfo.tierPricingHeader')} |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hardcoded text needs internationalization. Consider using:
| Pricing by service tier (price per 1M tokens) | |
| <div className="text-xs text-vscode-descriptionForeground mb-1"> | |
| {t('settings:modelInfo.tierPricingHeader')} | |
| </div> |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated 6 times across this component. Could we extract this into a helper function? For example:
const getTierPrice = (tier: string, priceType: 'inputPrice' | 'outputPrice' | 'cacheReadsPrice') => {
return tierPricing?.[tier]?.[priceType] ?? modelInfo?.[priceType]
}Then use it like: {fmt(getTierPrice('flex', 'inputPrice'))}
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tier pricing fallback logic is duplicated across multiple rows. Consider extracting this into a helper function that takes the tier name and price type to reduce duplication.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,19 +2,21 @@ import { useCallback, useState } from "react" | |||||||||
| import { Checkbox } from "vscrui" | ||||||||||
| import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react" | ||||||||||
|
|
||||||||||
| import type { ProviderSettings } from "@roo-code/types" | ||||||||||
| import type { ModelInfo, ProviderSettings } from "@roo-code/types" | ||||||||||
|
|
||||||||||
| import { useAppTranslation } from "@src/i18n/TranslationContext" | ||||||||||
| import { VSCodeButtonLink } from "@src/components/common/VSCodeButtonLink" | ||||||||||
| import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue, StandardTooltip } from "@src/components/ui" | ||||||||||
|
|
||||||||||
| import { inputEventTransform } from "../transforms" | ||||||||||
|
|
||||||||||
| type OpenAIProps = { | ||||||||||
| apiConfiguration: ProviderSettings | ||||||||||
| setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void | ||||||||||
| selectedModelInfo?: ModelInfo | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export const OpenAI = ({ apiConfiguration, setApiConfigurationField }: OpenAIProps) => { | ||||||||||
| export const OpenAI = ({ apiConfiguration, setApiConfigurationField, selectedModelInfo }: OpenAIProps) => { | ||||||||||
| const { t } = useAppTranslation() | ||||||||||
|
|
||||||||||
| const [openAiNativeBaseUrlSelected, setOpenAiNativeBaseUrlSelected] = useState( | ||||||||||
|
|
@@ -72,6 +74,44 @@ export const OpenAI = ({ apiConfiguration, setApiConfigurationField }: OpenAIPro | |||||||||
| {t("settings:providers.getOpenAiApiKey")} | ||||||||||
| </VSCodeButtonLink> | ||||||||||
| )} | ||||||||||
|
|
||||||||||
| {(() => { | ||||||||||
| const allowedTiers = (selectedModelInfo?.allowedServiceTiers || []).filter( | ||||||||||
| (t) => t === "flex" || t === "priority", | ||||||||||
| ) | ||||||||||
| if (allowedTiers.length === 0) return null | ||||||||||
|
|
||||||||||
| return ( | ||||||||||
| <div className="flex flex-col gap-1 mt-2" data-testid="openai-service-tier"> | ||||||||||
| <div className="flex items-center gap-1"> | ||||||||||
| <label className="block font-medium mb-1">Service tier</label> | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The service tier dropdown label, tooltip content, and option texts ('Service tier', 'Standard', 'Flex', 'Priority') are hard-coded. Please use the translation function (t()) for these UI strings.
Suggested change
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These UI strings need internationalization. The label "Service tier" and the dropdown options "Standard", "Flex", "Priority" should use translation keys:
Suggested change
|
||||||||||
| <StandardTooltip content="For faster processing of API requests, try the priority processing service tier. For lower prices with higher latency, try the flex processing tier."> | ||||||||||
| <i className="codicon codicon-info text-vscode-descriptionForeground text-xs" /> | ||||||||||
| </StandardTooltip> | ||||||||||
| </div> | ||||||||||
|
|
||||||||||
| <Select | ||||||||||
| value={apiConfiguration.openAiNativeServiceTier || "default"} | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical issue: The property |
||||||||||
| onValueChange={(value) => | ||||||||||
| setApiConfigurationField( | ||||||||||
| "openAiNativeServiceTier", | ||||||||||
| value as ProviderSettings["openAiNativeServiceTier"], | ||||||||||
| ) | ||||||||||
| }> | ||||||||||
| <SelectTrigger className="w-full"> | ||||||||||
| <SelectValue placeholder={t("settings:common.select")} /> | ||||||||||
| </SelectTrigger> | ||||||||||
| <SelectContent> | ||||||||||
| <SelectItem value="default">Standard</SelectItem> | ||||||||||
| {allowedTiers.includes("flex") && <SelectItem value="flex">Flex</SelectItem>} | ||||||||||
| {allowedTiers.includes("priority") && ( | ||||||||||
| <SelectItem value="priority">Priority</SelectItem> | ||||||||||
| )} | ||||||||||
| </SelectContent> | ||||||||||
| </Select> | ||||||||||
| </div> | ||||||||||
| ) | ||||||||||
| })()} | ||||||||||
| </> | ||||||||||
| ) | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this implementation complete? The UI is passing
selectedModelInfoto the OpenAI component, but there's no backend implementation to actually use the service tier selection. The following files mentioned in the PR description are missing their implementations:packages/types/src/providers/openai.ts- No service tier supportsrc/api/providers/openai-native.ts- No service tier handlingsrc/api/providers/__tests__/openai-native-service-tier.spec.ts- Test file doesn't existWithout these backend changes, the UI will set the service tier but it won't actually be used in API requests.