From 64f8f71e04475c2408a87aba686a4f4c44029813 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 12:38:54 +0100 Subject: [PATCH 1/3] implement: Check coverage, overlap and structure (t28) --- in2lambda/draft/__init__.py | 23 +- in2lambda/draft/report.py | 261 ++++++++++++++++++ in2lambda/main.py | 18 ++ in2lambda/source/__init__.py | 9 + tests/fixtures/drafts/README.md | 27 +- .../fixtures/drafts/empty_field/commands.json | 23 ++ .../fixtures/drafts/empty_field/expected.json | 38 +++ tests/fixtures/drafts/empty_field/report.json | 13 + tests/fixtures/drafts/empty_field/source.md | 3 + tests/fixtures/drafts/mark_ignore/report.json | 57 ++++ .../part_without_solution/commands.json | 24 ++ .../part_without_solution/expected.json | 38 +++ .../drafts/part_without_solution/report.json | 13 + .../drafts/part_without_solution/source.md | 5 + .../drafts/two_questions/commands.json | 7 + .../drafts/two_questions/expected.json | 12 + tests/test_cli.py | 1 + tests/test_draft.py | 68 ++++- 18 files changed, 622 insertions(+), 18 deletions(-) create mode 100644 in2lambda/draft/report.py create mode 100644 tests/fixtures/drafts/empty_field/commands.json create mode 100644 tests/fixtures/drafts/empty_field/expected.json create mode 100644 tests/fixtures/drafts/empty_field/report.json create mode 100644 tests/fixtures/drafts/empty_field/source.md create mode 100644 tests/fixtures/drafts/mark_ignore/report.json create mode 100644 tests/fixtures/drafts/part_without_solution/commands.json create mode 100644 tests/fixtures/drafts/part_without_solution/expected.json create mode 100644 tests/fixtures/drafts/part_without_solution/report.json create mode 100644 tests/fixtures/drafts/part_without_solution/source.md diff --git a/in2lambda/draft/__init__.py b/in2lambda/draft/__init__.py index 4a26cc9..06814c6 100644 --- a/in2lambda/draft/__init__.py +++ b/in2lambda/draft/__init__.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import Any +from in2lambda.draft.report import checks, overlapping from in2lambda.source import ( DRAFT, SourceError, @@ -129,13 +130,14 @@ def record( "Run in2lambda source add --start-over to begin the draft again." ) for filled, field in draft["fields"].items(): - for taken in field["ranges"]: - if any(taken[0] <= end and start <= taken[1] for start, end in ranges): - raise AlreadyFilled( - f"Lines {taken[0]}-{taken[1]} are where {filled} came from, so " - f"they cannot also be {key}. Run in2lambda source show to see " - "which lines are still free." - ) + if overlapping(ranges, field["ranges"]): + # A field is quoted from one range, so that is the range in the way. + taken = field["ranges"][0] + raise AlreadyFilled( + f"Lines {taken[0]}-{taken[1]} are where {filled} came from, so " + f"they cannot also be {key}. Run in2lambda source show to see " + "which lines are still free." + ) draft["fields"][key] = { "value": value, "layer": layer, @@ -215,6 +217,9 @@ def apply(draft: dict[str, Any], markdown: str, entry: Any) -> str: written = handler(draft, markdown, entry["args"], entry["by"]) # After the handler, so a command that was refused is not recorded as having run. draft["log"].append(entry) + # A report is about the draft as it was, so the command that changes it takes the + # report with it rather than leaving one that describes something else. + draft.pop("report", None) return written @@ -269,6 +274,10 @@ def replay(directory: str = ".") -> None: } for entry in draft["log"]: apply(rebuilt, markdown, entry) + # The one thing in a draft that no command wrote: the checks did, over the draft the + # commands left, so rebuilding it is running them again rather than copying it. + if "report" in draft: + rebuilt["report"] = checks(rebuilt) path = Path(directory) / DRAFT if serialise(rebuilt) != path.read_bytes(): diff --git a/in2lambda/draft/report.py b/in2lambda/draft/report.py new file mode 100644 index 0000000..d9f7aee --- /dev/null +++ b/in2lambda/draft/report.py @@ -0,0 +1,261 @@ +"""Checks a draft over as a whole, and writes what it finds into it. + +A draft is written one command at a time, and what a run of them left out is not +something any one command can see: a block nobody quoted, two fields taken from the same +lines, a question numbered 3 where there is no 2, a part with nothing answering it. So +the finished draft is looked over at once, and what the checks find is written into it as +its ``report``, which is what whoever is writing the draft - an agent or a person - reads +to find out what is left to do, without reading the draft itself. + +Everything here reports, never refuses: what the checks found may well be deliberate, and +deciding that is whoever is writing the draft's to do. Only what is in the draft is +looked at - its blocks, its field keys, their ranges and their values - because what the +text of a question says is `in2lambda.validation`'s, at export. +""" + +import re +from pathlib import Path +from typing import Any + +from in2lambda.source import DRAFT, frozen, save + +Finding = dict[str, Any] +"""One thing a check found: ``{"check", "field", "ranges", "message"}``. + +``check`` is which check found it, ``field`` the block id or field key it is about, +``ranges`` the lines in question as ``[[start, end], ...]``, and ``message`` a sentence +naming all of that, so that a line of the report can be acted on by itself. +""" + +_NUMBERED = re.compile(r"((?:q\d+\.p)|q)(\d+)\.text") +"""A question's or a part's text, split into what numbers it and the number.""" + +_PART = re.compile(r"(q\d+)\.p\d+\.text") +"""A part's text, and the question it belongs to.""" + +_UNPLACED = float("inf") +"""Where a finding about no particular line sorts: after every finding about one.""" + + +def overlapping(ranges: list[list[int]], other: list[list[int]]) -> bool: + """Whether any line falls in both sets of line ranges. + + Args: + ranges: Line ranges, as ``[[start, end], ...]``, each end inclusive. + other: The ranges to test them against. + + Returns: + Whether the two sets share a line. + + Examples: + >>> from in2lambda.draft.report import overlapping + >>> overlapping([[5, 6]], [[6, 8]]) + True + >>> overlapping([[5, 6]], [[7, 8]]) + False + """ + return any( + taken[0] <= end and start <= taken[1] + for taken in other + for start, end in ranges + ) + + +def _where(ranges: list[list[int]]) -> str: + """The lines something covers, as a message names them, or "" if it covers none.""" + if not ranges: + return "" + return " (lines " + ", ".join(f"{start}-{end}" for start, end in ranges) + ")" + + +def _runs(lines: list[int]) -> list[list[int]]: + """Line numbers in order, grouped into the ranges they run in.""" + runs: list[list[int]] = [] + for line in lines: + if runs and runs[-1][1] == line - 1: + runs[-1][1] = line + else: + runs.append([line, line]) + return runs + + +def _uncovered(draft: dict[str, Any]) -> list[Finding]: + """Blocks of the source that no field, and no `mark ignore`, accounts for. + + A block partly quoted is reported for the rest of it: a question taken from the first + line of a block leaves the other lines as much unaccounted for as a whole block would. + """ + claimed = { + line + for field in draft["fields"].values() + for start, end in field["ranges"] + for line in range(start, end + 1) + } + found = [] + for block in draft["blocks"]: + free = _runs( + [ + line + for line in range(block["start"], block["end"] + 1) + if line not in claimed + ] + ) + if free: + found.append( + { + "check": "uncovered", + "field": block["id"], + "ranges": free, + "message": f"{block['id']}{_where(free)} is in no field and not " + "marked ignore.", + } + ) + return found + + +def _overlaps(draft: dict[str, Any]) -> list[Finding]: + """Pairs of fields quoted from some of the same lines. + + No command writes such a pair - `record` refuses the second of them - so this is here + for a draft edited by hand, where one of the two fields is quoting the wrong thing. + """ + fields = draft["fields"] + keys = sorted(fields) + return [ + { + "check": "overlap", + "field": key, + "ranges": fields[key]["ranges"], + "message": f"{key}{_where(fields[key]['ranges'])} and {other}" + f"{_where(fields[other]['ranges'])} are taken from some of the same lines.", + } + for index, key in enumerate(keys) + for other in keys[index + 1 :] + if overlapping(fields[key]["ranges"], fields[other]["ranges"]) + ] + + +def _gaps(draft: dict[str, Any]) -> list[Finding]: + """Questions or parts numbered past one that was never written. + + Numbers are given out by `in2lambda.draft._next`, which leaves no gap, so this too is + a draft that was edited: a question renumbered, or one deleted out of the middle. + """ + numbered: dict[str, list[int]] = {} + for key in draft["fields"]: + if named := _NUMBERED.fullmatch(key): + numbered.setdefault(named[1], []).append(int(named[2])) + return [ + { + "check": "gap", + "field": f"{prefix}{missing}.text", + "ranges": [], + "message": f"There is no {prefix}{missing}.text, though " + f"{prefix}{max(numbers)}.text is written: the numbering skips it.", + } + for prefix, numbers in numbered.items() + for missing in range(1, max(numbers)) + if missing not in numbers + ] + + +def _without_solutions(draft: dict[str, Any]) -> list[Finding]: + """Parts that nothing in the draft answers. + + A part is answered by its own solution or by the solution of the question it belongs + to, since a sheet often writes one worked solution covering every part at once. + """ + fields = draft["fields"] + found = [] + for key in sorted(fields): + if (named := _PART.fullmatch(key)) is None: + continue + part = key.removesuffix(".text") + if f"{part}.solution" in fields or f"{named[1]}.solution" in fields: + continue + found.append( + { + "check": "no-solution", + "field": part, + "ranges": fields[key]["ranges"], + "message": f"{part}{_where(fields[key]['ranges'])} has no solution: " + f"neither {part}.solution nor {named[1]}.solution is written.", + } + ) + return found + + +def _empty(draft: dict[str, Any]) -> list[Finding]: + """Fields holding nothing, which is a quotation of the wrong lines or of none.""" + return [ + { + "check": "empty", + "field": key, + "ranges": field["ranges"], + "message": f"{key}{_where(field['ranges'])} is empty.", + } + for key, field in sorted(draft["fields"].items()) + if isinstance(field["value"], str) and not field["value"].strip() + ] + + +def checks(draft: dict[str, Any]) -> list[Finding]: + """Everything the checks find wrong with a draft, in the order of the source. + + Args: + draft: A draft, as `in2lambda.source.frozen` reads one. + + Returns: + One :data:`Finding` per thing found, earliest line first and then by what it is + about, with the findings about no particular line last. An empty list means the + draft covers its source once each, with nothing missing from its numbering. + + Examples: + >>> from in2lambda.draft.report import checks + >>> draft = { + ... "blocks": [{"id": "b1", "type": "paragraph", "start": 1, "end": 2}], + ... "fields": {}, + ... } + >>> [finding["message"] for finding in checks(draft)] + ['b1 (lines 1-2) is in no field and not marked ignore.'] + """ + found = ( + _uncovered(draft) + + _overlaps(draft) + + _gaps(draft) + + _without_solutions(draft) + + _empty(draft) + ) + return sorted( + found, + key=lambda finding: ( + finding["ranges"][0][0] if finding["ranges"] else _UNPLACED, + finding["field"], + ), + ) + + +def validate(directory: str = ".") -> list[Finding]: + """Checks the draft in a directory over and writes the report into it. + + The report replaces whatever one is there, and is dropped again by the next command + that changes the draft: it describes the draft as it stood, and a report saying + something else is worse than none at all. + + Args: + directory: Where the ``draft.json`` to check is. + + Returns: + What the checks found, as it was written into the draft. + + Raises: + DraftMissing: there is no draft in that directory. + DraftUnreadable: what is there is not a draft anything here wrote. + SourceUnreadable: the markdown the draft names has moved, or is not text. + DraftExists: the markdown has changed since the draft was written from it, so + the lines the report named would not be the lines it was written about. + """ + draft, _ = frozen(directory) + draft["report"] = checks(draft) + save(Path(directory) / DRAFT, draft) + return draft["report"] diff --git a/in2lambda/main.py b/in2lambda/main.py index 1d1ea2e..ff395e5 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -19,6 +19,7 @@ import rich_click as click import in2lambda.draft +import in2lambda.draft.report import in2lambda.filters import in2lambda.source from in2lambda.api.set import Set @@ -369,5 +370,22 @@ def draft_replay() -> None: click.echo("Replays as it stands.") +@cli.command("validate") +def validate() -> None: + """Checks the draft in this directory over and writes the report into it. + + Reports source blocks in no field and not marked ignore, two fields taken from the + same lines, gaps in the numbering of the questions or their parts, parts nothing + answers, and fields holding nothing. Finding something is not a failure: the report + is written into draft.json either way, and replaced by the next one. + """ + with _message_not_traceback(): + report = in2lambda.draft.report.validate() + for finding in report: + click.echo(finding["message"]) + if not report: + click.echo("Nothing to report.") + + if __name__ == "__main__": cli() diff --git a/in2lambda/source/__init__.py b/in2lambda/source/__init__.py index bd2d6dc..14abf2f 100644 --- a/in2lambda/source/__init__.py +++ b/in2lambda/source/__init__.py @@ -54,6 +54,10 @@ def _field_fault(field: Any) -> str: nothing here wrote: there is no command log to replay it from, and inventing an empty one would claim the fields in it came from nowhere. Freezing the source again is the way through, which is what the refusal says. + +A draft `in2lambda validate` has been run on also has a ``report``, which is not required +and not looked into: nothing here reads one back, and the next run of the checks writes +whatever is there over. """ _MARKDOWN = "commonmark_x" @@ -452,6 +456,9 @@ def add(file: str, start_over: bool = False) -> Path: # --start-over is the way to throw them away, and the only one. log: list[Any] = [] fields: dict[str, Any] = {} + # And what the checks found about it, which still holds for the same reason: this + # writes the draft back as it was, so a report of it is a report of what is saved. + report: Any = None # The blocks a draft already here has, which are not always what parsing the # markdown gives: `split block` cuts one in two, and parsing again would undo that # while keeping the log entry saying it happened, leaving the ids the fields were @@ -468,6 +475,7 @@ def add(file: str, start_over: bool = False) -> Path: "invalidates every line range taken from the old draft." ) found, log, fields = existing["blocks"], existing["log"], existing["fields"] + report = existing.get("report") elif frozen_path != source and frozen_path.exists(): raise DraftExists( f"{frozen_path.name} is already there and no {DRAFT} claims it, so it " @@ -495,6 +503,7 @@ def add(file: str, start_over: bool = False) -> Path: "blocks": found, "log": log, "fields": fields, + **({"report": report} if report is not None else {}), }, ) return draft diff --git a/tests/fixtures/drafts/README.md b/tests/fixtures/drafts/README.md index 5c7902b..0e5ec10 100644 --- a/tests/fixtures/drafts/README.md +++ b/tests/fixtures/drafts/README.md @@ -1,14 +1,21 @@ # Drafts built by commands Each folder here is one run: a `source.md` to freeze, the `commands.json` to apply to the draft -of it, and the `expected.json` those commands should leave in the draft's `fields`. The test -freezes the source, applies each command, compares the fields, and then replays the draft from -its log and checks the file is unchanged byte for byte - so a folder covers both what a command -writes and that it can be rebuilt from what it recorded. +of it, the `expected.json` those commands should leave in the draft's `fields`, and the +`report.json` that `in2lambda validate` should then find in it - a folder with no `report.json` +is a draft with nothing wrong with it. The test freezes the source, applies each command, checks +the draft over, compares the fields and the report, and then replays the draft from its log and +checks the file is unchanged byte for byte - so a folder covers both what a command writes and +that it can be rebuilt from what it recorded. -To cover another command, add a folder. `mark_ignore` is the `sources/markdown` document with two -of its blocks marked as nothing to take a question from. `two_questions` is a sheet with a title, a -rubric, two questions with a part each and a separate solutions section, written out by every -command there is: the second question runs into its part with no blank line between them, so the -parser makes one block of the two and `split block` cuts it, and the first question's part is typed -out rather than quoted, because the source writes it with an `(a)` the field should not carry. +To cover another command or another check, add a folder. `mark_ignore` is the `sources/markdown` +document with two of its blocks marked as nothing to take a question from, and the other five +reported as in no field. `two_questions` is a sheet with a title, a rubric, two questions with a +part each and a separate solutions section, written out by every command there is, and it is the +clean one: the second question runs into its part with no blank line between them, so the parser +makes one block of the two and `split block` cuts it, and the first question's part is typed out +rather than quoted, because the source writes it with an `(a)` the field should not carry - which +is why the block it was typed from is marked ignore rather than left unaccounted for. +`part_without_solution` and `empty_field` are the smallest drafts the other two checks have +anything to say about; an overlap and a gap in the numbering are not here, because no run of +commands can make one. diff --git a/tests/fixtures/drafts/empty_field/commands.json b/tests/fixtures/drafts/empty_field/commands.json new file mode 100644 index 0000000..6415553 --- /dev/null +++ b/tests/fixtures/drafts/empty_field/commands.json @@ -0,0 +1,23 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b2" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "text": "s2" + }, + "by": "tests", + "command": "question add" + } +] diff --git a/tests/fixtures/drafts/empty_field/expected.json b/tests/fixtures/drafts/empty_field/expected.json new file mode 100644 index 0000000..d37d178 --- /dev/null +++ b/tests/fixtures/drafts/empty_field/expected.json @@ -0,0 +1,38 @@ +{ + "b1.ignore": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 1, + 1 + ] + ], + "value": true + }, + "q1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 3, + 3 + ] + ], + "value": "Find the pressure at the bottom of the tank." + }, + "q2.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 2, + 2 + ] + ], + "value": "" + } +} diff --git a/tests/fixtures/drafts/empty_field/report.json b/tests/fixtures/drafts/empty_field/report.json new file mode 100644 index 0000000..015677f --- /dev/null +++ b/tests/fixtures/drafts/empty_field/report.json @@ -0,0 +1,13 @@ +[ + { + "check": "empty", + "field": "q2.text", + "message": "q2.text (lines 2-2) is empty.", + "ranges": [ + [ + 2, + 2 + ] + ] + } +] diff --git a/tests/fixtures/drafts/empty_field/source.md b/tests/fixtures/drafts/empty_field/source.md new file mode 100644 index 0000000..0153af9 --- /dev/null +++ b/tests/fixtures/drafts/empty_field/source.md @@ -0,0 +1,3 @@ +# Pressure in a tank + +Find the pressure at the bottom of the tank. diff --git a/tests/fixtures/drafts/mark_ignore/report.json b/tests/fixtures/drafts/mark_ignore/report.json new file mode 100644 index 0000000..d23d4f2 --- /dev/null +++ b/tests/fixtures/drafts/mark_ignore/report.json @@ -0,0 +1,57 @@ +[ + { + "check": "uncovered", + "field": "b2", + "message": "b2 (lines 3-3) is in no field and not marked ignore.", + "ranges": [ + [ + 3, + 3 + ] + ] + }, + { + "check": "uncovered", + "field": "b3", + "message": "b3 (lines 5-5) is in no field and not marked ignore.", + "ranges": [ + [ + 5, + 5 + ] + ] + }, + { + "check": "uncovered", + "field": "b4", + "message": "b4 (lines 6-6) is in no field and not marked ignore.", + "ranges": [ + [ + 6, + 6 + ] + ] + }, + { + "check": "uncovered", + "field": "b5", + "message": "b5 (lines 8-10) is in no field and not marked ignore.", + "ranges": [ + [ + 8, + 10 + ] + ] + }, + { + "check": "uncovered", + "field": "b6", + "message": "b6 (lines 12-12) is in no field and not marked ignore.", + "ranges": [ + [ + 12, + 12 + ] + ] + } +] diff --git a/tests/fixtures/drafts/part_without_solution/commands.json b/tests/fixtures/drafts/part_without_solution/commands.json new file mode 100644 index 0000000..6c30722 --- /dev/null +++ b/tests/fixtures/drafts/part_without_solution/commands.json @@ -0,0 +1,24 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b2" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "question": "q1", + "text": "b3" + }, + "by": "tests", + "command": "part add" + } +] diff --git a/tests/fixtures/drafts/part_without_solution/expected.json b/tests/fixtures/drafts/part_without_solution/expected.json new file mode 100644 index 0000000..1f0b546 --- /dev/null +++ b/tests/fixtures/drafts/part_without_solution/expected.json @@ -0,0 +1,38 @@ +{ + "b1.ignore": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 1, + 1 + ] + ], + "value": true + }, + "q1.p1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 5, + 5 + ] + ], + "value": "(a) Give the drag coefficient you used." + }, + "q1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 3, + 3 + ] + ], + "value": "A submarine is towed at speed $U$ through still water. Find the drag force on it." + } +} diff --git a/tests/fixtures/drafts/part_without_solution/report.json b/tests/fixtures/drafts/part_without_solution/report.json new file mode 100644 index 0000000..49e68ab --- /dev/null +++ b/tests/fixtures/drafts/part_without_solution/report.json @@ -0,0 +1,13 @@ +[ + { + "check": "no-solution", + "field": "q1.p1", + "message": "q1.p1 (lines 5-5) has no solution: neither q1.p1.solution nor q1.solution is written.", + "ranges": [ + [ + 5, + 5 + ] + ] + } +] diff --git a/tests/fixtures/drafts/part_without_solution/source.md b/tests/fixtures/drafts/part_without_solution/source.md new file mode 100644 index 0000000..6e0c25d --- /dev/null +++ b/tests/fixtures/drafts/part_without_solution/source.md @@ -0,0 +1,5 @@ +# Towing a submarine + +A submarine is towed at speed $U$ through still water. Find the drag force on it. + +(a) Give the drag coefficient you used. diff --git a/tests/fixtures/drafts/two_questions/commands.json b/tests/fixtures/drafts/two_questions/commands.json index 67a89e5..4fd2f4e 100644 --- a/tests/fixtures/drafts/two_questions/commands.json +++ b/tests/fixtures/drafts/two_questions/commands.json @@ -28,6 +28,13 @@ "by": "tests", "command": "part add" }, + { + "args": { + "block": "b4" + }, + "by": "tests", + "command": "mark ignore" + }, { "args": { "at": 12, diff --git a/tests/fixtures/drafts/two_questions/expected.json b/tests/fixtures/drafts/two_questions/expected.json index f81e303..f7373a9 100644 --- a/tests/fixtures/drafts/two_questions/expected.json +++ b/tests/fixtures/drafts/two_questions/expected.json @@ -23,6 +23,18 @@ ], "value": true }, + "b4.ignore": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 8, + 8 + ] + ], + "value": true + }, "b6.ignore": { "by": "tests", "edited": false, diff --git a/tests/test_cli.py b/tests/test_cli.py index 56515d0..c91daf3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -89,4 +89,5 @@ def test_completing_the_old_form_offers_the_subcommand() -> None: "convert", "draft", "source", + "validate", ] diff --git a/tests/test_draft.py b/tests/test_draft.py index 1f51e76..4d73563 100644 --- a/tests/test_draft.py +++ b/tests/test_draft.py @@ -17,6 +17,7 @@ from conftest import DRAFTS, DRAFTS_DIR import in2lambda.draft +import in2lambda.draft.report from in2lambda.main import cli MARK_IGNORE = DRAFTS_DIR / "mark_ignore" @@ -27,14 +28,23 @@ def _built(folder: Path, tmp_path: Path) -> Path: - """A folder's document, frozen in `tmp_path` with its commands applied to it.""" + """A folder's document, frozen in `tmp_path` with its commands applied and checked.""" shutil.copytree(folder, tmp_path, dirs_exist_ok=True) assert CliRunner().invoke(cli, ["source", "add", "source.md"]).exit_code == 0 for entry in json.loads((folder / "commands.json").read_text()): in2lambda.draft.execute(entry) + # Checked as well as built, so that what a folder's commands leave for the checks to + # find is fixture data like the fields they write are. + in2lambda.draft.report.validate() return tmp_path / "draft.json" +def _reported(folder: Path) -> list[dict[str, Any]]: + """What the checks should find in a folder's draft; nothing, where it says none.""" + report = folder / "report.json" + return json.loads(report.read_text()) if report.is_file() else [] + + @pytest.mark.parametrize("folder", DRAFTS, ids=lambda path: path.name) def test_a_draft_built_by_commands_replays_identically( folder: Path, tmp_path: Path, monkeypatch @@ -46,6 +56,7 @@ def test_a_draft_built_by_commands_replays_identically( draft = json.loads(draft_path.read_text()) assert draft["fields"] == json.loads((folder / "expected.json").read_text()) assert draft["log"] == json.loads((folder / "commands.json").read_text()) + assert draft["report"] == _reported(folder) written = draft_path.read_bytes() result = CliRunner().invoke(cli, ["draft", "replay"]) @@ -327,6 +338,61 @@ def test_a_command_says_what_it_wrote(tmp_path: Path, monkeypatch) -> None: assert result.output == "Wrote b3a and b3b.\n" +def test_a_draft_edited_into_an_overlap_or_a_gap_is_reported( + tmp_path: Path, monkeypatch +) -> None: + """Neither can be made by a command, so a hand-edited draft is the only way to one.""" + monkeypatch.chdir(tmp_path) + draft_path = _built(TWO_QUESTIONS, tmp_path) + draft = json.loads(draft_path.read_text()) + # Renumbering the second question leaves nothing numbered 2, and giving the first + # question's part the lines the question came from claims those lines twice. + for key in ("q2.text", "q2.p1.text", "q2.solution"): + draft["fields"][key.replace("q2", "q3")] = draft["fields"].pop(key) + draft["fields"]["q1.p1.text"]["ranges"] = [[5, 6]] + draft_path.write_text(json.dumps(draft)) + + result = CliRunner().invoke(cli, ["validate"]) + + assert result.exit_code == 0, result.output + report = json.loads(draft_path.read_text())["report"] + assert [(finding["check"], finding["field"]) for finding in report] == [ + ("overlap", "q1.p1.text"), + ("gap", "q2.text"), + ] + # Both sides of the overlap, so that either field can be looked at without the draft. + assert "q1.text" in report[0]["message"] + assert result.output == f"{report[0]['message']}\n{report[1]['message']}\n" + + +def test_a_clean_draft_is_reported_as_having_nothing_wrong_with_it( + tmp_path: Path, monkeypatch +) -> None: + """A report of nothing is still an answer, and is said rather than printed empty.""" + monkeypatch.chdir(tmp_path) + draft_path = _built(TWO_QUESTIONS, tmp_path) + + result = CliRunner().invoke(cli, ["validate"]) + + assert result.exit_code == 0, result.output + assert result.output == "Nothing to report.\n" + assert json.loads(draft_path.read_text())["report"] == [] + + +def test_a_command_run_after_a_report_leaves_none_behind( + tmp_path: Path, monkeypatch +) -> None: + """A report describes the draft it was run against, and that draft has changed.""" + monkeypatch.chdir(tmp_path) + draft_path = _built(MARK_IGNORE, tmp_path) + assert json.loads(draft_path.read_text())["report"] + + result = CliRunner().invoke(cli, ["draft", "mark", "ignore", "b2"]) + + assert result.exit_code == 0, result.output + assert "report" not in json.loads(draft_path.read_text()) + + def test_the_halves_of_a_split_block_are_blocks_like_any_other( tmp_path: Path, monkeypatch ) -> None: From 516e947f39f2aa66508c03dc33413d08101d2c7c Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 12:47:57 +0100 Subject: [PATCH 2/3] implement: Check coverage, overlap and structure (t28) --- in2lambda/source/__init__.py | 14 +++++++++----- tests/test_draft.py | 6 ++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/in2lambda/source/__init__.py b/in2lambda/source/__init__.py index 14abf2f..cec5778 100644 --- a/in2lambda/source/__init__.py +++ b/in2lambda/source/__init__.py @@ -27,11 +27,13 @@ def _field_fault(field: Any) -> str: """What is wrong with the shape of one field of a draft, or "" if nothing is. - Only ``ranges`` is looked inside for, because it is the only part of a field - anything here reads: `in2lambda.draft.record` compares the lines a command is - quoting against the lines every field was taken from. The value, the layer, whether - it was edited and by whom are written and read back whole, and an edit to any of - them is what a replay catches byte for byte. + ``ranges`` and ``value`` are what is looked for, because they are the parts of a + field anything here reads: `in2lambda.draft.record` compares the lines a command is + quoting against the lines every field was taken from, and + `in2lambda.draft.report.checks` reports a field whose value says nothing. Only + whether there is a value is asked, since the checks look at one as a string or not + at all. The layer, whether it was edited and by whom are written and read back + whole, and an edit to any of them is what a replay catches byte for byte. """ if not isinstance(field, dict): return "is not an object" @@ -44,6 +46,8 @@ def _field_fault(field: Any) -> str: for pair in field["ranges"] ): return f"has ranges {field['ranges']!r} rather than pairs of line numbers" + if "value" not in field: + return "has no value" return "" diff --git a/tests/test_draft.py b/tests/test_draft.py index 4d73563..10f6b1d 100644 --- a/tests/test_draft.py +++ b/tests/test_draft.py @@ -192,6 +192,7 @@ def test_a_log_entry_that_is_not_a_command_is_refused( ("fields", {"b1.ignore": 5}), ("fields", {"b1.ignore": {"ranges": "s1"}}), ("fields", {"b1.ignore": {"ranges": [[1]]}}), + ("fields", {"b1.ignore": {"ranges": [[1, 1]], "layer": 3}}), ], ids=[ "log", @@ -199,12 +200,13 @@ def test_a_log_entry_that_is_not_a_command_is_refused( "a field that is a number", "ranges that are not a list", "a range that is not a pair", + "a field with no value", ], ) @pytest.mark.parametrize( "arguments", - [["draft", "replay"], ["draft", "mark", "ignore", "b2"]], - ids=["replay", "mark"], + [["draft", "replay"], ["draft", "mark", "ignore", "b2"], ["validate"]], + ids=["replay", "mark", "validate"], ) def test_a_draft_whose_log_or_fields_is_the_wrong_shape_is_refused( field: str, value: Any, arguments: list[str], tmp_path: Path, monkeypatch From b98a59149e19ec939d2cf8cff13fb0b957e7368c Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 13:24:13 +0100 Subject: [PATCH 3/3] implement: Check coverage, overlap and structure (t28) --- docs/source/spec.md | 6 +++--- in2lambda/draft/__init__.py | 18 ++++++++++-------- tests/test_draft.py | 20 ++++++++++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/docs/source/spec.md b/docs/source/spec.md index cc7700c..8134a71 100644 --- a/docs/source/spec.md +++ b/docs/source/spec.md @@ -7,11 +7,11 @@ every question comes out of the source rather than being retyped: ```bash $ in2lambda source add questions.docx $ in2lambda spec run spec.yaml -b6 is in no field. +b6 (lines 12-13) is in no field and not marked ignore. ``` -The last line is the point of it: a spec run reports every block it made nothing of, so what is -left to account for is in front of you rather than quietly missing. +The last line is the point of it: a spec run reports every block it made nothing of, naming the +lines it is, so what is left to account for is in front of you rather than quietly missing. The fields a draft holds belong to the spec that wrote them, so a spec is run over a draft once. Running an edited one again is refused; freeze the document afresh and run it, which is two diff --git a/in2lambda/draft/__init__.py b/in2lambda/draft/__init__.py index f57c460..af86188 100644 --- a/in2lambda/draft/__init__.py +++ b/in2lambda/draft/__init__.py @@ -149,14 +149,16 @@ def record( "in2lambda source add --start-over to begin the draft again." ) for filled, field in draft["fields"].items(): - if overlapping(ranges, field["ranges"]): - # A field is quoted from one range, so that is the range in the way. - taken = field["ranges"][0] - raise AlreadyFilled( - f"Lines {taken[0]}-{taken[1]} are where {filled} came from, so " - f"they cannot also be {key}. Run in2lambda source show to see " - "which lines are still free." - ) + # Each of the field's ranges on its own, so that the refusal names the one in + # the way: a field edited by hand can be quoted from several, and the rest of + # them may be lines nobody wants. + for taken in field["ranges"]: + if overlapping(ranges, [taken]): + raise AlreadyFilled( + f"Lines {taken[0]}-{taken[1]} are where {filled} came from, so " + f"they cannot also be {key}. Run in2lambda source show to see " + "which lines are still free." + ) draft["fields"][key] = { "value": value, "layer": layer, diff --git a/tests/test_draft.py b/tests/test_draft.py index 79801c8..60bb9fa 100644 --- a/tests/test_draft.py +++ b/tests/test_draft.py @@ -303,6 +303,26 @@ def test_lines_another_field_was_taken_from_are_refused( assert draft_path.read_bytes() == built +def test_the_refusal_names_the_lines_that_are_in_the_way( + tmp_path: Path, monkeypatch +) -> None: + """A field edited by hand can be quoted from several ranges, only one of them clashing.""" + monkeypatch.setenv("COLUMNS", "200") + monkeypatch.chdir(tmp_path) + draft_path = _built(MARK_IGNORE, tmp_path) + draft = json.loads(draft_path.read_text()) + # The maths is part of the heading's block as far as this draft is concerned, which + # no command would write but an editor might. + draft["fields"]["b1.ignore"]["ranges"] = [[1, 1], [9, 10]] + draft_path.write_text(json.dumps(draft)) + + result = CliRunner().invoke(cli, ["draft", "question", "add", "--text", "s9:10"]) + + assert result.exit_code != 0 + # Lines 1-1 are free, so naming them would send whoever reads this to the wrong end. + assert "Lines 9-10" in result.output + + @pytest.mark.parametrize( ("arguments", "named"), [