Skip to content

chore: align all adapters with JobPhase SDK contract - #89

Merged
gnaulak-redhat merged 4 commits into
eval-hub:mainfrom
gnaulak-redhat:chore-jobphase-review
Jul 14, 2026
Merged

chore: align all adapters with JobPhase SDK contract#89
gnaulak-redhat merged 4 commits into
eval-hub:mainfrom
gnaulak-redhat:chore-jobphase-review

Conversation

@gnaulak-redhat

@gnaulak-redhat gnaulak-redhat commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What and why

Changes:

  • Review all adapters to implement all the supported JobPhases
  • use uv with Python 3.12 for local test venvs. This also addresses the uvloop segfault in guidellm on arm64 with python 3.14
  • update README with instructions on the JobPhases and implementation guide

Closes # https://redhat.atlassian.net/browse/RHOAIENG-66845

Type

  • feat
  • fix
  • docs
  • refactor / chore
  • test / ci

Testing

  • Tests added or updated
  • Tested manually

make tests passed

Screenshot 2026-07-13 at 3 14 14 PM

Testing in local the new lighteval driver
Screenshot 2026-07-13 at 6 39 59 PM
Screenshot 2026-07-13 at 6 40 21 PM

Breaking changes

No

Summary by CodeRabbit

  • New Features
    • Added OCI artifact export support across evaluation adapters, including persisted result files and artifact references in job results.
    • Job progress now follows a consistent lifecycle with initialization, loading, evaluation, post-processing, and artifact persistence phases.
  • Bug Fixes
    • Artifact persistence now occurs only when OCI export is configured and output files are available.
    • Failure reporting is more consistent and readable.
  • Documentation
    • Added guidance for job lifecycle phases and OCI export requirements.
  • Chores
    • Added configurable Python version support and streamlined test environment setup.

gnaulak-redhat and others added 2 commits July 10, 2026 14:30
Strip dead fields (progress, message, current_step, total_steps,
completed_steps) from JobStatusUpdate calls, migrate ErrorInfo to
error_message/MessageInfo on failure paths, and gate
PERSISTING_ARTIFACTS phase on OCI export configuration across all
8 adapters. Add OCI artifact persistence to the deepeval adapter.
Update test assertions for conditional PERSISTING_ARTIFACTS emission
and add a JobPhase lifecycle guide to the README.

Co-Authored-By: Claude <noreply@anthropic.com>
Avoid picking up the system python3 (currently 3.14) which triggers
a uvloop segfault in guidellm on arm64. Pin PYTHON_VERSION=3.12 by
default, overridable via make variable.

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

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adapters now use minimal job-phase status updates and error_message failure payloads, conditionally persist OCI artifacts, and attach artifact results. Integration tests cover OCI exports across adapters. The Makefile adds configurable uv-based Python environments, and the README documents the lifecycle contract.

Changes

Adapter lifecycle and OCI exports

Layer / File(s) Summary
Tooling and lifecycle contract
.gitignore, Makefile, README.md
Adds generated-fixture ignores, configurable uv-based test environments, and documentation for ordered job phases and conditional OCI persistence.
Lifecycle and failure status reporting
adapters/*/main.py
Simplifies lifecycle status payloads and replaces structured ErrorInfo failure data with error_message payloads across adapters.
Conditional OCI artifact persistence
adapters/*/main.py
Adds or conditions PERSISTING_ARTIFACTS, writes result files, creates OCI artifacts, and attaches returned artifact results.
OCI export integration coverage
adapters/*/tests/test_adapter.py
Adds end-to-end OCI export tests asserting phase reporting, persisted files, artifact creation, and returned digests.

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

Sequence Diagram(s)

sequenceDiagram
  participant Adapter
  participant Callbacks
  participant Results
  Adapter->>Adapter: write result files
  Adapter->>Callbacks: create OCI artifact
  Callbacks-->>Adapter: return OCI artifact result
  Adapter->>Results: attach OCI artifact
Loading

Possibly related PRs

Suggested labels: kind/fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title is concise and accurately summarizes the main change: aligning adapters with the JobPhase SDK contract.
Description check ✅ Passed The description covers what/why, type, testing, and breaking changes, and includes a related issue reference.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

The JobPhase alignment commit made PERSISTING_ARTIFACTS conditional on
exports.oci being configured but removed the phase assertion from happy-path
tests without adding coverage for the OCI-present path. Add
test_oci_export_persists_artifacts to deepeval, inspect, lighteval, and mteb
adapters verifying that the phase is emitted, create_oci_artifact is called
with a valid files_path, and the artifact is attached to JobResults.

Co-Authored-By: Claude <noreply@anthropic.com>
@gnaulak-redhat
gnaulak-redhat force-pushed the chore-jobphase-review branch from 0c58dc0 to 81863d6 Compare July 13, 2026 09:52
@gnaulak-redhat
gnaulak-redhat marked this pull request as ready for review July 13, 2026 09:56

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@adapters/clear/main.py`:
- Around line 373-374: Move the JobStatusUpdate call that reports
JobStatus.RUNNING with JobPhase.LOADING_DATA to immediately after input
validation, before traversing /test_data or /data and enumerating JSON files.
Remove it from its current later position while preserving the existing status
payload.

In `@adapters/swebench/main.py`:
- Around line 165-170: Update the OCI export flow around _persist_oci_artifact
to pass config.exports.oci.coordinates through and use those configured
coordinates as the publication destination instead of creating a new
host/job-specific target. Make _persist_oci_artifact return the created
artifact, capture that return value at the caller, and include it in the
resulting JobResults.

In `@Makefile`:
- Around line 263-265: Recreate .venv when PYTHON_VERSION changes instead of
only checking whether the directory exists. Apply this update to the adapter
test recipes at Makefile lines 263-265, 272-274, 281-283, 290-292, 299-300,
310-311, 321-322, and 333-335, preserving the existing dependency installation
and pytest commands.
🪄 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: c13ad5e2-b54b-409d-80e9-56e567f25a6b

📥 Commits

Reviewing files that changed from the base of the PR and between f9066bb and 81863d6.

📒 Files selected for processing (15)
  • .gitignore
  • Makefile
  • README.md
  • adapters/clear/main.py
  • adapters/deepeval/main.py
  • adapters/deepeval/tests/test_adapter.py
  • adapters/guidellm/main.py
  • adapters/inspect/main.py
  • adapters/inspect/tests/test_adapter.py
  • adapters/lighteval/main.py
  • adapters/lighteval/tests/test_adapter.py
  • adapters/mteb/main.py
  • adapters/mteb/tests/test_adapter.py
  • adapters/ragas/main.py
  • adapters/swebench/main.py

Comment thread adapters/clear/main.py Outdated
Comment thread adapters/swebench/main.py Outdated
Comment thread Makefile
…oordinates

Move CLEAR's LOADING_DATA status report to fire immediately after input
validation, before directory traversal begins. Fix SWE-bench OCI export
to use job-configured coordinates instead of hardcoded values, return the
artifact result, and include it in JobResults.

Co-Authored-By: Claude <noreply@anthropic.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.

3 participants