From ceab451d408a10d822d7b2c4da1c785787339bb7 Mon Sep 17 00:00:00 2001 From: SimonTaurus Date: Sun, 23 Aug 2026 14:37:22 +0200 Subject: [PATCH 1/2] fix(validation): classify every level, and cite the coverage rule - severity() treated anything not a MUST as advice, so a level added upstream was silently demoted; unrecognised levels now raise - _ADVICE_LEVELS is listed rather than inferred, and a test ties both sets to the level enum of every tracked version - context.coverage cites OOLD-SCH-21d7, so its warning is the catalogue's severity rather than one chosen in code --- src/oold/validation/check_registry.py | 29 ++++++++++++++--- tests/test_validation/test_check_registry.py | 33 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/oold/validation/check_registry.py b/src/oold/validation/check_registry.py index 649f2e3..2517de1 100644 --- a/src/oold/validation/check_registry.py +++ b/src/oold/validation/check_registry.py @@ -103,6 +103,13 @@ def keyword(self, name: str) -> Any: #: RFC 2119 levels that make a violation a failure. Everything else is advice, so it warns. _MUST_LEVELS = frozenset({"MUST", "MUST NOT", "SHALL", "SHALL NOT", "REQUIRED"}) +#: Levels that are advice. Listed rather than inferred from "not in _MUST_LEVELS", so that a level +#: the specification adds later is neither silently treated as advice nor silently treated as a +#: failure: `severity` raises on one it does not recognise. The two sets together must cover the +#: `level` enum in `oold-rules.schema.json`, which is asserted by +#: `test_the_severity_split_covers_the_whole_level_vocabulary`. +_ADVICE_LEVELS = frozenset({"SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED"}) + #: Used only when the meta version in use ships no catalogue to read the level from. DEFAULT_LEVEL: Status = FAIL @@ -113,10 +120,22 @@ def severity(rule: Rule | None, fallback: Status = DEFAULT_LEVEL) -> Status: The level is the specification's own, not a taste judgement made here, so relaxing a MUST to a SHOULD upstream changes the validator's behaviour with no code change. Without a catalogue there is nothing to read, and the caller's fallback applies. + + An unrecognised level raises rather than defaulting. Defaulting either way is silent and wrong + in one direction: as advice it demotes a requirement, as a failure it invents one. The + specification added `NOT RECOMMENDED` to its vocabulary in v1.0.0-rc.3, which the old + "anything not a MUST is advice" reading would have absorbed without a word. """ if not rule: return fallback - return FAIL if rule.level in _MUST_LEVELS else WARN + if rule.level in _MUST_LEVELS: + return FAIL + if rule.level in _ADVICE_LEVELS: + return WARN + raise ValueError( + f"{rule.id} carries the level {rule.level!r}, which this validator does not classify. " + "The specification's level vocabulary has grown; add it to _MUST_LEVELS or _ADVICE_LEVELS." + ) # ---------------------------------------------------------------------------- individual rules @@ -951,12 +970,14 @@ class CheckInfo: ), # Deliberately cites no rule. The specification permits an unmapped term - "an author MAY # leave it unmapped" - and treats deferring semantics as what distinguishes OO-LD from - # RDF/SHACL, so an unmapped property is not a conformance failure and there is no rule to - # borrow severity from. It is still worth reporting: it is usually an oversight, and the - # property will not reach RDF. Promote it with --strict where full coverage is intended. + # RDF/SHACL, so an unmapped property is not a conformance failure. It is still worth + # reporting: OOLD-SCH-21d7 says a schema should offer at least one complete mapping, which is + # a SHOULD, so the warning is the catalogue's own severity rather than one chosen here. + # Promote it with --strict where full coverage is intended. CheckInfo( "context.coverage", "every declared property carries a @context term", + rule="OOLD-SCH-21d7", default_status=WARN, detects=check_predicates, predates_catalog=True, diff --git a/tests/test_validation/test_check_registry.py b/tests/test_validation/test_check_registry.py index 92404b5..118a7e9 100644 --- a/tests/test_validation/test_check_registry.py +++ b/tests/test_validation/test_check_registry.py @@ -656,3 +656,36 @@ def test_every_check_runs_on_every_example(check, data_dir): report = validate_directory(data_dir, Options(meta=("latest",), offline=True)) produced = [c for c in report.checks if c.id == check.id] assert produced, f"{check.id} produced no finding at all" + + +def test_the_severity_split_covers_the_whole_level_vocabulary(): + """Every level the specification can state must be classified as a failure or as advice. + + `severity` raises on a level it does not recognise, which is the right behaviour at runtime + but a poor way to find out. This ties the two sets to the `level` enum of every tracked + version, so a vocabulary that grows upstream fails here, at vendoring time, naming the level. + + It is a real gap rather than a hypothetical: `NOT RECOMMENDED` entered the vocabulary in + v1.0.0-rc.3, and the previous "anything not a MUST is advice" reading would have swallowed it. + """ + import json + + from oold.validation.check_registry import _ADVICE_LEVELS, _MUST_LEVELS + from oold.validation.meta_store import RULES_SCHEMA_FILE, meta_dir, tracked_versions + + classified = _MUST_LEVELS | _ADVICE_LEVELS + assert not (_MUST_LEVELS & _ADVICE_LEVELS), "a level cannot be both a failure and advice" + + checked_any = False + for version in tracked_versions(): + path = meta_dir() / version / RULES_SCHEMA_FILE + if not path.is_file(): + continue + checked_any = True + declared = set(json.loads(path.read_text(encoding="utf-8"))["$defs"]["rule"]["properties"]["level"]["enum"]) + assert declared <= classified, ( + f"{version}/{RULES_SCHEMA_FILE} allows {sorted(declared - classified)}, which " + "check_registry classifies as neither a failure nor advice, so severity() would raise " + "on any rule stated with it." + ) + assert checked_any, "no tracked version ships a rule schema, so this test proved nothing" From 24216a292d52df9220184915c7673a4129c778b2 Mon Sep 17 00:00:00 2001 From: SimonTaurus Date: Mon, 24 Aug 2026 03:34:49 +0200 Subject: [PATCH 2/2] test(validation): cover the unclassified-level raise --- tests/test_validation/test_check_registry.py | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_validation/test_check_registry.py b/tests/test_validation/test_check_registry.py index 118a7e9..cabc079 100644 --- a/tests/test_validation/test_check_registry.py +++ b/tests/test_validation/test_check_registry.py @@ -689,3 +689,32 @@ def test_the_severity_split_covers_the_whole_level_vocabulary(): "on any rule stated with it." ) assert checked_any, "no tracked version ships a rule schema, so this test proved nothing" + + +def test_an_unclassified_level_raises_rather_than_defaulting(): + """Neither fallback is safe, so `severity` refuses to pick one. + + Treating it as advice demotes a requirement; treating it as a failure invents one. The error + names the rule and the level, because the fix is a one-line addition to one of the two sets. + """ + import pytest as _pytest + + from oold.validation.check_registry import severity + from oold.validation.meta_store import Rule + + rule = Rule( + id="OOLD-XXX-0000", + area="XXX", + level="MUST PROBABLY", + applies_to="document", + section="whatever", + summary="s", + text="t", + text_sha256="0" * 64, + machine_checkable=True, + since="9.9.9", + deprecated=False, + source="x.md:1", + ) + with _pytest.raises(ValueError, match="MUST PROBABLY"): + severity(rule)