Raised in review of #114 (#114 (comment)). Every item below re-verified against feat/validator-migration at b4e23ac, so the line numbers are current.
Duplication
- The
to_rdf -> count -> from_rdf -> frame-or-compact sequence is written three times, in roundtrip.py, instance_checks.py and compliance.py. "application/n-quads" appears 9 times across those three modules (5 / 2 / 2). Extract a project_and_restore(...) helper.
pipeline._check_variant (pipeline.py:353) is a compressed copy of pipeline._check_schema_jsonld (pipeline.py:260).
The third duplication from the same review comment, enforced_by = {v: k for k, v in rule_map().items()} written three times, is tracked in #125 together with the CLI/MCP filter-order divergence it belongs to.
Dead code
resolve.dereference_and_bound() (resolve.py:495) has no caller anywhere in src/ or tests/; pipeline.py and mcp_server.py open-code the same sequence. Use it or delete it.
- 13
to_dict() definitions against 10 call sites in src/; the review counted seven with no caller.
RoundtripResult.ok defaults to True (roundtrip.py:138) while InstanceRoundtripResult.ok defaults to False (instance_checks.py:43), and nothing reads either. Opposite defaults for the same concept is the kind of thing that becomes a bug the first time something does read it.
Naming and duplicated constants
DEFAULT_MAX_DEPTH is 6 in resolve.py:45 and 40 in context_resolution.py:46. Same package, same name, different meaning and an order of magnitude apart. Rename both to say what they bound.
"https://oo-ld.test/" is both loader.DEFAULT_HOST (loader.py:34) and compliance.RDF_BASE (compliance.py:48).
SCHEMA_SUFFIX = ".schema.json" is defined in pipeline.py:40 and context_graph.py:27.
- Two UUID regexes:
check_registry.py:55 and formats.py:78.
Check.line() (report.py:71) duplicates the live renderer at cli.py:89-93 and has already drifted from it - column width 24 against 22. One of them is dead; the human output only goes through cli.py.
Length
C901 is in ruff's ignore list (pyproject.toml:287), so nothing flags these today:
| Function |
Lines |
compliance._run_tests |
126 |
pipeline._check_schema |
92 |
pipeline._check_schema_jsonld |
92 |
resolve.bound_schema |
87 |
resolve.dereference |
84 |
Splitting these is worth more than re-enabling C901, which would fail the build on code nobody is about to touch.
Constraint
All of it is behaviour-preserving, so no verdict may move. Gate on make test plus OOLD_SCHEMA_DIR=../oold-schema uv run pytest -m parity.
Best done after #118 and #119, which rewrite parts of context_resolution.py and resolve.py and would otherwise conflict with the renames here.
Raised in review of #114 (#114 (comment)). Every item below re-verified against
feat/validator-migrationatb4e23ac, so the line numbers are current.Duplication
to_rdf-> count ->from_rdf-> frame-or-compact sequence is written three times, inroundtrip.py,instance_checks.pyandcompliance.py."application/n-quads"appears 9 times across those three modules (5 / 2 / 2). Extract aproject_and_restore(...)helper.pipeline._check_variant(pipeline.py:353) is a compressed copy ofpipeline._check_schema_jsonld(pipeline.py:260).The third duplication from the same review comment,
enforced_by = {v: k for k, v in rule_map().items()}written three times, is tracked in #125 together with the CLI/MCP filter-order divergence it belongs to.Dead code
resolve.dereference_and_bound()(resolve.py:495) has no caller anywhere insrc/ortests/;pipeline.pyandmcp_server.pyopen-code the same sequence. Use it or delete it.to_dict()definitions against 10 call sites insrc/; the review counted seven with no caller.RoundtripResult.okdefaults toTrue(roundtrip.py:138) whileInstanceRoundtripResult.okdefaults toFalse(instance_checks.py:43), and nothing reads either. Opposite defaults for the same concept is the kind of thing that becomes a bug the first time something does read it.Naming and duplicated constants
DEFAULT_MAX_DEPTHis6inresolve.py:45and40incontext_resolution.py:46. Same package, same name, different meaning and an order of magnitude apart. Rename both to say what they bound."https://oo-ld.test/"is bothloader.DEFAULT_HOST(loader.py:34) andcompliance.RDF_BASE(compliance.py:48).SCHEMA_SUFFIX = ".schema.json"is defined inpipeline.py:40andcontext_graph.py:27.check_registry.py:55andformats.py:78.Check.line()(report.py:71) duplicates the live renderer atcli.py:89-93and has already drifted from it - column width 24 against 22. One of them is dead; the human output only goes throughcli.py.Length
C901is in ruff's ignore list (pyproject.toml:287), so nothing flags these today:compliance._run_testspipeline._check_schemapipeline._check_schema_jsonldresolve.bound_schemaresolve.dereferenceSplitting these is worth more than re-enabling
C901, which would fail the build on code nobody is about to touch.Constraint
All of it is behaviour-preserving, so no verdict may move. Gate on
make testplusOOLD_SCHEMA_DIR=../oold-schema uv run pytest -m parity.Best done after #118 and #119, which rewrite parts of
context_resolution.pyandresolve.pyand would otherwise conflict with the renames here.