Skip to content

Use structured service tiers across core and app-server#20822

Closed
aibrahim-oai wants to merge 22 commits into
mainfrom
dev/service-tier-core-model-info
Closed

Use structured service tiers across core and app-server#20822
aibrahim-oai wants to merge 22 commits into
mainfrom
dev/service-tier-core-model-info

Conversation

@aibrahim-oai
Copy link
Copy Markdown
Collaborator

@aibrahim-oai aibrahim-oai commented May 2, 2026

Summary

  • Add structured ModelServiceTier metadata to shared Codex model info, presets, and app-server model/list payloads.
  • Make ServiceTier string-backed across config, session state, protocol ops, rollout/session events, app-server payloads, schemas, and generated SDK types.
  • Keep central known-id constants such as priority and flex, while allowing backend-provided ids to pass through without app-server enum edits.
  • Send the selected service-tier id directly in Responses requests, and keep the enterprise automatic default on priority.
  • Keep additionalSpeedTiers only as deprecated app-server model-list metadata while clients move to serviceTiers.

API impact

This intentionally changes app-server serviceTier from a closed enum to a string-backed value. Existing fast and flex literals are still accepted as strings, but clients should treat service-tier ids as backend-provided strings and read structured metadata from serviceTiers.

Verification

  • Schema/type fixtures updated.
  • Not run locally per repo guidance; CI will validate.

@aibrahim-oai
Copy link
Copy Markdown
Collaborator Author

aibrahim-oai commented May 2, 2026

@aibrahim-oai aibrahim-oai force-pushed the dev/service-tier-core-model-info branch from 6310040 to a01ae19 Compare May 3, 2026 04:28
@aibrahim-oai aibrahim-oai marked this pull request as ready for review May 3, 2026 21:21
@aibrahim-oai aibrahim-oai requested a review from a team as a code owner May 3, 2026 21:21
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a01ae19545

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/src/config/mod.rs
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

additional_speed_tiers: model.additional_speed_tiers,

P0 Badge Map app-server model tiers into ModelPreset.service_tiers

ModelPreset was changed to use service_tiers, but this constructor still initializes the removed additional_speed_tiers field. That leaves service_tiers unset and will fail to compile (unknown field / missing required field) when this module is built, blocking the TUI app-server session path.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4a5a23a58

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/src/config/mod.rs Outdated
Comment on lines +2678 to +2680
.service_tier_id
.or(cfg.service_tier_id)
.or(deprecated_profile_service_tier)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Preserve profile precedence for deprecated service tier

Reorder fallback resolution so profile-level legacy service_tier still overrides root config. Current order checks cfg.service_tier_id before deprecated_profile_service_tier, so a global service_tier_id can unexpectedly override a selected profile’s legacy tier. This regresses prior layering semantics (profile > root) and can silently switch request tier for users with older profile keys.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0286377ed7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 268 to +269
#[serde(default)]
pub additional_speed_tiers: Vec<String>,
pub service_tiers: Vec<ModelServiceTier>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Keep legacy additional_speed_tiers deserializable

ModelInfo now only reads service_tiers. Cached /models payloads from prior versions still contain additional_speed_tiers; on cache load those entries are dropped and service_tiers defaults to empty. In offline/OnlineIfUncached flows this removes fast-tier metadata and changes behavior until a successful network refresh. Add a serde alias/migration path for the old key.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ede4afd5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/src/config/mod.rs Outdated
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aef60ceb78

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Some(service_tier) => Some(service_tier.to_string()),
None => None,
},
service_tier: service_tier.map(|service_tier| service_tier.to_string()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Normalize legacy fast in runtime service-tier updates

ResponsesApiRequest now forwards service_tier verbatim. Turn-level overrides can carry ServiceTier("fast") directly and do not pass through Config::load normalization, so legacy fast can reach request construction unchanged. That creates inconsistent behavior versus config-based paths (which normalize to priority) and can bypass priority-based fast-mode handling.

Suggested change
service_tier: service_tier.map(|service_tier| service_tier.to_string()),
service_tier: service_tier.map(|service_tier| {
if service_tier.as_ref() == "fast" {
"priority".to_string()
} else {
service_tier.to_string()
}
}),

Useful? React with 👍 / 👎.

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.

1 participant