From d5a20870e762284bf82770d110637ed8cf405491 Mon Sep 17 00:00:00 2001 From: ecrum19 Date: Tue, 15 Sep 2026 11:08:01 +0200 Subject: [PATCH 1/3] Tell the validation oracle how the graph was actually built A conversion run with --info-representation raw could not pass validation. The oracle reported the entire structured layer missing -- the allele layer, value items, InfoFieldValue/InfoFieldDefinition, and all 256 buckets of the INFO value digest -- and the run exited 1. The oracle was right and already handled this: expected_census only adds that layer when info_representation == "structured". It just never learned which mode produced the graph. attach_census_expectations was called as attach_census_expectations(parser, args.representation) and both info_representation and header_representation are keyword-only with default "structured", so every run was scored against a structured oracle. The validation runner's CLI had no flag for either, so the wrapper had no way to say otherwise across the container boundary. - validation_runner.py gains --info-representation and --header-representation and passes them at the call site. - run_validation_mode takes both as required keyword arguments and forwards them to the runner. Full mode already had them and simply never passed them on; validate mode reads them from args like sample_representation does. - expected_census and attach_census_expectations lose their defaults. The default was the bug: it let a caller omit the parameter and silently get wrong expectations instead of a TypeError. The oracle's raw/structured branch was already exercised -- validation_fixtures passes both values explicitly -- so the logic was covered while the wiring between wrapper and runner was not. Added tests for the wiring itself: that the flags reach the runner's argv, that the runner's CLI accepts what the wrapper sends, and that the census helpers refuse to guess. Found by 11_covering_set, whose row 2 is the only row pairing raw INFO with single-strategy HDT. That row failing also aborted rows 3-6, so the sweep recorded 2 of 6 rows. Co-Authored-By: Claude Opus 5 --- src/validation/validation_runner.py | 40 ++++++++++++++++--- test/test_validation_engines_unit.py | 60 ++++++++++++++++++++++++++++ test/test_vcf_rdfizer_unit.py | 4 ++ vcf_rdfizer.py | 10 +++++ 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/src/validation/validation_runner.py b/src/validation/validation_runner.py index 3dfb94d..d7a0da3 100644 --- a/src/validation/validation_runner.py +++ b/src/validation/validation_runner.py @@ -760,8 +760,8 @@ def _count_definitions(predicates: dict[str, int], numbers: list[str]) -> None: def expected_census( - parser: dict[str, Any], representation: str, *, info_representation: str = "structured", - header_representation: str = "structured", + parser: dict[str, Any], representation: str, *, info_representation: str, + header_representation: str, ) -> dict[str, list[dict[str, Any]]]: """Predicate and class counts the graph must contain, and nothing else.""" records = parser["totalRecords"] @@ -1188,8 +1188,8 @@ def parse_header_metadata(raw_header: str) -> dict[str, Any]: def attach_census_expectations( - parser: dict[str, Any], representation: str, *, info_representation: str = "structured", - header_representation: str = "structured", + parser: dict[str, Any], representation: str, *, info_representation: str, + header_representation: str, ) -> dict[str, Any]: """Add the expected predicate/class inventory to a parser summary. @@ -3469,7 +3469,12 @@ def run_validation(args: argparse.Namespace) -> int: ) parse_seconds = time.monotonic() - oracle_started census_started = time.monotonic() - parser = attach_census_expectations(parser, args.representation) + parser = attach_census_expectations( + parser, + args.representation, + info_representation=args.info_representation, + header_representation=args.header_representation, + ) oracle_phases["censusSeconds"] = time.monotonic() - census_started oracle_seconds = { "parse": parse_seconds, @@ -3774,6 +3779,31 @@ def build_arg_parser() -> argparse.ArgumentParser: help="Override artifact format detection (default: infer from the filename)", ) parser.add_argument("--representation", choices=("expanded", "condensed"), required=True) + # The oracle's expectation depends on these exactly as much as it does on + # --representation: the allele layer, value items, SV carriers and parsed + # genotype layer are only emitted for structured INFO, and the '##' line + # detail only for structured headers. Without them the oracle expected a + # structured graph for every run and any --info-representation raw + # conversion failed validation with the whole structured layer reported + # missing. + parser.add_argument( + "--info-representation", + choices=("raw", "structured"), + default="structured", + help=( + "How the INFO column was emitted in the graph under test. Must match " + "the conversion; the wrapper passes it automatically." + ), + ) + parser.add_argument( + "--header-representation", + choices=("basic", "structured"), + default="structured", + help=( + "How the '##' meta-information lines were emitted in the graph under " + "test. Must match the conversion; the wrapper passes it automatically." + ), + ) parser.add_argument("--results-dir", type=Path, required=True) parser.add_argument("--dataset-id", required=True) parser.add_argument("--filter-oracle", choices=("auto", "bcftools", "cyvcf2"), default="auto") diff --git a/test/test_validation_engines_unit.py b/test/test_validation_engines_unit.py index d6c70d4..584ff79 100644 --- a/test/test_validation_engines_unit.py +++ b/test/test_validation_engines_unit.py @@ -726,6 +726,8 @@ def test_validation_command_carries_engine_and_format(self): vcf_path=vcf_path, rdf_path=hdt_path, representation="condensed", + info_representation="structured", + header_representation="structured", validation_id="cohort", results_dir=tmp_path / "results", metrics_dir=tmp_path / "metrics", @@ -744,6 +746,62 @@ def test_validation_command_carries_engine_and_format(self): self.assertEqual(command[command.index("--qlever-memory-gb") + 1], "12") self.assertIn("--qlever-index-arg", command) + def test_representation_flags_reach_the_validation_runner(self): + """The oracle's expectation depends on these; the wrapper must forward them. + + Regression: the runner defaulted info/header representation to + "structured" and the wrapper never passed either, so every + --info-representation raw conversion validated against a structured + oracle and failed with the whole structured layer reported missing + (allele layer, value items, InfoFieldValue, the INFO digest). The oracle + itself was already correct and unit-tested for raw -- only the wiring + between wrapper and runner was missing, which no test covered. + """ + with tempfile.TemporaryDirectory() as td: + tmp_path = Path(td) + vcf_path = tmp_path / "cohort.vcf" + vcf_path.write_text("##fileformat=VCFv4.2\n#CHROM\tPOS\n", encoding="utf-8") + nt_path = tmp_path / "cohort.nt" + nt_path.write_text("", encoding="utf-8") + commands = [] + + with mock.patch.object( + vcf_rdfizer, "run", side_effect=lambda cmd, **kw: commands.append(cmd) or 0 + ): + vcf_rdfizer.run_validation_mode( + vcf_path=vcf_path, + rdf_path=nt_path, + representation="expanded", + info_representation="raw", + header_representation="basic", + validation_id="cohort", + results_dir=tmp_path / "results", + metrics_dir=tmp_path / "metrics", + run_id="RID", + timestamp="TS", + image_ref="example/vcf-rdfizer:latest", + filter_oracle="auto", + wrapper_log_path=tmp_path / "wrapper.log", + ) + command = commands[0] + self.assertEqual(command[command.index("--info-representation") + 1], "raw") + self.assertEqual(command[command.index("--header-representation") + 1], "basic") + + def test_the_runner_cli_accepts_what_the_wrapper_sends(self): + """Both ends of the container boundary must agree on the flag names.""" + args = V.build_arg_parser().parse_args([ + "--vcf", "a.vcf", "--rdf", "a.nt", "--representation", "expanded", + "--info-representation", "raw", "--header-representation", "basic", + "--results-dir", "r", "--dataset-id", "d", + ]) + self.assertEqual(args.info_representation, "raw") + self.assertEqual(args.header_representation, "basic") + + def test_census_expectations_refuse_to_guess(self): + """No silent default: a caller that forgets must fail, not mis-expect.""" + with self.assertRaises(TypeError): + V.attach_census_expectations({}, "expanded") + def test_unsupported_artifact_is_rejected_by_the_wrapper(self): with tempfile.TemporaryDirectory() as td: tmp_path = Path(td) @@ -756,6 +814,8 @@ def test_unsupported_artifact_is_rejected_by_the_wrapper(self): vcf_path=vcf_path, rdf_path=bogus, representation="expanded", + info_representation="structured", + header_representation="structured", validation_id="cohort", results_dir=tmp_path / "results", metrics_dir=tmp_path / "metrics", diff --git a/test/test_vcf_rdfizer_unit.py b/test/test_vcf_rdfizer_unit.py index a6c4f2b..e68036f 100644 --- a/test/test_vcf_rdfizer_unit.py +++ b/test/test_vcf_rdfizer_unit.py @@ -467,6 +467,8 @@ def fake_run(cmd, cwd=None, env=None): vcf_path=vcf_path, rdf_path=rdf_path, representation="expanded", + info_representation="structured", + header_representation="structured", validation_id="sample", results_dir=results_dir, metrics_dir=metrics_dir, @@ -3110,6 +3112,8 @@ def fake_run(cmd, cwd=None, env=None): vcf_path=vcf_path, rdf_path=rdf_path, representation="expanded", + info_representation="structured", + header_representation="structured", validation_id="sample", results_dir=results_dir, metrics_dir=metrics_dir, diff --git a/vcf_rdfizer.py b/vcf_rdfizer.py index 144cfe5..23d9e2f 100644 --- a/vcf_rdfizer.py +++ b/vcf_rdfizer.py @@ -7578,6 +7578,8 @@ def fail_current(stage: str, message: str): rdf_path=target["path"], rdf_format=target["format"], representation=sample_workflow.representation, + info_representation=info_representation, + header_representation=header_representation, validation_id=target_id, results_dir=target_results_dir, metrics_dir=metrics_dir, @@ -8446,6 +8448,8 @@ def run_validation_mode( vcf_path: Path, rdf_path: Path, representation: str, + info_representation: str, + header_representation: str, validation_id: str, results_dir: Path, metrics_dir: Path, @@ -8570,6 +8574,10 @@ def run_validation_mode( *engine_args, "--representation", representation, + "--info-representation", + info_representation, + "--header-representation", + header_representation, "--results-dir", "/data/validation", "--dataset-id", @@ -9802,6 +9810,8 @@ def execute_mode(): rdf_path=validation_rdf_gzip_path, rdf_format=validation_rdf_format, representation=args.sample_representation, + info_representation=args.info_representation, + header_representation=args.header_representation, validation_id=validation_id, results_dir=validation_results_dir, metrics_dir=metrics_dir, From 3f93b4713b4bb00b3d62c5824aab4dac6438bb1d Mon Sep 17 00:00:00 2001 From: ecrum19 Date: Tue, 15 Sep 2026 11:13:27 +0200 Subject: [PATCH 2/3] Expect no INFO value digest when INFO was emitted raw With the representation threaded through, q09_predicate_census and q10_class_census came back clean but q12_info_value_digest still reported all 256 buckets missing against zero extra rows -- the graph produced nothing and the oracle expected a full digest. q12 hashes the decomposed INFO value items, which only the structured representation emits; under raw INFO the column stays one opaque literal and the query matches nothing. But the digest is computed from the VCF in parse_vcf, where the emitter's representation is not in scope, so it survived unconditionally. Gate it in attach_census_expectations, next to the existing q13 gate on sample representation. validation_fixtures already encodes this -- parser_summary(include_info=False) yields an empty digest -- so the fixture and the production oracle disagreed. The new test pins them together. Co-Authored-By: Claude Opus 5 --- src/validation/validation_runner.py | 6 ++++++ test/test_validation_engines_unit.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/validation/validation_runner.py b/src/validation/validation_runner.py index d7a0da3..c3962de 100644 --- a/src/validation/validation_runner.py +++ b/src/validation/validation_runner.py @@ -1206,6 +1206,12 @@ def attach_census_expectations( else "_condensedFormatValueDigest" ) parser["q13_format_value_digest"] = parser.get(key, []) + # The INFO value digest hashes the decomposed value items, which only the + # structured representation emits. Under raw INFO the column stays one + # opaque literal, so the query matches nothing and the expectation is + # empty -- not the digest of values the graph was never asked to produce. + if info_representation != "structured": + parser["q12_info_value_digest"] = [] return parser diff --git a/test/test_validation_engines_unit.py b/test/test_validation_engines_unit.py index 584ff79..45b6e4a 100644 --- a/test/test_validation_engines_unit.py +++ b/test/test_validation_engines_unit.py @@ -797,6 +797,26 @@ def test_the_runner_cli_accepts_what_the_wrapper_sends(self): self.assertEqual(args.info_representation, "raw") self.assertEqual(args.header_representation, "basic") + def test_raw_info_expects_no_value_digest(self): + """q12 hashes decomposed INFO value items, which raw mode never emits. + + Regression: with the representation threaded through, q09/q10 came back + clean but q12 still reported all 256 buckets missing, because the digest + is computed from the VCF in parse_vcf and was never gated on how the + INFO column was actually emitted. + """ + from test import validation_fixtures as fixtures + structured = V.attach_census_expectations( + fixtures.parser_summary("expanded"), "expanded", + info_representation="structured", header_representation="structured", + ) + self.assertNotEqual(structured["q12_info_value_digest"], []) + raw = V.attach_census_expectations( + fixtures.parser_summary("expanded"), "expanded", + info_representation="raw", header_representation="structured", + ) + self.assertEqual(raw["q12_info_value_digest"], []) + def test_census_expectations_refuse_to_guess(self): """No silent default: a caller that forgets must fail, not mis-expect.""" with self.assertRaises(TypeError): From a3679e1006c46d7a0caee80ef70d76e60ea21e19 Mon Sep 17 00:00:00 2001 From: ecrum19 Date: Tue, 15 Sep 2026 13:34:05 +0200 Subject: [PATCH 3/3] Pin pip in CI instead of installing whatever released most recently Every job began with `pip install --upgrade pip`, so each run resolved dependencies with a different, unannounced resolver. CI was non-reproducible by construction: the same commit could pass or fail depending on what pip shipped that morning, with nothing in this repository to show for it. Pin it once per workflow as PIP_VERSION and reference that from all six install steps, so a resolver change lands as its own reviewable commit. What this does not do is prevent transient index failures, and it should not be read as a fix for one. The case that prompted it: the pull_request run for PR #14 failed with no matching distributions available for your environment: mdurl while the identical commit had passed 15 minutes earlier on push, passed on ubuntu-22.04 and macos-latest in that same run, and passed again on re-run with no code change. mdurl is a pure-Python py3-none-any wheel and the job pins Python 3.11, so nothing about the environment made it uninstallable -- that was the index, not the resolver, and the answer to it is a re-run. Pinning was worth doing on its own merits, not because it would have prevented that failure. Note that build, twine and coverage are still unpinned in the publish and coverage jobs. Same argument applies to them; left alone here so this commit stays about the toolchain that resolves everything else. Co-Authored-By: Claude Opus 5 --- .github/workflows/publish-python.yml | 20 ++++++++++++++++- .github/workflows/tests.yml | 26 +++++++++++++++++++---- .github/workflows/validation-mutation.yml | 22 ++++++++++++++++++- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/.github/workflows/publish-python.yml b/.github/workflows/publish-python.yml index ba2d41b..9df3b1e 100644 --- a/.github/workflows/publish-python.yml +++ b/.github/workflows/publish-python.yml @@ -13,6 +13,24 @@ on: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + # Pin pip instead of taking whatever released most recently. Every job used to + # run `pip install --upgrade pip`, so each run resolved dependencies with a + # different, unannounced resolver: CI was non-reproducible by construction, + # and a pip release could break the build with no change in this repository. + # + # This does NOT prevent transient index failures, and should not be mistaken + # for a fix for one. A case is on record: the pull_request run for PR #14 + # failed with `no matching distributions available for your environment: + # mdurl` while the identical commit had passed 15 minutes earlier on push, + # passed on two other platforms in that same run, and passed again on re-run + # with no code change. mdurl is a pure-Python py3-none-any wheel, so nothing + # about the environment made it uninstallable -- that was the index, not the + # resolver. The right response there is a re-run. The right response to an + # unpinned toolchain is this pin. + # + # Bump deliberately, as its own commit, so a resolver change lands where it + # can be attributed instead of appearing inside an unrelated PR. + PIP_VERSION: "26.2.1" jobs: build: @@ -36,7 +54,7 @@ jobs: - name: Build package run: | - python -m pip install --upgrade pip build twine + python -m pip install "pip==${{ env.PIP_VERSION }}" build twine rm -rf dist build *.egg-info python -m build python -m twine check dist/* diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 47ff643..6990355 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,6 +8,24 @@ on: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + # Pin pip instead of taking whatever released most recently. Every job used to + # run `pip install --upgrade pip`, so each run resolved dependencies with a + # different, unannounced resolver: CI was non-reproducible by construction, + # and a pip release could break the build with no change in this repository. + # + # This does NOT prevent transient index failures, and should not be mistaken + # for a fix for one. A case is on record: the pull_request run for PR #14 + # failed with `no matching distributions available for your environment: + # mdurl` while the identical commit had passed 15 minutes earlier on push, + # passed on two other platforms in that same run, and passed again on re-run + # with no code change. mdurl is a pure-Python py3-none-any wheel, so nothing + # about the environment made it uninstallable -- that was the index, not the + # resolver. The right response there is a re-run. The right response to an + # unpinned toolchain is this pin. + # + # Bump deliberately, as its own commit, so a resolver change lands where it + # can be attributed instead of appearing inside an unrelated PR. + PIP_VERSION: "26.2.1" jobs: wrapper-cross-platform: @@ -36,7 +54,7 @@ jobs: # editable pulls exactly what pyproject declares and cannot drift from # it, while tests still run against the checkout. run: | - python -m pip install --upgrade pip + python -m pip install "pip==${{ env.PIP_VERSION }}" python -m pip install -e . - name: Run cross-platform wrapper tests (no real external tools) @@ -68,7 +86,7 @@ jobs: # editable pulls exactly what pyproject declares and cannot drift from # it, while tests still run against the checkout. run: | - python -m pip install --upgrade pip + python -m pip install "pip==${{ env.PIP_VERSION }}" python -m pip install -e . - name: Run full unit test suite @@ -91,7 +109,7 @@ jobs: - name: Run coverage run: | - python -m pip install --upgrade pip coverage + python -m pip install "pip==${{ env.PIP_VERSION }}" coverage python -m pip install -e . coverage run -m unittest discover -s test -p "test_*_unit.py" coverage xml -o coverage.xml @@ -127,7 +145,7 @@ jobs: - name: Build + install wheel run: | - python -m pip install --upgrade pip build + python -m pip install "pip==${{ env.PIP_VERSION }}" build python -m build python -c "import glob,subprocess,sys; wheels=sorted(glob.glob('dist/*.whl')); wheels or (_ for _ in ()).throw(SystemExit('No wheel files found in dist/')); print(f'Installing wheel: {wheels[-1]}'); subprocess.check_call([sys.executable,'-m','pip','install',wheels[-1]])" python -m vcf_rdfizer --help diff --git a/.github/workflows/validation-mutation.yml b/.github/workflows/validation-mutation.yml index e8fe117..b2ca616 100644 --- a/.github/workflows/validation-mutation.yml +++ b/.github/workflows/validation-mutation.yml @@ -12,6 +12,26 @@ on: pull_request: workflow_dispatch: +env: + # Pin pip instead of taking whatever released most recently. Every job used to + # run `pip install --upgrade pip`, so each run resolved dependencies with a + # different, unannounced resolver: CI was non-reproducible by construction, + # and a pip release could break the build with no change in this repository. + # + # This does NOT prevent transient index failures, and should not be mistaken + # for a fix for one. A case is on record: the pull_request run for PR #14 + # failed with `no matching distributions available for your environment: + # mdurl` while the identical commit had passed 15 minutes earlier on push, + # passed on two other platforms in that same run, and passed again on re-run + # with no code change. mdurl is a pure-Python py3-none-any wheel, so nothing + # about the environment made it uninstallable -- that was the index, not the + # resolver. The right response there is a re-run. The right response to an + # unpinned toolchain is this pin. + # + # Bump deliberately, as its own commit, so a resolver change lands where it + # can be attributed instead of appearing inside an unrelated PR. + PIP_VERSION: "26.2.1" + jobs: mutation-score: name: mutation score (rdflib) @@ -26,7 +46,7 @@ jobs: python-version: "3.11" - name: Install test dependencies - run: python -m pip install --upgrade pip rdflib + run: python -m pip install "pip==${{ env.PIP_VERSION }}" rdflib - name: Run the mutation harness env: