Skip to content

deps: upgrade all dependencies (Jul 2026) + OTLP migration - #182

Merged
ptimizeroracle merged 7 commits into
mainfrom
deps/upgrade-jul-2026
Jul 30, 2026
Merged

ptimizeroracle merged 7 commits into
mainfrom
deps/upgrade-jul-2026

Conversation

@ptimizeroracle

@ptimizeroracle ptimizeroracle commented Jul 12, 2026 •

Copy link
Copy Markdown
Owner

What

Upgrades 16 packages: litellm 1.91, instructor 1.15, anthropic 0.116, polars 1.42, pandas 2.3, pydantic 2.13 + dev tooling. Migrates Jaeger→OTLP (archived upstream). Adds rust-toolchain.toml, instructor compat tests, pandas CoW readiness fixture, dependency analysis docs.

Diff

+4334/-3008 (uv.lock bulk +6588/-2882)

Tests

1088 pass (+7 new), 97 skip, 0 regressions

Merge order

Independent — parallel with architecture train

Summary by CodeRabbit

  • New Features

    • Added OTLP over HTTP as the supported tracing export option.
    • Updated Langfuse integration for its latest observation model.
    • Improved compatibility with current LiteLLM and Instructor versions.
  • Bug Fixes

    • Ensured asynchronous LiteLLM clients are initialized correctly.
    • Improved tracing validation and handling of nested observations.
  • Documentation

    • Added dependency upgrade analysis, action plans, and merge guidance.
  • Tests

    • Added coverage for dependency compatibility and pandas Copy-on-Write behavior.

Hermes Builder added 3 commits July 10, 2026 10:01
Major bumps:
- litellm 1.83.13 -> 1.91.1
- anthropic 0.84.0 -> 0.116.0
- instructor 1.0.0 -> 1.15.4
- polars 0.20.0 -> 1.42.1
- pandas 1.5.0 -> 2.3.3
- pydantic 2.0.0 -> 2.13.4
- structlog 23.1.0 -> 26.1.0
- mypy 1.13.0 -> 2.2.0 (dev)
- ruff 0.8.0 -> 0.15.21 (dev)
- click 8.1.0 -> 8.4.2
- tiktoken 0.5.0 -> 0.13.0
- tenacity 8.2.0 -> 9.1.4
- tqdm 4.66.0 -> 4.68.4
- aiohttp 3.13.4 -> 3.14.1
- pytest 9.0.3 -> 9.1.1, pytest-asyncio 0.23 -> 1.4
- bandit 1.7.0 -> 1.9.4, pre-commit 4.0 -> 4.6
- pip-audit 2.7.0 -> 2.10.1, pip-licenses 4.3 -> 5.5
- prometheus-client 0.20 -> 0.25

Note: rich held at >=13.7.0 (instructor 1.15.4 caps rich<15.0.0).
All 120 outdated packages re-resolved via uv lock --upgrade.
Unit tests: 704 passed, 0 failed.
- Upgrade all direct + dev dependencies in pyproject.toml and re-resolve
  uv.lock (304 packages).
- Migrate observability tracing from the archived opentelemetry-exporter-jaeger
  to opentelemetry-exporter-otlp. enable_tracing(exporter="otlp", ...) now
  exports via OTLP/HTTP; Jaeger backends ingest this natively on :4318
  (see docker/docker-compose.yml). The Jaeger exporter was archived upstream
  and is incompatible with opentelemetry-sdk 1.43.
- Update tests/unit/test_observability.py: Jaeger test -> OTLP test, plus a
  guard that an unknown exporter name raises ValueError.
- Add DEPENDENCY_UPGRADE_ANALYSIS.md and DEPENDENCY_UPGRADE_ACTION_PLAN.md.

Observability suite: 15 passed, 0 failed.
@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ptimizeroracle, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac2e3709-688e-4a75-9f88-c3d47ac33c9d

📥 Commits

Reviewing files that changed from the base of the PR and between 017f3dc and 3c14931.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • pyproject.toml
📝 Walkthrough

Walkthrough

This change upgrades runtime and development dependencies, replaces Jaeger tracing with OTLP HTTP, migrates Langfuse observation APIs, pins the Rust toolchain, documents upgrade and merge plans, and adds Instructor–LiteLLM compatibility plus pandas Copy-on-Write tests.

