Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Sep 3, 2025

Related GitHub Issue

Closes: N/A

Roo Code Task Context (Optional)

No Roo Code task context for this PR

Description

Adds OpenAI Responses API service tier support (flex/priority) for the openai-native provider, including:

  • Types and schemas to model tiers
  • Provider settings to persist selected tier
  • Model metadata for allowed tiers and per-tier pricing
  • API handler logic to pass service_tier when appropriate and compute usage with tier pricing
  • UI: Moves the “Service tier” selector next to the pricing table for better context
  • Tests covering handler behavior for streaming and non-streaming paths

Key changes:

  • packages/types/src/model.ts — Introduce service tier types and schema
  • packages/types/src/provider-settings.ts — Add openAiNativeServiceTier to provider settings schema
  • packages/types/src/providers/openai.ts — Add allowedServiceTiers and serviceTierPricing metadata for supported models
  • src/api/providers/openai-native.ts — Respect openAiNativeServiceTier; include service_tier for non-stream requests; omit for streaming where ignored; usage reflects selected tier pricing
  • src/api/providers/tests/openai-native-service-tier.spec.ts — New tests for tier behavior (streaming vs non-streaming)
  • webview-ui/src/components/settings/ApiOptions.tsx — Plumb apiConfiguration and setApiConfigurationField to ModelInfoView
  • webview-ui/src/components/settings/ModelInfoView.tsx — Add tier selector beside pricing table; show only when model exposes allowedServiceTiers
  • webview-ui/src/components/settings/providers/OpenAI.tsx — Remove inline tier selector; keep API key/base URL controls

Test Procedure

  1. Open Settings, choose the openai-native provider.
  2. Select a model that supports tiers (e.g., o4-mini, o3-family, applicable gpt-4o variants).
  3. Verify the “Service tier” selector appears next to the pricing table in the model info panel.
  4. Toggle between Standard/Flex/Priority (as available).
  5. Confirm the selection persists via openAiNativeServiceTier and pricing reflects the selected tier.
  6. Functional checks:
    • Non-stream completion (e.g., completePrompt): request includes service_tier (e.g., flex).
    • Streaming createMessage: request omits service_tier, but usage reports costs using selected tier pricing.

Pre-Submission Checklist

  • Issue Linked: N/A for this PR (created from staged changes)
  • Scope: Focused on service-tier support for openai-native and related UI/metadata
  • Self-Review: Completed
  • Testing: New tests added for openai-native handler tier behavior
  • Documentation Impact: Considered (no end-user docs changes required)
  • Contribution Guidelines: Acknowledged

Screenshots / Videos

No UI screenshots included (UI change is a selector relocation/visibility).

Documentation Updates

  • No documentation updates are required.

Additional Notes

Created from staged local changes and validated by lint checks. If an approved issue exists, we can update this PR to link it.

Get in Touch

N/A


Important

Adds service tier support for OpenAI Responses API, including backend logic, UI updates, and tests for tier behavior.

  • Behavior:
    • Adds service tier support (flex/priority) for OpenAI Responses API in openai-native provider.
    • Updates openai-native.ts to handle service_tier in non-stream requests and adjust usage based on tier pricing.
    • UI changes in ModelInfoView.tsx and OpenAI.tsx to include tier selector and pricing table.
  • Schemas and Models:
    • Introduces serviceTierSchema in model.ts.
    • Updates provider-settings.ts to include openAiNativeServiceTier.
    • Adds tier metadata in openai.ts for supported models.
  • Tests:
    • Adds openai-native-service-tier.spec.ts to test tier behavior for streaming and non-streaming requests.

This description was created by Ellipsis for 6ca429d. You can customize this summary. It will automatically update as commits are pushed.

Copilot AI review requested due to automatic review settings September 3, 2025 23:39
@hannesrudolph hannesrudolph requested review from cte and jr as code owners September 3, 2025 23:39
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. enhancement New feature or request labels Sep 3, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive support for OpenAI Responses API service tiers (flex/priority) to the openai-native provider, including UI controls, pricing display, and backend integration.

  • Introduces service tier types and schema definitions with tier-specific pricing metadata
  • Implements provider logic to handle tier selection and compute costs using appropriate pricing
  • Moves service tier selector to model info view for better contextual placement alongside pricing tables

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/types/src/model.ts Defines ServiceTier types and schema with tier pricing structure
packages/types/src/provider-settings.ts Adds openAiNativeServiceTier to provider settings
packages/types/src/providers/openai.ts Updates model metadata with allowed tiers and tier-specific pricing
src/api/providers/openai-native.ts Implements tier handling in API requests and usage cost calculation
src/api/providers/tests/openai-native-service-tier.spec.ts Test coverage for tier behavior across streaming/non-streaming scenarios
webview-ui/src/components/settings/ApiOptions.tsx Passes configuration props to enable tier selector
webview-ui/src/components/settings/ModelInfoView.tsx Adds tier selector and pricing table display
webview-ui/src/components/settings/providers/OpenAI.tsx Removes inline tier selector, keeping only API key controls

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

