Skip to content

Follow-ups from the validation subsystem review #125

Description

@simontaurus

Findings from the review of #114 that are not covered by #118-#122. All verified against the branch.

These are five unrelated findings that happen to share an issue. They are separately assignable and do not have to land together. The ordering they fit into is #129.

Items

  • 1. Parity is measured but cannot fail - .github/workflows/main.yml only. Independent, can go first.
  • 2. The CLI and the MCP server disagree on filtering - check_registry.py, cli.py, mcp_server.py. Shares mcp_server.py with MCP tools cannot receive a document's dependencies from a remote client #122, so those two go adjacent or together.
  • 3. A corrupt rules schema disables catalogue validation silently - meta_store.py only. Independent, can go first.
  • 4. _MUST_LEVELS duplicates the level vocabulary - check_registry.py. Blocked on oold-schema rc.3 being vendored.
  • 5. Validation fetches by default - Makefile and docs. Coordinate with No fixture exercises a truly remote (HTTP) @context #128: a fixture that resolves a remote @context cannot run under a default of --offline.

1. Parity is measured but cannot fail

.github/workflows/main.yml:80 runs the parity tests with continue-on-error: true, so a parity regression is reported and never fails the build. It also clones oold-schema main at depth 1 rather than the tag the fixtures are pinned to.

Cloning main is right - it catches drift the fixture snapshot cannot see - but combined with continue-on-error the whole check is informational.

  • decide whether parity gates the build, and either drop continue-on-error or say in the docs that it does not

2. The CLI and the MCP server disagree on filtering

cli.py:246-251 and mcp_server.py:292-296 apply the area and unchecked filters in opposite orders, so area + unchecked together already return different results through the two front ends.

  • pull select_rules / select_checks / check_summary into check_registry.py, leaving both front ends with formatting only
  • enforced_by = {v: k for k, v in rule_map().items()} is written three times: cli.py:260, cli.py:295, mcp_server.py:291

3. A corrupt rules schema disables catalogue validation silently

meta_store.py. _read_rules separates absent from corrupt; _read_rules_schema ten lines below returns None for both. _catalog_problems then returns [], so a corrupt oold-rules.schema.json means the catalogue goes unvalidated with nothing reported.

  • report the corrupt case the way _read_rules does

4. _MUST_LEVELS duplicates the level vocabulary

check_registry.py:104. severity() states that the level is the specification's, but the split into failure and advice is a hardcoded frozenset here, and an unrecognised level silently becomes WARN.

Rule now rejects a missing level, which is the field case. The value case is still open: level: str accepts anything.

OO-LD/oold-schema#136 makes the upstream vocabulary explicit and guards it against drift, so once rc.3 is vendored:

  • constrain Rule.level to the enum in oold-rules.schema.json
  • derive _MUST_LEVELS rather than restating it, or fail loudly on a level outside the vocabulary

5. Validation fetches by default

offline defaults to False. That is what enables cross-directory @context resolution, so it is the point rather than an oversight, but it sits oddly beside the vendored meta-schemas, whose stated purpose is that a released version cannot change meaning between runs. Context resolution has no equivalent guarantee, so two runs of one fixture can disagree if a remote context moves.

  • default make validate to --offline, or state the asymmetry

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions