docs: upgrade docstrings as a first-class documentation surface for AI agents#1760
docs: upgrade docstrings as a first-class documentation surface for AI agents#1760jannikmaierhoefer wants to merge 9 commits into
Conversation
…I agents Coding agents treat the installed package as primary documentation (help(), inspect, grepping site-packages), so this improves the docstrings they hit most, verified against code and tests: - propagate_attributes: fix broken examples that called it as a client method (it is a top-level import) and used the nonexistent start_generation; document wrap-the-root-span ordering; add See also - Langfuse.api/async_api: new docstrings covering list-vs-get semantics (list endpoints return observation/score IDs as strings), asynchronous ingestion with a bounded retry snippet, and scores read methods (get_many/get_by_id; there is no scores.get) - flush(): note that server-side ingestion lags flush by a few seconds - @observe: document capture_input/capture_output/transform_to_string, as_type="generation" with update_current_generation example - run_experiment/Evaluation: add RAG LLM-as-a-judge example (faithfulness, answer relevance) and CI regression testing via langfuse/experiment-action; docstring for RunnerContext.run_experiment - package docstring/README/pyproject: lead with full capability map, current env var names, llms.txt pointer; fix stale v3 docs link and broken langfuse.otel import in the Langfuse class example - deprecation hygiene: emit DeprecationWarning when host= or LANGFUSE_HOST is the effective base URL source (behavior change); deprecation messages now name the exact replacement import Generated code under langfuse/api/ is untouched; list-vs-get and ingestion notes for it live on the non-generated api property and should also be mirrored into the fern spec in langfuse/langfuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@claude review |
Keep the change docs-only; the deprecation remains documented in the host parameter docstring and the LANGFUSE_HOST env var description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cross-checked against the langfuse-docs repo: - api/async_api/flush: align ingestion-lag wording with query-via-sdk docs (typically 15-30s, not "a few seconds"), replace retry snippet with the documented deadline-based pattern (capped backoff), note that a successful trace.get does not imply observations/scores are complete, recommend observations.get_many(trace_id=...) for row-level reads and the Metrics API for aggregation; add anchored doc links - run_experiment RAG example: use item.input (DatasetItem attribute) instead of item["input"], matching the documented dataset task shape - RegressionError: correct action input name to should_fail_on_regression (was should_fail_on_error; verified against experiment-action action.yml) - add See-also links: observation-types (observe/as_type) and environments (propagate_attributes) All 10 docstring URLs verified to map to content pages in langfuse-docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
langfuse/_client/client.py:312-331— The new DeprecationWarning guard (client.py:315-326) usesis Nonechecks while the actual_base_urlresolution three lines below (client.py:328-332) uses truthyorchaining, so they diverge on empty strings:LANGFUSE_BASE_URL=''+host=<url>silently skips the warning even thoughhostbecomes the effective URL, andhost=''fires a spurious warning even though it has no effect on resolution. Fix by mirroring the truthy checks used in the resolution.Extended reasoning...
The bug:
Langfuse.__init__now emits aDeprecationWarningwhenhost/LANGFUSE_HOSTis used instead ofbase_url/LANGFUSE_BASE_URL. The guard is:if base_url is None and os.environ.get(LANGFUSE_BASE_URL) is None: if host is not None: warnings.warn(...) # host deprecated elif os.environ.get(LANGFUSE_HOST) is not None: warnings.warn(...) # LANGFUSE_HOST deprecated
which uses is-None semantics. Three lines later, the actual URL resolution uses truthy
orchaining instead:self._base_url = ( base_url or os.environ.get(LANGFUSE_BASE_URL) or host or os.environ.get(LANGFUSE_HOST, "https://cloud.langfuse.com") )
Why this matters: the PR description explicitly states the warning should fire "only when they are the effective base-URL source." Because one branch checks
is Noneand the other checks truthiness, empty-string values ("") are treated inconsistently between the two blocks.Step-by-step proof (case 1 — false negative):
- User sets env var
LANGFUSE_BASE_URL=""(e.g. templated but unset in a k8s/docker deployment) and passeshost="https://custom.example.com"to the constructor. - Guard evaluates
os.environ.get(LANGFUSE_BASE_URL) is None→"" is None→False. The outerifis false, so neither branch runs — no warning is emitted. - Resolution:
base_url(None)oros.environ.get(LANGFUSE_BASE_URL)("", falsy)orhost("https://custom.example.com", truthy) →self._base_url = "https://custom.example.com". - Result:
hostsilently became the effective base URL with zero deprecation notice, even though it is exactly the case the warning is supposed to catch.
Step-by-step proof (case 2 — false positive):
- User passes
host=""(e.g. propagated from another empty env var) and neitherbase_urlnorLANGFUSE_BASE_URLis set. - Guard: outer
ifis true (both are None), innerif host is not None→"" is not None→True→ warning fires, telling the userhostis deprecated and effective. - Resolution:
host("", falsy) is skipped, falling through toos.environ.get(LANGFUSE_HOST, "https://cloud.langfuse.com").hostwas never the effective source. - Result: a spurious/misleading deprecation warning about a parameter that had no effect on the resolved URL.
Why existing code doesn't prevent it: the guard was written fresh in this PR and hand-duplicates the resolution logic with different semantics (
is Nonevs falsy-or) instead of deriving the warning conditions from the same expression used for resolution.Impact: purely cosmetic/advisory — the resolved
_base_urlis always correct in both cases; only the presence/absence of theDeprecationWarningis wrong. No crash, no data loss, no behavior change for the actual client. This is why all verifiers who confirmed the bug still rated it a nit.Fix: align the guard with the same truthy semantics as the resolution, e.g.:
if not base_url and not os.environ.get(LANGFUSE_BASE_URL): if host: warnings.warn(...) elif os.environ.get(LANGFUSE_HOST): warnings.warn(...)
- User sets env var
- api property: call out api.observations and api.metrics as the recommended high-performance v2 endpoints (SDK v4 defaults) and note their v1 equivalents under api.legacy.* are less performant, not recommended for new workflows, and will be deprecated - scores: point reads at api.scores_v3.get_many_v3; the v2 api.scores.get_many/get_by_id are deprecated per the API spec and no longer available on Langfuse v4+ servers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…first examples - propagate_attributes example: start_observation() returns a plain observation (no __enter__), so both `with` usages crashed at runtime; switch to start_as_current_observation (smoke-ran the example) - api/async_api/flush docstrings: use the v2 observations endpoint (api.observations.get_many with fields selection) as the primary example per review, matching the query-via-sdk docs page examples Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Current servers are v3.x; the spec's removal note is forward-looking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore the original @deprecated decorator strings on set_trace_io and set_current_trace_io so no runtime-visible string changes remain; the corrected propagate_attributes import guidance stays in the docstrings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| - **Ingestion is asynchronous.** `langfuse.flush()` only guarantees delivery to | ||
| the API, not read visibility: reads such as `api.trace.get(trace_id)` may | ||
| raise `langfuse.api.NotFoundError` until processing completes (typically | ||
| within 15-30 seconds; longer under load). The same applies to scores and | ||
| dataset run reads. Instead of a fixed sleep, retry with a deadline: |
There was a problem hiding this comment.
Dangling sentence — promised retry example is missing
The bullet ends with "Instead of a fixed sleep, retry with a deadline:" (colon implies a code block follows), but the next line is the next bullet point. The retry snippet that was meant to appear here was described in the PR as a key addition for agents, but it was never included. Any agent or developer reading this will be left without the concrete pattern to copy. The flush() docstring also cross-references "the api property docs for a bounded retry pattern", pointing at this non-existent snippet.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_client/client.py
Line: 432-436
Comment:
**Dangling sentence — promised retry example is missing**
The bullet ends with "Instead of a fixed sleep, retry with a deadline:" (colon implies a code block follows), but the next line is the next bullet point. The retry snippet that was meant to appear here was described in the PR as a key addition for agents, but it was never included. Any agent or developer reading this will be left without the concrete pattern to copy. The `flush()` docstring also cross-references "the `api` property docs for a bounded retry pattern", pointing at this non-existent snippet.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
LGTM — the two nits found (missing async_api docstring, dangling retry-snippet cross-reference) are trivial documentation completeness gaps, not functional defects, and don't block this docs-only PR.
Extended reasoning...
Overview
This PR is scoped to docstrings, README, and pyproject metadata across 7 files: langfuse/__init__.py, langfuse/_client/client.py, observe.py, propagation.py, span.py, README.md, and pyproject.toml. No signatures, control flow, or generated langfuse/api/ files are touched, consistent with the author's AST-diff verification claim. I re-checked the previously reported functional bug from my prior review on this PR (the propagate_attributes example using start_observation(...) as gen:, which would raise TypeError since that call doesn't return a context manager) — that has since been fixed in propagation.py (now correctly uses start_as_current_observation).
Security risks
None. This is a documentation-only change with no new code paths, auth, or data handling.
Level of scrutiny
Low-to-moderate is appropriate: docs-only changes carry no runtime risk, but since the PR's own thesis is that AI agents will copy-paste these examples verbatim, executability of examples matters more here than in a typical docs PR — which is why the earlier context-manager bug was worth flagging and is worth confirming was fixed (it was).
Other factors
The two nits found this run (async_api left undocumented despite the PR description claiming both api/async_api were documented; a dangling colon in the api docstring promising a retry snippet that never materializes, with a dead cross-reference from flush()) are real completeness gaps but purely cosmetic — nothing executes incorrectly. Given the author already responded to and fixed the one substantive bug from my previous pass, and the outstanding nits are trivial to fix in a follow-up, I'm comfortable approving.
| def api(self) -> LangfuseAPI: | ||
| """Synchronous client for the full Langfuse REST API (traces, observations, scores, datasets, prompts, ...). | ||
|
|
||
| Use this to read or manage data on the Langfuse server; use the tracing methods | ||
| (`start_observation`, `@observe`) to create traces. Use `async_api` for the | ||
| asyncio variant. |
There was a problem hiding this comment.
🟡 The PR description says property docstrings were added for both Langfuse.api and Langfuse.async_api (including scores get_many/get_by_id guidance), but only api (client.py:423-460) actually received one — async_api (client.py:473-478) remains undocumented, and the scores guidance the description mentions doesn't appear anywhere in the file.
Extended reasoning...
The PR's stated goal is to make the SDK's docstrings a first-class documentation surface, and its description explicitly frames the api/async_api work as a pair: "Langfuse.api / async_api — new property docstrings (previously none)", going on to list several semantics it claims were documented, including "scores are read via get_many/get_by_id — there is no api.scores.get".
In the actual diff, only the api property (client.py:423-460) gets the new docstring covering ingestion lag, list-vs-get semantics, and the v2 metrics/observations APIs. The async_api property, defined a few lines below right after the api setter (client.py:473-478), is completely unchanged by this PR and still has no docstring at all — its body goes straight from the @property decorator to the if self._resources is None guard. help(langfuse.async_api) / inspect.getdoc(langfuse.async_api) return None, while the same calls on langfuse.api now return the new text. Additionally, grepping client.py confirms the promised scores get_many/get_by_id/no-scores.get guidance was never actually written into either docstring — the new api docstring only mentions api.observations.get_many, not scores.
Concretely: (1) open client.py, jump to line 423 — the new docstring is present on api. (2) Scroll to line 473 — async_api's body has no docstring statement whatsoever. (3) Run import inspect; from langfuse import Langfuse; inspect.getdoc(Langfuse.async_api) — it returns None, contradicting the PR description's claim that both properties got documented. (4) Search the whole file for 'get_by_id' or 'scores are read' — no match, confirming that specific claimed addition was never written.
Since async_api is a first-class, identically-used surface for asyncio consumers (same underlying REST client, same ingestion-lag/list-vs-get caveats apply), leaving it silently undocumented while the sync twin gets detailed guidance is a real, in-scope completeness gap for a PR whose entire purpose is upgrading exactly this kind of documentation. It is not a functional defect — nothing crashes or behaves incorrectly at runtime, and the api docstring does at least point readers to async_api as 'the asyncio variant' — so it doesn't block merging. The fix is trivial: mirror the api docstring (or a short pointer with a note about awaiting the async client methods) onto async_api, and add the scores get_many/get_by_id guidance that was described but never written.
| the API, not read visibility: reads such as `api.trace.get(trace_id)` may | ||
| raise `langfuse.api.NotFoundError` until processing completes (typically | ||
| within 15-30 seconds; longer under load). The same applies to scores and | ||
| dataset run reads. Instead of a fixed sleep, retry with a deadline: | ||
|
|
There was a problem hiding this comment.
🟡 The api property docstring (langfuse/_client/client.py:436) ends its ingestion-lag bullet with "Instead of a fixed sleep, retry with a deadline:" — a colon that promises a code snippet, but no snippet follows anywhere in the docstring. The PR description itself claims a "bounded retry snippet" was added, and the flush() docstring added in this same PR cross-references "the api property docs for a bounded retry pattern", which now points to nothing. Either add the promised retry example or remove the dangling colon and the dead cross-reference.
Extended reasoning...
The bug: The new api property docstring in langfuse/_client/client.py (lines 424-460) has a bullet on ingestion lag that ends with:
Instead of a fixed sleep, retry with a deadline:
A trailing colon here unambiguously signals that a code example follows — that's the standard convention this same PR uses elsewhere (e.g. the Example: sections in observe.py and propagation.py always follow a colon with a fenced python block). But the very next line in this docstring is blank, and the line after that immediately starts the next, unrelated bullet ("List endpoints return lightweight views..."). Scanning the full docstring body (lines 424-460) confirms there is no code block anywhere in it.
Corroborating evidence this isn't just a stray colon: Two independent signals confirm a snippet was intended and simply never written. First, the PR description explicitly states the api property docs add ingestion-lag guidance "with a bounded retry snippet" — the author's own summary says this snippet exists. Second, this same PR adds a Note: block to flush() (client.py, ~line 2286) that says: "See the api property docs for a bounded retry pattern" — a cross-reference that, as written, points at nothing, since no retry pattern/snippet actually appears in the api docstring.
Why nothing else catches this: This is purely docstring text with no executable code, so none of the PR's own verification steps surface it — ruff format --check/ruff check/mypy don't parse docstring prose, the unit test suite doesn't inspect docstring content, and the PR's scripted check only verifies that embedded python examples compile — it doesn't check that every colon promising an example actually has one attached.
Impact: This PR's stated goal is to make docstrings a reliable, first-class documentation surface that AI coding agents and humans read via help()/inspect.getdoc()/pdoc instead of visiting langfuse.com. A reader (human or agent) who hits NotFoundError right after flush(), follows the flush() docstring's pointer to "the api property docs for a bounded retry pattern," and then follows the api docstring's own colon expecting a retry example, gets nothing actionable in either place. That's exactly the failure mode this PR is trying to eliminate, reintroduced in the very code it touches.
Step-by-step proof:
- Open
help(Langfuse.api)or readlangfuse/_client/client.pylines 424-460 directly. - Locate the first bullet: "Ingestion is asynchronous.
langfuse.flush()only guarantees delivery... Instead of a fixed sleep, retry with a deadline:" - Look at the next line: it is blank.
- Look at the line after that: "- List endpoints return lightweight views. ..." — a new, unrelated bullet has already started.
- Grep the whole docstring (lines 424-460) for a fenced code block or any indented Python snippet: there is none.
- Cross-check against
flush()'s docstring Note (added in the same PR): it says "See theapiproperty docs for a bounded retry pattern" — but step 5 already showed that pattern doesn't exist, so this cross-reference is dead. - Cross-check against the PR description: it explicitly claims the
apidocs were updated "with a bounded retry snippet" — confirming the snippet was intended but never actually written into the docstring.
Fix: Either add the promised bounded-retry example (e.g. a small python block using a deadline loop against api.trace.get(trace_id) catching NotFoundError), or, if the snippet is being deferred, change the trailing colon to a period and drop/rewrite the flush() cross-reference so it doesn't point at nonexistent content.
Hi @hassiebp, based on the code agent test I am currently running, I want to add and improve some of the docstrings to improve AX. Can you have a quick look if this is ok for you?
Context:
Trace analysis of frontier coding agents (Claude Code, Codex) shows they treat the installed package as primary documentation —
help(),inspect.signature, and grepping site-packages — often without visiting langfuse.com. This PR upgrades the docstrings those agents hit most, plus the README/PyPI description that feed the same surfaces. Every claim was verified against code and unit tests before writing.Changes
propagate_attributes— the examples were broken: they called it aslangfuse.propagate_attributes(...)(it is a top-level function, not a client method →AttributeError) and used the nonexistentlangfuse.start_generation(...). Fixed all examples, added an explicit "module-level function" note, strengthened the wrap-the-root-span ordering guidance, added See-also links. Also fixed theset_trace_io/set_current_trace_iodocstring deprecation notes that referenced it as:meth:(the runtime@deprecateddecorator strings are untouched).Langfuse.api/async_api— new property docstrings (previously none) covering the semantics agents repeatedly tripped over:observations/scoresare ID strings; full objects needapi.trace.get(id))NotFoundErrorright after a successfulflush()is expected) with a bounded retry snippetget_many/get_by_id— there is noapi.scores.getThese live on the non-generated property because
langfuse/api/is fern-generated and must not be hand-edited; the same descriptions should be mirrored into the fern spec in langfuse/langfuse so all generated SDKs pick them up.flush()— note that flush guarantees data is sent, not that it is queryable.@observe— documentedcapture_input/capture_output(default viaLANGFUSE_OBSERVE_DECORATOR_IO_CAPTURE_ENABLED) andtransform_to_string; extended theas_type="generation"example withupdate_current_generation(model=..., usage_details=...); documented generator semantics.run_experiment/Evaluation— added a RAG LLM-as-a-judge example (faithfulness + answer relevance evaluators), a CI regression-testing note pointing at langfuse/experiment-action andRegressionError, and See-also links.RunnerContext.run_experimentpreviously had no docstring at all.Package docstring / README / pyproject —
langfuse/__init__.pynow leads with the full capability map (tracing AND datasets, experiments, LLM-as-a-judge evaluation, prompt management), current env var names (LANGFUSE_PUBLIC_KEY/SECRET_KEY/BASE_URL), and a pointer to https://langfuse.com/llms.txt — while keeping the.. include:: ../README.mdso the pdoc reference site is unchanged (verified pdoc renders). README got the same capability lead, a quickstart, and the staledocs/sdk/python/sdk-v3link replaced with live URLs. PyPI description upgraded from "A client library for accessing langfuse".Deprecation hygiene — also fixed a broken import in the
Langfuseclass example (from langfuse.otel import Langfuse— nonexistent module) and itshost=usage.Behavior
Strictly docstrings/README/metadata only — verified by comparing every changed Python file against
mainat the AST level with docstrings stripped: the ASTs are identical (the only structural delta is the new docstring statement on theapiproperty). No signatures, logic, imports, warning strings, or generatedlangfuse/api/files are touched. Thehost/LANGFUSE_HOSTdeprecation stays documented in the parameter docstring and env var description, without a runtime warning.Verification
ruff format --check,ruff check,mypy langfuseall passtests/unit: 633 passed (the 18test_prompt.pyerrors require live env keys and fail identically on the unmodified tree)help()/inspect.getdocrender for every touched symbol; all 21 embedded ```python examples compile as writtenlangfuse/api/touched🤖 Generated with Claude Code
Greptile Summary
This PR upgrades docstrings, README, and PyPI metadata to serve as a first-class documentation surface for AI coding agents that inspect installed packages via
help()andinspect.signaturerather than visiting the web docs. All changes are documentation-only; no signatures, logic, or generated files are touched.propagate_attributes(wrong call site, nonexistent methods), addedcapture_input/capture_output/transform_to_stringdocs to@observe, fixed theLangfuseclass example import (langfuse.otel→langfuse), and correctedhost=tobase_url=.apiproperty docstring covering asynchronous ingestion lag, list-vs-detail semantics, and v2 API preference; added aflush()note pointing at the same guidance; updated README with a quickstart and live documentation URLs.Confidence Score: 4/5
Safe to merge — strictly documentation with no runtime behavior changes, but the
apiproperty docstring contains a dangling sentence that references a retry code example that was never included.The
apidocstring ends an ingestion-lag bullet with "Instead of a fixed sleep, retry with a deadline:" but no code example follows, andflush()explicitly cross-references "theapiproperty docs for a bounded retry pattern" that does not exist. Since the stated goal is to give agents copy-paste-ready patterns, the missing snippet is a meaningful gap in the primary deliverable of this PR. Everything else is accurate and well-structured.langfuse/_client/client.py — the
apiproperty docstring (missing retry snippet, async_api still undocumented) and the README quickstart (model name differs from the__init__.pyquickstart).Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "push" | Re-trigger Greptile