Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
264 changes: 264 additions & 0 deletions scripts/patch_generated_score_compat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
#!/usr/bin/env python3
"""Preserve the legacy score-create API after Fern moves it to ``scores``.

Fern owns ``langfuse/api`` and regenerates it from the main Langfuse repository.
This postprocessor is intentionally narrow and fail-closed: it only patches the
known generated shape where ``scores.create`` exists and
``legacy.score_v1.create`` no longer does.
"""

from __future__ import annotations

import argparse
from pathlib import Path

CLIENT_IMPORTS = """\
from ...commons.types.create_score_value import CreateScoreValue
from ...commons.types.score_data_type import ScoreDataType
from ...scores.client import (
AsyncScoresClient as CanonicalAsyncScoresClient,
ScoresClient as CanonicalScoresClient,
OMIT,
)
from ...scores.types.create_score_response import CreateScoreResponse
from ...scores.types.create_score_source import CreateScoreSource
"""

SYNC_CREATE_METHOD = '''\
def create(
self,
*,
name: str,
value: CreateScoreValue,
id: typing.Optional[str] = OMIT,
trace_id: typing.Optional[str] = OMIT,
session_id: typing.Optional[str] = OMIT,
observation_id: typing.Optional[str] = OMIT,
dataset_run_id: typing.Optional[str] = OMIT,
comment: typing.Optional[str] = OMIT,
metadata: typing.Optional[typing.Dict[str, typing.Any]] = OMIT,
environment: typing.Optional[str] = OMIT,
queue_id: typing.Optional[str] = OMIT,
data_type: typing.Optional[ScoreDataType] = OMIT,
config_id: typing.Optional[str] = OMIT,
source: typing.Optional[CreateScoreSource] = OMIT,
request_options: typing.Optional[RequestOptions] = None,
) -> CreateScoreResponse:
"""**Deprecated compatibility alias.** Use ``client.scores.create``."""
return CanonicalScoresClient(
client_wrapper=self._raw_client._client_wrapper
).create(
Comment on lines +48 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve create on the raw-response clients

After Fern moves the operation, callers using the existing client.legacy.score_v1.with_raw_response.create(...) or client.legacy.with_raw_response.score_v1.create(...) surfaces will receive a RawScoreV1Client that no longer defines create. This postprocessor only restores the decoded-response methods by constructing the canonical high-level client, so both synchronous and asynchronous raw-response users still break; add corresponding raw-client compatibility aliases as well.

Useful? React with 👍 / 👎.

name=name,
value=value,
id=id,
trace_id=trace_id,
session_id=session_id,
observation_id=observation_id,
dataset_run_id=dataset_run_id,
comment=comment,
metadata=metadata,
environment=environment,
queue_id=queue_id,
data_type=data_type,
config_id=config_id,
source=source,
request_options=request_options,
)

'''

ASYNC_CREATE_METHOD = '''\
async def create(
self,
*,
name: str,
value: CreateScoreValue,
id: typing.Optional[str] = OMIT,
trace_id: typing.Optional[str] = OMIT,
session_id: typing.Optional[str] = OMIT,
observation_id: typing.Optional[str] = OMIT,
dataset_run_id: typing.Optional[str] = OMIT,
comment: typing.Optional[str] = OMIT,
metadata: typing.Optional[typing.Dict[str, typing.Any]] = OMIT,
environment: typing.Optional[str] = OMIT,
queue_id: typing.Optional[str] = OMIT,
data_type: typing.Optional[ScoreDataType] = OMIT,
config_id: typing.Optional[str] = OMIT,
source: typing.Optional[CreateScoreSource] = OMIT,
request_options: typing.Optional[RequestOptions] = None,
) -> CreateScoreResponse:
"""**Deprecated compatibility alias.** Use ``client.scores.create``."""
return await CanonicalAsyncScoresClient(
client_wrapper=self._raw_client._client_wrapper
).create(
name=name,
value=value,
id=id,
trace_id=trace_id,
session_id=session_id,
observation_id=observation_id,
dataset_run_id=dataset_run_id,
comment=comment,
metadata=metadata,
environment=environment,
queue_id=queue_id,
data_type=data_type,
config_id=config_id,
source=source,
request_options=request_options,
)

'''

TYPE_NAMES = (
"CreateScoreRequest",
"CreateScoreResponse",
"CreateScoreSource",
)
LEGACY_ALIAS_MARKER = (
"**Deprecated compatibility alias.** Use ``client.scores.create``."
)


