Repository navigation
feat(api): update API spec from langfuse/langfuse 24f89b2 - #1000
langfuse-bot wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
There was a problem hiding this comment.
Claude Code Review
No review was started: this request came from a bot account. Manual reviews can only be requested by someone with write access to this repository. Ask a maintainer to comment @claude review, or have your automation post the comment from a user account with write access.
Tip: disable this comment in your organization's Code Review settings.
| public create( | ||
| request: LangfuseAPI.CreateScoreRequest, | ||
| request: LangfuseAPI.CreateScoresRequest, | ||
| requestOptions?: Scores.RequestOptions, | ||
| ): core.HttpResponsePromise<LangfuseAPI.CreateScoreResponse> { | ||
| ): core.HttpResponsePromise<LangfuseAPI.CreateScoresResponse> { |
There was a problem hiding this comment.
Single-score calls stop compiling
Scores.create() now returns CreateScoresResponse even for a single score. The existing ScoreV1.create() alias returns that result but promises CreateScoreResponse, which requires id. The batch response types have no id, so the alias no longer type-checks. Existing single-score callers also lose typed access to id.
Add separate overloads for single-score and batch requests to preserve the single-score return type.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/api/api/resources/scores/client/Client.ts
Line: 93-96
Comment:
**Single-score calls stop compiling**
`Scores.create()` now returns `CreateScoresResponse` even for a single score. The existing `ScoreV1.create()` alias returns that result but promises `CreateScoreResponse`, which requires `id`. The batch response types have no `id`, so the alias no longer type-checks. Existing single-score callers also lose typed access to `id`.
Add separate overloads for single-score and batch requests to preserve the single-score return type.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| * } | ||
| */ | ||
| export interface GetSkillFileContentsRequest { | ||
| /** Comma-separated list of one to 50 canonical base64-encoded SHA-256 hashes from skill file manifests. URL-encode the value, including +, /, and = characters. */ |
There was a problem hiding this comment.
The sha256Hashes documentation tells callers to URL-encode the value, but the client already does that. Following this instruction turns %2B, %2F, and %3D into %252B, %252F, and %253D. The request then carries percent escapes instead of the original base64 hashes, preventing callers from reading the requested files.
Tell callers to pass the raw comma-separated hashes; the SDK encodes them.
| /** Comma-separated list of one to 50 canonical base64-encoded SHA-256 hashes from skill file manifests. URL-encode the value, including +, /, and = characters. */ | |
| /** Comma-separated list of one to 50 canonical base64-encoded SHA-256 hashes from skill file manifests. Pass the raw hashes; the SDK URL-encodes the value. */ |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/api/api/resources/unstable/resources/skills/client/requests/GetSkillFileContentsRequest.ts
Line: 12
Comment:
**Hashes get encoded twice**
The `sha256Hashes` documentation tells callers to URL-encode the value, but the client already does that. Following this instruction turns `%2B`, `%2F`, and `%3D` into `%252B`, `%252F`, and `%253D`. The request then carries percent escapes instead of the original base64 hashes, preventing callers from reading the requested files.
Tell callers to pass the raw comma-separated hashes; the SDK encodes them.
```suggestion
/** Comma-separated list of one to 50 canonical base64-encoded SHA-256 hashes from skill file manifests. Pass the raw hashes; the SDK URL-encodes the value. */
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
FYI I have removed the code changes I am responsible from this PR. The following are remaining
This includes breaking changes:
🤖 Summaries Generated with Claude |
There was a problem hiding this comment.
Beyond the inline finding on Scores.create's widened return type breaking the legacy scoreV1 compatibility shim, I also checked the new expiresAt null-ability doc comment on ApiKeySummary/OrganizationApiKey — the field's type (string | undefined) is unchanged by this diff (only a comment was added), so that doc/type mismatch is pre-existing and not a regression introduced here.
Extended reasoning...
The diff is entirely Fern-generated API client code (types and Client.ts files), the largest piece being a new "skills" resource plus smaller additions to scores, ingestion, and API-key types; none of it is hand-written business logic and there is no injection/auth-surface change. I independently confirmed the already-flagged bug: scores/client/Client.ts now returns the union CreateScoresResponse from create(), while legacy/resources/scoreV1/client/Client.ts (lines 70-75, untouched by this diff) still declares HttpResponsePromise<CreateScoreResponse> and delegates to it — I traced this to scripts/patch-generated-score-create-alias.mjs, the repo's custom post-codegen patch that keeps the two create() methods in sync but did not account for the canonical method's return type widening. That is a real, non-trivial type-safety regression tied to this repo's custom generation pipeline, so a human should confirm the regen/patch script is updated before merging.
| public create( | ||
| request: LangfuseAPI.CreateScoreRequest, | ||
| request: LangfuseAPI.CreateScoresRequest, | ||
| requestOptions?: Scores.RequestOptions, | ||
| ): core.HttpResponsePromise<LangfuseAPI.CreateScoreResponse> { | ||
| ): core.HttpResponsePromise<LangfuseAPI.CreateScoresResponse> { |
There was a problem hiding this comment.
🔴 Widening Scores.create's return type to LangfuseAPI.CreateScoresResponse (a union of CreateScoreResponse | CreateScoreBatchResponse | CreateScoreBatchResults) breaks the unregenerated compatibility shim in legacy/resources/scoreV1/client/Client.ts:70-75, which still declares core.HttpResponsePromise<LangfuseAPI.CreateScoreResponse> while delegating to new Scores(this._options).create(...). Since HttpResponsePromise exposes withRawResponse(): Promise<WithRawResponse<T>> (data: T is covariant), returning the now-wider union where the narrower single type is declared is a type error, so tsc -b --noEmit (root typecheck/check:type scripts) and pnpm build's tsup dts: true declaration emit fail for @ langfuse/core. … [also at: packages/core/src/api/api/resources/legacy/resources/scoreV1/client/Client.ts:75 - Maintainers get a package that fails to type-check: Scores.create() widened its return type, but the legacy ScoreV1.create() alias (which just forwards to it) still declares the old narrow return type. scores/client/Client.ts:93-96 now returns…]
Why this was flagged
…Fix: regenerate/patch legacy/scoreV1/client/Client.ts's create() signature (via scripts/patch-generated-score-create-alias.mjs) whenever Scores.create's request/response types change, keeping both aliases' declared types consistent with the canonical implementation.
legacy/resources/scoreV1/client/Client.ts:70-75 (untouched by this diff; confirmed via git diff) declares public create(request: LangfuseAPI.CreateScoreRequest): core.HttpResponsePromise<LangfuseAPI.CreateScoreResponse> and returns new Scores(this._options).create(request, requestOptions). After this diff, Scores.create (scores/client/Client.ts:93-96) returns core.HttpResponsePromise<LangfuseAPI.CreateScoresResponse>, a union including CreateScoreBatchResponse/CreateScoreBatchResults.
Verification: normal. The diff widens Scores.create's return type at scores/client/Client.ts:93-96 to core.HttpResponsePromise<LangfuseAPI.CreateScoresResponse>, where CreateScoresResponse.ts:7-10 defines CreateScoreResponse | CreateScoreBatchResponse | CreateScoreBatchResults.
Fix the single-score return type before merging; it breaks TypeScript checking in the existing compatibility alias.
Summary
Updates the generated API client with batch score creation, an unstable skills API, prompt filters, and API-key names and expiration fields. It also refreshes ingestion sunset and real-time API documentation.
Reviews (1) · Last reviewed commit: "feat(api): update API spec from langfuse..." · Reviewed by Greptile