Changes

Dependency upgrade implementation

Layer / File(s) Summary
Upgrade analysis and action plan
DEPENDENCY_UPGRADE_ANALYSIS.md, DEPENDENCY_UPGRADE_ACTION_PLAN.md
Documents dependency version changes, compatibility findings, follow-up actions, and completion status.
Dependency and toolchain constraints
pyproject.toml, rust-toolchain.toml
Raises dependency minimums, switches optional observability support to OTLP, and pins stable Rust with rustfmt and clippy.
Dependency compatibility integration
ondine/adapters/unified_litellm_client.py, tests/unit/test_instructor_litellm_compat.py
Enables explicit asynchronous Instructor integration and tests LiteLLM module, callable, and mode compatibility.
Pandas Copy-on-Write test coverage
tests/conftest.py, tests/unit/test_pandas_cow_smoke.py
Adds environment-controlled CoW support and tests mutation isolation for assignments, slices, and chained assignment.
OTLP tracing exporter migration
ondine/observability/tracer.py, tests/unit/test_observability.py
Replaces Jaeger exporter wiring and documentation with OTLP HTTP configuration and validates unknown-exporter handling.
Langfuse v3 observation migration
ondine/observability/observers/langfuse_observer.py
Migrates pipeline, generation, cooldown, and recovery events to Langfuse v3 observations with explicit lifecycle closure.

Branch documentation

Layer / File(s) Summary
Branch descriptions and merge order
docs/PR_DESCRIPTIONS.md
Adds branch descriptions, test and diff summaries, merge constraints, merge trains, and branches marked not to merge.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Pipeline
  participant LangfuseObserver
  participant LangfuseClient
  participant LangfuseObservation
  Pipeline->>LangfuseObserver: start pipeline
  LangfuseObserver->>LangfuseClient: start root span observation
  Pipeline->>LangfuseObserver: record LLM call
  LangfuseObserver->>LangfuseClient: start generation observation
  LangfuseClient->>LangfuseObservation: end generation
  Pipeline->>LangfuseObserver: end pipeline
  LangfuseObserver->>LangfuseObservation: update metrics and end root span
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the dependency upgrade effort and the Jaeger-to-OTLP observability migration.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch deps/upgrade-jul-2026
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch deps/upgrade-jul-2026

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
DEPENDENCY_UPGRADE_ACTION_PLAN.md (1)

28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a language specifier to the fenced code block.

