fix(api): preserve score create compatibility - #888
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
There was a problem hiding this comment.
LGTM — verified locally (tests pass, script parses cleanly).
What was reviewed:
- The new
Scores.create()delegation toScoreV1in the generated client, and confirmed the legacyCreateScoreRequest/CreateScoreResponse/CreateScoreSourcetypes are still exported and re-exported correctly. - The postprocessor script's fail-closed logic (throws on ambiguous or missing
create()on both clients) rather than silently guessing — ran it as valid JS and traced through the four branch cases. - Ran
tests/unit/score-create-api.test.tslocally; both the canonical and legacy request paths post to/api/public/scoreswith the same body.
Extended reasoning...
Overview
This PR is a targeted compatibility shim for the generated Langfuse API client (@langfuse/core). It (1) manually adds a create() method to Scores in the currently-checked-in generated Client.ts that delegates to the legacy ScoreV1.create(), and (2) adds a new postprocessor script (scripts/patch-generated-score-create-alias.mjs) intended to be invoked by a future Fern code-generation workflow (coordinated with a companion PR in langfuse/langfuse) once the create operation is promoted out of legacy in the API spec. A new unit test exercises both the canonical (client.scores.create) and legacy (client.legacy.scoreV1.create) code paths against a stubbed fetch.
Security risks
None identified. This only affects client-side request construction/dispatch for an already-public API operation (score creation); no auth, crypto, or permission logic is touched. The delegation just forwards the same request/options to the underlying HTTP call.
Level of scrutiny
Moderate-low is appropriate: this is generated/scaffolding code with a narrow, mechanical purpose (keep both API surface names working across a coordinated spec migration), not application logic. The postprocessor script itself isn't wired into this repo's build/CI yet (its invocation lives in the coordinated core Fern PR's generation workflow), so its blast radius today is limited to being unused until that companion change lands — it is effectively documentation-as-code for the next regeneration. I verified it parses and its guard logic (unique-anchor matching, throwing on ambiguous/missing create()) fails closed rather than corrupting generated output.
Other factors
Local verification: pnpm install + pnpm exec vitest run tests/unit/score-create-api.test.ts --project=unit passes (2/2), matching the PR's stated verification. Confirmed LangfuseAPI.legacy is exported from the resources index so the delegating type references resolve. No outstanding review comments to address, and no bugs were surfaced by the bug-hunting pass.
Summary
client.scores.create()immediately while the Fern operation is promoted out oflegacyclient.legacy.scoreV1.create()as a deprecated delegating alias after regenerationCoordination
The postprocessor was also exercised against the exact TypeScript output generated from the coordinated Fern change. After the existing generated-auth patch and formatting, the future
@langfuse/coredeclaration build and the two-path request test pass.Impacted packages
@langfuse/coreVerification
pnpm build—Tasks: 8 successful, 8 totalpnpm exec vitest run "tests/unit/score-create-api.test.ts" --project=unit—Tests 2 passed (2)pnpm lint— passedpnpm typecheck— passedpnpm format:check— passedpnpm precommit:check—Tasks: 3 successful, 3 totalGreptile Summary
Adds a compatibility layer for score creation.
client.scores.create()by delegating to the existing legacy score client.Confidence Score: 5/5
The PR appears safe to merge with no actionable defects identified.
The new canonical method preserves the existing request implementation through delegation, while the postprocessor explicitly validates generated-client states and the tests cover both public access paths.
Reviews (1): Last reviewed commit: "fix(api): preserve score create compatib..." | Re-trigger Greptile
Context used: