Remove upstream synthetic.new source - #41
Conversation
Reviewer's GuideRemoves the upstream File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/tokscale-core/src/pricing/custom.rs" line_range="252" />
<code_context>
}
- let normalized_key = normalize_synthetic_model(model_id).to_lowercase();
+ let normalized_key = normalize_gateway_model_path(model_id).to_lowercase();
if normalized_key != raw_key {
if let Some(pricing) = self.models.get_key_value(&normalized_key) {
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid double-lowercasing in model id normalization to reduce unnecessary allocation.
`normalize_gateway_model_path` already lowercases its input (it begins with `let lower = model_id.to_lowercase();` and only uses substrings of that), so the extra `.to_lowercase()` here just adds an unnecessary allocation. You can set `normalized_key` to `normalize_gateway_model_path(model_id)` directly to keep behavior identical while avoiding the extra work.
```suggestion
let normalized_key = normalize_gateway_model_path(model_id);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request completely removes the "Synthetic" (synthetic.new) client integration, including its meta-client filters, SQLite parsing, documentation, and frontend assets across the codebase. In crates/tokscale-core/src/pricing/custom.rs, the normalize_synthetic_model function was replaced with normalize_gateway_model_path. A review comment correctly points out a redundant .to_lowercase() call on the result of normalize_gateway_model_path which can be removed to avoid unnecessary heap allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/tokscale-cli/src/main.rs (1)
1143-1157:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift不要把已移除的 client 配置静默回退成“全部客户端”。
如果用户的
settings.json里还残留defaultClients: ["synthetic"],这里会先把它当作 unknown 丢掉,然后把“解析后为空”解释成“未配置默认值”,最终让tokscale tui/tokscale models/ warm-cache 都退回到全量 client。Issue#34明确要求移除 Synthetic 时不要引入 silent fallback;这里至少需要把“配置里引用了已移除 client”变成用户可见的错误或迁移提示,而不是悄悄扩大结果集。build_client_filter_with_defaults()和resolve_default_tui_filter_set_with()需要一起修,不然两条路径还会继续同步地做错事。Also applies to: 5116-5124, 6033-6038
crates/tokscale-cli/src/tui/cache.rs (1)
614-667:⚠️ Potential issue | 🟠 Major | ⚡ Quick win移除
includeSynthetic后需要升级缓存 schema。现在这里把客户端精确匹配完全收敛到
enabledClients,但旧 JSON 里的includeSynthetic会被serde静默忽略。这样旧的 v13 缓存如果是enabledClients=["claude"]且includeSynthetic=true,本次读取会被当成精确命中,直接把已经删除的 Synthetic 数据和统计值展示出来,直到后台刷新覆盖。既然磁盘键语义已经变了,这里至少应该 bumpCACHE_SCHEMA_VERSION,并补一个“旧 v13 + includeSynthetic => Miss”的回归测试。Also applies to: 720-747
🤖 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/tui/cache.rs` around lines 614 - 667, Bump the disk cache schema version and add a regression test so old v13 JSONs that contained the removed includeSynthetic field do not falsely hit the cache: increment CACHE_SCHEMA_VERSION, update any schema constants, and modify load_cache/CachedTUIData expectations so that caches serialized with the old includeSynthetic are treated as CacheResult::Miss (you can detect this in tests by deserializing a v13 JSON string containing enabledClients=["claude"] plus includeSynthetic=true and asserting load_cache returns Miss); also add a unit test covering the same scenario referenced near the logic that calls cache_clients_match_exact to ensure the old payloads are rejected.
🤖 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-cli/src/tui/mod.rs`:
- Around line 176-180: The bg_clients vector is built directly from
enabled_clients (HashSet<ClientFilter>) which yields an arbitrary order, causing
cold-start ordering to differ from the stable order produced by
App::scan_clients() and thus causing DataLoader::aggregate_messages (which uses
first-seen ordering) to produce inconsistent client orderings; fix by
constructing bg_clients using the same stable ordering as App::scan_clients()
(e.g., call or reuse App::scan_clients() result or apply the same sort/ordering
routine used there and then filter by enabled_clients) so bg_clients is
deterministic and matches the reload path used by
DataLoader::aggregate_messages.
In `@crates/tokscale-core/src/pricing/custom.rs`:
- Around line 320-324: The current normalization only strips
"accounts/.../models/..." so router gateway paths like
"accounts/fireworks/routers/kimi-k2p6-turbo" never resolve to the base model id;
update the logic in the same function that uses lower.strip_prefix("accounts/")
to also check rest.split_once("/routers/") and return the segment after
"/routers/" when present (mirroring the existing "/models/" branch), and add a
unit/regression test that calls
lookup("accounts/fireworks/routers/kimi-k2p6-turbo") and asserts it matches the
base key "kimi-k2p6-turbo".
---
Outside diff comments:
In `@crates/tokscale-cli/src/tui/cache.rs`:
- Around line 614-667: Bump the disk cache schema version and add a regression
test so old v13 JSONs that contained the removed includeSynthetic field do not
falsely hit the cache: increment CACHE_SCHEMA_VERSION, update any schema
constants, and modify load_cache/CachedTUIData expectations so that caches
serialized with the old includeSynthetic are treated as CacheResult::Miss (you
can detect this in tests by deserializing a v13 JSON string containing
enabledClients=["claude"] plus includeSynthetic=true and asserting load_cache
returns Miss); also add a unit test covering the same scenario referenced near
the logic that calls cache_clients_match_exact to ensure the old payloads are
rejected.
🪄 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: b03224f8-acd5-417e-a021-e86e77d95993
📒 Files selected for processing (21)
README.ja.mdREADME.ko.mdREADME.mdREADME.zh-cn.mdcrates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-cli/src/main.rscrates/tokscale-cli/src/tui/app.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/data/mod.rscrates/tokscale-cli/src/tui/mod.rscrates/tokscale-cli/src/tui/settings.rscrates/tokscale-cli/src/tui/ui/dialog/source_picker.rscrates/tokscale-core/src/lib.rscrates/tokscale-core/src/pricing/aliases.rscrates/tokscale-core/src/pricing/custom.rscrates/tokscale-core/src/scanner.rscrates/tokscale-core/src/sessions/mod.rscrates/tokscale-core/src/sessions/synthetic.rspackages/frontend/src/components/SourceLogo.tsxpackages/frontend/src/lib/constants.tspackages/frontend/src/lib/types.ts
💤 Files with no reviewable changes (6)
- crates/tokscale-core/src/sessions/synthetic.rs
- packages/frontend/src/lib/constants.ts
- packages/frontend/src/lib/types.ts
- packages/frontend/src/components/SourceLogo.tsx
- crates/tokscale-cli/src/commands/wrapped.rs
- crates/tokscale-core/src/sessions/mod.rs
关联 issue:Closes #34
依赖关系:建议先合入 #40。这个 PR 的实现语义与 #40 中的 ADR 一致,但 base 仍按要求指向
personal/local-clients。变更
synthetic.newsource/client/filter/session reader。--client synthetic、隐藏--syntheticlegacy flag、source picker Synthetic entry、TUI cacheincludeSynthetickey。model = "<synthetic>"placeholder drop logic。sessions::synthetic迁到 custom pricing helper,不再依赖 Synthetic source/client vocabulary。原因
synthetic.newsource/client 与 Claude Code 的<synthetic>placeholder 只是名字撞车,语义完全不同。把synthetic.new做成 first-class source 会污染 core、CLI、TUI、frontend 和文档,并让调用方误以为存在一个真实 client。本分支选择删除这个 source/client 概念,而不是保留 fallback、compat flag 或 silent re-attribution。
验证
cargo check -p tokscale-clicargo test -p tokscale-corecargo test -p tokscale-clibun run --cwd packages/frontend lintbun run --cwd packages/frontend buildbun run --cwd packages/frontend lint仅有既有 warning,没有 error。Summary by Sourcery
Remove the Synthetic meta-client/source and its associated parsing, filtering, and documentation, while preserving gateway model path normalization for pricing overrides.
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
Documentation
Changes