The code block on line 28 lacks a language tag, triggering MD040. Since it shows Python packaging metadata, use text or toml.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@DEPENDENCY_UPGRADE_ACTION_PLAN.md` around lines 28 - 31, Add a language
specifier to the fenced code block containing the litellm dependency entries in
DEPENDENCY_UPGRADE_ACTION_PLAN.md, using text or toml while leaving the
dependency content unchanged.

Source: Linters/SAST tools

pyproject.toml (1)

41-41: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider enforcing the rich<15.0.0 upper bound.

The comment documents that instructor 1.15.4 caps rich<15.0.0, but the constraint is only rich>=13.7.0 with no upper bound. A downstream consumer or fresh install without the lockfile could pull rich 15.0+ and break instructor compatibility. Adding rich>=13.7.0,<15.0.0 would enforce the documented cap.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyproject.toml` at line 41, Update the rich dependency constraint in
pyproject.toml to include the documented upper bound, enforcing rich>=13.7.0 and
rich<15.0.0 so instructor 1.15.4 compatibility is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@DEPENDENCY_UPGRADE_ANALYSIS.md`:
- Line 39: Verify the actual litellm upper bound imposed by instructor 1.15.0,
then update the version reference in the note around “instructor 1.15.0” and the
corresponding action-plan entry so both documents use the same constraint and
compromised-version assessment.

In `@docs/PR_DESCRIPTIONS.md`:
- Around line 3-5: Update the introductory description in PR_DESCRIPTIONS.md to
avoid stating that all 12 branches are ready to merge; describe them as “12
branch descriptions” or explicitly distinguish the 11 merge candidates from the
optional wt/t6-plan v2 branch.

In `@tests/unit/test_pandas_cow_smoke.py`:
- Around line 44-58: The CoW smoke tests currently use independent copies, so
they do not exercise detachment. In tests/unit/test_pandas_cow_smoke.py lines
44-58, replace original.copy() with a view such as original.iloc[:] and perform
the assignment through .loc[:, "text"]; in lines 60-75, remove chunk.copy() and
modify chunk directly through .loc[:, "value"], preserving the assertions that
the source remains unchanged and the view receives the new values.

---

Nitpick comments:
In `@DEPENDENCY_UPGRADE_ACTION_PLAN.md`:
- Around line 28-31: Add a language specifier to the fenced code block
containing the litellm dependency entries in DEPENDENCY_UPGRADE_ACTION_PLAN.md,
using text or toml while leaving the dependency content unchanged.

In `@pyproject.toml`:
- Line 41: Update the rich dependency constraint in pyproject.toml to include
the documented upper bound, enforcing rich>=13.7.0 and rich<15.0.0 so instructor
1.15.4 compatibility is preserved.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c52cdfd-cb8f-4ba9-9138-7caf999c5262

📥 Commits

Reviewing files that changed from the base of the PR and between cdc14f1 and 7294eb5.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • DEPENDENCY_UPGRADE_ACTION_PLAN.md
  • DEPENDENCY_UPGRADE_ANALYSIS.md
  • docs/PR_DESCRIPTIONS.md
  • ondine/observability/tracer.py
  • pyproject.toml
  • rust-toolchain.toml
  • tests/conftest.py
  • tests/unit/test_instructor_litellm_compat.py
  • tests/unit/test_observability.py
  • tests/unit/test_pandas_cow_smoke.py

- Security is the standout: **CVE-2025-69872** (diskcache) mitigation by making it optional, SSRF blocks for Bedrock image/PDF (remote URLs blocked, only `data:` + `s3://` accepted), auth header redaction in debug logs.
- Model support: Claude 4 (Opus/Sonnet/Haiku), GPT-4.1, o3/o4, Grok 3, DeepSeek R1/V3 added to `KnownModelName`.
- Bug fixes directly relevant to structured output: `list[Model]` scalar response-model crashes, PEP 604 union streaming, Anthropic reasoning tools routing, partial-streaming Literal defaults, Gemini truncated-response detection.
- Note: instructor 1.15.0 **pins litellm ≤ 1.82.6** (blocks compromised 1.82.7/1.82.8) — but ondine now requires litellm ≥ 1.91.1, so the instructor pin is effectively superseded by the new litellm. No conflict at install time (tests pass).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Inconsistent litellm upper-bound version between analysis and action plan.

Line 39 states instructor 1.15.0 "pins litellm ≤ 1.82.6", but DEPENDENCY_UPGRADE_ACTION_PLAN.md lines 29-30 show the actual constraint as litellm<=1.83.7. Verify which is correct and align both documents.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@DEPENDENCY_UPGRADE_ANALYSIS.md` at line 39, Verify the actual litellm upper
bound imposed by instructor 1.15.0, then update the version reference in the
note around “instructor 1.15.0” and the corresponding action-plan entry so both
documents use the same constraint and compromised-version assessment.

Comment thread docs/PR_DESCRIPTIONS.md
Comment on lines +3 to +5
Copy-paste-ready descriptions for the 12 ready branches. Ordered by merge
dependency. Each block is self-contained: title, summary, test status, and
merge order note.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify that not all 12 branches are ready to merge.

wt/t6-plan is explicitly marked as a v2 candidate that may be held, so calling all 12 entries “ready branches” is misleading. Rename this to “12 branch descriptions” or identify the 11 merge candidates plus the optional v2 branch.

Suggested wording
-Copy-paste-ready descriptions for the 12 ready branches. Ordered by merge
+Copy-paste-ready descriptions for 12 branches, including one optional v2
+candidate. Ordered by merge
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Copy-paste-ready descriptions for the 12 ready branches. Ordered by merge
dependency. Each block is self-contained: title, summary, test status, and
merge order note.
Copy-paste-ready descriptions for 12 branches, including one optional v2
candidate. Ordered by merge
dependency. Each block is self-contained: title, summary, test status, and
merge order note.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/PR_DESCRIPTIONS.md` around lines 3 - 5, Update the introductory
description in PR_DESCRIPTIONS.md to avoid stating that all 12 branches are
ready to merge; describe them as “12 branch descriptions” or explicitly
distinguish the 11 merge candidates from the optional wt/t6-plan v2 branch.

