fix(client): point the dataset-run helpers at the v4 read path in docs and logs - #1912
Open
passionworkeer wants to merge 1 commit into
Conversation
…s and logs
Fixes langfuse-python#1906.
A Langfuse v4 deployment rejects the legacy dataset-run read endpoints with
404, so `get_dataset_run()`, `get_dataset_runs()` and `delete_dataset_run()`
cannot work there. Two things made that worse than merely inconvenient.
The docstrings said nothing about v4, and the raised error's `str()` starts
with the response headers, so the only signal a caller got was a bare 404
whose explanation sits in `exc.body`.
The error handlers were also dead code. All three were written as
`except Error as e:`, and `Error` here is the fern-generated
`langfuse.api.Error` -- a *subclass* of `langfuse.api.core.api_error.ApiError`.
The errors the SDK actually raises (`NotFoundError` and the rest of the
exported set) are `ApiError` subclasses that are not `Error`, so the clause
never fired and `handle_fern_exception` was never called either:
>>> issubclass(NotFoundError, ApiError)
True
>>> issubclass(NotFoundError, Error)
False
This change:
- adds a v4 note to each docstring naming the replacement read path, notes
that the `dataset_run_id` from `run_experiment()` is the same value those
return, and links the migration guide. `from_start_time` is called out as
required, since a literal follower otherwise gets a TypeError;
- makes the three clauses catch `ApiError`;
- routes all three failures through one `_handle_dataset_run_error()`, which
emits the v4 guidance and otherwise leaves the exception alone.
`handle_fern_exception` and `generate_error_message_fern` now take `ApiError`
instead of the narrower `Error`. Widening a parameter is safe for every
existing caller, `generate_error_message_fern` already dispatched on
`isinstance(..., ApiError)`, and leaving the annotations alone made mypy fail
with three `arg-type` errors on the newly reachable calls.
Two deliberate omissions in `_handle_dataset_run_error`:
- 404 is not routed to `handle_fern_exception`. Its 404 entry reads
"Internal error occurred. This is an unusual occurrence and we are monitoring
it closely" -- these helpers raise `NotFoundError` routinely, because asking
for a run that does not exist is an ordinary outcome, and a false claim at
ERROR level is also what error alerting keys on.
- a refused delete gets its own hint, not the read hint. There is no delete
counterpart on `client.api.experiments`, so telling a caller whose delete
was refused to "read the run" could lead them to conclude it was removed. It
was not.
Routing the helpers themselves onto the v4 read path is not part of this
change and is not a drop-in: `DatasetRunItem.id` has no experiment
counterpart, `dataset_name` needs a `datasets.list()` lookup, `metadata`
needs `fields=metadata`, `startTime`/`endTime` are clipped to the requested
`from_start_time` window, and pagination is cursor-based rather than
page-based. That mapping is a product decision.
Five other `except Error` clauses in this file share the latent bug --
`get_dataset`, `auth_check`, `create_dataset`, `create_dataset_item` and
`create_prompt` -- but they are left alone here: widening them changes
error-logging behaviour across unrelated methods.
Verified against a real v4 events_only deployment (langfuse 4.42.0): before,
all three helpers raised with no guidance and the error logger never ran;
after, the two read helpers log the read guidance, the delete helper logs the
delete-specific guidance, nothing is reported as an internal error, a
non-404 API error still reaches the original logger, and
`client.api.experiments.list(...)` returns ExperimentsResponse.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2608
to
+2611
| ``client.api.experiments.list(from_start_time=...)`` and match on ``id`` | ||
| or ``name`` instead -- ``from_start_time`` is required, and | ||
| ``run_experiment()`` returns that same value as ``dataset_run_id``. See | ||
| https://langfuse.com/docs/v4. |
Contributor
There was a problem hiding this comment.
dataset_name and run_name, but this guidance suggests matching an experiment by name alone. If two datasets use the same run name, that match is ambiguous and a caller could select the wrong run. Advise callers to include dataset_id when matching by name, or to use the run ID.
Knowledge Base Used: Dataset and run management
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_client/client.py
Line: 2608-2611
Comment:
**Run name lacks dataset scope** The old lookup uses both `dataset_name` and `run_name`, but this guidance suggests matching an experiment by name alone. If two datasets use the same run name, that match is ambiguous and a caller could select the wrong run. Advise callers to include `dataset_id` when matching by name, or to use the run ID.
**Knowledge Base Used:** [Dataset and run management](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/dataset-and-run-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(client): point the dataset-run helpers at the v4 read path in docs and logs
Fixes #1906.
What is wrong
On a Langfuse v4 deployment the three dataset-run read helpers cannot work: the
endpoints behind them are rejected with 404 in
events_onlymode. Two things madethat a bad experience rather than a merely inconvenient one.
The docstrings said nothing about v4. So the only signal a caller got was a
404 whose Python
str()begins with the response headers — the server's ownexplanation is only reachable through
exc.body.The error handlers were dead code. All three helpers were written as:
where
Errorislangfuse.api.Error, the fern-generated base class. The errorsthe SDK actually raises derive from
langfuse.api.core.api_error.ApiError:So the clause never fired for a server rejection, and
handle_fern_exception—which only logs — was never called either.
What this changes
Each of the three docstrings now states that the endpoint is unavailable on v4,
names
client.api.experiments.list()/list_items()as the replacement, notesthat the
dataset_run_idreturned byrun_experiment()is the same value thosereturn, and links the migration guide.
The three clauses become
except (Error, ApiError), which also lets thepre-existing
handle_fern_exceptioncall do the job it was written for._warn_if_v4_dataset_run_rejected(e)adds oneWARNINGnaming the replacement —only when the server's body actually carries the v4 rejection. The exception type
is unchanged, so anything already catching
NotFoundErrorkeeps working.Routing the helpers themselves onto the v4 read path is deliberately not part of
this change. It is not a drop-in:
DatasetRunItem.idhas no experiment counterpart,dataset_nameneeds adatasets.list()lookup,metadataneedsfields=metadata,startTime/endTimeare clipped to the requestedfrom_start_timewindow, andpagination is cursor-based instead of page-based. That mapping is a product decision
and belongs to a maintainer.
Scope note
client.pycontains eightexcept Errorclauses and all eight have this latentbug. The five left alone are
get_dataset,auth_check,create_dataset,create_dataset_itemandcreate_prompt. Widening them alters error-loggingbehaviour across unrelated methods, so it is left for a separate decision.
404 is not routed through the error logger
handle_fern_exceptionmaps a bare status onto a generic message, and its 404 entryreads "Internal error occurred. This is an unusual occurrence and we are monitoring it
closely." These three helpers raise
NotFoundErrorroutinely — asking for a run thatdoes not exist is an ordinary outcome — so routing 404 through it would ship a false
claim at ERROR level, which is also what error alerting keys on. A v4 refusal and a 404
therefore log the actionable warning (or nothing), while every other status still goes
through
handle_fern_exceptionas originally intended.Verification
Against a real self-hosted Langfuse v4
events_onlydeployment (langfuse-web4.42.0) on base
0bc5897b:run_experiment()returns a populateddataset_run_idwithitem_count=2and adataset_run_url; the same client then fails to read that run back throughget_dataset_run()with the v4 404.get_dataset_run,get_dataset_runsanddelete_dataset_runall raisedNotFoundErrorwith no guidance reachable, and the error-log handler never ran.client.api.experiments.list(...)— thereplacement the guidance names — returns
ExperimentsResponseon the samedeployment.
New unit tests in
tests/unit/test_dataset_run_v4_guidance.py:ordinary 404, on missing bodies and on non-dict bodies;
client.api.experiments, so pointing the caller at the read path could lead them toconclude the run had been removed;
ApiError/Errorclass relationship that caused the dead handlers;a v4 refusal logs the guidance and never "Internal error", an ordinary 404 logs neither,
and a non-404 API error still reaches the original logger.
The
dataset_run_id== experimentidclaim was checked against the live deployment:the experiment returned by
experiments.list()carries the same id, and it resolves bothby
idand byname.ruff checkandruff format --checkare clean on the new test file.client.pyhas563 pre-existing
rufffindings, identical before and after this change.The runtime change appears safe to merge, but the repository’s import-placement requirement must be satisfied first.
Summary
The PR adds v4 migration guidance to three legacy dataset-run helpers and catches generated API errors so a recognized v4 rejection produces an actionable warning. It also adds focused regression tests.
Reviews (1) · Last reviewed commit: "fix(client): point the dataset-run helpe..."