def _replace_once(contents: str, marker: str, replacement: str, *, path: Path) -> str:
count = contents.count(marker)
if count != 1:
raise RuntimeError(
f"Expected exactly one compatibility marker in {path}: {marker!r}; "
f"found {count}"
Comment on lines +126 to +128

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Construct the exception message before raising

When a generated marker is missing or duplicated, this fail-closed path inlines an f-string in the raise statement. Build the formatted message in a variable first to comply with the repository's explicit exception-message convention.

AGENTS.md reference: AGENTS.md:L149-L149

Useful? React with 👍 / 👎.

)
return contents.replace(marker, replacement, 1)


def _append_type_exports(path: Path, imports_by_type: dict[str, str]) -> None:
exports_marker = "# Score-create compatibility aliases (LFE-10397)."
contents = path.read_text()
if exports_marker in contents:
return

imports = "\n".join(
f"from {module_name} import {type_name}"
for type_name, module_name in imports_by_type.items()
)
names = ", ".join(repr(type_name) for type_name in TYPE_NAMES)
path.write_text(
f"{contents.rstrip()}\n\n{exports_marker}\n{imports}\n"
f"__all__ = [*__all__, {names}]\n"

Check failure on line 146 in scripts/patch_generated_score_compat.py

View check run for this annotation

Claude / Claude Code Review

_append_type_exports crashes on Fern header-only __init__.py (no __all__)

_append_type_exports() writes `__all__ = [*__all__, ...]` to legacy/score_v1/__init__.py and legacy/__init__.py with no fallback if `__all__` doesn't already exist, unlike legacy_types_path which has one. Fern emits header-only __init__.py files (no __all__) for packages with zero exported types — confirmed live in this repo at langfuse/api/prompt_version/__init__.py — and since legacy/score_v1/__init__.py currently exports only the three create-related types, it will very likely regenerate into
Comment on lines +133 to +146

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 _append_type_exports() writes __all__ = [*__all__, ...] to legacy/score_v1/init.py and legacy/init.py with no fallback if __all__ doesn't already exist, unlike legacy_types_path which has one. Fern emits header-only init.py files (no all) for packages with zero exported types — confirmed live in this repo at langfuse/api/prompt_version/init.py — and since legacy/score_v1/init.py currently exports only the three create-related types, it will very likely regenerate into that same header-only shape once create moves off legacy, causing a NameError at import time.

Extended reasoning...

The bug: _append_type_exports() (scripts/patch_generated_score_compat.py:133-146) unconditionally appends a trailing line __all__ = [*__all__, {names}] to whatever file it's given. This line assumes __all__ is already bound as a module global in that file. It is called on three targets: legacy_types_path, legacy_package_path (legacy/score_v1/__init__.py), and legacy_parent_package_path (legacy/__init__.py). Only the first of these has a fallback — lines 222-226 write a default __all__: list[str] = [] to legacy_types_path, but only in the 'file doesn't exist yet' branch. legacy_package_path and legacy_parent_package_path get no such treatment, regardless of whether they define __all__.

Why this isn't hypothetical: Fern emits a specific 'empty package' shape for subpackages that have a client but export zero types — just the auto-generated header comment and # isort: skip_file, with no __all__, no _dynamic_imports, no __getattr__. I verified this is exactly the current shape of langfuse/api/prompt_version/__init__.py in this repo, even though prompt_version/client.py and raw_client.py still exist:

# This file was auto-generated by Fern from our API Definition.

# isort: skip_file

I also verified the current legacy/score_v1/__init__.py exports only CreateScoreRequest, CreateScoreResponse, CreateScoreSource — i.e. exactly the types tied to create, which the coordinated core Fern PR (referenced in this PR's description as merge step 2) moves off legacy.score_v1 onto scores. Once that happens, score_v1 retains only delete(score_id), which has no request/response types to export. Fern will very likely regenerate legacy/score_v1/__init__.py into the same header-only, no-__all__ shape as prompt_version.

Step-by-step proof of the crash:

  1. Core Fern PR merges, moving create's types off legacy.score_v1.
  2. Fern regenerates legacy/score_v1/__init__.py as header-only (no __all__), mirroring prompt_version/__init__.py's current shape.
  3. This postprocessor runs, calls _append_type_exports(legacy_package_path, {...}), which reads the header-only file, sees no marker, and appends: __all__ = [*__all__, 'CreateScoreRequest', 'CreateScoreResponse', 'CreateScoreSource'].
  4. At Python import time, evaluating [*__all__, ...] looks up the name __all__ in the module's own namespace before the assignment completes — since it was never previously bound, this raises NameError: name '__all__' is not defined. I reproduced this exact failure in isolation: exec(\"__all__ = [*__all__, 'X']\") raises NameError: name '__all__' is not defined.
  5. Any access to client.legacy.score_v1 (the parent package's __getattr__ dynamic-import machinery lazily imports the submodule) now raises at import time instead of working — breaking the exact backward-compatible entry point this PR exists to preserve.

Why the tests don't catch it: test_patch_generated_score_compat.py's _write_generated_fixture always pre-seeds __all__ = [] in legacy/score_v1/__init__.py, legacy/__init__.py, and the types __init__.py (see the fixture around line 35). This never exercises Fern's real 'header-only, no-__all__' output shape, so the suite passes while the production regeneration path is unguarded.

Fix: apply the same fallback used for legacy_types_path to legacy_package_path and legacy_parent_package_path — write a default __all__: list[str] = [] (or otherwise ensure __all__ is bound) before calling _append_type_exports, or have _append_type_exports itself detect the absence of __all__ in the file's source and initialize it instead of assuming it exists.

)


def _create_type_aliases(types_dir: Path) -> None:
types_dir.mkdir(parents=True, exist_ok=True)
for type_name in TYPE_NAMES:
module_name = "".join(
f"_{character.lower()}" if character.isupper() else character
for character in type_name
).lstrip("_")
alias_path = types_dir / f"{module_name}.py"
alias_path.write_text(
"# This compatibility alias is applied after Fern generation.\n\n"
f"from ....scores.types.{module_name} import {type_name}\n\n"
f'__all__ = ["{type_name}"]\n'
)


def patch_generated_api(api_root: Path) -> bool:
canonical_client_path = api_root / "scores" / "client.py"
legacy_client_path = api_root / "legacy" / "score_v1" / "client.py"

canonical_client = canonical_client_path.read_text()
legacy_client = legacy_client_path.read_text()

canonical_has_create = (
"\n def create(" in canonical_client
and "\n async def create(" in canonical_client
)
legacy_has_create = (
"\n def create(" in legacy_client
and "\n async def create(" in legacy_client
)

if legacy_has_create:
if LEGACY_ALIAS_MARKER in legacy_client and canonical_has_create:
return False
if canonical_has_create:
Comment on lines +183 to +184

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Partial patches bypass recovery

When a run stops after writing the legacy client but before creating every type alias and package export, the marker-based early return treats that partial output as complete on retry, causing generation to report "No patch needed" while required legacy imports remain missing.

Knowledge Base Used: API Client (langfuse/api/)

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/patch_generated_score_compat.py
Line: 183-184

Comment:
**Partial patches bypass recovery**

When a run stops after writing the legacy client but before creating every type alias and package export, the marker-based early return treats that partial output as complete on retry, causing generation to report "No patch needed" while required legacy imports remain missing.

**Knowledge Base Used:** [API Client (langfuse/api/)](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/api-client.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

raise RuntimeError(
"Both canonical and legacy generated score clients define create; "
"refusing to introduce a third compatibility surface"
)
return False

if not canonical_has_create:
raise RuntimeError(
"Neither generated score client defines both sync and async create methods"
)

Check warning on line 194 in scripts/patch_generated_score_compat.py

View check run for this annotation

Claude / Claude Code Review

Compat shim does not patch raw_client.py — with_raw_response.create breaks

The postprocessor only patches `legacy/score_v1/client.py`, adding a delegating `create`/`async create`, but never touches `legacy/score_v1/raw_client.py`. Once the coordinated Fern PR removes `create` from the raw clients, `client.legacy.score_v1.with_raw_response.create(...)` will raise `AttributeError` even though the plain `client.legacy.score_v1.create(...)` keeps working. This is a real but narrow gap in legacy API preservation, with no test coverage for `with_raw_response`.
Comment on lines +172 to +194

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The postprocessor only patches legacy/score_v1/client.py, adding a delegating create/async create, but never touches legacy/score_v1/raw_client.py. Once the coordinated Fern PR removes create from the raw clients, client.legacy.score_v1.with_raw_response.create(...) will raise AttributeError even though the plain client.legacy.score_v1.create(...) keeps working. This is a real but narrow gap in legacy API preservation, with no test coverage for with_raw_response.

Extended reasoning...

patch_generated_api() in scripts/patch_generated_score_compat.py (lines 172-194) only reads and rewrites legacy/score_v1/client.py — it inserts SYNC_CREATE_METHOD/ASYNC_CREATE_METHOD before the delete methods and appends type re-exports to the package __init__.py files. It never opens or modifies legacy/score_v1/raw_client.py.

Today, raw_client.py defines both RawScoreV1Client.create (line 31) and AsyncRawScoreV1Client.create (line 297), and client.py's with_raw_response property (lines 22 and 176) returns self._raw_client, i.e. the Raw*Client instance. So client.legacy.score_v1.with_raw_response.create(...) currently works and returns the raw HTTP response wrapper — confirmed by grepping the live tree.

Once the coordinated core Fern PR (langfuse/langfuse#15528) moves the create operation off score_v1 in the OpenAPI spec, Fern will remove create from both the public client (ScoreV1Client/AsyncScoreV1Client) and the raw client (RawScoreV1Client/AsyncRawScoreV1Client), since Fern generates both from the same operation. This postprocessor only re-adds the method to the public client via a delegating shim that calls CanonicalScoresClient(client_wrapper=self._raw_client._client_wrapper).create(...) — it bypasses self._raw_client.create entirely rather than patching it. Nothing in the script reconstructs create on the raw client.

Proof walkthrough:

  1. Core Fern PR merges, regenerating langfuse/api with create removed from ScoreV1Client, AsyncScoreV1Client, RawScoreV1Client, and AsyncRawScoreV1Client.
  2. This postprocessor runs, sees legacy_has_create == False and canonical_has_create == True, and patches legacy/score_v1/client.py to add a delegating create/async create — but does not touch raw_client.py.
  3. A user calls client.legacy.score_v1.create(...) → works, via the new shim delegating to CanonicalScoresClient.
  4. A user calls client.legacy.score_v1.with_raw_response.create(...)with_raw_response returns RawScoreV1Client, which no longer has create (Fern removed it and the postprocessor never restored it) → AttributeError: 'RawScoreV1Client' object has no attribute 'create'.

This silently breaks part of the legacy API surface that previously worked, and the existing test suite (tests/unit/test_patch_generated_score_compat.py) only exercises the delegating method on the plain client fixture — it never instantiates or checks raw_client.py, so this regression would not be caught by CI even after the core Fern PR lands.

Fix: extend patch_generated_api() to also insert delegating create/async create methods into RawScoreV1Client/AsyncRawScoreV1Client in raw_client.py (likely returning the wrapped HttpResponse[CreateScoreResponse] type to match the raw client's convention), and add a fixture/test asserting with_raw_response.create still works after patching. Given the PR's stated scope is preserving the plain client.legacy.score_v1.create(...) call — which remains intact — and with_raw_response is a niche, low-level accessor with a clear migration path (client.scores.with_raw_response.create), this does not block the primary documented use case, so it is best flagged as a nit for the author to address, either now or in a quick follow-up before the core Fern PR lands.


legacy_client = _replace_once(
legacy_client,
"from ...core.client_wrapper import AsyncClientWrapper, SyncClientWrapper\n",
"from ...core.client_wrapper import AsyncClientWrapper, SyncClientWrapper\n"
f"{CLIENT_IMPORTS}",
path=legacy_client_path,
)
legacy_client = _replace_once(
legacy_client,
"\n def delete(",
f"\n{SYNC_CREATE_METHOD} def delete(",
path=legacy_client_path,
)
legacy_client = _replace_once(
legacy_client,
"\n async def delete(",
f"\n{ASYNC_CREATE_METHOD} async def delete(",
path=legacy_client_path,
)
legacy_client_path.write_text(legacy_client)

legacy_package_path = api_root / "legacy" / "score_v1" / "__init__.py"
legacy_parent_package_path = api_root / "legacy" / "__init__.py"
legacy_types_path = api_root / "legacy" / "score_v1" / "types" / "__init__.py"
_create_type_aliases(legacy_types_path.parent)

if not legacy_types_path.exists():
legacy_types_path.write_text(
"# This package is generated by Fern and extended for compatibility.\n\n"
"__all__: list[str] = []\n"
)

_append_type_exports(
legacy_types_path,
{
"CreateScoreRequest": ".create_score_request",
"CreateScoreResponse": ".create_score_response",
"CreateScoreSource": ".create_score_source",
},
)
_append_type_exports(
legacy_package_path, {type_name: ".types" for type_name in TYPE_NAMES}
)
_append_type_exports(
legacy_parent_package_path,
{type_name: ".score_v1" for type_name in TYPE_NAMES},
)
return True


def main() -> None:
parser = argparse.ArgumentParser()
parser.add_argument(
"--api-root",
type=Path,
default=Path("langfuse/api"),
help="Path to the freshly generated Fern Python package",
)
args = parser.parse_args()
changed = patch_generated_api(args.api_root)
print(
"Patched legacy score create compatibility alias."
if changed
else "No patch needed."
)


if __name__ == "__main__":
main()
Loading