diff --git a/changelog.md b/changelog.md index a2621a0..cf5a184 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,103 @@ # Changelog +## 2026-09-05 — Fix a unit test that inverted under CI + +`shell+pipeline-unit` failed on all three OS matrix legs of the `dev` -> `main` +merge (ubuntu-latest, ubuntu-22.04, macos-latest) while passing on developer +machines. One test, three red checks. + +### Fixed + +- `test_validation_forwards_progress_sidecar_and_quiet_flag` asserted that + `run_validation_mode` forwards `--progress-path` to the container. It does — + but `progress_events_enabled()` returns `False` whenever `CI` is set, and + GitHub Actions sets `CI=true`, so the sidecar is deliberately not requested + there. The test pinned `_PROGRESS_ALLOWED`, `_PROGRESS_EVENTS_ALLOWED` and + `_QUIET` but not the environment, so it tested a different code path in CI + than the one it was written for. Reproducible locally with + `CI=true python -m unittest discover -s test -p "test_*_unit.py"`. + + The tool's behaviour is correct and unchanged: the sidecar exists only to + drive the terminal display and `ProgressSession.__exit__` unlinks it, so + suppressing it where nothing renders costs nothing. + +### Changed + +- `stable_progress_env()` replaces the `{"VCF_RDFIZER_NO_PROGRESS": "", "CI": ""}` + dict that two sibling progress tests already carried inline. The third test + omitted it, which is how this happened; a named helper with the reason in its + docstring makes the requirement visible to the next test author. +- The validation-command setup is factored into + `_run_validation_mode_capturing_commands()`, shared by both sides of the + contract. + +### Added + +- `test_validation_omits_progress_sidecar_under_ci` pins the CI behaviour that + was previously only implicit: no `--progress-path` is requested, and `--quiet` + still is. Verified as a real guard by removing the `CI` check from + `progress_events_enabled()` and confirming the test fails. + +## 2026-09-05 — Vocabulary alignment for the condensed representation (upstream) + +The two open items in [`docs/roadmap.md`](docs/roadmap.md) under "Blocking +publication" are closed. Both were work in the +[vocabulary repository](https://github.com/ecrum19/VCF-RDFizer-vocabulary), +released there as **v1.1.0**; **no change was needed in this repository**, and +this entry records what the conversion may now rely on. + +### Condensed graphs are ontology-backed + +The conversion emitted 17 terms the vocabulary did not define — `SampleSet`, +`VCFSample`, `CohortCallMatrix`, `FormatValueVector`, `representationProfile`, +`sampleIndex`, `encodedValues` and the rest — so a condensed graph could not be +dereferenced or meaningfully SHACL-checked. All 17 are now defined, with two +superclasses (`RepresentationProfile`, `VectorEncoding`) giving the enumerations +a range, plus SHACL shapes for the profile and a worked example that is the +expanded example's own record in the other profile. + +The condensed shapes constrain what positional decoding depends on: exactly one +`sampleIndex` per sample, a sample set on every matrix, and an encoding and +FORMAT declaration on every vector. Ten targeted corruptions of a condensed +graph are each reported as a violation, so the shapes are load-bearing. + +`--shacl-shapes` therefore now covers `--sample-representation condensed`, which +it previously could not. + +### The missing-value contradiction is resolved + +`vcfr:missingValuePolicy` required `"."^^vcfr:Null` for a missing token while +`VCFRecordShape` constrained `vcfr:alt` to `sh:datatype xsd:string`, so a record +with `ALT=.` could satisfy neither and the tool could not be conformant on any +VCF containing one. + +The fix draws the boundary the policy was missing rather than relaxing every +field: ALT and ID admit the missing token in VCF 4.5, so their shapes now accept +`xsd:string` or `vcfr:Null` as `qual` already did; CHROM, POS and REF are +required and have no missing form, so their shapes stay exact — a `.` there is +malformed input and should still be reported. The policy also now states that a +missing value inside a `FormatValueVector` stays a `.` character at its sample's +position, rather than becoming a typed literal that would break alignment. + +The conversion already followed the policy, so it needs no change. Reproduced +against the 1.0.1 shapes before the fix and confirmed gone after. + +### Caveat + +The terms only dereference for a third-party consumer once vocabulary v1.1.0 is +published to `https://w3id.org/vcf-rdfizer/vocab#`. Until then the definitions +exist but are not yet live, which is why the docs below say "pending release" +rather than "done". + +### Documentation + +- [`docs/vcf-coverage.md`](docs/vcf-coverage.md) — the "Vocabulary alignment" + section now records both gaps as closed, with what was verified. +- [`docs/roadmap.md`](docs/roadmap.md) — both publication blockers marked done. +- [`docs/limitations.md`](docs/limitations.md), + [`docs/conversion.md`](docs/conversion.md), + [`docs/validation.md`](docs/validation.md) updated to match. + ## 2026-09-04 — Multi-engine validation, native HDT/COTTAS querying, and benchmarking Validation was one engine against one N-Triples file. It is now up to four diff --git a/docs/conversion.md b/docs/conversion.md index 31d9d4c..a691188 100644 --- a/docs/conversion.md +++ b/docs/conversion.md @@ -204,11 +204,14 @@ The vocabulary's `vcfr:missingValuePolicy` says a missing token should be reports a plain `"."` literal as an anomaly, and `--strict-conformance` promotes that from a report to a failure. -This policy currently **conflicts with the published SHACL shapes** for -`ref`/`alt`/`chrom`, which constrain those to `sh:datatype xsd:string`. A record -with `ALT=.` therefore cannot satisfy both. The conflict is documented in -[`vcf-coverage.md`](vcf-coverage.md#an-open-conflict-inside-the-vocabulary) and -needs a decision in the vocabulary repository, not here. +This policy **conflicted with the published SHACL shapes** for `alt`, which +constrained it to `sh:datatype xsd:string` so that a record with `ALT=.` could +satisfy neither. Vocabulary v1.1.0 resolves it by drawing the boundary the +policy was missing: ALT and ID admit the missing token in VCF 4.5 and their +shapes now accept `xsd:string` or `vcfr:Null`, while CHROM, POS and REF are +required, have no missing form, and keep an exact datatype. The conversion +already followed the policy and is unchanged. See +[`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). ## 8. What conversion does *not* do diff --git a/docs/limitations.md b/docs/limitations.md index cf0dcb3..9af0d37 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -164,17 +164,19 @@ argv is recorded in every report so a future divergence is diagnosable. ## 7. Vocabulary -**Condensed graphs are not ontology-backed.** The tool emits 17 terms that -`https://w3id.org/vcf-rdfizer/vocab#` does not define, all belonging to the -condensed representation. Dereferencing any of them returns nothing. This is the -one remaining publication blocker, and the work is in the vocabulary repository. - -**The published SHACL shapes contradict the missing-value policy.** -`vcfr:missingValuePolicy` says a missing token should be `"."^^vcfr:Null`, while -`VCFRecordShape` constrains `vcfr:alt` to `sh:datatype xsd:string`. A record with -`ALT=.` cannot satisfy both. The conversion follows the missing-value policy. -The vocabulary needs a decision; see -[`vcf-coverage.md`](vcf-coverage.md#an-open-conflict-inside-the-vocabulary). +**Condensed vocabulary coverage lands with the vocabulary's next release.** The +tool emits 17 condensed-representation terms that `vcf-rdfizer/vocab#` did not +define; all are defined in the vocabulary repository as of v1.1.0, together with +SHACL shapes for the profile. Until that version is published to +`https://w3id.org/vcf-rdfizer/vocab#`, those terms still do not dereference for +a third-party consumer. + +**The SHACL / missing-value contradiction is resolved** in the same release. +`vcfr:alt` and `vcfr:recordId` now accept `xsd:string` or `vcfr:Null`, matching +where VCF 4.5 actually permits the missing token, while CHROM, POS and REF stay +exact because they have no missing form. The conversion already followed the +policy and needs no change. See +[`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). **Condensed mode has no query-time decoder.** Reconstructing sample *i*'s value means splitting a tab-separated literal, which SPARQL cannot do portably. The diff --git a/docs/roadmap.md b/docs/roadmap.md index fdf3c14..0bf7b95 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -11,28 +11,32 @@ block something else, not by effort. ## Blocking publication -### 1. Vocabulary coverage for the condensed representation +### 1. Vocabulary coverage for the condensed representation — **done, pending release** -The conversion emits 17 terms that `https://w3id.org/vcf-rdfizer/vocab#` does +The conversion emitted 17 terms that `https://w3id.org/vcf-rdfizer/vocab#` did not define — `CohortCallMatrix`, `FormatValueVector`, `SampleSet`, `VCFSample`, -`VCFTextVector`, `representationProfile`, `sampleIndex` and the rest. They do -not dereference, so **condensed graphs are not ontology-backed**. - -The work is in the vocabulary repository, not here. Until it lands, the -condensed representation is usable but not citable as linked data. -Full list: [`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). - -### 2. The SHACL / missing-value contradiction - -`vcfr:missingValuePolicy` requires `"."^^vcfr:Null` for a missing token; -`VCFRecordShape` constrains `vcfr:alt` to `sh:datatype xsd:string`. A record with -`ALT=.` cannot satisfy both, so it is impossible for the tool to be conformant. - -The fix is a decision in the vocabulary: either relax the `alt`/`ref`/`chrom` -shapes to `sh:or([xsd:string] [vcfr:Null])` — as the `qual` shape already does — -or drop the missing-value policy for those fields. The conversion currently -follows the policy. -[Detail](vcf-coverage.md#an-open-conflict-inside-the-vocabulary). +`VCFTextVector`, `representationProfile`, `sampleIndex` and the rest — so +condensed graphs were not ontology-backed. + +All 17 are now defined in the vocabulary repository (v1.1.0), with two +superclasses giving the enumerations a range, SHACL shapes for the condensed +profile, and a worked example. What remains is publication: the terms only +dereference for a third-party consumer once v1.1.0 is deployed to the w3id +namespace. Nothing further is required in this repository. +Detail: [`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). + +### 2. The SHACL / missing-value contradiction — **resolved** + +`vcfr:missingValuePolicy` required `"."^^vcfr:Null` for a missing token while +`VCFRecordShape` constrained `vcfr:alt` to `sh:datatype xsd:string`, so a record +with `ALT=.` could satisfy neither and the tool could not be conformant. + +Vocabulary v1.1.0 resolves it by stating where VCF actually permits the token: +`alt` and `recordId` now accept `xsd:string` or `vcfr:Null`, as `qual` already +did, while `chrom`, `pos` and `ref` keep an exact datatype because they are +required and have no missing form. The conversion already followed the policy, +so no change was needed here. +Detail: [`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). ## Known defects diff --git a/docs/validation.md b/docs/validation.md index 5b45242..e1fb75e 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -439,11 +439,12 @@ report-only and the run rests on the aggregate comparisons alone. That is correct behaviour, not a bug, but it means a custom mapping is validated more weakly than the default one. -**Condensed graphs are not ontology-backed.** The condensed representation uses -17 terms the published vocabulary does not define, so SHACL cannot meaningfully -check it and dereferencing those terms returns nothing. The list is in -[`vcf-coverage.md`](vcf-coverage.md); closing it is work in the vocabulary -repository. +**Condensed SHACL coverage depends on the vocabulary's next release.** The +condensed representation used 17 terms the published vocabulary did not define, +so SHACL could not meaningfully check it. All 17 are defined in vocabulary +v1.1.0, which also ships condensed shapes, but a third-party consumer only sees +them once that version is published. See +[`vcf-coverage.md`](vcf-coverage.md#vocabulary-alignment). **A high mutation score is a lower bound on blindness, not a proof.** It says "almost every corruption we thought to write down is caught". Corruptions nobody diff --git a/docs/vcf-coverage.md b/docs/vcf-coverage.md index 37d1a6d..12d34f2 100644 --- a/docs/vcf-coverage.md +++ b/docs/vcf-coverage.md @@ -109,8 +109,15 @@ a subclass would put a term in the graph that the vocabulary does not define. ## Vocabulary alignment -The tool emits 17 terms that `https://w3id.org/vcf-rdfizer/vocab#` does **not** -define — all belonging to the condensed representation: +Both gaps recorded here have been closed in the vocabulary repository +(`https://github.com/ecrum19/VCF-RDFizer-vocabulary`, v1.1.0). They are kept +below because the fix is only live for consumers once that version is published +to `https://w3id.org/vcf-rdfizer/vocab#`. + +### Condensed terms are now defined + +The conversion emitted 17 terms the vocabulary did not define, all belonging to +the condensed representation, so **condensed graphs were not ontology-backed**: `CohortCallMatrix`, `CondensedRepresentation`, `ExpandedRepresentation`, `FormatValueVector`, `SampleSet`, `VCFSample`, `VCFTextVector`, @@ -118,27 +125,41 @@ define — all belonging to the condensed representation: `hasSample`, `hasSampleSet`, `representationProfile`, `sampleIndex`, `sampleName`, `valueEncoding` -Dereferencing any of these returns nothing, so **condensed graphs are not -ontology-backed**. This is the one remaining publication blocker and is work in -the vocabulary repository, not here. - -### An open conflict inside the vocabulary - -Running SHACL surfaced a contradiction between two parts of the published -vocabulary that the tool cannot satisfy simultaneously: - -- `vcfr:missingValuePolicy` says a missing token SHOULD be `"."^^vcfr:Null`. -- `VCFRecordShape` constrains `vcfr:alt` to `sh:datatype xsd:string`. - -A record with `ALT=.` therefore violates the shape no matter which rule the -conversion follows. The tool currently follows the missing-value policy. This -needs a decision in the vocabulary: either relax the `alt`/`ref`/`chrom` shapes -to `sh:or([xsd:string] [vcfr:Null])`, as the `qual` shape already does, or drop -the missing-value policy for those fields. - -Two shape violations found the same way have already been fixed here: QUAL is -now `xsd:decimal`/`vcfr:Null` rather than a plain literal, and `##fileDate` is -typed `xsd:date` when its form allows. +All 17 are now defined, along with two superclasses (`RepresentationProfile`, +`VectorEncoding`) that give the two enumerations a range. The vocabulary also +ships SHACL shapes for the condensed profile, constraining what positional +decoding depends on: exactly one `sampleIndex` per sample, a sample set on every +matrix, and an encoding and FORMAT declaration on every vector. + +Verified: the project's own expanded and condensed fixture graphs conform +against the new shapes, and ten targeted corruptions of a condensed graph +(dropped index, duplicated index, untyped index, missing encoding, missing +declaration, missing values, missing sample set, unknown profile, empty sample +set, missing name) are each reported as a violation. + +### The SHACL / missing-value contradiction is resolved + +Running SHACL had surfaced a contradiction between two parts of the published +vocabulary that the tool could not satisfy simultaneously: + +- `vcfr:missingValuePolicy` said a missing token SHOULD be `"."^^vcfr:Null`. +- `VCFRecordShape` constrained `vcfr:alt` to `sh:datatype xsd:string`. + +A record with `ALT=.` therefore violated the shape no matter which rule the +conversion followed. The resolution draws the boundary the policy was missing +rather than relaxing every field: **ALT and ID admit the missing token in VCF +4.5, so their shapes now accept `xsd:string` or `vcfr:Null`**, as `qual` already +did. **CHROM, POS and REF are required and have no missing form**, so their +shapes stay exact — a `.` in one of them is malformed input, and the shape +should say so. The policy text now states that boundary explicitly, and adds +that a missing value inside a `FormatValueVector` stays a `.` character at its +sample's position rather than becoming a typed literal. + +The tool needs no change: it already followed the missing-value policy. + +Two shape violations found the same way had already been fixed here: QUAL is +`xsd:decimal`/`vcfr:Null` rather than a plain literal, and `##fileDate` is typed +`xsd:date` when its form allows. --- diff --git a/test/test_vcf_rdfizer_unit.py b/test/test_vcf_rdfizer_unit.py index 5dea3b7..9a7a68f 100644 --- a/test/test_vcf_rdfizer_unit.py +++ b/test/test_vcf_rdfizer_unit.py @@ -271,6 +271,23 @@ def latest_metrics_run_dir(metrics_root: Path) -> Path: return run_dirs[-1][1] + +def stable_progress_env(): + """Pin the ambient progress opt-outs for one test. + + ``progress_events_enabled()`` and ``progress_ui_enabled()`` both consult + ``CI`` and ``VCF_RDFIZER_NO_PROGRESS``, so any assertion about progress + behaviour written on a developer machine silently inverts under GitHub + Actions, which sets ``CI=true``. Every test that asserts on progress has to + pin both variables; use this rather than repeating the dict, and see + ``test_validation_omits_progress_sidecar_under_ci`` for the other side of + the contract. + """ + return mock.patch.dict( + os.environ, {"VCF_RDFIZER_NO_PROGRESS": "", "CI": ""}, clear=False + ) + + class WrapperUnitTests(VerboseTestCase): def test_metrics_directory_uses_recognizable_input_label(self): """Metrics runs keep the full input stem alongside their timestamp.""" @@ -323,11 +340,7 @@ def test_plain_progress_is_shown_when_rich_or_a_tty_is_unavailable(self): previous_progress_setting = vcf_rdfizer._PROGRESS_ALLOWED try: vcf_rdfizer._PROGRESS_ALLOWED = True - with tempfile.TemporaryDirectory() as td, mock.patch.dict( - os.environ, - {"VCF_RDFIZER_NO_PROGRESS": "", "CI": ""}, - clear=False, - ), mock.patch.object(vcf_rdfizer, "Progress", None), mock.patch.object( + with tempfile.TemporaryDirectory() as td, stable_progress_env(), mock.patch.object(vcf_rdfizer, "Progress", None), mock.patch.object( vcf_rdfizer, "Console", None ): progress_path = Path(td) / "partitioned.jsonl" @@ -364,11 +377,7 @@ def test_quiet_progress_consumes_sidecar_without_terminal_output(self): vcf_rdfizer._PROGRESS_ALLOWED = False vcf_rdfizer._PROGRESS_EVENTS_ALLOWED = True vcf_rdfizer._QUIET = True - with tempfile.TemporaryDirectory() as td, mock.patch.dict( - os.environ, - {"VCF_RDFIZER_NO_PROGRESS": "", "CI": ""}, - clear=False, - ), mock.patch.object(vcf_rdfizer, "Progress", None), mock.patch.object( + with tempfile.TemporaryDirectory() as td, stable_progress_env(), mock.patch.object(vcf_rdfizer, "Progress", None), mock.patch.object( vcf_rdfizer, "Console", None ): progress_path = Path(td) / "validation.jsonl" @@ -395,8 +404,13 @@ def test_quiet_progress_consumes_sidecar_without_terminal_output(self): vcf_rdfizer._PROGRESS_EVENTS_ALLOWED = previous_events vcf_rdfizer._QUIET = previous_quiet - def test_validation_forwards_progress_sidecar_and_quiet_flag(self): - """Validation uses the shared sidecar protocol and propagates --quiet.""" + def _run_validation_mode_capturing_commands(self): + """Invoke ``run_validation_mode`` with Docker stubbed; return (rc, argv list). + + The sidecar decision is made inside the call, so a caller that cares + about ``CI`` must pin the environment *around* this helper rather than + before it. + """ previous_allowed = vcf_rdfizer._PROGRESS_ALLOWED previous_events = vcf_rdfizer._PROGRESS_EVENTS_ALLOWED previous_quiet = vcf_rdfizer._QUIET @@ -418,9 +432,9 @@ def fake_run(cmd, cwd=None, env=None): commands.append(cmd) return 0 - with mock.patch.object(vcf_rdfizer, "run", side_effect=fake_run), redirect_stdout( - StringIO() - ): + with mock.patch.object( + vcf_rdfizer, "run", side_effect=fake_run + ), redirect_stdout(StringIO()): rc = vcf_rdfizer.run_validation_mode( vcf_path=vcf_path, rdf_path=rdf_path, @@ -434,20 +448,42 @@ def fake_run(cmd, cwd=None, env=None): filter_oracle="auto", wrapper_log_path=tmp_path / "wrapper.log", ) - - self.assertEqual(rc, 0) - self.assertEqual(len(commands), 1) - command = commands[0] - self.assertIn("--progress-path", command) - self.assertIn( - "/data/metrics/.progress/validation-sample.jsonl", command - ) - self.assertIn("--quiet", command) + return rc, commands finally: vcf_rdfizer._PROGRESS_ALLOWED = previous_allowed vcf_rdfizer._PROGRESS_EVENTS_ALLOWED = previous_events vcf_rdfizer._QUIET = previous_quiet + def test_validation_forwards_progress_sidecar_and_quiet_flag(self): + """Validation uses the shared sidecar protocol and propagates --quiet.""" + with stable_progress_env(): + rc, commands = self._run_validation_mode_capturing_commands() + + self.assertEqual(rc, 0) + self.assertEqual(len(commands), 1) + command = commands[0] + self.assertIn("--progress-path", command) + self.assertIn("/data/metrics/.progress/validation-sample.jsonl", command) + self.assertIn("--quiet", command) + + def test_validation_omits_progress_sidecar_under_ci(self): + """CI keeps captured logs clean: no sidecar is requested, --quiet still is.""" + with mock.patch.dict( + os.environ, + {"VCF_RDFIZER_NO_PROGRESS": "", "CI": "true"}, + clear=False, + ): + rc, commands = self._run_validation_mode_capturing_commands() + + self.assertEqual(rc, 0) + self.assertEqual(len(commands), 1) + command = commands[0] + # No consumer renders the sidecar under CI, and ProgressSession unlinks + # it on exit, so the wrapper does not ask the container to write one. + self.assertNotIn("--progress-path", command) + # --quiet is independent of the sidecar decision and must survive it. + self.assertIn("--quiet", command) + def test_validation_runner_emits_query_progress_in_quiet_mode(self): """The container validator still writes lifecycle events when stdout is quiet.""" validator = load_validation_runner_module()