feat(api): update API spec from langfuse/langfuse be747a7 - #956
langfuse-bot wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| request: LangfuseAPI.legacy.GetObservationRequest = {}, | ||
| requestOptions?: ObservationsV1.RequestOptions, |
There was a problem hiding this comment.
Adding request before requestOptions breaks the existing two-argument public API. Calls such as get(id, { timeoutInSeconds, headers }) or the aliased fetchObservation(id, options) now treat the options object as GetObservationRequest. Since only startTime is read from that argument, the caller's timeout, retry settings, headers, abort signal, and query parameters are silently ignored. Please preserve the old overload or distinguish request options at runtime.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/api/api/resources/legacy/resources/observationsV1/client/Client.ts
Line: 83-84
Comment:
**Request options silently ignored**
Adding `request` before `requestOptions` breaks the existing two-argument public API. Calls such as `get(id, { timeoutInSeconds, headers })` or the aliased `fetchObservation(id, options)` now treat the options object as `GetObservationRequest`. Since only `startTime` is read from that argument, the caller's timeout, retry settings, headers, abort signal, and query parameters are silently ignored. Please preserve the old overload or distinguish request options at runtime.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because this regenerates a large slice of the auto-generated API client (30 files) and includes a public method signature change, a human look would still be worthwhile.
- Verified the SdkLog type removal is fully consistent across
ingestion/types/index.tsand theIngestionEventunion (no dangling references). - Checked
ObservationsV1.get()'s newstartTimequery param wiring — correctly merged into_queryParamsalongsiderequestOptions?.queryParams. - Reviewed the doc/comment-only deprecation and guidance updates in Ingestion, Metrics, Observations, OpenTelemetry, Scores, Trace, and Experiments clients — no behavioral changes there.
- Noted
ObservationsV1.get(observationId, requestOptions?)gained a new middlerequestparameter (with a= {}default); this can silently break existing callers passingrequestOptionspositionally as the 2nd argument.
Extended reasoning...
Overview
This is a Fern-regenerated update to the Langfuse API client in packages/core/src/api/api/, spanning 30 files: deprecation/sunset doc comments for the legacy v3 ingestion endpoint, removal of the SdkLog event type, a new optional objectStartTime field on CreateCommentRequest, a new startTime query parameter on the legacy ObservationsV1.get() endpoint, and various comment-only clarifications elsewhere. All files carry Fern's "auto-generated" header, so hand-authored logic risk is low, but the changes still alter the public TypeScript SDK surface that downstream consumers depend on.
Security risks
None identified. No auth, crypto, or permission logic is touched; changes are limited to request/response type shapes, query-parameter wiring, and doc comments.
Level of scrutiny
Medium. Most of the diff is mechanical (comments, added optional fields, one type removal that's consistently threaded through), which would normally support a quick approval. However, the ObservationsV1.get() change inserts a new request: GetObservationRequest = {} parameter between observationId and requestOptions in a public method. Although it has a default value, this is a source-breaking change for any caller currently invoking get(observationId, requestOptionsObject) positionally — such a call now silently passes a RequestOptions-shaped object where GetObservationRequest is expected. TypeScript would likely catch this if the shapes don't overlap, but with structurally permissive types (e.g., an empty object or overlapping optional fields) it could compile without error and behave incorrectly at runtime (the intended requestOptions values would be dropped). This was already flagged and investigated as a candidate issue and deemed not a reported bug, but it's a legitimate API-compatibility concern that benefits from a human's judgment on whether this matches the intended Fern-generated migration pattern.
Other factors
The rest of the diff (SdkLog removal, new optional fields, deprecation comments) is internally consistent and low-risk. There is no existing test coverage change to speak of since this is generated code, and the PR has no description from the author beyond the auto-generated title referencing an upstream spec commit. Given the breadth of the regeneration and the one behavior-relevant signature change, a defer with a concise "what was reviewed" summary is more appropriate than a full approval.
This branch was successfully deployed
1 participant
The PR should not merge until legacy observation retrieval preserves the existing two-argument request-options behavior.
Summary
This PR synchronizes generated API definitions and documentation with the latest server specification.
sdk-logingestion event types and adds v3 ingestion sunset guidance.Reviews (1) · Last reviewed commit: "feat(api): update API spec from langfuse..."