-
Notifications
You must be signed in to change notification settings - Fork 224
feat(api): update API spec from langfuse/langfuse 41f064c #1492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 files reviewed, 14 comments
| model_parameters: typing.Any = pydantic_v1.Field(alias="modelParameters") | ||
| """ | ||
| The parameters of the model used for the observation | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from Optional[typing.Dict[str, MapValue]] with default=None to required typing.Any field. If the API response doesn't include modelParameters, Pydantic validation will fail during deserialization.
| model_parameters: typing.Any = pydantic_v1.Field(alias="modelParameters") | |
| """ | |
| The parameters of the model used for the observation | |
| """ | |
| model_parameters: typing.Any = pydantic_v1.Field(default=None, alias="modelParameters") |
Does the API always return modelParameters, metadata, input, output, usage, usageDetails, costDetails, and environment fields in observation responses?
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/observation.py
Line: 57:60
Comment:
**logic:** Changed from `Optional[typing.Dict[str, MapValue]]` with `default=None` to required `typing.Any` field. If the API response doesn't include `modelParameters`, Pydantic validation will fail during deserialization.
```suggestion
model_parameters: typing.Any = pydantic_v1.Field(default=None, alias="modelParameters")
```
Does the API always return modelParameters, metadata, input, output, usage, usageDetails, costDetails, and environment fields in observation responses?
How can I resolve this? If you propose a fix, please make it concise.| input: typing.Any = pydantic_v1.Field() | ||
| """ | ||
| The input data of the observation | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from optional to required field without default value. This will cause validation errors if API responses don't include input.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/observation.py
Line: 62:65
Comment:
**logic:** Changed from optional to required field without default value. This will cause validation errors if API responses don't include `input`.
How can I resolve this? If you propose a fix, please make it concise.| metadata: typing.Any = pydantic_v1.Field() | ||
| """ | ||
| Additional metadata of the observation | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from optional to required without default. Will fail validation if metadata is missing from API response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/observation.py
Line: 72:75
Comment:
**logic:** Changed from optional to required without default. Will fail validation if `metadata` is missing from API response.
How can I resolve this? If you propose a fix, please make it concise.| output: typing.Any = pydantic_v1.Field() | ||
| """ | ||
| The output data of the observation | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from optional to required without default. Will fail validation if output is missing from API response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/observation.py
Line: 77:80
Comment:
**logic:** Changed from optional to required without default. Will fail validation if `output` is missing from API response.
How can I resolve this? If you propose a fix, please make it concise.| usage: Usage = pydantic_v1.Field() | ||
| """ | ||
| (Deprecated. Use usageDetails and costDetails instead.) The usage data of the observation | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from optional to required without default. Will fail validation if deprecated usage field is missing from API response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/observation.py
Line: 82:85
Comment:
**logic:** Changed from optional to required without default. Will fail validation if deprecated `usage` field is missing from API response.
How can I resolve this? If you propose a fix, please make it concise.| environment: str = pydantic_v1.Field() | ||
| """ | ||
| The environment from which this score originated. Can be any lowercase alphanumeric string with hyphens and underscores that does not start with 'langfuse'. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed from Optional[str] with default=None to required str without default. Will fail validation if environment is missing from API response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/base_score.py
Line: 71:74
Comment:
**logic:** Changed from `Optional[str]` with `default=None` to required `str` without default. Will fail validation if `environment` is missing from API response.
How can I resolve this? If you propose a fix, please make it concise.| input: typing.Any = pydantic_v1.Field() | ||
| """ | ||
| Input data for the dataset item | ||
| """ | ||
|
|
||
| expected_output: typing.Any = pydantic_v1.Field(alias="expectedOutput") | ||
| """ | ||
| Expected output for the dataset item | ||
| """ | ||
|
|
||
| metadata: typing.Any = pydantic_v1.Field() | ||
| """ | ||
| Metadata associated with the dataset item | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed input, expected_output, and metadata from optional to required without defaults. These fields now must be present in all API responses or deserialization will fail.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/dataset_item.py
Line: 14:27
Comment:
**logic:** Changed `input`, `expected_output`, and `metadata` from optional to required without defaults. These fields now must be present in all API responses or deserialization will fail.
How can I resolve this? If you propose a fix, please make it concise.| input: int = pydantic_v1.Field() | ||
| """ | ||
| Number of input units (e.g. tokens) | ||
| """ | ||
|
|
||
| output: typing.Optional[int] = pydantic_v1.Field(default=None) | ||
| output: int = pydantic_v1.Field() | ||
| """ | ||
| Number of output units (e.g. tokens) | ||
| """ | ||
|
|
||
| total: typing.Optional[int] = pydantic_v1.Field(default=None) | ||
| total: int = pydantic_v1.Field() | ||
| """ | ||
| Defaults to input+output if not set | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed input, output, and total from optional to required without defaults. Since this is a deprecated type, existing API responses that omit these fields will now fail validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/usage.py
Line: 15:28
Comment:
**logic:** Changed `input`, `output`, and `total` from optional to required without defaults. Since this is a deprecated type, existing API responses that omit these fields will now fail validation.
How can I resolve this? If you propose a fix, please make it concise.| tags: typing.List[str] = pydantic_v1.Field() | ||
| """ | ||
| The tags associated with the trace. Can be an array of strings or null. | ||
| The tags associated with the trace. | ||
| """ | ||
|
|
||
| public: typing.Optional[bool] = pydantic_v1.Field(default=None) | ||
| public: bool = pydantic_v1.Field() | ||
| """ | ||
| Public traces are accessible via url without login | ||
| """ | ||
|
|
||
| environment: typing.Optional[str] = pydantic_v1.Field(default=None) | ||
| environment: str = pydantic_v1.Field() | ||
| """ | ||
| The environment from which this trace originated. Can be any lowercase alphanumeric string with hyphens and underscores that does not start with 'langfuse'. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed tags, public, and environment from optional to required without defaults. API responses missing these fields will fail validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/trace.py
Line: 63:76
Comment:
**logic:** Changed `tags`, `public`, and `environment` from optional to required without defaults. API responses missing these fields will fail validation.
How can I resolve this? If you propose a fix, please make it concise.| metadata: typing.Any | ||
| config_id: typing.Optional[str] = pydantic_v1.Field(alias="configId", default=None) | ||
| queue_id: typing.Optional[str] = pydantic_v1.Field(alias="queueId", default=None) | ||
| environment: typing.Optional[str] = None | ||
| environment: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Changed metadata from Optional[typing.Any] = None to required typing.Any without default, and environment from Optional[str] = None to required str without default. This pattern is repeated in all three Score variants (Numeric, Categorical, Boolean). API responses missing these fields will fail validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/resources/commons/types/score.py
Line: 35:38
Comment:
**logic:** Changed `metadata` from `Optional[typing.Any] = None` to required `typing.Any` without default, and `environment` from `Optional[str] = None` to required `str` without default. This pattern is repeated in all three Score variants (Numeric, Categorical, Boolean). API responses missing these fields will fail validation.
How can I resolve this? If you propose a fix, please make it concise.
Important
Add
expand_metadataparameter toget_manyinObservationsV2ClientandAsyncObservationsV2Clientfor full metadata retrieval and update models to ensuremetadatais non-optional.expand_metadataparameter toget_many()inObservationsV2ClientandAsyncObservationsV2Clientto retrieve full metadata values.reference.mdto documentexpand_metadatausage.metadatafield to non-optional inBaseScore,BaseScoreV1,Comment,Dataset,DatasetItem,DatasetRun,DatasetRunItem,Model,Observation,Score_Numeric,Score_Categorical,Score_Boolean,ScoreConfig,ScoreV1_Numeric,ScoreV1_Categorical,ScoreV1_Boolean,Session,Trace,TraceWithDetails,TraceWithFullDetails,Usage, andGetScoresResponseDataclasses.client.pyto includeexpand_metadataparameter details.This description was created by
for fb0e3dc. You can customize this summary. It will automatically update as commits are pushed.
Disclaimer: Experimental PR review
Greptile Summary
Updated API spec from langfuse/langfuse commit 41f064c, bringing type definitions in sync with the backend API. The changes include added documentation strings, a new
expandMetadataparameter for the observations API, and critically, multiple fields changed from optional to required across core types (Observation,Trace,Score,DatasetItem,Usage,BaseScore).Key Changes:
expandMetadataparameter to observations API for retrieving full metadata values without truncationOptional[Type]withdefault=Noneto requiredTypewithout defaults, including:Observation:model_parameters,input,metadata,output,usage,usage_details,cost_details,environmentTrace:tags,public,environmentScorevariants:metadata,environmentDatasetItem:input,expected_output,metadataUsage:input,output,totalBaseScore:metadata,environmentCritical Concern:
If the Langfuse API doesn't guarantee these fields are always present in responses, this update will cause Pydantic validation errors during deserialization, breaking existing client code. The removal of
default=Nonefrompydantic_v1.Field()makes these fields strictly required.Recommendation:
Verify that the backend API (commit 41f064c) always returns these now-required fields in all responses, or revert the fields to optional with appropriate defaults to maintain backward compatibility.
Confidence Score: 2/5
Observation,Trace,Score, and other core types affect critical SDK functionality.langfuse/api/resources/commons/types/observation.py,langfuse/api/resources/commons/types/trace.py,langfuse/api/resources/commons/types/score.py,langfuse/api/resources/commons/types/base_score.py,langfuse/api/resources/commons/types/dataset_item.py, andlangfuse/api/resources/commons/types/usage.pyrequire verification that the API always returns the newly required fieldsImportant Files Changed
Optionalto required (typing.Anyor specific types), which could break deserialization if API doesn't return these fieldsmetadataandenvironmentfrom optional to required fieldsinput,expected_output, andmetadatafrom optional to required, added docstringsinput,output, andtotalfrom optional to required, removedModelUsageUnitimport and changedunittype toOptional[str]tags,public, andenvironmentfrom optional to required fieldsmetadataandenvironmentfrom optional to required in all three Score union variants (Numeric, Categorical, Boolean)expand_metadataparameter to allow retrieving full metadata values without truncationSequence Diagram
sequenceDiagram participant API as Langfuse API participant Client as Python SDK Client participant Models as Pydantic Models participant User as User Application Note over API,Models: API Spec Update Flow API->>Client: API spec updated (41f064c) Client->>Models: Generate updated types Note over Models: Key Changes:<br/>- Fields: Optional → Required<br/>- Added docstrings<br/>- New expandMetadata param User->>Client: Fetch observations/scores/traces Client->>API: GET request with params API-->>Client: JSON response Client->>Models: Deserialize to Pydantic models alt All required fields present Models-->>Client: Success Client-->>User: Typed objects returned else Required field missing Models-->>Client: ValidationError Client-->>User: Error propagated end Note over User,Models: Breaking Change Risk:<br/>If API doesn't return newly<br/>required fields, deserialization fails