Skip to content

feat(api): update API spec from langfuse/langfuse 2cb8041 - #2095

Closed
langfuse-bot wants to merge 1 commit into
mainfrom
api-spec-bot-2cb8041-41830756409-1
Closed

langfuse-bot wants to merge 1 commit into
mainfrom
api-spec-bot-2cb8041-41830756409-1

Conversation

@langfuse-bot

@langfuse-bot langfuse-bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Generated from langfuse/langfuse main at 2cb8041.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable issues were found in the changed code.

Summary

Updates the generated API client from the upstream API definition.

  • Python callers can read Topics rules, summaries, and assignments.
  • Observation pages explain when callers should keep following a cursor.

Reviews (1) · Last reviewed commit: "feat(api): update API spec from langfuse..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +22 to 27
rule_ids: typing_extensions.Annotated[
typing.List[str], FieldMetadata(alias="ruleIds")
] = pydantic.Field()
"""
The rules that run the facet.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Callers on a Langfuse server that does not yet send ruleIds now get a pydantic ValidationError from list_facets and get_facet. On the base branch those facets parsed fine. TopicFacet.rule_ids is declared required (pydantic.Field() with no default). parse_obj_as validates strictly, and the raw client only catches JSONDecodeError. Fix: make rule_ids optional (typing.Optional[typing.List[str]] = None) in the API definition, the same way the sibling Topic.rule_id and TopicFacetCounts.rule_id were added, so responses without ruleIds still parse.

Why this was flagged

The trigger is a server deployment, such as a self-hosted Langfuse older than spec 2cb8041, whose topic-facets responses have no ruleIds key. I could not enumerate which server versions callers run. client.unstable.topics.list_facets() / get_facet() in raw_client.py call parse_obj_as(type_=TopicFacetList/TopicFacet, ...). On pydantic v2 that is TypeAdapter.validate_python (core/pydantic_utilities.py:58-59), which raises on the missing required field rule_ids (topic_facet.py:22-24). The raw client's try block only handles JSONDecodeError, so the ValidationError reaches the caller. On the base branch TopicFacet had no rule_ids and the same response parsed. The model's extra="allow" config tolerates unknown keys but does not help with a missing required key. The other fields added in this change (Topic.rule_id, TopicFacetCounts.rule_id, ObservationsV2Meta.comment) all default to None.

Verification: The diff adds rule_ids with pydantic.Field() and no default at langfuse/api/unstable/topics/types/topic_facet.py:22-24, which makes it required. parse_obj_as in core/pydantic_utilities.py:53-60 raises a ValidationError when a required field is missing. The try blocks catch only JSONDecodeError, so payloads without ruleIds that parsed on the base now fail.

Comment on lines +14 to +17
comment: typing.Optional[str] = pydantic.Field(default=None)
"""
Present when an empty page has a cursor; continue requesting pages until the cursor is absent.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟣 pre-existing, not blocking: Batch evaluations over observations can stop partway and still report completed=True with no resume token. The cause is the server's new empty pages that carry a cursor. This spec says a page can be empty while meta.cursor is set, and paging must continue until the cursor is absent. The SDK's own loop at langfuse/batch_evaluation.py:1057 stops on cursor is None or not items, so it treats an empty page as the end. Fix: end the loop only when next_cursor is None, while keeping the max_items stop and the per-page dedupe working. [also at: langfuse/api/observations/client.py:1 - pre-existing: Batch evaluations over observations can stop early and report completion, skipping the remaining observations.]
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

The trigger is a server that sends the contract added at observations_v2meta.py:14-17 and client.py docstrings: data is empty, meta.cursor is set, and comment is present. The spec calls this normal for requests whose bounded scan finds no matches in a slice. The SDK caller run_batched_evaluation on observations fetches pages through _fetch_batch_with_retry (batch_evaluation.py:1115-1123) and at batch_evaluation.py:1057 runs if cursor is None or not items: has_more = False; break. After the break it returns completed=True and has_more_items=False (batch_evaluation.py:1081-1085), and the resume token holds a live cursor that nothing asks the user to follow. Observations after the empty slice are never evaluated and no error is shown. This PR updates the generated client to document the contract but leaves the only in-repo consumer unchanged, so nothing guards against the early stop.

Verification: pre-existing. The trigger is a Langfuse server that returns a page with empty data while meta.cursor is still set. In langfuse/batch_evaluation.py the loop runs if cursor is None or not items: has_more = False; break (lines 1056-1059), so an empty page with a non-null cursor ends the run. The caller gets a "complete" result with no resume token. The base has the same loop.

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.

1 participant