feat(api): update API spec from langfuse/langfuse 2db6c30 - #1887
langfuse-bot wants to merge 1 commit into
Conversation
|
@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.
| @@ -147,7 +134,6 @@ class IngestionEvent_ObservationUpdate(UniversalBaseModel): | |||
| IngestionEvent_GenerationCreate, | |||
There was a problem hiding this comment.
Removing IngestionEvent_SdkLog, SdkLogBody, and SdkLogEvent from the ingestion union and package exports makes previously valid imports fail immediately. Raw sdk-log dictionaries still pass through batch serialization unchanged, so this specifically breaks callers using the typed public API. Preserve compatibility exports or provide an explicit migration boundary before deleting these models.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/ingestion/types/ingestion_event.py
Line: 134
Comment:
**Public SDK-log types removed**
Removing `IngestionEvent_SdkLog`, `SdkLogBody`, and `SdkLogEvent` from the ingestion union and package exports makes previously valid imports fail immediately. Raw `sdk-log` dictionaries still pass through batch serialization unchanged, so this specifically breaks callers using the typed public API. Preserve compatibility exports or provide an explicit migration boundary before deleting these models.
**Knowledge Base Used:**
- [API client and service surface](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/api-client-and-service-surface.md)
- [Shared models and data serialization](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/shared-models-and-data-serialization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| - `stringObject`: `"="`, `contains`, `does not contain`, `starts with`, `ends with`, `is set`, `is not set` (use `is set` / `is not set` for key presence; an empty value for `contains`, `starts with`, or `ends with` is treated as `is set`) | ||
| - `boolean`: `"="`, `"<>"` | ||
| - `null`: `is null`, `is not null` | ||
|
|
There was a problem hiding this comment.
🟡 (optional) Docstring for EvaluationRuleFilter_StringObject now advertises is set/is not set as valid stringObject operators for evaluation rule filters (line 416), but the operator field at line 456 is still typed EvaluationRuleStringFilterOperator, an unchanged enum with only =, contains, does not contain, starts with, ends with. Any caller following the new docs and constructing EvaluationRuleFilter_StringObject(..., operator="is set") (or the sibling StringObjectEvaluationRuleFilter) gets a pydantic ValidationError, so the newly documented feature is unusable through this SDK's typed request models even though the server accepts it. …
Extended reasoning...
…Fix: add IS_SET/IS_NOT_SET members to EvaluationRuleStringFilterOperator (evaluation_rule_string_filter_operator.py) so both EvaluationRuleFilter_StringObject and StringObjectEvaluationRuleFilter accept the documented values.
This file (evaluation_rule_filter.py) is in the diff; its docstrings at lines 37,100,163,226,289,353,416,480,544,607 were all updated to list is set/is not set for stringObject filters used when creating/updating evaluation rules via evaluation_rules/client.py. The operator field type EvaluationRuleStringFilterOperator (imported from evaluation_rule_string_filter_operator.py, not touched by this diff) is a plain Python StrEnum with only 5 members. UniversalBaseModel/pydantic validates enum fields strictly, so passing the string 'is set' raises pydantic.ValidationError at construction time, before any request is sent. Contrast: in observations/client.py, scores/client.py, trace/client.py the same new operator strings were added to docstrings too, but those filter params are plain str (raw JSON), so no type mismatch there - only the…
Verification: nit. evaluation_rule_filter.py:416 (added in this diff) documents stringObject operators including is set / is not set, but the field at line 456 is operator: EvaluationRuleStringFilterOperator. That enum (evaluation_rule_string_filter_operator.py, unchanged by the diff) has only 5 members: "=", "contains", "does not contain", "starts with", "ends with".… | nit — real…
The PR should not merge until the public SDK-log compatibility break is addressed; the inaccurate ingestion-header guidance should also be corrected.
Summary
This PR regenerates the API surface from the latest upstream specification, adding comment and observation lookup performance hints, exposing ingestion deprecation metadata, expanding documented filter operators, and updating v3-to-v4 migration guidance. It also removes the typed SDK-log ingestion models.
_deprecationmetadata to ingestion responses.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Application] --> B{API path} B --> C[Legacy batch ingestion] B --> D[OTLP trace ingestion] B --> E[V2 observations and metrics] C --> F[IngestionResponse with deprecation metadata] D --> G[x-langfuse-ingestion-version: 4 required] E --> H[Real-time read APIs] A --> I[Typed SDK-log imports] I --> J[Removed public models]Reviews (1) · Last reviewed commit: "feat(api): update API spec from langfuse..."