Skip to content

fix(pipeline): don't hand the pipeline's argv to a step body - #28

Merged
hallelx2 merged 1 commit into
mainfrom
halleluyaholudele/hal-4194-step-bodies-parse-the-pipelines-own-argv
Oct 25, 2026
Merged

hallelx2 merged 1 commit into
mainfrom
halleluyaholudele/hal-4194-step-bodies-parse-the-pipelines-own-argv

Conversation

@hallelx2

Copy link
Copy Markdown
Collaborator

The rebuild completed ten steps and then died one from the end:

usage: pipeline.py [-h] [--dataset DATASET] [--holdout HOLDOUT]
                   [--output-dir OUTPUT_DIR] [--no-umap]
pipeline.py: error: unrecognized arguments: --from dataset_assembly --force --no-zenml

_run_module calls each step's entrypoint in-process. Those are written as main(argv=None) and fall through to parse_args(argv) — and with argv=None, argparse reads sys.argv[1:], which belongs to the pipeline, not the step. So clustering was handed --from dataset_assembly --force --no-zenml, didn't recognise them, and SystemExit-ed. figures never ran.

Why four hours of work hit this and nothing before did

The systemd unit invoked the pipeline with no arguments:

ExecStart=/usr/local/bin/uv run --with mlflow python -m cancerag.pipeline

sys.argv[1:] was empty, argparse was satisfied, and every historical run took the one path that works. The first run with --from/--force hit it immediately. A latent bug in a step that only fires when you use the CLI as designed.

Change

The guard goes in _run_module, not the call sites. Three modules parse args this way — unsupervised_clustering, chain_verification, binding_site — and patching three places leaves the next one written exposed. The step is invoked as a function; it should not see a command line at all.

sys.argv is restored in a finally, so a failing step cannot leave it clobbered for the steps after it.

_script and _module_cli use subprocess.run([sys.executable, "-m", module]), which gets a fresh argv — unaffected.

Verification

  • Tests checked against the unfixed code. Reverting the guard reproduces the production error verbatim:
    FAILED test_step_with_argparse_survives_pipeline_flags - SystemExit: 2
    usage: pipeline.py ... error: unrecognized arguments: --from dataset_assembly --force --no-zenml
    
  • pytest tests/unit — 481 passed, 31 skipped (was 474; +7 here)
  • ruff check clean on both touched files
  • Covers: argparse step survives pipeline flags; argv restored normally; argv restored when the step raises; real **kwargs still reach the entrypoint; and a guard test that flags if any of the three exposed modules stops parsing args, so the issue's exposure list can't silently rot

Next: re-run --from clustering to finish clustering and figures.

Closes HAL-4194

`_run_module` calls each step's entrypoint in-process. Those entrypoints are
written as `main(argv=None)` and fall through to `parse_args(argv)`, and with
`argv=None` argparse reads `sys.argv[1:]` -- which belongs to the pipeline,
not the step.

So running `--from dataset_assembly --force --no-zenml` killed the clustering
step with "unrecognized arguments: --from dataset_assembly --force", one step
from the end of a four-hour chain, and took `figures` with it.

It had never surfaced because the systemd unit invoked the pipeline with no
arguments at all, leaving `sys.argv[1:]` empty and argparse satisfied. Every
historical run took the single path that happens to work; the first run with
flags hit it immediately.

Three modules parse args this way -- unsupervised_clustering,
chain_verification and binding_site -- so the guard goes in `_run_module`
rather than at the call sites, where the next one written would miss it. argv
is restored in a `finally`, so a failing step cannot leave it clobbered for
the steps after it.

The tests were checked against the unfixed code: reverting the guard
reproduces the production error verbatim.
@hallelx2
hallelx2 merged commit 863529d into main Oct 25, 2026
2 checks passed
@hallelx2
hallelx2 deleted the halleluyaholudele/hal-4194-step-bodies-parse-the-pipelines-own-argv branch October 25, 2026 11:15
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.

1 participant