Treat a figure as content, not as a block to ignore

docs/how-it-works.md

@@ -93,7 +93,7 @@ spec in2lambda accepts.
 ### `coverage`
 
 `in2lambda.draft.execute` runs the spec over the frozen source and fills the draft's
-layer 1 fields. The message has one form:
+layer 1 fields. The message begins with one line about the spec run:
 
 ```
 PartsSepSol: 14 blocks, 9 fields at layer 1, 4 ignored, b13 unassigned
@@ -105,6 +105,18 @@ stage prints `no fields` where the spec wrote none. `4 ignored` is the number of
 blocks the spec's `ignore` selector matched. The unassigned blocks are listed by id,
 and the stage prints `none unassigned` where every block reached a field.
 
+A block marked `ignore` whose lines hold a markdown image is a figure the spec
+discarded, and the set is built without that image. The stage reads the ignored
+blocks back and adds one of two phrases to the line where any of them holds a `![`:
+
+* `; b10 (lines 29-30) holds an image and is marked ignore. — writing the set's spec
+  again` — `--rounds` is 1 or more and the run has not written the spec again yet.
+  The run writes the spec again and prints `freeze`, `spec` and `coverage` a second
+  time before it reaches `validate` at all.
+* `; 2 images dropped: b10 (lines 29-30), b12 (lines 40-41)` — the rewritten spec
+  marks an image ignored too, or the run has already written the spec again. The run
+  continues to `validate`, and the set it builds holds those images in no question.
+
 ### `validate`
 
 `in2lambda.draft.report.validate` checks the draft and writes its report into the
@@ -120,7 +132,9 @@ prints one of six messages:
   ends on this line, with no `fix` line after it.
 * `ERRORS — writing the set's spec again` — the run reused a saved spec, the checks
   fault the draft, and `--rounds` is 1 or more. The run writes the spec again and
-  prints `freeze`, `spec`, `coverage` and `validate` a second time.
+  prints `freeze`, `spec`, `coverage` and `validate` a second time. A spec that marks
+  a figure ignored draws the same rewrite from the `coverage` line. A run writes the
+  spec again once, for either reason.
 * `ERRORS — left by round 2, no zip` — round 2 answered no error it was given, so the
   run ends with those errors and writes no zip.
 * `ERRORS — round limit 3 reached, no zip` — the last round of `--rounds` ran and the

in2lambda_agent/package.py

@@ -249,6 +249,89 @@ def spec_run(draft: Path, spec: Path) -> Coverage:
     return coverage
 
 
+def ignored_images(draft: Path) -> list[Finding]:
+    """The blocks a spec marked ignore whose lines hold an image.
+
+    A figure belongs to the question or part it illustrates. Mathpix writes a
+    figure as one paragraph, the image line and then its caption, so an `ignore`
+    selector matching the caption's `Figure n:` marks the image ignored and the
+    set is built without it. in2lambda's checks report nothing about an ignored
+    block, so the agent reads the ignored blocks back and reports each image.
+
+    Args:
+        draft: The draft file, after a spec has been run over it.
+
+    Returns:
+        One finding per ignored block whose lines hold a markdown image, in the
+        order the blocks appear in the sources. The message follows the wording
+        of in2lambda's own coverage findings, so that a spec-writing prompt
+        reads the same for either finding. A source whose bytes are not text —
+        a docx, frozen as itself — has no such block to report.
+    """
+    found = _frozen(draft)
+    read: dict[int, list[str] | None] = {}
+
+    def source_lines(number: int) -> list[str] | None:
+        """The lines of one source, read the first time a block of it is ignored."""
+        if number not in read:
+            path = Path(draft).parent / found["sources"][number]["source"]
+            read[number] = _source_lines(path)
+        return read[number]
+
+    dropped = []
+    for key, written in found["fields"].items():
+        if not key.endswith(".ignore"):
+            continue
+        block = key[: -len(".ignore")]
+        # `2/b3` is the second source's block; `b3` is the first source's.
+        number, _, _ = block.rpartition("/")
+        lines = source_lines(int(number) - 1 if number else 0)
+        if lines is None:
+            continue
+        ranges = written["ranges"]
+        held = "\n".join("\n".join(lines[start - 1 : end]) for start, end in ranges)
+        if "![" in held:
+            dropped.append(
+                Finding(
+                    check="ignored-image",
+                    level=ERROR,
+                    field=block,
+                    ranges=ranges,
+                    message=f"{where(block, ranges)} holds an image and is "
+                    "marked ignore.",
+                )
+            )
+    return sorted(dropped, key=lambda one: (one.field.rpartition("/")[0], one.ranges))
+
+
+def where(field: str, ranges: list[list[int]]) -> str:
+    """A block and the lines it covers, as a finding's message names one.
+
+    Args:
+        field: The block id or field key.
+        ranges: The lines it covers, as `[[start, end], ...]`.
+
+    Returns:
+        `b10 (lines 29-30)`, which is how in2lambda's own findings name one.
+    """
+    covered = ", ".join(f"{start}-{end}" for start, end in ranges)
+    return f"{field} (lines {covered})"
+
+
+def _source_lines(path: Path) -> list[str] | None:
+    """A frozen source read as text, or None where its bytes are not text.
+
+    A docx is frozen as itself and is a zip, and a tex sheet of a real set need
+    not be UTF-8, so a source is decoded the way `corpus.is_document` decodes
+    one and a source holding a NUL byte is left alone: it has no line of
+    markdown to find an image in.
+    """
+    raw = path.read_bytes()
+    if b"\x00" in raw:
+        return None
+    return raw.decode("utf-8", errors="replace").splitlines()
+
+
 def _frozen(draft: Path) -> dict[str, Any]:
     """A draft read off disk, as in2lambda writes one."""
     return json.loads(Path(draft).read_text())

in2lambda_agent/pipeline.py

@@ -16,7 +16,9 @@
 earlier sheet gets one rewrite before
 any of that, since a spec that covers the set is worth more than a field
 repaired in one sheet of it; that rewrite is layer 1, and is not one of the
-rounds.
+rounds. A spec that marks a figure ignored draws the same one rewrite, saved or
+new. The checks report nothing about an ignored block, so the coverage stage
+reports each ignored image.
 
 A run asked for a review stops once the checks are quiet: it renders the
 questions the reviewer is to see, leaves a record of them in the cache, and
@@ -208,8 +210,10 @@ def run(
     result.add_stage("ocr", message)
 
     # One pass, or two where a saved spec leaves something for the checks to
-    # find: the second writes the spec again with the report in the prompt.
+    # find, or where the spec marked a figure ignored: the second writes the
+    # spec again with the report in the prompt. One rewrite, for either cause.
     reused = saved.is_file()
+    rewritten = False
     report = package.Report(clean=False, errors=[])
     while True:
         draft = result.draft = package.source_add(
@@ -263,7 +267,37 @@ def run(
                 else:
                     saved.write_text(replaced, encoding="utf-8")
             raise
-        result.add_stage("coverage", str(result.coverage))
+        # in2lambda's checks report nothing about an ignored block, so a spec
+        # that marks a figure ignored builds a set without that image and the
+        # `validate` stage reports nothing. The coverage stage reports it and
+        # asks for the one rewrite.
+        dropped = package.ignored_images(draft)
+        if dropped and not rewritten and rounds >= 1:
+            said = "; ".join(one.message for one in dropped)
+            result.add_stage(
+                "coverage",
+                f"{result.coverage}; {said} — writing the set's spec again",
+            )
+            report = package.Report(
+                clean=False,
+                errors=[one.message for one in dropped],
+                findings=dropped,
+            )
+            reused = False
+            rewritten = True
+            continue
+        if dropped:
+            # The rewritten spec marks an image ignored too, or the run has
+            # already written the spec again. The run continues to `validate`,
+            # and the line names each image the set will not hold.
+            where = ", ".join(package.where(one.field, one.ranges) for one in dropped)
+            images = "image" if len(dropped) == 1 else "images"
+            result.add_stage(
+                "coverage",
+                f"{result.coverage}; {len(dropped)} {images} dropped: {where}",
+            )
+        else:
+            result.add_stage("coverage", str(result.coverage))
 
         report = package.validate(draft)
         if report.clean:
@@ -273,6 +307,7 @@ def run(
         if reused and rounds >= 1:
             result.add_stage("validate", f"{errors} — writing the set's spec again")
             reused = False
+            rewritten = True
             continue
         result.add_stage("validate", errors)
         break

in2lambda_agent/spec.py

@@ -63,10 +63,15 @@
 the question stems are paragraphs too, the question selector needs a constraint
 that the solutions fail.
 
-Three things about blocks to write selectors against:
+Four things about blocks to write selectors against:
 
   * One block fills one field. A question's text is the block holding its stem,
-    not the heading above it — headings usually belong in `ignore`.
+    not the heading above it: `ignore` is for headings, rubric and page
+    furniture, and those are all it is for.
+  * A paragraph holding an image — `![...](...)` — is content, never `ignore`:
+    it belongs to the question or part it illustrates. A figure is written as
+    one paragraph, the image line and then its caption, so a selector matching
+    the caption's `Figure 1:` marks the image ignored too.
   * A lettered or numbered item — `(a) ...`, `a. ...`, `1. ...` — is a
     ListItem, and its marker is not part of the text a constraint matches. The
     marker is still in the field's value, so `strip` is what takes it off.

tests/fixtures/figure-paragraph-spec.yaml

@@ -0,0 +1,5 @@
+ignore:   Para text~'Figure 1'
+question: Para text~'^[A-Z]'
+part:     ListItem
+strip:    ['^\(a\) ', '^\(b\) ']
+layout:   PartsSepSol

tests/fixtures/figure-paragraph.md

@@ -0,0 +1,8 @@
+A ball is thrown straight up at $20\,\mathrm{m/s}$.
+
+![the ball](figures/ball.png)
+Figure 1: the ball leaving the hand.
+
+(a) Find the greatest height it reaches.
+
+(b) Find its time of flight.

tests/test_package.py

@@ -98,6 +98,51 @@ def test_a_part_with_no_solution_is_a_warning_the_report_is_still_clean_for(
     assert report.warnings == [one.message for one in report.findings]
 
 
+def test_a_block_marked_ignore_whose_lines_hold_an_image_is_reported(tmp_path):
+    # The spec marks the figure paragraph ignored by matching its caption. The
+    # draft is clean, and the set built from it holds no image.
+    folder = tmp_path / "figure-paragraph"
+    folder.mkdir()
+    shutil.copy(FIXTURES / "figure-paragraph.md", folder / "figure-paragraph.md")
+    (folder / "figures").mkdir()
+    shutil.copy(FIXTURES / "ball.png", folder / "figures" / "ball.png")
+    written = package.source_add(folder / "figure-paragraph.md")
+    package.spec_run(written, FIXTURES / "figure-paragraph-spec.yaml")
+
+    (dropped,) = package.ignored_images(written)
+
+    assert (dropped.check, dropped.level) == ("ignored-image", package.ERROR)
+    assert (dropped.field, dropped.ranges) == ("b2", [[3, 4]])
+    assert dropped.message == "b2 (lines 3-4) holds an image and is marked ignore."
+
+
+def test_a_source_whose_bytes_are_not_text_has_no_ignored_image_to_read(tmp_path):
+    # A docx source is frozen as itself, so the file beside the draft is a zip.
+    # Reading it for a `![` is not what it is for, and must not end the run.
+    folder = tmp_path / "figure-paragraph"
+    folder.mkdir()
+    shutil.copy(FIXTURES / "figure-paragraph.md", folder / "figure-paragraph.md")
+    (folder / "figures").mkdir()
+    shutil.copy(FIXTURES / "ball.png", folder / "figures" / "ball.png")
+    written = package.source_add(folder / "figure-paragraph.md")
+    package.spec_run(written, FIXTURES / "figure-paragraph-spec.yaml")
+    package.frozen_source(written).write_bytes((FIXTURES / "ball.png").read_bytes())
+
+    assert package.ignored_images(written) == []
+
+
+def test_an_image_inside_a_question_is_nothing_to_report(tmp_path):
+    folder = tmp_path / "figure"
+    folder.mkdir()
+    shutil.copy(FIXTURES / "figure.md", folder / "figure.md")
+    (folder / "figures").mkdir()
+    shutil.copy(FIXTURES / "ball.png", folder / "figures" / "ball.png")
+    written = package.source_add(folder / "figure.md")
+    package.spec_run(written, FIXTURES / "sheet-spec.yaml")
+
+    assert package.ignored_images(written) == []
+
+
 def test_a_second_source_is_frozen_into_the_same_draft(tmp_path):
     folder = tmp_path / "sheets"
     folder.mkdir()

tests/test_spec.py

@@ -1,13 +1,16 @@
 """Layer 1: where a set's spec lives, and the one call that writes it."""
 
 import json
+import shutil
 from pathlib import Path
 
 import pytest
-from conftest import FakeBackend
+from conftest import PNG, FakeBackend
 
+from in2lambda_agent import pipeline
 from in2lambda_agent.model import Usage
 from in2lambda_agent.package import Coverage, Report
+from in2lambda_agent.settings import Settings
 from in2lambda_agent.spec import (
     SPEC_NAME,
     BadSpec,
@@ -16,7 +19,13 @@
     write_spec,
 )
 
-SPEC = (Path(__file__).parent / "fixtures" / "sheet-spec.yaml").read_text()
+FIXTURES = Path(__file__).parent / "fixtures"
+SPEC = (FIXTURES / "sheet-spec.yaml").read_text()
+
+# A spec that marks the figure paragraph ignored by matching its caption, as
+# the spec the model wrote for UCL_MechEng/Worksheet_2 marked every figure of
+# that sheet ignored.
+IGNORES_THE_FIGURE = (FIXTURES / "figure-paragraph-spec.yaml").read_text()
 
 
 def test_the_sets_spec_is_beside_the_source(tmp_path):
@@ -98,6 +107,35 @@ def test_a_spec_naming_something_that_is_not_a_layout_is_refused_by_name():
         write_spec("b1  1  # Sheet", backend)
 
 
+def test_a_spec_that_ignores_a_figure_is_written_again_and_the_drop_is_said(tmp_path):
+    # Both specs the model answers with mark the figure ignored. The run writes
+    # the spec again once, builds a set holding no image, and names that image
+    # on the coverage line.
+    folder = tmp_path / "figure-paragraph"
+    (folder / "figures").mkdir(parents=True)
+    shutil.copy(FIXTURES / "figure-paragraph.md", folder / "figure-paragraph.md")
+    (folder / "figures" / "ball.png").write_bytes(PNG)
+    backend = FakeBackend(IGNORES_THE_FIGURE, IGNORES_THE_FIGURE)
+
+    result = pipeline.run(
+        folder / "figure-paragraph.md",
+        out_dir=tmp_path / "out",
+        settings=Settings(),
+        backend=backend,
+    )
+
+    _, rewrite = backend.calls[1]
+    assert len(backend.calls) == 2
+    assert "b2 (lines 3-4) holds an image and is marked ignore." in rewrite
+    first, again = (one for one in result.stages if one.name == "coverage")
+    assert first.message.endswith(
+        "b2 (lines 3-4) holds an image and is marked ignore. "
+        "— writing the set's spec again"
+    )
+    assert again.message.endswith("1 image dropped: b2 (lines 3-4)")
+    assert result.zip_path.is_file()
+
+
 def test_each_run_appends_one_line_saying_what_the_spec_covered(tmp_path):
     record = tmp_path / "runs.jsonl"
     coverage = Coverage(