fix(core): canonicalize release suffixes across local model families - #86
Conversation
Walkthrough本次变更扩展了模型来源 ID 归一化与发布日期后缀剥离逻辑，并同步调整 Changes模型 ID 归一化逻辑扩展
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
makoMakoGo
left a comment
makoMakoGo
left a comment
There was a problem hiding this comment.
Reviewed. Overall this is clean: putting release/build suffix cleanup in source-model canonicalization keeps the aggregation and pricing layers simple, and the parser-level expectation changes line up with the new canonical IDs. No blocking issues from this diff.
One small coverage note inline: Longcat is the only touched family whose canonicalizer still doesn't take the terminal model segment first, so provider-scoped source IDs won't canonicalize the same way Qwen/Mimo/DeepSeek/Mistral do.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/tokscale-core/src/model_aliases.rs (1)
287-337: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win非 ASCII 模型名会在这里触发 panic
model_id没有 ASCII 约束；这里直接按字节下标切&str，只要输入里带多字节字符且长度落到这些分支，就会在解析时 panic。入口处先拒绝非 ASCII，或改用str::get。🤖 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 287 - 337, Non-ASCII model names can panic in the suffix-stripping helpers because strip_full_release_date_suffix and strip_short_release_date_suffix index into model by byte offsets without validating ASCII. Fix this by either rejecting non-ASCII input before these helpers are reached or replacing the direct slice/index access with safe str::get-based checks. Keep the existing date-suffix matching behavior intact while ensuring the code never assumes single-byte characters in model_id.
🤖 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.
Inline comments:
In `@crates/tokscale-core/src/model_aliases.rs`:
- Around line 148-152: The Kimi source model canonicalization in
canonicalize_kimi_source_model is too strict because strip_release_suffix can
reduce values like kimi-k2-0711 to kimi-k2, which no longer matches
starts_with("kimi-k2-") and causes them to be skipped. Update the matching logic
to accept the base kimi-k2 form as well (for example by loosening the prefix
check in canonicalize_kimi_source_model), and add or adjust a test around this
path to cover the date-only variant and confirm the intended normalization
behavior.
---
Outside diff comments:
In `@crates/tokscale-core/src/model_aliases.rs`:
- Around line 287-337: Non-ASCII model names can panic in the suffix-stripping
helpers because strip_full_release_date_suffix and
strip_short_release_date_suffix index into model by byte offsets without
validating ASCII. Fix this by either rejecting non-ASCII input before these
helpers are reached or replacing the direct slice/index access with safe
str::get-based checks. Keep the existing date-suffix matching behavior intact
while ensuring the code never assumes single-byte characters in model_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc268311-8412-4133-b82c-6b0db97a4a3b
📒 Files selected for processing (3)
crates/tokscale-core/src/lib.rscrates/tokscale-core/src/model_aliases.rscrates/tokscale-core/src/sessions/junie.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
c6ed20a to
16aa4bd
Compare
|
/juya review |
![juya-review-bot[bot]](https://avatars.githubusercontent.com/in/4074416?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Summary
Tests
Summary by cubic
Canonicalizes local source model IDs by stripping release/build date suffixes across OpenAI, Qwen, Kimi, Grok, Mimo, DeepSeek, and Longcat. Tightens matching to avoid false positives, enforces ASCII on the terminal segment, and supports scoped Longcat IDs (e.g., meituan/longcat-flash-3b-all-quant-0203-eagle3 → longcat-flash-3b; gpt-4.1-2025-04-14 → gpt-4.1; deepseek-v3-0324 → deepseek-v3).
New Features
k2.*(incl. instruct builds), Grokgrok-code-fast-1, Mimo variants, and DeepSeek (v3 andr1distill forms); handles scoped Longcat IDs.Bug Fixes
mistral-small-2603).tokscale-coreandtokscale-clito expect canonical IDs (e.g.,gpt-4.1,deepseek-v3).Written for commit fc2ab2c. Summary will update on new commits.
Summary by CodeRabbit
model_id更简洁一致（如归一为基础模型名）。model_id过长或不一致的问题，避免出现未归一化的长标识。