docs(evals): mark batch evaluation as legacy - #1867
Conversation
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| input=cls._parse_io_value(observation.input), | ||
| output=cls._parse_io_value(observation.output), | ||
| session_id=observation.session_id, | ||
| release=observation.release, | ||
| version=observation.version, | ||
| user_id=observation.user_id, | ||
| metadata=observation.metadata, | ||
| tags=observation.tags or [], | ||
| public=observation.public or False, | ||
| environment=observation.environment or "default", | ||
| html_path="", | ||
| latency=observation.latency, | ||
| total_cost=observation.total_cost, |
There was a problem hiding this comment.
Trace-scope records now copy input, output, metadata, version, latency, and total_cost from one root observation. These are observation-level values, while the previous trace endpoint returned trace-level payloads and metrics aggregated across all observations. When a root span differs from its trace or the trace has child costs and latency, existing mappers evaluate different data and create incorrect trace scores.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1351-1363
Comment:
**Root Data Replaces Trace Data**
Trace-scope records now copy `input`, `output`, `metadata`, `version`, `latency`, and `total_cost` from one root observation. These are observation-level values, while the previous trace endpoint returned trace-level payloads and metrics aggregated across all observations. When a root span differs from its trace or the trace has child costs and latency, existing mappers evaluate different data and create incorrect trace scores.
**Knowledge Base Used:**
- [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)
- [Observations, spans, and traces](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/observations-spans-and-traces.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Addressed by making the v2 path explicitly opt-in via observation_read_api="v2". The default remains "legacy", so existing trace mappers continue receiving trace-level data from GET /api/public/traces; the docstring now explains that v2 trace items are derived from root observations.
There was a problem hiding this comment.
Follow-up: the v2 trace adaptation has now been removed entirely. observation_read_api="v2" rejects trace scope immediately, so root-observation data can no longer be substituted for trace data.
There was a problem hiding this comment.
Final update: all v2 read support and trace adaptation have been removed from this PR. It now changes docstrings only, so this concern no longer applies.
| observations=[], | ||
| scores=[], |
There was a problem hiding this comment.
scores and observations remain valid fetch_trace_fields groups, but the v2 field mapping ignores both and this adapter always returns empty lists. A mapper that explicitly requests either group therefore receives no data, which can fail the item or produce an incorrect evaluation.
Knowledge Base Used: Experiments and batch evaluation
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1364-1365
Comment:
**Requested Trace Fields Are Empty**
`scores` and `observations` remain valid `fetch_trace_fields` groups, but the v2 field mapping ignores both and this adapter always returns empty lists. A mapper that explicitly requests either group therefore receives no data, which can fail the item or produce an incorrect evaluation.
**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Addressed. Legacy remains the default and preserves all trace field groups. The opt-in v2 path now rejects explicit scores, observations, or metrics groups with a clear ValueError instead of returning silently incomplete data.
There was a problem hiding this comment.
Follow-up: v2 is now observation-scope only. Trace scope with observation_read_api="v2" is rejected, eliminating the possibility of returning empty legacy trace field groups.
There was a problem hiding this comment.
Final update: all v2 read support has been removed. Legacy field handling is unchanged; this PR now documents the existing compatibility only.
| column_aliases = { | ||
| "timestamp": "startTime", | ||
| "start_time": "startTime", | ||
| "user_id": "userId", | ||
| "session_id": "sessionId", | ||
| } |
There was a problem hiding this comment.
The previous trace endpoint accepts filters such as release, bookmarked, observation-level counts, and score aggregates. This adapter translates only four column names before sending the filter to the v2 observations endpoint, which does not support those trace-only columns. Existing batch evaluations that use these documented filters now fail to fetch their matching traces.
Knowledge Base Used: Experiments and batch evaluation
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1300-1305
Comment:
**Trace Filters Lose Compatibility**
The previous trace endpoint accepts filters such as `release`, `bookmarked`, observation-level counts, and score aggregates. This adapter translates only four column names before sending the filter to the v2 observations endpoint, which does not support those trace-only columns. Existing batch evaluations that use these documented filters now fail to fetch their matching traces.
**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Addressed for backward compatibility: legacy is again the default, so existing trace filters continue going to the trace endpoint unchanged. The v2 path is explicit opt-in and its docstring calls out that not all legacy trace filters are available.
There was a problem hiding this comment.
Follow-up: v2 trace reads are now disallowed altogether. Trace filters are therefore always evaluated by the legacy trace endpoint and retain their existing semantics.
There was a problem hiding this comment.
That resolves this issue. With v2 reads disallowed for trace scope, trace filters continue to be handled by the legacy trace endpoint and retain their existing semantics. No further change is needed for this comment.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
There was a problem hiding this comment.
Final update: all v2 read and filter translation code has been removed. Existing legacy trace filter behavior is unchanged.
| if "io" in requested_fields: | ||
| selected_fields.update({"io", "metadata"}) |
There was a problem hiding this comment.
The default fetch_trace_fields="io" maps to the v2 metadata group, but that endpoint truncates metadata values to 200 characters unless expanded metadata is requested. The new request does not enable that option, so existing trace mappers can receive partial metadata and score different content than before.
Knowledge Base Used: Experiments and batch evaluation
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1282-1283
Comment:
**Trace Metadata Is Truncated**
The default `fetch_trace_fields="io"` maps to the v2 `metadata` group, but that endpoint truncates metadata values to 200 characters unless expanded metadata is requested. The new request does not enable that option, so existing trace mappers can receive partial metadata and score different content than before.
**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Addressed as a default-path regression: batch evaluation now uses the legacy APIs unless the caller explicitly selects observation_read_api="v2". Thus existing mappers retain the legacy endpoint's metadata behavior; v2 retains its native observation metadata semantics.
There was a problem hiding this comment.
Final update: the v2 metadata path has been removed. This PR now makes no runtime changes and only documents legacy platform compatibility.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 334ac151c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| input=cls._parse_io_value(observation.input), | ||
| output=cls._parse_io_value(observation.output), |
There was a problem hiding this comment.
When scope="traces" and callers have used set_trace_io() independently of the root span's input/output, these lines substitute observation I/O for trace I/O. Those values use distinct OpenTelemetry attributes, and set_trace_io() is specifically retained for legacy evaluators, so existing mappers such as input=trace.input silently grade the wrong content. Obtain the actual trace-level I/O rather than treating root-observation I/O as equivalent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed by restoring the legacy trace endpoint as the default. Root-observation I/O is now used only when callers explicitly select the platform-v4-compatible observation_read_api="v2"; that distinction is documented on the public keyword argument.
There was a problem hiding this comment.
Follow-up: the synthetic v2 trace path has been removed. Selecting v2 with trace scope now raises a clear ValueError; actual trace-level I/O always comes from the legacy trace endpoint.
There was a problem hiding this comment.
Final update: the synthetic trace path was removed with all v2 support. Trace I/O behavior remains exactly as it was before this PR.
| "column": "isRootObservation", | ||
| "operator": "=", | ||
| "value": True, |
There was a problem hiding this comment.
When one OpenTelemetry trace has multiple logical application roots—for example, tests/unit/test_app_root_detection.py::test_filtered_parent_marks_exported_children_as_app_roots marks both siblings—this predicate returns every root. _observation_to_trace then assigns each item the same trace_id, so a trace-scoped run evaluates the trace repeatedly and writes duplicate trace scores. Deduplicate or group by trace ID, or select one canonical root, before processing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. The opt-in v2 trace path now deduplicates fetched root observations by trace ID across batches, and unit coverage verifies that two logical roots produce one evaluation and one trace score.
There was a problem hiding this comment.
Follow-up: rather than deduplicating synthetic roots, v2 trace scope is now rejected entirely. This removes the duplicate-score path and avoids synthesizing incomplete traces.
There was a problem hiding this comment.
Final update: root-observation trace handling was removed entirely. This PR is documentation-only, so duplicate v2 trace evaluation is no longer possible.
| and condition.get("column") in column_aliases | ||
| ): | ||
| condition["column"] = column_aliases[condition["column"]] |
There was a problem hiding this comment.
For scope="traces", every unlisted condition is forwarded with observation semantics. A legacy trace filter on id, name, or metadata therefore targets the root observation's corresponding field even though observation IDs and data can differ from the trace, while trace aggregate filters such as scores_avg are unsupported by the v2 endpoint. Targeted runs can silently select no or incorrect traces, or fail with a 400 response, so trace filters need complete translation or explicit validation rather than only these aliases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed for existing users by keeping observation_read_api="legacy" as the default and forwarding legacy filters unchanged. The v2 path is now explicit and documented as not supporting every legacy trace-specific filter.
There was a problem hiding this comment.
Follow-up: the v2 trace path no longer exists. All trace-scope filters continue through the legacy trace API; v2 is restricted to observation scope.
There was a problem hiding this comment.
Final update: the v2 trace/filter implementation was removed. Legacy trace filters continue through the unchanged legacy endpoint.
| latency=observation.latency, | ||
| total_cost=observation.total_cost, | ||
| observations=[], | ||
| scores=[], |
There was a problem hiding this comment.
When callers leave fetch_trace_fields=None—documented as returning all fields—or explicitly request observations, scores, or metrics, this adapter always returns empty child/score lists and root-observation metrics rather than trace aggregates. Mappers and composite evaluators using these fields consequently produce incorrect results on any server exposing v2, including v3 servers whose legacy trace endpoint could still provide the data. Fetch or enrich the requested groups, or reject unsupported groups instead of fabricating empty values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed. Default legacy reads continue honoring all existing field groups. For explicit v2 trace reads, scores, observations, and aggregate metrics are rejected with a clear error; only core and io are documented as supported.
There was a problem hiding this comment.
Follow-up: v2 trace reads are now rejected instead of supporting a reduced field set. This preserves the complete legacy trace-field contract for every allowed trace-scope run.
There was a problem hiding this comment.
Final update: all v2 field adaptation was removed. Existing legacy trace field behavior remains unchanged.
| def _build_v2_filter(*, filter: Optional[str], scope: str) -> Optional[str]: | ||
| """Adapt legacy filter columns and select root observations for traces.""" | ||
| try: | ||
| filters = json.loads(filter) if filter else [] | ||
| except json.JSONDecodeError: | ||
| return filter | ||
|
|
||
| if not isinstance(filters, list): | ||
| return filter | ||
|
|
||
| column_aliases = { | ||
| "timestamp": "startTime", | ||
| "start_time": "startTime", | ||
| "user_id": "userId", | ||
| "session_id": "sessionId", | ||
| } | ||
| for condition in filters: | ||
| if ( | ||
| isinstance(condition, dict) | ||
| and condition.get("column") in column_aliases | ||
| ): | ||
| condition["column"] = column_aliases[condition["column"]] | ||
|
|
||
| if scope == "traces": | ||
| filters.append( | ||
| { | ||
| "type": "boolean", | ||
| "column": "isRootObservation", | ||
| "operator": "=", | ||
| "value": True, | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
🔴 Traces scope now selects items via an isRootObservation=true filter on the v2 observations API, but a single trace can have multiple app-root observations (per the API's own docs), so one trace can now yield several synthetic "trace" items sharing the same trace_id. Each is evaluated and scored independently, so the same trace can get duplicate scores/evaluator runs that never happened when trace.list returned one row per trace. Fix: dedupe by trace_id after mapping (or pick a single canonical root per trace, e.g. earliest start_time) before processing.
Extended reasoning...
_build_v2_filter (line ~1317) appends {"column":"isRootObservation","value":true} for scope=='traces'; observations.get_many can legitimately return >1 observation with isRootObservation=true for the same trace_id (per the SDK's own docstring: 'An app-root observation may have isRootObservation=true and a non-null parentObservationId'). _observation_to_trace (line 1344) maps each such observation to its own TraceWithFullDetails with id=trace_id. The main loop (lines ~1041-1082) has no dedupe by item id across the batch/run, so _process_batch_evaluation_item runs and create_score fires once per duplicate root, silently multiplying scores for one trace — a regression versus the legacy trace.list path which returned exactly one row per trace.
Verification: normal. The v2 traces path can return multiple rows per trace, which the base (trace.list) never did. Trigger: _build_v2_filter appends {"column":"isRootObservation","operator":"=","value":True} for scope=='traces'. Per the SDK's own docstring (langfuse/api/observations/client.py:123-125), isRootObservation=true matches "observations without a physical parent AND observations marked…
There was a problem hiding this comment.
Fixed. V2 trace results are deduplicated by trace ID across the full run, including cursor pages, before mapping/evaluation. Added a regression test asserting multiple logical roots create only one score.
There was a problem hiding this comment.
Follow-up: the root-observation-to-trace conversion was removed. scope="traces", observation_read_api="v2" now fails validation before any request or score creation.
There was a problem hiding this comment.
Final update: all v2 trace support was removed. The PR now contains documentation changes only, so this issue no longer applies.
| return json.dumps(filters) | ||
|
|
||
| @classmethod | ||
| def _observation_to_trace(cls, observation: ObservationV2) -> TraceWithFullDetails: | ||
| """Adapt a v2 root observation to the established trace mapper contract.""" | ||
| trace_id = observation.trace_id or observation.id | ||
| return TraceWithFullDetails.model_construct( | ||
| id=trace_id, | ||
| timestamp=observation.start_time, | ||
| name=observation.trace_name or observation.name, | ||
| input=cls._parse_io_value(observation.input), | ||
| output=cls._parse_io_value(observation.output), | ||
| session_id=observation.session_id, | ||
| release=observation.release, | ||
| version=observation.version, | ||
| user_id=observation.user_id, | ||
| metadata=observation.metadata, | ||
| tags=observation.tags or [], | ||
| public=observation.public or False, | ||
| environment=observation.environment or "default", | ||
| html_path="", | ||
| latency=observation.latency, | ||
| total_cost=observation.total_cost, | ||
| observations=[], | ||
| scores=[], |
There was a problem hiding this comment.
🔴 _observation_to_trace hardcodes scores=[] and observations=[] on every mapped TraceWithFullDetails, so when the v2 path is used (now the default, not just a fallback) requesting fetch_trace_fields='scores' or 'observations' silently returns empty lists instead of the actual data the docstring still promises. Fix: either populate these from a supplementary v2/legacy call when those field groups are requested, or update the docstring/validation to reject 'scores'/'observations' field groups for the v2 path so callers don't silently get wrong data. [also at: langfuse/batch_evaluation.py:1280 - The fetch_trace_fields docstring (line 876) still documents 'scores' and 'observations' as requestable field groups…]
Extended reasoning...
run_async(scope='traces', fetch_trace_fields='core,scores') calls _fetch_batch_with_retry -> observations.get_many (v2) succeeds -> _observation_to_trace maps each ObservationV2 to TraceWithFullDetails with scores=[] and observations=[] unconditionally, regardless of fetch_trace_fields. Previously (v3 trace.list with fields='scores') the real score/observation arrays were returned. A user's mapper/evaluator that reads trace.scores to check for prior scores or dedupe now always sees an empty list, with no error or log indicating the field group was dropped.
Verification: normal. The v2 path is now the default (only falls back to legacy on a 404, langfuse/batch_evaluation.py:1197-1215). For scope=="traces", every ObservationV2 is mapped through _observation_to_trace, which hardcodes observations=[] and scores=[] unconditionally (lines 1364-1365), ignoring fetch_trace_fields. ObservationV2 carries no scores field (grep for "score" in… | Severity: normal…
There was a problem hiding this comment.
Fixed. The legacy API is now the default and retains the established field behavior. Explicit v2 trace reads validate fetch_trace_fields and reject scores, observations, and metrics rather than fabricating empty or non-aggregate values.
There was a problem hiding this comment.
Follow-up: resolved by disallowing the combination rather than returning partial trace objects. V2 is observation-scope only; trace scope always uses the legacy API and honors its field groups.
There was a problem hiding this comment.
Final update: the v2 trace adapter and field mapping were removed entirely. Runtime behavior is unchanged from the base branch.
| response = self.client.api.observations.get_many( | ||
| fields=self._get_v2_observation_fields( | ||
| scope=scope, trace_fields=fields | ||
| ), | ||
| cursor=cursor, | ||
| limit=limit, | ||
| filter=self._build_v2_filter(filter=filter, scope=scope), | ||
| request_options={"max_retries": max_retries}, |
There was a problem hiding this comment.
🔴 The v2 observations.get_many call requests the 'metadata' field group but never sets expand_metadata, so the server truncates every observation/trace metadata value to 200 chars by default (per langfuse/api/observations/client.py docstring) — a limit the legacy trace.list/observations_v1.get_many APIs this replaces never applied. Since metadata is included whenever fetch_trace_fields (default 'io') includes 'io' for traces, and always for scope='observations', every default run now silently hands mappers/evaluators truncated metadata instead of the full value they got pre-diff. Fix: pass expand_metadata for the metadata keys needed, or drop 'metadata' from the requested field groups unless the caller explicitly opts into truncation-aware handling.
Extended reasoning...
_get_v2_observation_fields adds 'metadata' to selected_fields whenever 'io' is requested (default fetch_trace_fields='io') or unconditionally for scope='observations'. _fetch_batch_with_retry then calls self.client.api.observations.get_many(fields=..., cursor=..., limit=..., filter=...) at line 1198 without expand_metadata. Per the v2 API's own documented contract, metadata values over 200 chars are truncated unless expand_metadata names the keys to expand. _observation_to_trace/_observation_to_legacy_view then copy this truncated value straight into TraceWithFullDetails.metadata/ObservationsView.metadata (lines 1357, 1386), so a user mapper that does trace.metadata.get('input') or similar (as shown in the module's own mapper docstring example) silently receives truncated/garbled data on any server new enough to serve v2, with no error surfaced.
Verification: normal. The v2 call omits expand_metadata, so metadata is server-truncated to 200 chars on the now-default path, whereas the legacy APIs it replaces returned full metadata — a real regression. At langfuse/batch_evaluation.py:1198-1205 the call is: ``` response = self.client.api.observations.get_many( fields=self._get_v2_observation_fields(scope=scope, trace_fields=fields),… | normal. The v2…
There was a problem hiding this comment.
Addressed as a compatibility regression by restoring legacy reads as the default. Metadata from v2 is now only used after explicit observation_read_api="v2" selection, while existing batch evaluations retain the legacy endpoint's complete metadata behavior.
There was a problem hiding this comment.
Final update: the v2 observations implementation was removed. This PR now only documents that batch evaluation uses legacy platform-v3 endpoints.
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
cursor
Bot
changed the title
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
cursor
Bot
changed the title
2 participants
What does this PR do?
Marks
run_batched_evaluationas legacy in its docstrings without deprecating it or changing runtime behavior.The documentation now explicitly states that it reads:
GET /api/public/tracesGET /api/public/observationsendpointIt also states that batch evaluation is supported with Langfuse platform v3 and is not yet supported with platform v4.
Type of change
Verification
uv run --frozen ruff check . uv run --frozen ruff format --check langfuse/_client/client.py langfuse/batch_evaluation.py uv run --frozen mypy langfuse --no-error-summaryThe repository-wide format check is currently blocked by the pre-existing formatting of
tests/unit/test_media.py; the changed files pass the format check.Checklist
code_review.md.Linear Issue: LFE-16015
Greptile Summary
This PR moves batch trace and observation reads to the cursor-paginated v2 Observations API, adapts v2 records to the existing mapper models, and retains a 404-triggered fallback to legacy APIs. The compatibility layer currently does not preserve several established trace contracts:
Confidence Score: 1/5
This PR is not safe to merge until the v2 trace path preserves trace-level mapper data, requested field groups, filter compatibility, and complete metadata.
Four independent compatibility failures can make established trace-scope evaluations fail to fetch records or evaluate incomplete and semantically different data.
Files Needing Attention: langfuse/batch_evaluation.py
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Batch evaluation] --> B[V2 observations API] B --> C{Scope} C -->|observations| D[ObservationsView adapter] C -->|traces| E[Logical root observation] E --> F[Synthetic TraceWithFullDetails] D --> G[Mapper and evaluators] F --> G B -->|404| H[Legacy trace or observations API] H --> G G --> I[Scores on source entity]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "test(evals): correct fallback filter fix..." | Re-trigger Greptile
Context used (3)