Comment on lines +44 to +58
def test_column_assignment_does_not_mutate_source(self):
"""``df[col] = value`` must not propagate back to a sharing parent.

This mirrors pipeline_composer.py:254 (``df[col_name] = result.data[col_name]``).
Under CoW, assigning into a DataFrame that shares memory with another
triggers a copy — the original must remain unchanged.
"""
original = pd.DataFrame({"text": ["a", "b", "c"]})
working = original.copy()
working["text"] = ["x", "y", "z"]

assert list(original["text"]) == ["a", "b", "c"], (
"CoW violation: writing to a copy mutated the original DataFrame"
)
assert list(working["text"]) == ["x", "y", "z"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Two CoW tests don't actually exercise Copy-on-Write — they use .copy() which is always independent. Both tests create an independent copy before modifying, so they pass with CoW disabled too, giving false confidence about CoW readiness for these patterns.

  • tests/unit/test_pandas_cow_smoke.py#L44-L58: Replace original.copy() with a view (e.g., original.iloc[:]) and use .loc[:, "text"] for in-place modification so CoW's copy-on-write is actually triggered.
  • tests/unit/test_pandas_cow_smoke.py#L60-L75: Remove chunk.copy() and modify the view chunk directly via .loc[:, "value"] so the first write into the view triggers CoW detachment.
📍 Affects 1 file
  • tests/unit/test_pandas_cow_smoke.py#L44-L58 (this comment)
  • tests/unit/test_pandas_cow_smoke.py#L60-L75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_pandas_cow_smoke.py` around lines 44 - 58, The CoW smoke
tests currently use independent copies, so they do not exercise detachment. In
tests/unit/test_pandas_cow_smoke.py lines 44-58, replace original.copy() with a
view such as original.iloc[:] and perform the assignment through .loc[:,
"text"]; in lines 60-75, remove chunk.copy() and modify chunk directly through
.loc[:, "value"], preserving the assertions that the source remains unchanged
and the view receives the new values.

Langfuse 4.13.2 (bumped from 2.x) dropped the v2 trace()/generation()/span()
API in favor of an OTEL-based model. Migrate langfuse_observer.py to
client.start_observation(as_type="span"|"generation", ...) / observation.
start_observation(...) / .update(...) / .end(), using trace_context={"trace_id"}
(32-char hex, derived from event.run_id/trace_id) to pin the root span in place
of the old explicit trace id. This preserves the existing trace/span naming,
metadata, and cost/token fields reported to Langfuse.

- ondine/observability/observers/langfuse_observer.py: rewrite all five event
  handlers onto the v3+ API; annotate self._client/self._current_trace as
  `Langfuse | None` / `LangfuseSpan | None` (fixes the None-vs-Langfuse mypy
  errors); add a `cast("TraceContext", {...})` at the two trace_context call
  sites because mypy's overload resolution for start_observation() produces a
  spurious "no overload matches" error when a trace_context dict literal is
  passed inline (a NotRequired-key false positive in this mypy/langfuse
  combination) -- the cast keeps the rest of each call fully type-checked
  instead of falling back to `Any`.
- ondine/adapters/unified_litellm_client.py:300: instructor 1.15's from_litellm
  has separate Instructor/AsyncInstructor overloads selected by `async_client`;
  the completion_func passed here is always a coroutine function
  (router.acompletion or litellm.acompletion), so pass `async_client=True`
  explicitly to select the AsyncInstructor overload matching actual runtime
  usage (this only fixes the static type -- instructor's own
  inspect.iscoroutinefunction() check already inferred the same at runtime).
- ondine/observability/tracer.py:114: annotate `span_exporter: SpanExporter`
  (the shared base class, now imported alongside ConsoleSpanExporter/
  OTLPSpanExporter) so the console/otlp branches assign a common type instead
  of narrowing to ConsoleSpanExporter.

Verified: uv run mypy ondine/ (0 errors), uv run pytest tests/unit/ -q
(975 passed, 11 skipped, 1 pre-existing unrelated MLX/transformers failure),
uv run ruff check/format (clean). Also ran a live smoke test of all five
LangfuseObserver event handlers against the installed langfuse 4.13.2 SDK to
confirm the new call surface works at runtime, not just under mypy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
ondine/adapters/unified_litellm_client.py (1)

300-306: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the explicit async-client contract in compatibility tests.

The current tests call instructor.from_litellm(...) without async_client=True and do not assert AsyncInstructor, so they would still pass if this change were accidentally reverted. Update tests/unit/test_instructor_litellm_compat.py to use the exact callable path and assert the async client type. Instructor 1.15.4 uses this flag to select AsyncInstructor. (raw.githubusercontent.com)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ondine/adapters/unified_litellm_client.py` around lines 300 - 306, Update the
compatibility tests in test_instructor_litellm_compat.py to invoke
instructor.from_litellm with the same callable path and async_client=True used
by the production initialization, then assert the result is an AsyncInstructor.
Ensure the test would fail if the explicit async-client flag or async client
type were removed.
ondine/observability/observers/langfuse_observer.py (1)

261-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Recovery events are dropped without an active trace.

Unlike on_provider_cooldown, this handler early-returns when _current_trace is None, so circuit-breaker recoveries outside a pipeline run are silently lost — an asymmetry that makes cooldown/recovery pairs unbalanced in Langfuse. The nest-or-standalone logic is now duplicated in both handlers; extracting a small helper would fix both at once.

♻️ Extract a shared span helper
+    def _start_span(self, name: str, metadata: dict[str, Any], level: str) -> Any:
+        parent = self._current_trace or self._client
+        return parent.start_observation(
+            name=name, as_type="span", metadata=metadata, level=level
+        )
+
     def on_provider_recovered(self, event: ProviderRecoveredEvent) -> None:
         """
         Log provider recovery as a span in Langfuse.
         """
-        if not self._client or not self._current_trace:
+        if not self._client:
             return
 
         try:
-            span = self._current_trace.start_observation(
-                name="provider-recovered",
-                as_type="span",
-                metadata={
+            span = self._start_span(
+                "provider-recovered",
+                {
                     "provider": event.provider,
                     "deployment_id": event.deployment_id,
                     "cooldown_duration": event.cooldown_duration,
                     "event_type": "circuit_breaker_recovered",
                     **event.metadata,
                 },
-                level="DEFAULT",
+                "DEFAULT",
             )
             span.end()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ondine/observability/observers/langfuse_observer.py` around lines 261 - 281,
Update on_provider_recovered and on_provider_cooldown to use a shared helper
that creates and ends Langfuse observations with the current trace when
available or as standalone spans otherwise. Remove the recovery handler’s early
return on missing _current_trace while preserving the existing provider-recovery
metadata and event naming.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ondine/observability/observers/langfuse_observer.py`:
- Line 219: Update the cleanup logic around self._current_trace.end() to clear
_current_trace immediately after ending the root observation. Ensure subsequent
events or pipeline runs cannot reuse the closed trace while preserving the
existing close behavior.
- Around line 129-133: Update the root trace context in the pipeline-start
handling to use the event’s trace_id, matching on_llm_call’s
_to_trace_id(event.trace_id) fallback; keep the observation setup unchanged so
standalone generations remain grouped with the pipeline root.

---

Nitpick comments:
In `@ondine/adapters/unified_litellm_client.py`:
- Around line 300-306: Update the compatibility tests in
test_instructor_litellm_compat.py to invoke instructor.from_litellm with the
same callable path and async_client=True used by the production initialization,
then assert the result is an AsyncInstructor. Ensure the test would fail if the
explicit async-client flag or async client type were removed.

In `@ondine/observability/observers/langfuse_observer.py`:
- Around line 261-281: Update on_provider_recovered and on_provider_cooldown to
use a shared helper that creates and ends Langfuse observations with the current
trace when available or as standalone spans otherwise. Remove the recovery
handler’s early return on missing _current_trace while preserving the existing
provider-recovery metadata and event naming.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bf6bf6b2-2c8f-4184-a72f-3e950c753d82

📥 Commits

Reviewing files that changed from the base of the PR and between 7294eb5 and 017f3dc.

📒 Files selected for processing (4)
  • ondine/adapters/unified_litellm_client.py
  • ondine/observability/observers/langfuse_observer.py
  • ondine/observability/tracer.py
  • tests/unit/test_observability.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/unit/test_observability.py
  • ondine/observability/tracer.py

Comment on lines +129 to +133
trace_context = cast("TraceContext", {"trace_id": event.run_id.hex})
self._current_trace = self._client.start_observation(
name="ondine-pipeline",
as_type="span",
trace_context=trace_context,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -H -t f 'pyproject.toml' --exec rg -n -C2 'langfuse'

Repository: ptimizeroracle/ondine

Length of output: 471


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "find langfuse observer files"
fd -H -t f 'langfuse_observer\.py|langfuse' . || true

echo
echo "listener file outline"
if [ -f ondine/observability/observers/langfuse_observer.py ]; then
  ast-grep outline ondine/observability/observers/langfuse_observer.py || true
  echo
  echo "context 1-240"
  cat -n ondine/observability/observers/langfuse_observer.py | sed -n '1,240p'
fi

echo
echo "events containing trace/run id declarations"
rg -n "class PipelineStartEvent|PipelineStartEvent|trace_id|run_id|`@dataclass`|dataclass" ondine | head -n 200

Repository: ptimizeroracle/ondine

Length of output: 18856


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "events.py"
cat -n ondine/observability/events.py | sed -n '15,60p'

echo
echo "execution_context.py relevant fields"
cat -n ondine/orchestration/execution_context.py | sed -n '20,135p'

echo
echo "pipeline.py relevant start event construction"
cat -n ondine/api/pipeline.py | sed -n '340,465p' | sed -n '1,130p'

echo
echo "observability event class definitions"
rg -n "class PipelineStartEvent|class LLMCallEvent|pipeline_id|run_id|trace_id|metadata" ondine/observability/events.py ondine/api/pipeline.py

Repository: ptimizeroracle/ondine

Length of output: 19186


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "execution_context id fields construction"
rg -n -C 3 "session_id|trace_id|span_id|ExecutionContext\\(" ondine/orchestration/execution_context.py ondine | head -n 220

echo
echo "observer dispatcher imports/events"
cat -n ondine/observability/dispatcher.py | sed -n '1,220p'

Repository: ptimizeroracle/ondine

Length of output: 20234


🌐 Web query:

Langfuse Python SDK v2 start_observation as_type trace_context observation root generation

💡 Result:

In the Langfuse Python SDK, start_observation is the unified method for creating observations [1][2]. As of v2 (and continuing into later versions), you use the as_type parameter to specify the observation type, such as "span" (the default) or "generation" [1][3][4]. To manage trace hierarchy and context: 1. Trace Context (trace_context): This parameter allows you to manually specify a trace_id and parent_span_id [5][6]. When provided, the SDK links the new observation to the specified parent or trace, effectively allowing you to override the default OpenTelemetry context inheritance [2][3]. 2. Root Observations: If you need to create a new, disconnected root observation (i.e., not a child of the current active trace), you typically use trace_context to initiate a new trace ID [5][3]. Internally, when an observation is created as a root, it may be marked with isRootObservation: true in the Langfuse platform [7]. 3. Context Managers: While start_observation creates an observation directly (requiring a manual .end() call) [2][6], the preferred approach for lifecycle management is using the context manager start_as_current_observation, which automatically handles the lifecycle and sets the observation as the active span in the OpenTelemetry context [2][8]. Example usage with trace_context: from langfuse import Langfuse langfuse = Langfuse # Start an observation with a custom trace context with langfuse.start_as_current_observation( name="my-operation", as_type="span", trace_context={ "trace_id": "your-trace-id-32-hex-chars", "parent_span_id": "your-parent-span-id-16-hex-chars" }) as observation: # Application code pass [5][3]

Citations:


Pin the root Langfuse trace id to trace_id.

PipelineStartEvent uses run_id for the pipeline session, but the LLM event carries trace_id; when on_llm_call falls back to a root generation, it uses _to_trace_id(event.trace_id), so that standalone generation will not be grouped with the run_id-based pipeline root. Use the same field consistently for the root trace id.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ondine/observability/observers/langfuse_observer.py` around lines 129 - 133,
Update the root trace context in the pipeline-start handling to use the event’s
trace_id, matching on_llm_call’s _to_trace_id(event.trace_id) fallback; keep the
observation setup unchanged so standalone generations remain grouped with the
pipeline root.

"duration_ms": event.total_duration_ms,
},
)
self._current_trace.end()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

