Repository navigation
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedToo many files! This PR contains 263 files, which is 113 over the limit of 150. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (263)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdded a new provider entry for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/models/src/models/zai.ts`:
- Around line 30-45: The mapping for providerId "canopywave" and modelName
"zai/glm-5.1" must be gated until the upstream model is live: update that object
to mirror other unstable Canopywave entries by adding test: "skip" and
stability: "unstable" and include a deactivatedAt timestamp (or otherwise mark
it inactive) so runtime routing and E2E tests won’t hit the unavailable
endpoint; alternatively remove/hold this entry until the provider reports
healthy — locate the object with providerId "canopywave" and modelName
"zai/glm-5.1" and apply the gating fields (test, stability, deactivatedAt)
consistent with the patterns used for "glm-5" and "glm-4.7".
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d6a0f141-21e0-4a96-b1c8-d39ab7bd42e1
📒 Files selected for processing (1)
packages/models/src/models/zai.ts
| { | ||
| providerId: "canopywave", | ||
| modelName: "zai/glm-5.1", | ||
| inputPrice: 1.4 / 1e6, | ||
| cachedInputPrice: 0.26 / 1e6, | ||
| outputPrice: 4.4 / 1e6, | ||
| discount: 0.3, | ||
| requestPrice: 0, | ||
| contextSize: 200000, | ||
| maxOutput: 128000, | ||
| streaming: true, | ||
| reasoning: true, | ||
| vision: false, | ||
| tools: true, | ||
| jsonOutput: false, | ||
| }, |
There was a problem hiding this comment.
Gate this mapping until Canopywave brings glm-5.1 online.
Per the PR description, Canopywave currently lists glm-5.1 as "Coming Soon" and the endpoint returns No available workers (all circuits open or unhealthy). If this is merged before the upstream model is live, live traffic routed here (and E2E tests) will fail. Consider mirroring the pattern already used for other not-yet-stable Canopywave entries in this file — e.g. test: "skip" (as on canopywave glm-5 at line 75) and/or stability: "unstable" with a deactivatedAt (as on canopywave glm-4.7 at lines 451-453) — so the mapping can land safely and be flipped on once the provider is healthy. Alternatively, keep the PR unmerged until the endpoint is functional, as the description suggests.
🛡️ Suggested guard while upstream is unavailable
{
providerId: "canopywave",
+ test: "skip",
+ stability: "unstable",
modelName: "zai/glm-5.1",
inputPrice: 1.4 / 1e6,
cachedInputPrice: 0.26 / 1e6,
outputPrice: 4.4 / 1e6,
discount: 0.3,
requestPrice: 0,
contextSize: 200000,
maxOutput: 128000,
streaming: true,
reasoning: true,
vision: false,
tools: true,
jsonOutput: false,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| providerId: "canopywave", | |
| modelName: "zai/glm-5.1", | |
| inputPrice: 1.4 / 1e6, | |
| cachedInputPrice: 0.26 / 1e6, | |
| outputPrice: 4.4 / 1e6, | |
| discount: 0.3, | |
| requestPrice: 0, | |
| contextSize: 200000, | |
| maxOutput: 128000, | |
| streaming: true, | |
| reasoning: true, | |
| vision: false, | |
| tools: true, | |
| jsonOutput: false, | |
| }, | |
| { | |
| providerId: "canopywave", | |
| test: "skip", | |
| stability: "unstable", | |
| modelName: "zai/glm-5.1", | |
| inputPrice: 1.4 / 1e6, | |
| cachedInputPrice: 0.26 / 1e6, | |
| outputPrice: 4.4 / 1e6, | |
| discount: 0.3, | |
| requestPrice: 0, | |
| contextSize: 200000, | |
| maxOutput: 128000, | |
| streaming: true, | |
| reasoning: true, | |
| vision: false, | |
| tools: true, | |
| jsonOutput: false, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/models/src/models/zai.ts` around lines 30 - 45, The mapping for
providerId "canopywave" and modelName "zai/glm-5.1" must be gated until the
upstream model is live: update that object to mirror other unstable Canopywave
entries by adding test: "skip" and stability: "unstable" and include a
deactivatedAt timestamp (or otherwise mark it inactive) so runtime routing and
E2E tests won’t hit the unavailable endpoint; alternatively remove/hold this
entry until the provider reports healthy — locate the object with providerId
"canopywave" and modelName "zai/glm-5.1" and apply the gating fields (test,
stability, deactivatedAt) consistent with the patterns used for "glm-5" and
"glm-4.7".
There was a problem hiding this comment.
Pull request overview
Adds a new canopywave provider mapping for the ZAI glm-5.1 model so it can be routed/priced as an additional upstream option alongside the existing zai provider entry.
Changes:
- Adds
providerId: "canopywave"mapping forglm-5.1with pricing, context/output limits, and capability flags.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| inputPrice: 1.4 / 1e6, | ||
| cachedInputPrice: 0.26 / 1e6, | ||
| outputPrice: 4.4 / 1e6, | ||
| discount: 0.3, |
There was a problem hiding this comment.
This Canopywave mapping has a larger discount (0.3) than the existing zai mapping (0.1) while keeping the same base token prices, which makes Canopywave the cheapest route for glm-5.1. In production routing, getCheapestFromAvailableProviders() uses discount in its price calculation, and when metrics are missing it will pick the cheapest provider purely by price—so this entry is likely to become the default route even while the Canopywave endpoint is still returning "No available workers".
To avoid shipping a default route to an unavailable upstream, gate this mapping until the model is live (e.g., set stability: "experimental"/"unstable" so it’s excluded from auto-routing, or temporarily remove/zero the discount so it won’t win price-based selection).
| discount: 0.3, | |
| discount: 0, |
| providerId: "canopywave", | ||
| modelName: "zai/glm-5.1", | ||
| inputPrice: 1.4 / 1e6, | ||
| cachedInputPrice: 0.26 / 1e6, | ||
| outputPrice: 4.4 / 1e6, |
There was a problem hiding this comment.
This provider mapping is not marked with test: "skip". The gateway E2E suite enumerates provider-specific test cases from model.providers and will attempt requests for each provider unless test: "skip" (or stability exclusion in non-FULL_MODE) is set; given Canopywave currently returns "No available workers", this is likely to make CI/E2E fail if the suite reaches this model.
Consider adding test: "skip" (and/or stability: "experimental"/"unstable") until the upstream is actually available, then remove the skip flag when it’s ready to be exercised by tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1e5c3de to
6e1329e
Compare
Merged origin/main into canopywave-glm-5.1. Resolved conflict in packages/models/src/models/zai.ts by keeping both the canopywave provider entry (from PR) and the together-ai provider entry (from main) for glm-5.1. Also restored canopywave provider definition in providers.ts and endpoint URL in get-provider-endpoint.ts, which were removed from main (#2162) but are needed by this PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Resolved merge conflict with origin/main. Conflict in
Additional fixes needed:
TypeScript compilation passes. 94 unit tests pass. |
Summary
canopywaveprovider entry forglm-5.1at $1.40/$4.40/$0.26 per MTok (in/out/cached), 200K context, discount 0.3.Note
Canopywave currently lists this model as "Coming Soon" — the endpoint returns "No available workers (all circuits open or unhealthy)" on requests. Leaving this PR open until canopywave brings the model online so we don't ship a broken route.
🤖 Generated with Claude Code
Summary by CodeRabbit