Skip to content

Conversation

@cynthia-lixinyi
Copy link

@cynthia-lixinyi cynthia-lixinyi commented Nov 25, 2025

Ticket

Link to the ticket(s) this pull request addresses.

Component/Service

What part of Helicone does this affect?

  • Web (Frontend)
  • Jawn (Backend)
  • Worker (Proxy)
  • Bifrost (Marketing)
  • AI Gateway
  • Packages
  • Infrastructure/Docker
  • Documentation

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Performance improvement
  • Refactoring

Testing

  • Added/updated unit tests
  • Added/updated integration tests
  • Tested locally
  • Verified in staging environment
  • E2E tests pass (if applicable)

Technical Considerations

  • Database migrations included (if needed)
  • API changes documented
  • Breaking changes noted
  • Performance impact assessed
  • Security implications reviewed

Dependencies

  • No external dependencies added
  • Dependencies added and documented
  • Environment variables added/modified

Deployment Notes

  • No special deployment steps required
  • Database migrations need to run
  • Environment variable changes required
  • Coordination with other teams needed

Context

Why are you making this change?
Integrate Canopy Wave as a provider for ZAI GLM-4.6

Screenshots / Demos

Before After

Misc. Review Notes

@vercel
Copy link

vercel bot commented Nov 25, 2025

@cynthia-lixiny07 is attempting to deploy a commit to the Helicone Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 25, 2025

Greptile Overview

Greptile Summary

Integrates Canopy Wave as a new provider for ZAI GLM-4.6 model with pricing, configuration, and comprehensive test coverage.

Key Changes:

  • Added CanopyWaveProvider class with API endpoint configuration
  • Registered provider across cost calculation, usage processing, and UI systems
  • Configured glm-4.6 endpoint with Canopy Wave pricing ($0.00000045 input / $0.0000015 output)
  • Added 400+ lines of integration tests covering success and error scenarios

Issues Found:

  • Critical: buildUrl method signature in packages/cost/models/providers/canopywave.ts is missing required parameters and type imports, which will cause TypeScript compilation errors
  • Model ID inconsistency: tests expect zai/glm-4.6 while Novita provider uses zai-org/glm-4.6
  • Parameter discrepancy: logit_bias supported by Novita but not Canopy Wave

Confidence Score: 1/5

  • This PR has critical syntax errors that will prevent compilation
  • The CanopyWaveProvider.buildUrl method has an incorrect signature that doesn't match the BaseProvider abstract class, missing both required parameters (endpoint, requestParams) and the necessary type imports. This will cause TypeScript compilation to fail. Additionally, there's a potential model ID mismatch in tests that needs verification.
  • packages/cost/models/providers/canopywave.ts requires immediate fixes for TypeScript compilation errors. Verify model ID consistency in worker/test/ai-gateway/registry-zai.spec.ts

Important Files Changed

File Analysis

Filename Score Overview
packages/cost/models/providers/canopywave.ts 1/5 New provider class with critical issues: incorrect buildUrl signature and missing type imports
packages/cost/models/authors/zai/glm-4/endpoints.ts 3/5 Added Canopy Wave endpoint config for glm-4.6 with potential parameter inconsistency (logit_bias)
packages/cost/models/provider-helpers.ts 5/5 Added Canopy Wave provider mapping - implementation looks correct and consistent with existing patterns
worker/test/ai-gateway/registry-zai.spec.ts 3/5 Added comprehensive test suite for Canopy Wave integration with potential model ID mismatch

Sequence Diagram