_current_trace is ended but never cleared, leaving a stale ended span.

After end(), _current_trace still references the closed root observation. Any event arriving afterwards (trailing on_llm_call, on_provider_cooldown, or a second pipeline run on the same observer instance before close()) will create children on an already-ended span, producing dropped or misparented observations. Reset the attribute alongside the end() call.

🔧 Clear the root observation after closing it
-            self._current_trace.end()
+            self._current_trace.end()
+            self._current_trace = None
             logger.debug("Updated Langfuse trace with final metrics")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self._current_trace.end()
self._current_trace.end()
self._current_trace = None
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ondine/observability/observers/langfuse_observer.py` at line 219, Update the
cleanup logic around self._current_trace.end() to clear _current_trace
immediately after ending the root observation. Ensure subsequent events or
pipeline runs cannot reuse the closed trace while preserving the existing close
behavior.

Resolves two conflicts:
- pyproject.toml: keep this branch's upgraded dev toolchain (pytest 9.1.1,
  ruff 0.15.21, mypy 2.2.0, ...) AND main's types-PyYAML stubs from #184.
- uv.lock: regenerated from scratch with `uv lock` rather than textually
  merged — a 3-way-merged lockfile is internally inconsistent.

Also refreshes this branch against README/enrich/plan work merged in
#174 #175 #176 #181 #183 #184.
@ptimizeroracle
ptimizeroracle merged commit 6d92c60 into main Jul 30, 2026
38 checks passed
@ptimizeroracle
ptimizeroracle deleted the deps/upgrade-jul-2026 branch July 30, 2026 13:56
ptimizeroracle added a commit that referenced this pull request Jul 31, 2026
…to Anthropic (#188)

* fix(api): repair enrich() data path and stop sending OpenAI-only kwarg to Anthropic

Three bugs shipped in 1.11.0, all found by installing the published wheel
into a clean venv and calling the real APIs. None were visible to CI.

1. enrich(schema=...) was completely broken — ValueError: "Either dataframe
   or source_path must be provided", for BOTH DataFrame and CSV-path input.
   PipelineBuilder.from_specifications() copies the five spec objects but
   never carried the data, and QuickPipeline attaches an in-memory frame even
   for a path. The structured-output rebuild therefore always dropped it.
   from_specifications() now takes an explicit dataframe= argument.

   Missed by tests because test_enrich.py mocks Pipeline.execute, so the
   rebuilt pipeline never reached the loader stage — configuration was
   asserted, behaviour was not.

2. enrich(polars_df, ...) raised ModuleNotFoundError: No module named
   'pyarrow'. polars is a core dependency and polars support is documented,
   but DataFrame.to_pandas() goes through Arrow and pyarrow ships only in the
   parquet/all extras. Both conversion directions now fall back to a
   column-wise copy that needs no extra dependency.

   Missed by tests because `uv sync --all-extras` installs pyarrow in the dev
   venv, masking the missing declaration.

3. Anthropic structured output failed with "AsyncMessages.create() got an
   unexpected keyword argument 'parallel_tool_calls'" (3 integration tests).
   instructor >=1.15 normalises from_anthropic(mode=ANTHROPIC_TOOLS) down to
   Mode.TOOLS, so the existing mode check matched the native Anthropic client
   and injected an OpenAI-only parameter its SDK rejects outright. The guard
   now excludes the direct-Anthropic path.

   Introduced by #182 raising instructor's floor from >=1.0.0 to >=1.15.4.

Adds regression tests for (1) and (2) that exercise the real failure — both
verified to fail without the fix and pass with it.

Not fixed here, filed as #187: structured output has no mode fallback, and a
run where every row fails is silently returned as a DataFrame of None.

Verified: 1066 unit tests pass, mypy clean (126 files), ruff clean. Polars
round-trip confirmed live against a pyarrow-free venv.

* style: apply ruff 0.16 formatting to test_enrich.py

CI resolves ruff 0.16.0 (floor is >=0.15.21); the appended regression
test class was formatted under an older local resolution.

---------

Co-authored-by: ptimizeroracle <git@binblok.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant