feat(cli): add stale --exit-code to gate CI on stale episodes - #81
Conversation
|
👋 Hi @chiruu12 — thank you so much for your first contribution to HFlow! A maintainer will review your pull request as soon as possible. In the meantime:
💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game. We are excited to have you here and appreciate your help making the project better! 🙌 |
kstonekuan
left a comment
There was a problem hiding this comment.
Thank you @chiruu12, and worth the wait. This is the plan you laid out on #30, delivered exactly as described, including the part that mattered most: leaving the default exit code and stdout alone so the xargs hflow ingest pipe keeps working.
What I validated locally:
- Full quality gate is clean (
ruff check,ruff format --check,ty check) and all 310 tests pass, on your branch merged with currentmain(#80 landed a few minutes ago, so I checked the combination rather than your branch alone). - Confirmed both new tests fail against pre-fix
cli.pywithunrecognized arguments: --exit-code, so they pin the behavior. - Confirmed
test_cli_stale_prints_source_uris_for_ingeststill passes, which is the real guard on the pipe: exit 0 on a stale catalog when the flag is absent. - lychee reports 231 links with 0 errors for the
docs/CATALOG.mdaddition.
On the failures you saw in tests/test_ffmpeg.py and test_media_stage_records_a_contact_sheet_artifact: those need real ffmpeg/ffprobe binaries and pass here. Thank you for checking them against unmodified main on the same host and reporting the base count rather than just the delta. That is exactly the right way to report an environment failure, and it made this review faster.
Two details I appreciated:
- Naming it after
git diff --exit-codeand saying so in the help text. Anyone who has gated CI on a diff already knows what this flag does. - The assertion that stdout is still exactly
["episodes-in/run_0001.mcap"]with the flag set. The flag changing only the exit code is the whole contract, and that line is what keeps a future refactor from quietly printing a summary into the pipe.
Merging now. Sorry about the traffic on this one: someone else opened a pull request for #30 while it was assigned to you, and I closed it because the assignment was yours. Thank you for seeing it through.
If you want another, the open good first issues have plenty left, and #8 (curate --dry-run) is the closest neighbor to what you just did. We hang out on Discord.
Closes #30.
_command_stalereturns 0 whatever it finds, sohflow stalecannot fail a CI job or a pre-training checklist the wayhflow doctoralready does.Adds
--exit-codeto thestalesubparser, named aftergit diff --exit-code. With the flag set and at least one stale episode printed, the command returns 1. Everything else is untouched: no flag means exit 0 as before, the parse and load failure paths still return 2, and stdout stays the bare URI list, so thexargs hflow ingestpipe indocs/CATALOG.mdis unaffected. Documented next to that pipe.Tests cover both directions against the existing catalog fixtures: an episode behind the pipeline version exits 1 and still prints the URI, an up to date catalog exits 0. The default path was already covered by
test_cli_stale_prints_source_uris_for_ingest, which asserts exit 0 on a stale catalog with no flag, so that one is the guard on the pipe.Both new tests fail on unmodified
mainwithunrecognized arguments: --exit-code, so they pin the behavior rather than just describing it.Validation
macOS,
uv sync --locked --all-extras.The 5 failures are all in
tests/test_ffmpeg.pyandtests/test_run_profiles.py::test_media_stage_records_a_contact_sheet_artifact, and they fail identically on unmodifiedmainon this host. Base is 299 passed with the same 5 red, so this takes the suite to 301 and moves nothing else.