Repository navigation
fix(protocols): match the z.ai profile's GLM-5.3 family and effort sets to the vendor - #2657
Conversation
…ts to the vendor The GLM-5.3 markers now cover the dash-spelled ids the multimodal registry already treats as the same family (glm-5-3-flash), and the other GLM generations take the effort set docs.z.ai documents for them and reject the rest, as the vendor does (turbo is a 400 there too). GLM-5-Next is deliberately not pinned to the enabled-only rules: its thinking contract is not recorded. The module header describes the rules as they now are and states the file_url decision. Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Limit details: You’ve used all 6 included reviews currently available. Your 46 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour. 📝 SummarySummary by CodeRabbit
WalkthroughThe Z.AI profile now recognizes additional GLM-5.3 model spellings and validates effort values using separate sets for GLM-5.3 and other generations. Tests cover the added spellings and rejection of unsupported effort values. ChangesZ.AI profile validation
Estimated code review effort: 2 (Simple) | ~12 minutes Merge Risk: ⚪ Minimal · up to The updated model matching and effort validation are ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // The documented set is wider than GLM-5.3's, but still a set. | ||
| for effort in ["turbo", "ultra"] { | ||
| assert_eq!( | ||
| validate(&with_model(model, json!({"reasoning_effort": effort}))), | ||
| Err("reasoning_effort_not_allowed".into()), | ||
| "{model} {effort}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 Nit: The wider documented set is only positively exercised through none (line 405). If an entry of REASONING_EFFORTS were dropped or misspelled (e.g. xhigh), no test would catch it. A cheap loop over ["minimal", "medium", "xhigh"] (or the whole constant) asserting Ok(()) here would pin the accepted set the same way the rejected one is pinned.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
2 participants
Description
Problem
From the review of #2634 (verified live on the served id
glm-5.3-flash, wherereasoning_effort: none→ 400 already works): two gaps against the measured vendor behaviour.GLM53_MARKERSmatchedglm-5.3/glm5.3/glm_5.3only, while the multimodal registry (registry/glm53_flash.rs) also treats dash-spelled ids such asglm-5-3-flashas the same family, so a checkpoint served under that spelling got the wider rules.reasoning_effortvalues on the other GLM generations too (turbo→ 400), while the profile accepted anything there.Solution
The 5.3 markers gain
glm-5-3/glm5-3. Effort validation takes an allowed set per generation:low|high|maxon the 5.3 series, the set docs.z.ai documents (none|minimal|low|medium|high|xhigh|max) elsewhere, unknown values a 400 in both. Deliberately not done: pinning GLM-5-Next to the enabled-only rules, which the write-up asked for by analogy with the registry's media grouping; its thinking contract is not recorded, and the registry says nothing about thinking. The module header describes the rules as they are and states thefile_urldecision (refused at ingress on every backend: the vendor fetches the file, no self-hosted engine does, and validation runs before the backend is known).Test Plan
profile::zai::tests::glm53_thinking_rules_do_not_reach_other_generations: dash-spelled ids are 5.3,GLM-5-Nextandglm-5.30are not; GLM-5.2, GLM-5 and GLM-4.6 keepdisabledandnonewhileturbo/ultraare rejected.cargo test -p openai-protocol: 347 passed, 0 failed.cargo +nightly fmt --all -- --checkclean;cargo clippy -p openai-protocol --all-targets -- -D warningsclean.Checklist
cargo +nightly fmtpassescargo clippy --all-targets -- -D warningspasses