Skip to content

fix(trace): restore Phoenix collector compatibility - #339

Open
Jake Present (jakepresent) wants to merge 3 commits into
mainfrom
jake/fix-phoenix-collector
Open

fix(trace): restore Phoenix collector compatibility#339
Jake Present (jakepresent) wants to merge 3 commits into
mainfrom
jake/fix-phoenix-collector

Conversation

@jakepresent

Copy link
Copy Markdown
Collaborator

Summary

  • Restore PhoenixCollector against the supported Phoenix 15.0–19.17 client API.
  • Convert ASSERT's ISO turn bounds to timezone-aware datetimes so Phoenix applies them instead of silently querying unbounded project history.
  • Convert the DataFrame shape Phoenix actually returns, including indexed span IDs, direct span_kind, timezone-aware timestamps, and missing Pandas values.
  • Run compatibility tests against the actual Phoenix client installed by the public phoenix extra.

Problem

PhoenixCollector could not be constructed with any supported Phoenix release because it called the removed phoenix.Client(endpoint=...) surface. After correcting construction, retrieval still used the removed top-level get_spans_dataframe method. The supported API is phoenix.client.Client(base_url=...) followed by client.spans.get_spans_dataframe(...).

The collector protocol supplies ISO timestamp strings, while Phoenix expects datetime objects and silently normalizes strings to None. That dropped the intended turn bounds. The old DataFrame conversion also called int() on Phoenix Timestamp values and missed span IDs stored in the DataFrame index.

Verification

  • Focused collector, error, Protocol, and session tests: 19 passed.
  • Full Python suite with the Phoenix extra installed: 1492 passed, 21 skipped, 840 subtests passed.
  • Clean built-wheel assert-ai[phoenix] environment: all 5 compatibility tests passed against Phoenix 19.17.0, importing ASSERT from site-packages rather than the checkout.
  • Supported lower bound Phoenix 15.0.0: all 5 compatibility tests passed.
  • Base wheel without the optional extra: actionable missing-dependency test passed and the 4 installed-Phoenix tests skipped cleanly.
  • Live Phoenix server probe: exported an old span and a current span, then verified the bounded collector query returned only the current span with its real trace ID, indexed span ID, kind, nanosecond timestamps, session ID, and output.
  • Negative control: all 4 installed-Phoenix compatibility tests fail against the previous implementation.
  • uv lock --check, diff hygiene, focused Ruff, Python compilation, wheel/sdist build, and twine check passed.

Scope

This is the focused follow-up identified while reviewing #336. It does not wire PhoenixCollector into the CLI/config path or change the SpanCollector protocol; it repairs the existing public programmatic adapter and locks its real dependency contract in required CI.

@changliu2 Chang Liu (changliu2) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found four issues tightly coupled to restoring the previously unreachable Phoenix adapter:

  1. assert_ai/core/collector.py:273-288 still discards Phoenix span events. A minimal event-only gen_ai.choice span produces no ASSERT trace_events; preserving the event produces the expected assistant message. This breaks GenAI output and tool evidence after the adapter becomes operational.
  2. assert_ai/core/collector.py:250-288 preserves Phoenix's default newest-first DataFrame ordering. ASSERT consumes spans sequentially, so a minimal two-span trace is emitted as final answer followed by initial answer. Sort converted spans chronologically.
  3. assert_ai/core/collector.py:162-185 applies trace_ids only after retrieving Phoenix's capped unfiltered DataFrame. Existing traces outside that page silently disappear. Phoenix 15.0 and 19.17 both provide server-side trace_ids filtering with cursor pagination through client.spans.get_spans(...).
  4. .github/workflows/build.yml:103-160 runs the installed-wheel compatibility tests only against the resolver-selected Phoenix 19.17.0. There is no required job pinning the supported 15.0.0 lower bound.

I am not retaining the concurrent-session finding: that behavior is broader pre-existing design, and this PR improves rather than worsens cross-run isolation by restoring effective time bounds.

@jakepresent

Copy link
Copy Markdown
Collaborator Author

Addressed all four findings on exact head 7505f3f. The collector now uses Phoenix’s raw event-bearing get_spans API, pushes trace_ids server-side, drains cursor pages without the DataFrame cap, preserves gen_ai.choice and tool-message evidence, and orders spans chronologically with deterministic parent-first ties; the DataFrame path is now only a capability-detected legacy fallback. Added installed-wheel CI coverage for Phoenix 15.0/client 2.1 and Phoenix 19.17/current, with regressions for events, tool evidence, pagination, newest-first results, equal timestamps, deep/cyclic parent graphs, and timestamp precision. Exact-head verification is green: local full suite 1498 passed, 21 skipped, 840 subtests passed, and every GitHub check passes, including both Phoenix compatibility jobs, Tier 1, and real-Docker containment. Chang Liu (@changliu2) ready for re-review.

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.

2 participants