</StandardTooltip>
</div>
<Select
value={(apiConfiguration?.openAiNativeServiceTier as any) || "default"}
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Using as any type assertions bypasses TypeScript's type safety. Consider using proper type casting with ServiceTier type or creating a type-safe conversion function.

Copilot uses AI. Check for mistakes.
<Select
value={(apiConfiguration?.openAiNativeServiceTier as any) || "default"}
onValueChange={(value) =>
setApiConfigurationField("openAiNativeServiceTier", value as any)
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Using as any type assertions bypasses TypeScript's type safety. Consider using proper type casting with ServiceTier type or creating a type-safe conversion function.

Copilot uses AI. Check for mistakes.
Comment on lines 34 to 35
const allowedTiers =
(modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") ?? []
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

The filter condition hardcodes tier names and uses redundant nullish coalescing. Since the expression already provides a default empty array with || [], the ?? [] is unnecessary. Consider: (modelInfo?.allowedServiceTiers || []).filter((tier) => tier !== 'default')

Suggested change
const allowedTiers =
(modelInfo?.allowedServiceTiers || []).filter((tier) => tier === "flex" || tier === "priority") ?? []
(modelInfo?.allowedServiceTiers || []).filter((tier) => ["flex", "priority"].includes(tier))

Copilot uses AI. Check for mistakes.
Comment on lines 294 to 319
(requestBody.service_tier &&
(/service[_ ]tier/i.test(errorMessage) ||
errorMessage.toLowerCase().includes("unsupported service tier") ||
errorMessage.toLowerCase().includes("invalid service tier"))) ||
false
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

The error detection logic is overly complex and the || false at the end is redundant since the boolean expression already evaluates to a boolean. Consider simplifying to: const isTierError = requestBody.service_tier && (/service[_ ]tier/i.test(errorMessage) || errorMessage.toLowerCase().includes('unsupported service tier') || errorMessage.toLowerCase().includes('invalid service tier'))

Suggested change
(requestBody.service_tier &&
(/service[_ ]tier/i.test(errorMessage) ||
errorMessage.toLowerCase().includes("unsupported service tier") ||
errorMessage.toLowerCase().includes("invalid service tier"))) ||
false
requestBody.service_tier &&
(/service[_ ]tier/i.test(errorMessage) ||
errorMessage.toLowerCase().includes("unsupported service tier") ||
errorMessage.toLowerCase().includes("invalid service tier"));

Copilot uses AI. Check for mistakes.
Comment on lines 527 to 552
const isTierError =
requestBody.service_tier &&
(/service[_ ]tier/i.test(errorDetails) ||
errorDetails.toLowerCase().includes("unsupported service tier") ||
errorDetails.toLowerCase().includes("invalid service tier"))
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

This error detection logic is duplicated from the earlier instance (lines 293-298). Consider extracting this into a shared helper function to avoid code duplication and ensure consistent error handling.

Copilot uses AI. Check for mistakes.
const is400Error = sdkErr?.status === 400 || sdkErr?.response?.status === 400
const isPreviousResponseError =
errorMessage.includes("Previous response") || errorMessage.includes("not found")
const isTierError =
Copy link

Choose a reason for hiding this comment

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

The retry logic for handling 400 errors due to an invalid or unsupported 'service_tier' is duplicated. Consider refactoring this block into a shared utility and adding logging to aid debugging.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

{setApiConfigurationField && (
<div className="flex flex-col gap-1 mb-2" data-testid="openai-service-tier">
<div className="flex items-center gap-1">
<label className="block font-medium mb-1">Service tier</label>
Copy link

Choose a reason for hiding this comment

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

Hardcoded strings for service tier labels ('Service tier', 'Standard', 'Flex', 'Priority') and table headers are not internationalized. Please replace them with translation keys to support multiple languages.

Suggested change
<label className="block font-medium mb-1">Service tier</label>
<label className="block font-medium mb-1">{t("settings:modelInfo.serviceTier")}</label>

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I've reviewed the OpenAI Responses API service tier implementation and found it to be well-structured overall. The feature adds valuable flexibility for users to choose between different service tiers based on their needs. I've left some suggestions inline that could improve code maintainability and test coverage.

}
}

if (is400Error && requestBody.service_tier && isTierError) {
Copy link

Choose a reason for hiding this comment

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

I noticed the service tier retry logic is duplicated between the SDK path (lines 333-358) and the SSE fallback path (lines 527-557). Could we extract this into a shared helper method to reduce duplication and ensure consistency?

For example:

}
})

