Skip to content

feat: log project annotations as a zipped MLflow artifact - #420

Merged
gbeane merged 2 commits into
mainfrom
mlflow-annotations-artifact
Jul 31, 2026
Merged

feat: log project annotations as a zipped MLflow artifact#420
gbeane merged 2 commits into
mainfrom
mlflow-annotations-artifact

Conversation

@gbeane

@gbeane gbeane commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

jabs-cli cross-validation --mlflow now zips the project's jabs/annotations directory and uploads it as an annotations.zip run artifact, alongside the training report. This captures the label set a run's metrics were computed from, so a run can be traced back to (and reproduced from) the exact annotations.

Changes

  • jabs/classifier/mlflow_logging.py — new archive_annotations() helper. Zips with ZIP_DEFLATED, storing members under a single top-level annotations/ prefix so unpacking recreates the directory rather than scattering JSON into the cwd. Files are added in sorted order for reproducible archives. Returns None (no error) when the directory is missing or empty.
  • log_cross_validation_to_mlflow() gained annotations_dir and log_annotations_artifact; the archive is staged in a TemporaryDirectory inside the run and logged as annotations.zip — a constant name, so the artifact is easy to find and compare across runs.
  • jabs/scripts/cli/cross_validation.py — passes project.annotation_dir through, plus a mlflow_log_annotations parameter.
  • jabs/scripts/cli/cli.py — new --mlflow-no-annotations opt-out flag.
  • Docs — synopsis, options list, "What gets logged", and the skip-artifacts section updated in both the online docs and the in-app user guide copy.

Notes for review

Two judgment calls worth a look:

  1. New --mlflow-no-annotations flag. --mlflow-no-report already existed and its help text promised "metrics + params only", so an unconditional second artifact would have contradicted it. A skip flag also matters for projects with a large annotations directory, since the archive uploads on every run. The --mlflow-no-report help text was updated to drop the now-inaccurate parenthetical.
  2. Archiving is best-effort. An OSError while zipping logs a warning and the run keeps its metrics, params, and report artifact, consistent with this module's existing stance that a logging failure never costs you the results. The alternative would be exit code 3 on a run whose metrics already landed.

Testing

  • 7 new unit tests: archive contents/prefix, missing directory, empty directory, upload alongside the report, both opt-out paths, and archive-failure tolerance. Plus a CLI test for the new flag.
  • The fake mlflow client now captures zip members at log_artifact time, since the staging directory is gone by the time assertions run — this is what proves the archive is still on disk when uploaded.
  • Full root suite passes (875 tests); ruff check and format clean.
  • Verified end to end against a live MLflow tracking server.

Copilot AI 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.

Pull request overview

Adds optional provenance logging for cross-validation runs by zipping the project’s jabs/annotations directory and uploading it as a consistent MLflow run artifact (annotations.zip). This strengthens reproducibility by allowing metrics to be traced back to the exact label set used for a run, and integrates the behavior into jabs-cli cross-validation with an explicit opt-out.

Changes:

  • Add archive_annotations() and extend log_cross_validation_to_mlflow() to (optionally) upload annotations.zip alongside the report.
  • Plumb new CLI wiring/flags (--mlflow-no-annotations) through jabs-cli cross-validation into the cross-validation runner.
  • Update documentation and tests to cover the new artifact and opt-out behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/jabs/classifier/mlflow_logging.py Adds annotation zipping + MLflow artifact upload support.
src/jabs/scripts/cli/cross_validation.py Passes project.annotation_dir into MLflow logging and adds a toggle param.
src/jabs/scripts/cli/cli.py Introduces --mlflow-no-annotations and forwards the opt-out into CV execution.
tests/classifier/test_mlflow_logging.py Adds unit tests for archive creation, opt-outs, and failure tolerance.
tests/scripts/test_cross_validation_cli.py Adds CLI test coverage for the new opt-out flag and default behavior.
docs/user-guide/cli-tools.md Documents the new artifact and opt-out flag for the published user guide.
src/jabs/resources/docs/user_guide/cli-tools.md Mirrors the same documentation update for in-app docs resources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/jabs/resources/docs/user_guide/cli-tools.md Outdated
Comment thread src/jabs/classifier/mlflow_logging.py Outdated
Comment thread docs/user-guide/cli-tools.md Outdated
@gbeane
gbeane requested review from bergsalex and keithshep July 30, 2026 18:39
@gbeane gbeane self-assigned this Jul 30, 2026
@gbeane
gbeane merged commit df9999c into main Jul 31, 2026
5 checks passed
@gbeane
gbeane deleted the mlflow-annotations-artifact branch July 31, 2026 20:36
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