diff --git a/CHANGELOG.md b/CHANGELOG.md index fe860b6..d21dae2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to this project are documented in this file. +## Unreleased + +### Added +- **The final graph QC now asserts no field in the emitted NDJSON is null or empty, and that every node carries a name.** `build-kg --qc`'s stage-7 study pass (in the spirit of `studyKGtsvs.pl`) gained two assertions. `empty-or-null-values`: any field in the nodes or edges file whose value is JSON `null`, a string that strips to empty, or an empty container — checked recursively, so a null or blank nested inside an `attributes` list counts — fails the build. The check is deliberately stricter than the NDJSON writer's `strip_nulls` (`rust/src/json.rs`), which scrubs dict entries at every depth but passes array scalars (`["x", ""]`) and emptied nested objects (`[{}]`) through verbatim; the study stage now asserts the stronger contract — no null or empty value anywhere — so the first such shape to reach an emitted file fails the build loudly instead of shipping silently. Null-like *strings* (`NA`/`NaN`/`null`/`none`) are also dropped by the writer but are neither null nor empty, and are deliberately not flagged; the `original_*` whitespace exemption does not extend to emptiness. `unnamed-nodes`: a node record whose `name` key is missing, `null`, or strips to empty fails the build — the missing-key case is what pipeline output surfaces, since `strip_nulls` deletes empty and null-like names before the file is written. Both report offenders per field or per node id, capped at 10 examples like the other assertions. + ## 12.1.0 - 2026-08-18 ### Added diff --git a/docs/cli.md b/docs/cli.md index d311c90..6644c00 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -146,7 +146,7 @@ The positional `GRAPH-CONFIGURATION-FILE` (also `--configuration-file`, `-f`) is | --- | --- | --- | --- | --- | | `GRAPH-CONFIGURATION-FILE` (`--configuration-file`, `-f`) | Path | Yes | — | Graph YAML | | `--release`, `-r` | Flag | No | `False` | Emit a slim, significant-only graph (drops `biolink:not_significant` edges before resolution) | -| `--qc`, `-q` | Flag | No | `False` | Audit resolved mappings (exact → fuzzy → abbreviation → SapBERT) so low-confidence edges are flagged; requires the `[qc]` extra, checked before the build starts. Also runs a final study stage that asserts over the emitted NDJSON — no duplicate node ids, no undeclared or isolated nodes, no malformed lines or stray whitespace (verbatim `original_*` fields excepted, since they are faithful source copies) — and fails the build (non-zero exit) on any violation | +| `--qc`, `-q` | Flag | No | `False` | Audit resolved mappings (exact → fuzzy → abbreviation → SapBERT) so low-confidence edges are flagged; requires the `[qc]` extra, checked before the build starts. Also runs a final study stage that asserts over the emitted NDJSON — no duplicate node ids, no nodes with no name or an empty name, no undeclared or isolated nodes, no malformed lines, no null or empty values in any field (checked recursively), and no stray whitespace — verbatim `original_*` fields excepted from the whitespace check, since they are faithful source copies — and fails the build (non-zero exit) on any violation | | `--log`, `-l` | Flag | No | `False` | Enable verbose per-section logging | | `--head`, `-hd` | Flag | No | `False` | Fast output-shape preview: ≤5 random rows/section, cached to `.head.parquet`, never clobbers a full build | | `--threads`, `-t` | int | No | `None` (auto) | Worker threads for the parallel fullmap reads behind entity resolution. Readers fan out across the 16 record-shard files, and values above the (non-empty) shard count further split the busiest shards' term buckets across more concurrent readers of the same shard — redb readers share-lock, so they never contend with each other. Unset keeps the auto behavior: large batches (≥ 1024 terms) fan out, small ones stay serial. Results are identical at any worker count | diff --git a/src/tablassert/cli.py b/src/tablassert/cli.py index b8d4b3c..6a962bf 100644 --- a/src/tablassert/cli.py +++ b/src/tablassert/cli.py @@ -665,7 +665,8 @@ def build_kg( "tablassert[qc]"``); it is checked before the build starts, because the audit stage runs LAST and a missing extra would otherwise surface only after entity resolution has finished. It also runs a final study stage that asserts over the emitted NDJSON - -- no duplicate node ids, no undeclared or isolated nodes, no malformed lines or + -- no duplicate node ids, no nodes with no name or an empty name, no undeclared or + isolated nodes, no malformed lines, no null or empty values in any field, and no stray whitespace -- and fails the build (non-zero exit) when any assertion is violated. """ diff --git a/src/tablassert/study.py b/src/tablassert/study.py index 84bd768..1b7d823 100644 --- a/src/tablassert/study.py +++ b/src/tablassert/study.py @@ -14,7 +14,9 @@ "file-missing": "file not found", "malformed-lines": "empty or malformed JSON lines", "whitespace-values": "values with leading/trailing whitespace", + "empty-or-null-values": "null or empty values", "duplicate-node-ids": "duplicate node ids", + "unnamed-nodes": "nodes with no name or an empty name", "undeclared-nodes": "nodes referenced by edges but not declared in the nodes file", "isolated-nodes": "declared nodes participating in no edge", } @@ -44,24 +46,50 @@ class _FileScan: ids: set[str] duplicate_ids: Counter[str] whitespace: Counter[str] + empty_null: Counter[str] + unnamed: Counter[str] malformed: int missing: bool path: Path +def _is_empty_or_null(value: object) -> bool: + """True for JSON null, strings that strip to empty, and empty containers (recursively). + + Deliberately STRICTER than the writer's strip_nulls (``rust/src/json.rs``): the + writer scrubs dict entries at every depth but passes array scalars + (``["x", ""]``) and emptied nested objects (``[{}]``) through verbatim, and + the study asserts the stronger contract -- no null or empty value anywhere + -- so the first such shape to reach an emitted file fails loudly instead of + shipping. Null-like *strings* (``"NA"``, ``"NaN"``, ``"null"``, ``"none"``) + are also dropped by the writer but are neither null nor empty, so they are + deliberately not flagged. + """ + if value is None: + return True + if isinstance(value, str): + return not value.strip() + if isinstance(value, list): + return not value or any(_is_empty_or_null(item) for item in value) + if isinstance(value, dict): + return not value or any(_is_empty_or_null(item) for item in value.values()) + return False + + def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: """Stream one NDJSON file, collecting the facts the study assertions need. Args: path: Path to a ``.nodes.ndjson`` or ``.edges.ndjson`` file. edge: ``True`` to collect referenced ids from ``subject``/``object``; - ``False`` to collect declared node ``id``s and track duplicates. + ``False`` to collect declared node ``id``s and track duplicates and + nodes with no name or an empty name. Returns: A :class:`_FileScan`; ``missing`` is set (and nothing else) when the file does not exist, so a typo'd path can never read as a clean pass. """ - scan: _FileScan = _FileScan(set(), Counter(), Counter(), 0, not path.is_file(), path) + scan: _FileScan = _FileScan(set(), Counter(), Counter(), Counter(), Counter(), 0, not path.is_file(), path) if scan.missing: return scan with path.open(encoding="utf-8") as handle: @@ -84,10 +112,15 @@ def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: # faithful to the source, not a defect to flag. Safe only because every # `original_` producer is such a verbatim copy; a future slot that merely # starts with `original_` would escape this check and must be revisited here. - if key.startswith("original_"): - continue - if isinstance(value, str) and value != value.strip(): + if not key.startswith("original_") and isinstance(value, str) and value != value.strip(): scan.whitespace[key] += 1 + # The original_* exemption covers whitespace only: the writer drops + # null and strip-empty strings everywhere -- verbatim copies included + # -- so an empty original_* value is never legitimate output. A + # whitespace-only value intentionally trips this check; on non-original + # fields it also trips the whitespace check above. + if _is_empty_or_null(value): + scan.empty_null[key] += 1 if edge: for role in ("subject", "object"): ident: object = record.get(role) @@ -101,17 +134,32 @@ def _scan_ndjson(path: Path, *, edge: bool) -> _FileScan: if ident in scan.ids: scan.duplicate_ids[ident] += 1 scan.ids.add(ident) + # A node with no name is unusable downstream: KGX consumers key display + # and merging off `name`. Flag a missing key, a null, or a string that + # strips to empty. On pipeline output the writer's strip_nulls + # (rust/src/json.rs is_bad_token) has already removed empty and null-like + # names ("NA"/"NaN"/"null"/"none"), so the missing-key branch is what + # fires there; the other branches guard hand-crafted files. Non-string, + # non-null names pass -- no writer emits them. Offenders are keyed by + # node id (or `` when the record has no string id) for the + # examples list. + node_id: str = ident if isinstance(ident, str) else "" + name: object = record.get("name") + if name is None or (isinstance(name, str) and not name.strip()): + scan.unnamed[node_id] += 1 return scan def study_kgx(nodes_path: Path, edges_path: Path, *, example_limit: int = 10) -> list[StudyViolation]: """Assert over the final KGX NDJSON files, in the spirit of studyKGtsvs.pl. - Streams both files once each and checks: duplicate node ids, nodes referenced - by edges but never declared (``undeclared``), declared nodes participating in - no edge (``isolated``), empty/malformed lines, and string values carrying - leading/trailing whitespace. Every check is an assertion -- the caller decides - whether violations fail the build. + Streams both files once each and checks: duplicate node ids, nodes with no + name or an empty name, nodes referenced by edges but never declared + (``undeclared``), declared nodes participating in no edge (``isolated``), + empty/malformed lines, string values carrying leading/trailing whitespace, + and null or empty values in any field (a stronger contract than the writer's + strip_nulls). Every check is an assertion -- the caller decides whether + violations fail the build. Args: nodes_path: Path to ``_.nodes.ndjson``. @@ -134,9 +182,15 @@ def study_kgx(nodes_path: Path, edges_path: Path, *, example_limit: int = 10) -> if scan.whitespace: examples: list[str] = [f"{field_name} ({n})" for field_name, n in scan.whitespace.most_common(example_limit)] violations.append(StudyViolation("whitespace-values", label, sum(scan.whitespace.values()), examples)) + if scan.empty_null: + examples = [f"{field_name} ({n})" for field_name, n in scan.empty_null.most_common(example_limit)] + violations.append(StudyViolation("empty-or-null-values", label, sum(scan.empty_null.values()), examples)) if nodes.duplicate_ids: examples = [ident for ident, _ in nodes.duplicate_ids.most_common(example_limit)] violations.append(StudyViolation("duplicate-node-ids", "nodes", len(nodes.duplicate_ids), examples)) + if nodes.unnamed: + examples = [ident for ident, _ in nodes.unnamed.most_common(example_limit)] + violations.append(StudyViolation("unnamed-nodes", "nodes", sum(nodes.unnamed.values()), examples)) if not nodes.missing and not edges.missing: undeclared: list[str] = sorted(edges.ids - nodes.ids) if undeclared: diff --git a/tests/test_study.py b/tests/test_study.py index abf4299..c57322f 100644 --- a/tests/test_study.py +++ b/tests/test_study.py @@ -21,7 +21,7 @@ def _records(*records: dict[str, Any]) -> list[str]: def _clean(tmp_path: Path) -> tuple[Path, Path]: - nodes: Path = _write_ndjson(tmp_path / "g_1.nodes.ndjson", _records({"id": "HGNC:5"}, {"id": "HGNC:6"})) + nodes: Path = _write_ndjson(tmp_path / "g_1.nodes.ndjson", _records({"id": "HGNC:5", "name": "insulin"}, {"id": "HGNC:6", "name": "IGF1"})) edges: Path = _write_ndjson(tmp_path / "g_1.edges.ndjson", _records({"subject": "HGNC:5", "object": "HGNC:6", "predicate": "biolink:related_to"})) return nodes, edges @@ -38,7 +38,7 @@ def test_clean_files_pass(tmp_path: Path) -> None: def test_duplicate_node_ids(tmp_path: Path) -> None: """A node id appearing on more than one line fails the duplicate assertion.""" - nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "HGNC:5"}, {"id": "HGNC:5", "name": "different"})) + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "HGNC:5", "name": "a"}, {"id": "HGNC:5", "name": "different"})) _, edges = _clean(tmp_path) checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) assert checks["duplicate-node-ids"].count == 1 @@ -47,16 +47,158 @@ def test_duplicate_node_ids(tmp_path: Path) -> None: def test_duplicate_examples_capped(tmp_path: Path) -> None: """Duplicate-id examples are capped while the count stays exact.""" - nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records(*({"id": f"X:{i}"} for i in range(30)), *({"id": f"X:{i}"} for i in range(30)))) + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", + _records(*({"id": f"X:{i}", "name": f"n{i}"} for i in range(30)), *({"id": f"X:{i}", "name": f"n{i}"} for i in range(30))), + ) _, edges = _clean(tmp_path) checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges, example_limit=10)) assert checks["duplicate-node-ids"].count == 30 assert len(checks["duplicate-node-ids"].examples) == 10 +def test_unnamed_nodes(tmp_path: Path) -> None: + """A missing, null, empty, or whitespace-only node name fails the unnamed assertion.""" + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", + _records( + {"id": "A:1"}, {"id": "A:2", "name": None}, {"id": "A:3", "name": ""}, {"id": "A:4", "name": " "}, {"id": "A:5", "name": "insulin"} + ), + ) + edges: Path = _write_ndjson( + tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:5"}, {"subject": "A:2", "object": "A:3"}, {"subject": "A:4", "object": "A:5"}) + ) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) + violation: study.StudyViolation = checks["unnamed-nodes"] + assert violation.label == "nodes" + assert violation.count == 4 + assert sorted(violation.examples) == ["A:1", "A:2", "A:3", "A:4"] + + +def test_unnamed_node_without_id(tmp_path: Path) -> None: + """A nameless record with no string id is still counted, keyed as ````.""" + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"name": ""}, {"id": "A:1", "name": "x"})) + edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:1"})) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) + violation: study.StudyViolation = checks["unnamed-nodes"] + assert violation.count == 1 + assert violation.examples == [""] + + +def test_unnamed_examples_capped(tmp_path: Path) -> None: + """Unnamed-node examples are capped while the count stays exact.""" + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records(*({"id": f"X:{i}"} for i in range(30)))) + _, edges = _clean(tmp_path) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges, example_limit=10)) + assert checks["unnamed-nodes"].count == 30 + assert len(checks["unnamed-nodes"].examples) == 10 + + +def test_non_string_name_is_not_unnamed(tmp_path: Path) -> None: + """A non-string, non-null name passes; the assertion targets absent/empty names only.""" + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "A:1", "name": 5}, {"id": "A:2", "name": "x"})) + edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:2"})) + assert study.study_kgx(nodes, edges) == [] + + +def test_empty_or_null_values(tmp_path: Path) -> None: + """Nulls, strip-empty strings, and empty containers fail the empty-or-null assertion. + + The check asserts the stronger no-null-or-empty-anywhere contract (stricter than + the writer's strip_nulls, which keeps array scalars verbatim), so these shapes + fail loudly instead of shipping. Nested hits (an empty attribute value inside a + list) are counted under the top-level field. + """ + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", + _records( + {"id": "A:1", "name": "a", "category": None, "provided_by": []}, + {"id": "A:2", "name": "b", "description": " "}, + {"id": "A:3", "name": "c", "synonym": ["x", ""]}, + ), + ) + edges: Path = _write_ndjson( + tmp_path / "e.ndjson", + _records( + {"subject": "A:1", "object": "A:2", "predicate": "biolink:related_to", "p_value": None}, + {"subject": "A:1", "object": "A:3", "predicate": "biolink:related_to", "attributes": [{"value": None}]}, + ), + ) + violations: list[study.StudyViolation] = [v for v in study.study_kgx(nodes, edges) if v.check == "empty-or-null-values"] + by_label: dict[str, study.StudyViolation] = {v.label: v for v in violations} + assert len(violations) == 2 + assert by_label["nodes"].count == 4 + assert sorted(by_label["nodes"].examples) == ["category (1)", "description (1)", "provided_by (1)", "synonym (1)"] + assert by_label["edges"].count == 2 + assert sorted(by_label["edges"].examples) == ["attributes (1)", "p_value (1)"] + + +def test_empty_or_null_writer_pass_through_shapes(tmp_path: Path) -> None: + """Shapes the writer passes verbatim are still flagged: array scalars, emptied dicts. + + `strip_nulls` scrubs dict entries but keeps array scalars verbatim and leaves a + nested dict that empties as `{}`; the study asserts the stricter contract, so + `["x", ""]`, `["x", null]`, `[{}]`, and `[[]]` all fail. + """ + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", + _records( + {"id": "A:1", "name": "a", "synonym": ["x", ""]}, + {"id": "A:2", "name": "b", "synonym": ["x", None]}, + {"id": "A:3", "name": "c", "attributes": [{}]}, + {"id": "A:4", "name": "d", "nested": [[]]}, + ), + ) + edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:2"}, {"subject": "A:3", "object": "A:4"})) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) + violation: study.StudyViolation = checks["empty-or-null-values"] + assert violation.count == 4 + assert sorted(violation.examples) == ["attributes (1)", "nested (1)", "synonym (2)"] + + +def test_falsy_meaningful_values_pass(tmp_path: Path) -> None: + """Zero and false are meaningful Biolink values, not absent ones. + + The writer deliberately keeps `0` and `false` (rust/src/json.rs is_present); the + empty-or-null assertion must agree or every zero-effect-size edge would fail. + """ + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "A:1", "name": "a"}, {"id": "A:2", "name": "b"})) + edges: Path = _write_ndjson( + tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:2", "predicate": "biolink:related_to", "p_value": 0, "negated": False}) + ) + assert study.study_kgx(nodes, edges) == [] + + +def test_original_fields_empty_still_flagged(tmp_path: Path) -> None: + """The `original_*` exemption covers whitespace only, not emptiness. + + The writer drops null and strip-empty strings everywhere -- verbatim copies + included -- so an empty `original_*` value is never legitimate output. + """ + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", _records({"id": "A:1", "name": "a", "original_subject": None}, {"id": "A:2", "name": "b", "original_object": ""}) + ) + edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:2"})) + checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) + violation: study.StudyViolation = checks["empty-or-null-values"] + assert violation.count == 2 + assert sorted(violation.examples) == ["original_object (1)", "original_subject (1)"] + + +def test_null_like_strings_are_not_empty_or_null(tmp_path: Path) -> None: + """Null-like strings (NA/NaN/null/none) are dropped by the writer but are not null/empty. + + The assertion targets absent values, not their string spellings; the writer's + bad-token sweep already guarantees the spellings never reach the final output. + """ + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "A:1", "name": "NA"}, {"id": "A:2", "name": "b", "source": "none"})) + edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "A:1", "object": "A:2"})) + assert study.study_kgx(nodes, edges) == [] + + def test_undeclared_nodes(tmp_path: Path) -> None: """Edge subject/object ids missing from the nodes file fail the undeclared assertion.""" - nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "HGNC:5"})) + nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "HGNC:5", "name": "a"})) edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "HGNC:5", "object": "HGNC:6"}, {"subject": "HGNC:7", "object": "HGNC:5"})) checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) assert checks["undeclared-nodes"].count == 2 @@ -65,7 +207,9 @@ def test_undeclared_nodes(tmp_path: Path) -> None: def test_isolated_nodes(tmp_path: Path) -> None: """Declared nodes participating in no edge fail the isolated assertion.""" - nodes: Path = _write_ndjson(tmp_path / "n.ndjson", _records({"id": "HGNC:5"}, {"id": "HGNC:6"}, {"id": "HGNC:7"})) + nodes: Path = _write_ndjson( + tmp_path / "n.ndjson", _records({"id": "HGNC:5", "name": "a"}, {"id": "HGNC:6", "name": "b"}, {"id": "HGNC:7", "name": "c"}) + ) edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "HGNC:5", "object": "HGNC:6"})) checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges)) assert checks["isolated-nodes"].count == 1 @@ -100,7 +244,7 @@ def test_whitespace_allowed_in_original_fields(tmp_path: Path) -> None: even as the same record carries genuinely padded values on other keys. """ nodes: Path = _write_ndjson( - tmp_path / "n.ndjson", _records({"id": "HGNC:5", "original_name": " padded source ", "name": " padded"}, {"id": "HGNC:6"}) + tmp_path / "n.ndjson", _records({"id": "HGNC:5", "original_name": " padded source ", "name": " padded"}, {"id": "HGNC:6", "name": "b"}) ) edges: Path = _write_ndjson(tmp_path / "e.ndjson", _records({"subject": "HGNC:5", "object": "HGNC:6", "original_subject": " raw gene "})) checks: dict[str, study.StudyViolation] = _checks(study.study_kgx(nodes, edges))