sequenceDiagram
    participant Client
    participant Gateway as AI Gateway
    participant Registry as Model Registry
    participant Provider as CanopyWaveProvider
    participant API as Canopy Wave API

    Client->>Gateway: POST /chat/completions<br/>model: glm-4.6/canopywave
    Gateway->>Registry: Lookup model config
    Registry->>Registry: Match "glm-4.6:canopywave"<br/>from endpoints.ts
    Registry-->>Gateway: Return config<br/>(providerModelId: zai-org/glm-4.6)
    
    Gateway->>Provider: buildUrl(endpoint, params)
    Provider-->>Gateway: https://inference.canopywave.io/v1/chat/completions
    
    Gateway->>Provider: authenticate(context)
    Provider-->>Gateway: Authorization: Bearer <api-key>
    
    Gateway->>Provider: buildRequestBody(endpoint, context)
    Provider-->>Gateway: JSON with model: zai-org/glm-4.6
    
    Gateway->>API: POST with transformed request
    API-->>Gateway: Response
    Gateway-->>Client: Return response with usage/cost
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 10 to 12
buildUrl(): string {
return `${this.baseUrl}v1/chat/completions`;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: buildUrl method signature doesn't match BaseProvider interface - missing required endpoint and requestParams parameters

Suggested change
buildUrl(): string {
return `${this.baseUrl}v1/chat/completions`;
}
buildUrl(endpoint: Endpoint, requestParams: RequestParams): string {
return `${this.baseUrl}v1/chat/completions`;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cost/models/providers/canopywave.ts
Line: 10:12

Comment:
**syntax:** `buildUrl` method signature doesn't match `BaseProvider` interface - missing required `endpoint` and `requestParams` parameters

```suggestion
  buildUrl(endpoint: Endpoint, requestParams: RequestParams): string {
    return `${this.baseUrl}v1/chat/completions`;
  }
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Author

@cynthia-lixinyi cynthia-lixinyi Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. (18a828d)

@@ -0,0 +1,13 @@
import { BaseProvider } from "./base";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: missing imports for Endpoint and RequestParams types that are required by the buildUrl method signature

Suggested change
import { BaseProvider } from "./base";
import { BaseProvider } from "./base";
import type { Endpoint, RequestParams } from "../types";
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cost/models/providers/canopywave.ts
Line: 1:1

Comment:
**syntax:** missing imports for `Endpoint` and `RequestParams` types that are required by the `buildUrl` method signature

```suggestion
import { BaseProvider } from "./base";
import type { Endpoint, RequestParams } from "../types";
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. (18a828d)

Comment on lines 58 to 75
supportedParameters: [
"functions",
"structured_outputs",
"reasoning",
"tool_choice",
"tools",
"response_format",
"max_tokens",
"temperature",
"top_p",
"stop",
"frequency_penalty",
"presence_penalty",
"seed",
"top_k",
"min_p",
"repetition_penalty",
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: check that logit_bias parameter (supported by Novita on line 37) should be included or excluded from Canopy Wave's supportedParameters list

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cost/models/authors/zai/glm-4/endpoints.ts
Line: 58:75

Comment:
**style:** check that `logit_bias` parameter (supported by Novita on line 37) should be included or excluded from Canopy Wave's `supportedParameters` list

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. (39a1136)

{
url: "https://inference.canopywave.io/v1/chat/completions",
response: "success",
model: "zai/glm-4.6",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: verify the correct model ID - Novita tests use zai-org/glm-4.6 (line 34-35) but Canopy Wave tests use zai/glm-4.6

Prompt To Fix With AI
This is a comment left during a code review.
Path: worker/test/ai-gateway/registry-zai.spec.ts
Line: 448:448

Comment:
**style:** verify the correct model ID - Novita tests use `zai-org/glm-4.6` (line 34-35) but Canopy Wave tests use `zai/glm-4.6`

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Canopy Wave's cluster, the model ID for ZAI GLM 4.6 is zai/glm-4.6.

yarn.lock Outdated
version "2.0.4"
resolved "https://registry.yarnpkg.com/zwitch/-/zwitch-2.0.4.tgz#c827d4b0acb76fc3e685a4c6ec2902d51070e9d7"
integrity sha512-bXE4cR/kVZhKZX/RjPEflHaKVhUVl85noU3v6b8apfQEc1x4A+zBxjZ4lN8LqGd6WZ3dl98pY4o717VFmoPp+A==
integrity sha512-bXE4cR/kVZhKZX/RjPEflHaKVhUVl85noU3v6b8apfQEc1x4A+zBxjZ4lN8LqGd6WZ3dl98pY4o717VFmoPp+A== No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this file from the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants