Skip to content

Add offline Phoenix recorded-score example - #30

Merged
thangldw merged 3 commits into
thangldw:mainfrom
uuzzrm:codex/phoenix-recorded-scores
Aug 11, 2026
Merged

thangldw merged 3 commits into
thangldw:mainfrom
uuzzrm:codex/phoenix-recorded-scores

Conversation

@uuzzrm

@uuzzrm uuzzrm commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #23.

This adds a small offline example for converting already-recorded Phoenix trace annotations into RAGOps' existing external-metrics envelope.

  • Uses the existing custom provider so the published external-metrics-0.1 schema remains unchanged.
  • Preserves Phoenix's recorded score value, scale, and direction; the output metric is namespaced as phoenix.<annotation_name>.
  • Joins records through an explicit metadata.ragops_case_id field instead of guessing from array order or Phoenix trace IDs.
  • Fails closed for incomplete pagination, missing or unknown case IDs, duplicate case IDs, invalid annotator data, and non-finite or non-numeric scores.
  • Includes synthetic input, expected output, documentation, and tests.
  • Does not import the Phoenix SDK, initialize tracing, call a hosted service, or require credentials.

The input shape follows Phoenix's trace-annotation REST model and annotation concepts:

Verification

  • ruff check .: passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest -q -k not symlinked: passed, 184 passed, 1 skipped, 2 deselected
  • The two excluded tests require Windows symlink creation privileges (WinError 1314) in this environment; they are existing repository tests unrelated to this change.
  • git diff --check: passed

AI assistance

This patch was developed with AI assistance and reviewed against the repository contribution guidance and the Phoenix sources linked above. The implementation is intentionally limited to local, deterministic conversion and does not claim equivalence between Phoenix and RAGOps metrics.

@thangldw thangldw left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed at 880a3b1. The implementation satisfies issue #23: it stays offline, preserves producer-owned scores, joins through explicit case IDs, rejects incomplete coverage, and keeps the external-metrics schema unchanged. Local verification passed: ruff check, full test suite (200 passed), targeted tests on Python 3.11 and 3.13, and git diff --check.

Non-blocking follow-up: catch PhoenixAnnotationError and output OSError at the CLI boundary and report them with parser.exit(2, ...). Invalid or incomplete exports currently produce a full traceback even though they are expected user-facing validation failures. Please add a CLI-level negative test when addressing this.

Signed-off-by: Ruiming Zhao <uuzzrm@gmail.com>
@uuzzrm

uuzzrm commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the non-blocking CLI follow-up in 69be899.

The Phoenix example now catches PhoenixAnnotationError and output-file OSError at the CLI boundary and exits through parser.exit(2, "error: ..."). The conversion function still raises PhoenixAnnotationError for library callers.

Added CLI-level regression coverage for:

  • incomplete/invalid Phoenix input;
  • an unwritable output path;
  • exit code 2 and a concise error line without a traceback.

Validation:

  • tests/test_phoenix_recorded_scores.py: 12 passed
  • related external-metrics/public API tests: 14 passed
  • the example CLI produced the expected envelope
  • git diff --check: passed

Thanks for the specific follow-up.

@uuzzrm

uuzzrm commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

The PR is approved and CLEAN at head 69be8994, but my uuzzrm token cannot execute the upstream merge. The approval was submitted on the earlier 880a3b1 head; please review the current follow-up commit and merge if it still looks good.

@thangldw thangldw left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed current head 69be8994 (follow-up to the approval at 880a3b1). The follow-up is correctly scoped to the CLI boundary: PhoenixAnnotationError and input/output OSError now exit with code 2 and a concise error: line without a traceback, while convert_annotations keeps raising PhoenixAnnotationError for library callers. The added CLI tests cover invalid/incomplete Phoenix input and an output I/O failure.

Local verification on 69be8994:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest -q tests/test_phoenix_recorded_scores.py: 12 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest -q: 202 passed
  • .venv/bin/ruff check .: passed
  • git diff --check origin/main...HEAD: passed

No blocking findings. Approved for merge.

@thangldw
thangldw merged commit 6d05934 into thangldw:main Aug 11, 2026
@thangldw

Copy link
Copy Markdown
Owner

Thank you, @uuzzrm, for contributing this implementation. The offline Phoenix recorded-score adapter is now merged into main as 6d05934.

I appreciate the careful fail-closed validation, explicit case-ID mapping, preservation of producer-owned score semantics, and the prompt follow-up on CLI error handling and regression coverage. GitHub now recognizes your contribution on the repository's Contributors page.

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.

[Good first issue]: Add a Phoenix recorded-score adapter example

2 participants