From 226f155f8aae6cebecc91b211bd1e04e2458726c Mon Sep 17 00:00:00 2001 From: man4ish Date: Fri, 13 Nov 2026 09:27:00 -0600 Subject: [PATCH 1/2] fix(download_references): refuse to run without a legacy-layout flag The downloader writes the pre-contract reference layout (organisms//, indexes//, dataset_registry.json), and some of its URLs (rat release 112) now 404. The reference tree under /mnt/nas/omnibioai/reference-genomes follows contract v1 instead. main() now exits 2 with a pointer to the contract unless --i-know-this-writes-legacy-layout is passed. --help still works. The existing CLI tests pass the flag; a new test checks that --status, --scaffold, --dry-run and a bare run all refuse before touching the base directory. Co-Authored-By: Claude Opus 5.5 (1M context) --- download_references.py | 23 +++++++++++++++++++++++ tests/test_coverage_gaps.py | 3 ++- tests/test_python_utilities.py | 28 +++++++++++++++++++++++----- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/download_references.py b/download_references.py index 913a955..42d0be9 100644 --- a/download_references.py +++ b/download_references.py @@ -9,6 +9,11 @@ --include-variants --include-databases --threads 8 python download_references.py --status --base-dir $OMNIBIOAI_ROOT/data/reference python download_references.py --dry-run --species human --assemblies GRCh38 + +LEGACY: this writes the pre-contract layout (organisms//, indexes//, +dataset_registry.json), and some of its URLs no longer exist. The reference tree now follows +contract v1 (ROOT/README.md and ROOT/_schema/ under /mnt/nas/omnibioai/reference-genomes). +Every run except --help therefore needs --i-know-this-writes-legacy-layout. """ import argparse @@ -26,6 +31,15 @@ REGISTRY_FILENAME = "dataset_registry.json" +LEGACY_FLAG = "--i-know-this-writes-legacy-layout" +LEGACY_MESSAGE = ( + "download_references.py writes the legacy reference layout (organisms//,\n" + "indexes//, dataset_registry.json), not the contract v1 layout now used under\n" + "/mnt/nas/omnibioai/reference-genomes: see README.md and _schema/ there, and\n" + "scripts/reference/validate_reference.py in omnibioai-ecosystem.\n" + f"Refusing to run. Pass {LEGACY_FLAG} to run it anyway." +) + def load_registry(registry_path: Path) -> dict: if registry_path.exists(): @@ -703,6 +717,12 @@ def parse_args() -> argparse.Namespace: action="store_true", help="Print a status report of what files exist and exit", ) + parser.add_argument( + LEGACY_FLAG, + dest="legacy_layout_ok", + action="store_true", + help="Required: acknowledge that this writes the legacy (pre contract v1) layout", + ) parser.add_argument( "--scaffold", action="store_true", @@ -743,6 +763,9 @@ def resolve_assemblies(species: list[str] | None, assemblies: list[str] | None) def main() -> None: args = parse_args() + if not args.legacy_layout_ok: + print(LEGACY_MESSAGE, file=sys.stderr) + sys.exit(2) base = Path(args.base_dir).expanduser().resolve() dl = ReferenceDownloader(base_dir=base, threads=args.threads, dry_run=args.dry_run) diff --git a/tests/test_coverage_gaps.py b/tests/test_coverage_gaps.py index 7d221f9..eaf53b3 100644 --- a/tests/test_coverage_gaps.py +++ b/tests/test_coverage_gaps.py @@ -133,7 +133,8 @@ def test_download_only_flag_and_unknown_assembly(monkeypatch, tmp_path, capsys): monkeypatch.setattr(d, "ASSEMBLY_MAP", {}) monkeypatch.setattr( sys, "argv", - ["x", "--base-dir", str(tmp_path), "--species", "human", "--only", "genome"], + ["x", "--base-dir", str(tmp_path), "--species", "human", "--only", "genome", + "--i-know-this-writes-legacy-layout"], ) d.main() out = capsys.readouterr().out diff --git a/tests/test_python_utilities.py b/tests/test_python_utilities.py index bef17e0..ffa93a1 100644 --- a/tests/test_python_utilities.py +++ b/tests/test_python_utilities.py @@ -260,7 +260,7 @@ def test_reference_cli_parser_and_main_dry_run(monkeypatch, capsys): monkeypatch.setattr(sys, "argv", ["download_references.py", "--species", "human", "--dry-run"]) args = mod.parse_args() assert args.species == ["human"] and args.dry_run - monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", "/tmp/refs", "--status"]) + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", "/tmp/refs", "--status", "--i-know-this-writes-legacy-layout"]) monkeypatch.setattr(mod.ReferenceDownloader, "print_status", lambda self: print("status")) mod.main() assert "status" in capsys.readouterr().out @@ -270,7 +270,7 @@ def test_reference_main_dry_run_dispatches_optional_sections(monkeypatch, tmp_pa """Dispatch main() to the genome, annotation, variant, and database download methods requested via --include-* flags, in order.""" mod = load("download_references") - monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path), "--assemblies", "GRCh38", "--include-annotation", "--include-variants", "--include-databases", "--dry-run"]) + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path), "--assemblies", "GRCh38", "--include-annotation", "--include-variants", "--include-databases", "--dry-run", "--i-know-this-writes-legacy-layout"]) calls = [] for name in ["download_human_GRCh38", "download_annotation_human", "download_annotation_mouse", "download_variants_human", "download_variants_mouse", "download_databases"]: monkeypatch.setattr(mod.ReferenceDownloader, name, lambda self, name=name: calls.append(name)) @@ -836,12 +836,13 @@ def test_setup_main_and_reference_cli_control_paths(monkeypatch, tmp_path, capsy assert "DRY RUN complete" in capsys.readouterr().out refs = load("download_references") - monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path), "--scaffold"]) + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path), "--scaffold", "--i-know-this-writes-legacy-layout"]) monkeypatch.setattr(refs.ReferenceDownloader, "scaffold_directories", lambda self: None) monkeypatch.setattr(refs.ReferenceDownloader, "flush_registry", lambda self: None) refs.main() - monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path)]) - with pytest.raises(SystemExit): refs.main() + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path), "--i-know-this-writes-legacy-layout"]) + with pytest.raises(SystemExit) as nothing: refs.main() + assert nothing.value.code == 0 def test_prepare_download_parse_reservoir_and_empty_main(monkeypatch, tmp_path): @@ -930,3 +931,20 @@ def test_small_remaining_branches(monkeypatch): monkeypatch.setattr(refs, "ASSEMBLY_MAP", {"GRCh38": ("human", "missing_downloader")}) monkeypatch.setattr(refs.ReferenceDownloader, "scaffold_directories", lambda self: None) refs.main() + + +def test_reference_main_refuses_without_legacy_flag(monkeypatch, tmp_path, capsys): + """Without --i-know-this-writes-legacy-layout, main() exits 2 before + touching the base dir, and points at contract v1.""" + refs = load("download_references") + called = [] + monkeypatch.setattr(refs.ReferenceDownloader, "__init__", lambda self, *a, **k: called.append("init")) + for extra in (["--status"], ["--scaffold"], ["--species", "human", "--dry-run"], []): + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", str(tmp_path / "refs"), *extra]) + with pytest.raises(SystemExit) as exc: + refs.main() + assert exc.value.code == 2 + assert called == [] + assert not (tmp_path / "refs").exists() + err = capsys.readouterr().err + assert "contract v1" in err and "--i-know-this-writes-legacy-layout" in err From 0f0ce8135b70ddaf22bc3db7eb5b51dfc8fb17bc Mon Sep 17 00:00:00 2001 From: man4ish Date: Fri, 13 Nov 2026 09:29:39 -0600 Subject: [PATCH 2/2] test(download_references): pass the legacy-layout flag in the remaining CLI tests test_download_cli_branches and test_small_remaining_branches call main() directly; give them the flag, and assert the no-selection exit is code 0 so it cannot pass by being refused. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_high_coverage.py | 7 ++++--- tests/test_python_utilities.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_high_coverage.py b/tests/test_high_coverage.py index b3f8cda..d8aff82 100644 --- a/tests/test_high_coverage.py +++ b/tests/test_high_coverage.py @@ -414,11 +414,12 @@ def test_download_cli_branches(monkeypatch,tmp_path,capsys): """Report an empty registry, require at least one selection flag, reject an unknown assembly at argument-parsing time, and report a missing downloader for an unmapped assembly.""" - d=load("download_references"); dl=d.ReferenceDownloader(tmp_path); dl.registry={}; dl.print_status(); assert "Registry: empty" in capsys.readouterr().out; assert d._now(); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--scaffold","--dry-run"]); d.main(); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path)]); - with pytest.raises(SystemExit): d.main() + d=load("download_references"); dl=d.ReferenceDownloader(tmp_path); dl.registry={}; dl.print_status(); assert "Registry: empty" in capsys.readouterr().out; assert d._now(); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--scaffold","--dry-run","--i-know-this-writes-legacy-layout"]); d.main(); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--i-know-this-writes-legacy-layout"]); + with pytest.raises(SystemExit) as nothing: d.main() + assert nothing.value.code == 0 monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--assemblies","UNKNOWN"]); # parser rejects this before business logic; validate separately with pytest.raises(SystemExit): d.parse_args() - monkeypatch.setattr(d,"ASSEMBLY_MAP",{"X":("x","missing")}); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--assemblies","X"]); d.main(); assert "No downloader" in capsys.readouterr().out + monkeypatch.setattr(d,"ASSEMBLY_MAP",{"X":("x","missing")}); monkeypatch.setattr(sys,"argv",["x","--base-dir",str(tmp_path),"--assemblies","X","--i-know-this-writes-legacy-layout"]); d.main(); assert "No downloader" in capsys.readouterr().out def test_public_radio_and_submit_failures(monkeypatch): diff --git a/tests/test_python_utilities.py b/tests/test_python_utilities.py index ffa93a1..16e8976 100644 --- a/tests/test_python_utilities.py +++ b/tests/test_python_utilities.py @@ -927,7 +927,7 @@ def test_small_remaining_branches(monkeypatch): with pytest.raises(SystemExit): agent.load_corpus("/missing/tools.yaml", None, None, None, [], False) assert load("download_references")._now() refs = load("download_references") - monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", "/tmp/refs", "--assemblies", "GRCh38", "--only", "genome", "--dry-run"]) + monkeypatch.setattr(sys, "argv", ["download_references.py", "--base-dir", "/tmp/refs", "--assemblies", "GRCh38", "--only", "genome", "--dry-run", "--i-know-this-writes-legacy-layout"]) monkeypatch.setattr(refs, "ASSEMBLY_MAP", {"GRCh38": ("human", "missing_downloader")}) monkeypatch.setattr(refs.ReferenceDownloader, "scaffold_directories", lambda self: None) refs.main()