fix(models): canonicalize GPT-5.6 reasoning efforts - #136
Conversation
Recognize Sol, Terra, and Luna as GPT-5.6 model identities so Factory custom-entry effort suffixes are removed before grouping and pricing. Handle max only for the GPT-5.6 family to preserve model identities such as gpt-5.1-codex-max, and bump the canonicalization revision so cached source rows are rebuilt.
Walkthrough本次变更新增 GPT-5.6 变体的 canonicalization 与 reasoning effort 识别,更新解析修订号,补充解析和定价测试,并调整 wrapped 数据的模型聚合及显示名称生成。 ChangesGPT-5.6 规范化与解析
Wrapped 模型聚合与显示
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will degrade performance by 16.56%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
Exercise Droid discovery, parsing, cache-version selection, model canonicalization, and exact pricing from a Factory session fixture instead of testing the finalizer in isolation.
Key wrapped model aggregation by canonical model ID instead of formatted display text, and render Sol, Terra, and Luna names explicitly so their usage and cost remain separate.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Parse the same Factory session twice through one cache home and require the warm result to retain the same canonical model, provider, reasoning tokens, and exact price as the cold result.
Resolve the official gpt-5.6 alias to gpt-5.6-sol before stripping reasoning effort, so bare, dashed, parenthesized, and Factory custom-entry forms share one grouping and exact-pricing identity. Cover end-to-end pricing and wrapped aggregation while preserving gpt-5.1-codex-max.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/tokscale-cli/src/commands/wrapped.rs (1)
381-407: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win避免对已存在的模型条目重复计算
format_model_name/get_provider_from_model。当
model_id已存在于model_map中时,format_model_name(model_id)与get_provider_from_model(model_id)仍会被无条件计算,仅在or_insert_with闭包内才实际使用;同时if model_entry.provider.is_none()回填分支因两函数均为关于model_id的纯函数、且 key 不变而永远是空操作,属死代码。建议把这两次计算下沉到or_insert_with闭包内,仅在首次插入时执行。♻️ 建议的重构
fn accumulate_wrapped_model( model_map: &mut HashMap<String, WrappedRankedEntry>, model_id: &str, cost: f64, tokens: i64, ) { - let model_name = format_model_name(model_id); - let provider = get_provider_from_model(model_id); let model_entry = model_map .entry(model_id.to_string()) .or_insert_with(|| WrappedRankedEntry { - name: model_name, + name: format_model_name(model_id), client_id: None, - provider, + provider: get_provider_from_model(model_id), cost: 0.0, tokens: 0, }); - if model_entry.provider.is_none() { - model_entry.provider = provider; - } model_entry.cost += cost; model_entry.tokens = model_entry .tokens .checked_add(tokens) .expect("wrapped model token total exceeds i64::MAX"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-cli/src/commands/wrapped.rs` around lines 381 - 407, Update accumulate_wrapped_model so format_model_name and get_provider_from_model are evaluated only inside the or_insert_with initializer for newly inserted entries. Remove the provider-is-none backfill branch, since existing entries should retain their initialized provider; leave the cost and checked token accumulation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/tokscale-cli/src/commands/wrapped.rs`:
- Around line 381-407: Update accumulate_wrapped_model so format_model_name and
get_provider_from_model are evaluated only inside the or_insert_with initializer
for newly inserted entries. Remove the provider-is-none backfill branch, since
existing entries should retain their initialized provider; leave the cost and
checked token accumulation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3555c4c-5244-4a93-a499-b3198e8c74ea
📒 Files selected for processing (5)
crates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-core/src/adapters/mod.rscrates/tokscale-core/src/lib_tests.rscrates/tokscale-core/src/model_aliases.rscrates/tokscale-core/src/sessions/droid.rs
Keep the reported custom:gpt-5.6-sol-xhigh Factory session alongside the official alias fixture so cold and warm end-to-end parsing and exact pricing cover both source identities.
Strip global free-channel and release decorations before source-specific semantic canonicalization so decorated GPT-5.6 aliases reach gpt-5.6-sol in one pass. Keep combined GLM tiers canonical and lock the affected identities to an idempotent public normalization result.
Summary
gpt-5.6alias togpt-5.6-sol, including reasoning efforts, free-channel tags, release suffixes, and Factory custom-entry formsmaxas a reasoning effort only for the GPT-5.6 family, preserving model identities such asgpt-5.1-codex-maxRoot cause
Factory stores the selected custom-entry ID in Droid session records, for example
custom:gpt-5.6-sol-xhigh. Tokscale already removed thecustom:prefix and recognizedxhigh, but its OpenAI base-model predicate did not recognize the new Sol, Terra, and Luna family identities. The decorated ID therefore reached exact pricing unchanged and remained unpriced.OpenAI also defines
gpt-5.6as an alias that routes togpt-5.6-sol. Leaving the alias as a separate canonical ID would split grouping, exact pricing, and wrapped rankings from the explicit Sol model.Global free-channel and release suffixes were removed only after source-specific alias resolution. Inputs such as
gpt-5.6-free,gpt-5.6-2607, andgpt-5.6-high-freetherefore needed a second canonicalization pass to reach Sol. The pipeline now removes lexical decorations before semantic alias and reasoning-effort resolution.Wrapped reporting also used formatted display text as its aggregation key. Because the GPT-5 formatter did not render the three GPT-5.6 family names, their canonical IDs could collapse into one displayed model.
Impact
Droid and other local-client sessions using GPT-5.6 family entries now group under
gpt-5.6-sol,gpt-5.6-terra, orgpt-5.6-lunaand resolve their catalog-backed token cost. Thegpt-5.6alias and its decorated forms share the Sol identity in one idempotent canonicalization pass. Wrapped output retains the three family tiers separately while merging the alias with explicit Sol. Reasoning token accounting remains unchanged.Validation
cargo test— 2,117 passed, 5 ignoredcargo clippy --workspace --all-targets -- -D warningscargo build --release -p tokscale-cli-free,:free,(free), short release, effort-plus-free, and idempotencecustom:gpt-5.6-high-freetogpt-5.6-soland matched an exact Sol-only custom price at$0.000024References