describe("OpenAiNativeHandler - service tier + pricing", () => {
Copy link

Choose a reason for hiding this comment

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

Great test coverage for the basic scenarios! Could we add a few more edge cases to make the tests more robust?

  • Test what happens when the API returns a different tier than requested (e.g., request "flex" but API returns "priority")
  • Test error handling when an invalid tier is provided
  • Test the retry logic when service_tier causes a 400 error

These additional tests would help ensure the implementation handles all possible scenarios gracefully.

</div>
)}
<div className="text-xs text-vscode-descriptionForeground mb-1">
Pricing by service tier (price per 1M tokens)
Copy link

Choose a reason for hiding this comment

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

Should this string be internationalized for consistency with the rest of the UI? I noticed other UI strings use the translation system.

</StandardTooltip>
</div>
<Select
value={(apiConfiguration?.openAiNativeServiceTier as any) || "default"}
Copy link

Choose a reason for hiding this comment

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

Could we improve type safety here by properly typing the openAiNativeServiceTier field instead of casting to ? This would help catch potential type mismatches at compile time.

* Returns a shallow-cloned ModelInfo with pricing overridden for the given tier, if available.
* If no tier or no overrides exist, the original ModelInfo is returned.
*/
private applyServiceTierPricing(info: ModelInfo, tier?: ServiceTier): ModelInfo {
Copy link

Choose a reason for hiding this comment

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

Would it be helpful to add JSDoc comments explaining when service_tier is included vs omitted in requests? This would help future maintainers understand the streaming vs non-streaming behavior difference.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 3, 2025

// Only include reasoningTokens if present
if (reasoningTokens !== undefined) {
;(result as any).reasoningTokens = reasoningTokens
Copy link

Choose a reason for hiding this comment

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

Consider extending the ApiStreamUsageChunk type to include 'reasoningTokens' instead of casting to any for better type safety.

@hannesrudolph hannesrudolph force-pushed the feat/openai-service-tiers-ui branch from ca50ad6 to 31f5205 Compare September 4, 2025 01:08
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 4, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 4, 2025
@daniel-lxs daniel-lxs force-pushed the feat/openai-service-tiers-ui branch from 21e137d to d5833f3 Compare September 4, 2025 16:59

// Validate requested tier against model support; if not supported, omit.
const requestedTier = (this.options.openAiNativeServiceTier as ServiceTier | undefined) || undefined
const allowedTierNames = new Set(model.info.tiers?.map((t) => t.name).filter(Boolean) || [])
Copy link

Choose a reason for hiding this comment

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

Consider extracting the logic that builds the set of allowed tier names (allowedTierNames) into a shared helper function. This pattern is repeated (e.g. in buildRequestBody and completePrompt), and a helper would improve maintainability if the logic ever changes.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

@daniel-lxs daniel-lxs force-pushed the feat/openai-service-tiers-ui branch from d5833f3 to 3e2aed9 Compare September 4, 2025 17:22
hannesrudolph and others added 6 commits September 4, 2025 12:29
- Fixed documentation comment about streaming behavior (service_tier IS sent for streaming)
- Added internationalization for all UI strings (Service tier, Standard, Flex, Priority, pricing table)
- Fixed TypeScript type safety by using proper ServiceTier type instead of 'any' casts
- All tests passing, TypeScript compilation successful
- Added translations for service tier UI strings to all 17 supported languages
- Fixes CI check-translations failure
- Ensures complete i18n coverage for the new feature
@daniel-lxs daniel-lxs force-pushed the feat/openai-service-tiers-ui branch from 3e2aed9 to 6ca429d Compare September 4, 2025 17:50
@dosubot dosubot bot removed the size:XL This PR changes 500-999 lines, ignoring generated files. label Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants