perf(models): avoid redundant canonicalization work - #139
Conversation
Keep lexical suffix cleanup borrow-preserving so dated and free-channel model IDs allocate only their final identity. Short-circuit canonical OpenAI base models and stop dispatch after the OpenAI branch, removing the extra work that regressed gpt4o_mini_date while preserving the terminal-to-lexical-to-semantic pipeline.
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough本次变更重构模型 ID 的全局后缀清洗，改用 Changes模型 ID 规范化
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 improve performance by 16.16%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | canonical_model_id_cleanup[gpt4o_mini_date] |
7.4 µs | 6.4 µs | +16.16% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing agent/optimize-model-canonicalization (248c4b3) with personal/local-clients (802bb29)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/tokscale-core/src/model_aliases.rs (1)
234-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
canonical_gpt_5_6_base尾部兜底分支已变为死代码。第 234-236 行新增的早返回已经处理了
canonical_gpt_5_6_base(model)返回Some的所有情况（无论是否与model相同都会 return）。函数体内model（第 232 行绑定后）此后从未被重新赋值，因此第 274-277 行再次调用canonical_gpt_5_6_base(model)时必然返回None（否则第 234 行早已 return），.filter(...).map(...)也永远不会产生Some。这段代码是本次重构留下的死代码，与本 PR「避免冗余分配/计算」的目标本身相悖。建议直接移除该冗余尾部分支，用显式
None收尾，减少误导性代码。♻️ 移除死代码
- - canonical_gpt_5_6_base(model) - .filter(|canonical| *canonical != model) - .map(str::to_string) + + None }Also applies to: 274-277
🤖 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-core/src/model_aliases.rs` around lines 234 - 243, Remove the redundant trailing canonical_gpt_5_6_base(model) fallback after the existing early return, since it can never produce Some. End the function explicitly with None while preserving the preceding gpt-4.1 and OpenAI base-model handling.
🤖 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-core/src/model_aliases.rs`:
- Around line 234-243: Remove the redundant trailing
canonical_gpt_5_6_base(model) fallback after the existing early return, since it
can never produce Some. End the function explicitly with None while preserving
the preceding gpt-4.1 and OpenAI base-model handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31131c77-f39c-48e1-8249-7b65a5bac3cf
📒 Files selected for processing (1)
crates/tokscale-core/src/model_aliases.rs
1 participant
Summary
Root cause
The GPT-5.6 canonicalization fix correctly established the pipeline as terminal normalization, lexical decoration cleanup, then semantic canonicalization. Its lexical stage eagerly converted the normalized model into an owned
String, copied the full decorated ID, truncated it in place, and then sent an already-canonical OpenAI base through the remaining parser checks.That extra allocation work and parsing caused the
gpt4o_mini_dateCodSpeed benchmark in PR #136 to regress by 16.56%.Impact
Canonical model behavior remains unchanged, including GPT-5.6 alias idempotence and the existing Claude, GLM, free-channel, release-date, and reasoning-effort identities. Terminal suffix removal now borrows the canonical slice until a final owned ID is required, and canonical OpenAI base IDs avoid redundant parsing.
Validation
cargo test— 2,137 passed, 5 ignoredcargo clippy --workspace --all-targets -- -D warningscargo build --release -p tokscale-clicanonical_model_id_cleanup/gpt4o_mini_daterepeated local baseline: approximately 273–276 nsReferences
Summary by CodeRabbit
free等模型后缀及其变体，减少模型名称解析异常。