From c3c8be7834d90900beb32533687f2f75f0251eb5 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Mon, 21 Sep 2026 01:02:36 +0100 Subject: [PATCH 1/2] implement: Relock in2lambda and offer field set and part solution to the fix loop (t26) --- README.md | 12 -- in2lambda_agent/cli.py | 12 +- in2lambda_agent/corpus.py | 24 +-- in2lambda_agent/package.py | 22 +-- in2lambda_agent/pair.py | 129 --------------- in2lambda_agent/pipeline.py | 138 ++-------------- in2lambda_agent/spec.py | 30 +--- tests/fixtures/marked_solutions.md | 13 -- tests/fixtures/paired-spec.yaml | 10 -- tests/fixtures/paired.md | 17 -- tests/fixtures/paired_solutions.md | 11 -- tests/test_corpus.py | 59 +------ tests/test_package.py | 16 -- tests/test_pair.py | 86 ---------- tests/test_pipeline.py | 250 +---------------------------- 15 files changed, 34 insertions(+), 795 deletions(-) delete mode 100644 in2lambda_agent/pair.py delete mode 100644 tests/fixtures/marked_solutions.md delete mode 100644 tests/fixtures/paired-spec.yaml delete mode 100644 tests/fixtures/paired.md delete mode 100644 tests/fixtures/paired_solutions.md delete mode 100644 tests/test_pair.py diff --git a/README.md b/README.md index 23b8ca2..4762694 100644 --- a/README.md +++ b/README.md @@ -65,14 +65,6 @@ agent keeps the markdown and the images under the PDF's hash in `--cache` (defau `--fresh-ocr` converts the PDF again and restarts the run from the new markdown. `--out` defaults to `./out`, where in2lambda writes the set's JSON folder and its zip. -A sheet whose solutions are a document of their own is one run. The solutions file is -the one beside `SOURCE` whose name is the sheet's with `_solutions`, `-solutions` or -` Solutions` after it, in any case, and whose suffix is the same: `Worksheet_1.pdf` -and `Worksheet_1_solutions.pdf`. Both are frozen into the one draft, the questions -first and the solutions second, and the run is named after the questions file whether -you name that file or the solutions one. A solutions file with no questions file -beside it stops the run, which exits 1 saying `solutions without questions`. - The run writes a spec — the YAML selectors naming which blocks of the source are questions, parts and solutions — in one model call, and saves it as `in2lambda-spec.yaml` beside `SOURCE`. A folder of sheets is one document set and @@ -261,10 +253,6 @@ all of it. `--suffix` is repeatable and defaults to `tex`, `md` and `docx`; `--s pdf` runs the PDFs too, which needs Mathpix credentials and one call per PDF. Every run is review mode `none`. -A sheet and the solutions file beside it are one run and one row, named after the -questions file. A solutions file with no questions file beside it is a `skipped` row -with the reason `solutions without questions`. - The sweep never writes to the corpus. It copies each set's folder into `--work` (default `./.in2lambda-agent/corpus`), empties that copy first, and runs the documents there. It keeps the sets' specs in `--specs` (default `./corpus-specs`) in a tree diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 7664e72..139d35b 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -11,7 +11,6 @@ from in2lambda_agent.model import ModelUnavailable, choose_backend from in2lambda_agent.ocr import ocr_pdf from in2lambda_agent.package import CommandRefused, SpecRejected -from in2lambda_agent.pair import SolutionsWithoutQuestions from in2lambda_agent.review import ReviewError from in2lambda_agent.settings import load_settings from in2lambda_agent.spec import BadSpec @@ -78,12 +77,7 @@ def build_parser() -> argparse.ArgumentParser: subcommands = parser.add_subparsers(dest="command", required=True) run = subcommands.add_parser("run", help="Convert SOURCE into a set.") - run.add_argument( - "source", - type=Path, - help="The question file to convert. A solutions file beside it, named " - "after it, is frozen with it.", - ) + run.add_argument("source", type=Path, help="The question file to convert.") run.add_argument( "--spec", type=Path, @@ -359,12 +353,10 @@ def main(argv: Optional[Sequence[str]] = None) -> int: SpecRejected, ReviewError, CommandRefused, - SolutionsWithoutQuestions, ) as error: # Missing credentials among them: the message names the variables, or # the login to run, or what a spec says that a spec cannot say, or the - # question a review command names that is not under review, or the - # questions file a solutions file was run without. + # question a review command names that is not under review. print(f"in2lambda-agent: {error}", file=sys.stderr) return 1 diff --git a/in2lambda_agent/corpus.py b/in2lambda_agent/corpus.py index 494d19c..95fb16e 100644 --- a/in2lambda_agent/corpus.py +++ b/in2lambda_agent/corpus.py @@ -6,9 +6,6 @@ is one run of the pipeline per document and one row of a table per run, written where two runs can be diffed against each other. -A questions document and the solutions document beside it are one run and one -row, which is the questions document's. - Nothing here writes into the corpus. A run leaves a draft beside its source, and a spec and a record beside that, so each set's folder is copied into a work directory and run there, and the specs are kept in a tree of their own @@ -24,7 +21,7 @@ from pathlib import Path from typing import Optional, Sequence -from in2lambda_agent import package, pair, pipeline +from in2lambda_agent import package, pipeline from in2lambda_agent.model import Backend, ModelUnavailable from in2lambda_agent.package import SpecRejected from in2lambda_agent.settings import Settings @@ -60,8 +57,7 @@ class Row: outcome: `built`, `build refused` where the checks came clean and in2lambda still would not write the set out, `faulted` for a draft the checks still fault and no zip, `skipped` for a file that is not - a document and for a solutions document with no questions document - beside it, `no spec` for a replay with nothing saved to replay, + a document, `no spec` for a replay with nothing saved to replay, `no model`, `spec rejected`, `bad spec`, or `error: `. reason: What the run had to say for itself, in the words of whatever said it: the refusal, the first error the checks were still finding, @@ -425,22 +421,6 @@ def sweep( print(f"{row.outcome:<20} {row.source}") rows.append(row) continue - # A solutions document is frozen into the run of the questions document - # it answers, so the pair is one row, which is the questions document's. - # One with no questions document beside it has no questions to attach - # its solutions to, and is a row of its own saying so. - if pair.questions_stem(document) is not None: - if pair.questions_beside(document) is not None: - continue - row = Row( - source=relative.as_posix(), - set=relative.parent.as_posix(), - outcome="skipped", - reason="solutions without questions", - ) - print(f"{row.outcome:<20} {row.source}") - rows.append(row) - continue # Staging is per set and the row is per document, so the guard is here # rather than in `run_one`: what a copy raises — an unreadable folder, a # file that goes while it is being read — is a row for every sheet of diff --git a/in2lambda_agent/package.py b/in2lambda_agent/package.py index 3fcd69f..43f4e9a 100644 --- a/in2lambda_agent/package.py +++ b/in2lambda_agent/package.py @@ -168,29 +168,25 @@ class Report: findings: list[Finding] = field(default_factory=list) -def source_add(source: Path, *more: Path) -> Path: - """Freezes one or more source documents into the draft beside the first. +def source_add(source: Path) -> Path: + """Freezes a source document and returns the draft written beside it. Always from the beginning: the agent's run owns the draft it writes, so a second run over the same file is a second run and not a continuation of - the first one's fields. + the first one's fields. One document: a draft can hold a second source — + the solutions written separately — and nothing the agent does asks for one. Args: - source: The markdown, tex or docx file to freeze. The draft is named - after it, and its blocks are `b1` onwards. - more: Further documents to freeze into the same draft, in the order - they are to be numbered: a sheet's solutions written as a file of - their own. The blocks of the second source are `2/b1` onwards. + source: The markdown, tex or docx file to freeze. Returns: The `FILE.draft.json` that was written, which every command below is given. Raises: - SourceError: pandoc or panflute is missing, the files are not all in - one directory, or a file cannot be read. + SourceError: pandoc or panflute is missing, or the file cannot be read. """ - return in2lambda.source.add([str(source), *(str(one) for one in more)], True) + return in2lambda.source.add([str(source)], True) def source_show(draft: Path) -> str: @@ -200,9 +196,7 @@ def source_show(draft: Path) -> str: draft: The draft file. Returns: - What the model is shown to write a spec from. A draft of two sources - heads each with `Source N: NAME`, and the ids of the second source's - blocks carry its number: `2/b1`. + What the model is shown to write a spec from. Raises: SourceError: there is no draft there, or its source has moved on. diff --git a/in2lambda_agent/pair.py b/in2lambda_agent/pair.py deleted file mode 100644 index 1e5c25a..0000000 --- a/in2lambda_agent/pair.py +++ /dev/null @@ -1,129 +0,0 @@ -"""Which document holds the solutions to which, going by the file names. - -A folder of worksheets often writes the questions and the solutions as separate -documents: `Worksheet_1.pdf` beside `Worksheet_1_solutions.pdf`. A solutions -document alone holds no questions for its solutions to answer, so the run -freezes the two into one draft, the questions first and the solutions second, -and is named after the questions document. - -The pairing is by name. A solutions document is one whose stem ends in -`solutions` after a space, an underscore or a hyphen, in any case. Its questions -document is the file beside it whose stem is the stem before that ending and -whose suffix is the same. -""" - -import re -from pathlib import Path -from typing import Optional - -SOLUTIONS = re.compile(r"^(?P.+?)[ _-]solutions$", re.IGNORECASE) -"""A solutions document's stem, and the questions document's stem within it. - -The separator is required, so `resolutions.pdf` is not a solutions document and -`Solutions.pdf` names no questions document. -""" - - -class SolutionsWithoutQuestions(ValueError): - """A solutions document has no questions document beside it.""" - - -def questions_stem(document: Path) -> Optional[str]: - """The stem of the questions document a solutions document answers. - - Args: - document: Any file. - - Returns: - The stem before the `solutions` ending, or None where the name has no - such ending. - """ - found = SOLUTIONS.match(Path(document).stem) - return found.group("stem") if found else None - - -def questions_beside(solutions: Path) -> Optional[Path]: - """The questions document of a solutions document, in the same folder. - - Args: - solutions: A solutions document, as `questions_stem` reads one. - - Returns: - The file with that stem and the same suffix, or None where the folder - holds no such file. The suffix is compared without regard to case. - """ - stem = questions_stem(solutions) - if stem is None: - return None - solutions = Path(solutions) - return next( - ( - path - for path in _files_in(solutions.parent) - if path.stem == stem and path.suffix.lower() == solutions.suffix.lower() - ), - None, - ) - - -def solutions_beside(questions: Path) -> Optional[Path]: - """The solutions document of a questions document, in the same folder. - - Args: - questions: A document that is not itself a solutions document. - - Returns: - The first file, in name order, whose `questions_stem` is this - document's stem and whose suffix is the same, or None where the folder - holds no such file. - """ - questions = Path(questions) - return next( - ( - path - for path in _files_in(questions.parent) - if path.suffix.lower() == questions.suffix.lower() - and questions_stem(path) == questions.stem - ), - None, - ) - - -def _files_in(folder: Path) -> list[Path]: - """The files of a folder, in name order, and none where there is no folder. - - The pairing is the first thing a run does, before anything has read the - source, so it is where a mistyped path arrives first. It has nothing to say - about one: a source that is not there is in2lambda's to complain about, in - the words it uses for every file it cannot read. - """ - if not folder.is_dir(): - return [] - return sorted(path for path in folder.iterdir() if path.is_file()) - - -def of(source: Path) -> tuple[Path, Optional[Path]]: - """The two documents a run freezes, whichever of them the user named. - - Args: - source: The document the user named, the questions or the solutions. - - Returns: - The questions document, and the solutions document to freeze after it, - or None where the folder holds no solutions document for it. - - Raises: - SolutionsWithoutQuestions: `source` is a solutions document and the - folder holds no questions document for it. - """ - source = Path(source) - stem = questions_stem(source) - if stem is None: - return source, solutions_beside(source) - questions = questions_beside(source) - if questions is None: - raise SolutionsWithoutQuestions( - f"solutions without questions: nothing named {stem}{source.suffix} " - f"beside {source.name}" - ) - return questions, source diff --git a/in2lambda_agent/pipeline.py b/in2lambda_agent/pipeline.py index fe721d1..564e517 100644 --- a/in2lambda_agent/pipeline.py +++ b/in2lambda_agent/pipeline.py @@ -26,17 +26,15 @@ """ import random -import re -import shutil from dataclasses import dataclass, field from pathlib import Path from typing import Callable, Optional -from in2lambda_agent import package, pair +from in2lambda_agent import package from in2lambda_agent.fix import RoundResult, fix_round, summary, unrepaired from in2lambda_agent.mathpix import MathpixClient from in2lambda_agent.model import Backend, ModelUnavailable, Usage, choose_backend -from in2lambda_agent.ocr import MEDIA_NAME, ocr_pdf +from in2lambda_agent.ocr import ocr_pdf from in2lambda_agent.review import RECORD, Question, Review, choose from in2lambda_agent.settings import Settings from in2lambda_agent.spec import RECORD_NAME, record_run, spec_path, write_spec @@ -44,11 +42,6 @@ # Where the OCR of each PDF is kept, under the directory the user ran from. DEFAULT_CACHE_DIR = Path(".in2lambda-agent") -# A markdown image whose file is in the OCR's media folder, as far as the folder -# name: `![a plot](media/plot.png)`. What a second source's images are renamed -# by when they are copied beside the first source's. -MEDIA_REFERENCE = re.compile(rf"(!\[[^\]]*\]\(){MEDIA_NAME}/") - REVIEW_MODES = ("none", "sample", "per-question") @@ -127,8 +120,7 @@ def run( """Drives in2lambda over one source file. Args: - source: The question file to convert, or the solutions file beside it, - which runs the questions file it answers. + source: The question file to convert. out_dir: Where in2lambda writes the set's JSON folder and zip. settings: The environment the run has available. spec: The set's spec file, when it is not the one beside the source. @@ -160,8 +152,6 @@ def run( BadSpec: If what the model answers with is not a spec. SpecRejected: If in2lambda will not run the spec. SourceError: If in2lambda cannot freeze or check the source. - SolutionsWithoutQuestions: If `source` is a solutions file and no - questions file is beside it. """ # A relative --out means the directory the user ran from, whatever in2lambda # does with the working directory along the way. @@ -169,11 +159,6 @@ def run( out_dir = Path(out_dir).resolve() cache_dir = Path(cache_dir).resolve() - # A sheet whose solutions are written as a file of their own is one run and - # one draft, named after the questions file. So the run is the questions - # file's from here on, whichever of the two the user named. - source, solutions = pair.of(source) - # The set is the folder the user's file is in, so this is settled before # OCR moves a PDF's markdown off into the cache. saved = spec_path(source, spec) @@ -181,30 +166,19 @@ def run( result = RunResult(on_stage=on_stage) # The rest of the pipeline reads markdown, so a PDF becomes markdown first. - frozen, _, message = _markdown( - source, cache_dir=cache_dir, settings=settings, mathpix=mathpix, fresh=fresh_ocr - ) - frozen_solutions = None - if solutions is not None: - frozen_solutions, solutions_media, said = _markdown( - solutions, - cache_dir=cache_dir, - settings=settings, - mathpix=mathpix, - fresh=fresh_ocr, + if source.suffix.lower() == ".pdf": + client = mathpix or MathpixClient.from_settings(settings) + ocr = ocr_pdf(source, cache_dir=cache_dir, client=client, fresh=fresh_ocr) + frozen = ocr.markdown + # A fresh pass is a restart: every stage below reads the new markdown. + message = ( + f"fresh pass, restarting from {frozen}" + if ocr.fresh + else f"cached {frozen}" ) - message += f"; {said}" - # in2lambda freezes into one draft the documents of one directory, and - # the OCR of each PDF is cached in an entry named after its own hash. So - # the solutions markdown, and the images it refers to, are copied beside - # the questions markdown. - if frozen_solutions.parent != frozen.parent: - frozen_solutions = _copy_beside( - frozen_solutions, - solutions_media, - name=solutions.stem, - into=frozen.parent, - ) + else: + frozen = source + message = f"not needed for {source.name}" result.add_stage("ocr", message) # One pass, or two where a saved spec leaves something for the checks to @@ -212,15 +186,8 @@ def run( reused = saved.is_file() report = package.Report(clean=False, errors=[]) while True: - draft = result.draft = package.source_add( - frozen, *([frozen_solutions] if frozen_solutions is not None else []) - ) - result.add_stage( - "freeze", - f"{draft}, with {solutions.name} as source 2" - if solutions is not None - else str(draft), - ) + draft = result.draft = package.source_add(frozen) + result.add_stage("freeze", str(draft)) # What the set's spec said before this pass wrote over it, where it # said anything: a rewrite in2lambda then refuses puts it back. @@ -235,7 +202,6 @@ def run( package.source_show(draft), backend, report if report.errors else None, - sources=2 if frozen_solutions is not None else 1, ) if saved.is_file(): replaced = saved.read_text(encoding="utf-8") @@ -498,76 +464,6 @@ def resume( return result -def _markdown( - document: Path, - *, - cache_dir: Path, - settings: Settings, - mathpix: Optional[MathpixClient], - fresh: bool, -) -> tuple[Path, Optional[Path], str]: - """The markdown a document is frozen from, and the OCR stage's line for it. - - Args: - document: The file the user named, or the solutions file beside it. - cache_dir: Where the OCR of each PDF is kept. - settings: The environment the run has available. - mathpix: The client to convert with, built from the settings if absent. - fresh: Convert a PDF again even if it is already cached. - - Returns: - The markdown, which is the document itself where it is not a PDF, the - folder holding the images it refers to where OCR wrote any, and what - the OCR stage says about it. - - Raises: - MathpixError: If the PDF cannot be converted. - """ - if document.suffix.lower() != ".pdf": - return document, None, f"not needed for {document.name}" - client = mathpix or MathpixClient.from_settings(settings) - ocr = ocr_pdf(document, cache_dir=cache_dir, client=client, fresh=fresh) - # A fresh pass is a restart: every stage below reads the new markdown. - if ocr.fresh: - return ocr.markdown, ocr.media, f"fresh pass, restarting from {ocr.markdown}" - return ocr.markdown, ocr.media, f"cached {ocr.markdown}" - - -def _copy_beside( - markdown: Path, media: Optional[Path], *, name: str, into: Path -) -> Path: - """Copies a second source, and the images it refers to, beside the first. - - The images cannot come across under the name their folder has, because each - PDF's OCR calls its own folder `media` and two sheets can each hold a - `plot.png`: the second copy would be the first one gone. So they arrive in a - folder named after their document, and the references in the copy are - rewritten to it. in2lambda resolves a reference from the folder the draft is - in, which is the folder copied into, so any name does. - - Args: - markdown: The markdown to copy. - media: The folder its images are in, where the document has any. - name: What the copies are named after: the solutions document's stem. - into: The folder the first source's markdown is in. - - Returns: - The copy, which is what the draft freezes as its second source. - """ - folder = f"{name}-{MEDIA_NAME}" - copied = into / f"{name}.md" - copied.write_text( - MEDIA_REFERENCE.sub( - lambda found: f"{found.group(1)}{folder}/", - markdown.read_text(encoding="utf-8"), - ), - encoding="utf-8", - ) - if media is not None and media.is_dir(): - shutil.copytree(media, into / folder, dirs_exist_ok=True) - return copied - - def _fix_rounds( draft: Path, report: package.Report, diff --git a/in2lambda_agent/spec.py b/in2lambda_agent/spec.py index 2269fd2..07fcd07 100644 --- a/in2lambda_agent/spec.py +++ b/in2lambda_agent/spec.py @@ -73,20 +73,6 @@ * A block indented under a list item is inside it, not beside it: such a question and its parts are one block, and there is nothing to select. -A draft holds two documents where the solutions are written as a file of their -own. The questions file is the first source, with block ids `b1` onwards, and -the solutions file is the second, with ids `2/b1` onwards. The same selectors -run over both. In the solutions file, every block `part` or `solution` matches is -a solution, and the solutions answer the questions of the first source in order: -each question's parts, or the question itself where it has none. A block -`question` matches there is a marker — the `Q2.` or `## Question 2` written above -the solutions to the second question. A marker is no question of its own, and its -text reaches no field. The first marker in the file assigns the solutions after -it to the first question, the second marker to the second question, so a -`question` selector that matches the marker above one question's solutions and -not the marker above another's assigns every solution after it to the wrong -question. `layout` describes the questions file alone. - The layout says which question or part a solution answers: PartsSepSol every solution together at the end, in part order: each @@ -134,10 +120,7 @@ def spec_path(source: Path, spec: Optional[Path] = None) -> Path: def write_spec( - shown: str, - backend: Backend, - report: Optional[Report] = None, - sources: int = 1, + shown: str, backend: Backend, report: Optional[Report] = None ) -> tuple[str, Reply]: """Writes a spec for a source, in one model call with no tools. @@ -146,8 +129,6 @@ def write_spec( backend: The backend to call, already known to be available. report: What the checks found about the draft a previous spec made, where this is the rewrite that follows a dirty validate. - sources: How many documents the draft holds: 2 where the solutions are - a file of their own, which the prompt then says before the source. Returns: The spec, and the reply it came in. @@ -156,14 +137,7 @@ def write_spec( BadSpec: the reply is not YAML, is not a mapping, or names no layout or one that is not a layout. """ - prompt = "" - if sources > 1: - prompt = ( - "The draft holds two documents: the questions file, whose blocks " - "are `b1` onwards, and its solutions file, whose blocks are `2/b1` " - "onwards. Every solution is in the second.\n\n" - ) - prompt += f"Here is the source, one line each with its block id:\n\n{shown}\n" + prompt = f"Here is the source, one line each with its block id:\n\n{shown}\n" if report is not None: prompt += ( "\nA previous spec for this set left the draft with this to answer " diff --git a/tests/fixtures/marked_solutions.md b/tests/fixtures/marked_solutions.md deleted file mode 100644 index 4b9549e..0000000 --- a/tests/fixtures/marked_solutions.md +++ /dev/null @@ -1,13 +0,0 @@ -# Tutorial Sheet 3: answers - -## Solutions - -Q1. - -(a) $\omega = v / r$ - -Q2. - -(a) $T = 2\pi\sqrt{m/k}$ - -(b) $v = A\sqrt{k/m}$ diff --git a/tests/fixtures/paired-spec.yaml b/tests/fixtures/paired-spec.yaml deleted file mode 100644 index 0dae92a..0000000 --- a/tests/fixtures/paired-spec.yaml +++ /dev/null @@ -1,10 +0,0 @@ -# The spec the fake backend answers with for a sheet whose solutions are a file -# of their own. The same selectors run over both sources: the questions file -# holds no `## Solutions` heading, so nothing in it is a solution, and every -# solution comes from the second source. -ignore: Header -question: Para text~'^[A-Z]' -part: ListItem -solution: after Header text=Solutions, Para -strip: ['^\([a-z]\) ', '^\d+\([a-z]\) '] -layout: PartsSepSol diff --git a/tests/fixtures/paired.md b/tests/fixtures/paired.md deleted file mode 100644 index 7cf326b..0000000 --- a/tests/fixtures/paired.md +++ /dev/null @@ -1,17 +0,0 @@ -# Tutorial Sheet 3 - -## Question 1 - -A cylinder of radius $r$ rolls along the ground without slipping. - -(a) Find its angular velocity at speed $v$. - -(b) Find its kinetic energy. - -## Question 2 - -A spring of stiffness $k$ carries a mass $m$. - -(a) Find the period of small oscillations. - -(b) Find the greatest speed of the mass in an oscillation of amplitude $A$. diff --git a/tests/fixtures/paired_solutions.md b/tests/fixtures/paired_solutions.md deleted file mode 100644 index 02f3d19..0000000 --- a/tests/fixtures/paired_solutions.md +++ /dev/null @@ -1,11 +0,0 @@ -# Tutorial Sheet 3: answers - -## Solutions - -1(a) $\omega = v / r$ - -1(b) $E = \tfrac{3}{4} m v^2$ - -2(a) $T = 2\pi\sqrt{m/k}$ - -2(b) $v = A\sqrt{k/m}$ diff --git a/tests/test_corpus.py b/tests/test_corpus.py index 38d2f46..70a1376 100644 --- a/tests/test_corpus.py +++ b/tests/test_corpus.py @@ -1,13 +1,12 @@ """The sweep over a corpus: one row per document, and a replay with no model in it.""" -import json import shutil from dataclasses import asdict from pathlib import Path import pytest from conftest import FakeBackend -from test_pipeline import FAULTY_SPEC, FIXES, PAIRED_SPEC, SPEC, TEX_SPEC +from test_pipeline import FAULTY_SPEC, FIXES, SPEC, TEX_SPEC from in2lambda_agent import corpus, pipeline from in2lambda_agent.settings import Settings @@ -460,59 +459,3 @@ def without_clocks(row): for key, value in asdict(row).items() if key not in ("model_seconds", "wall_seconds") } - - -def test_a_sheet_and_its_solutions_file_are_one_row(tmp_path): - # UCL_MechEng writes each worksheet as two documents: Worksheet_1.pdf and - # Worksheet_1_solutions.pdf beside it. - root = tmp_path / "corpus" - folder = root / "worksheets" - folder.mkdir(parents=True) - shutil.copy(FIXTURES / "paired.md", folder / "Sheet_3.md") - shutil.copy(FIXTURES / "paired_solutions.md", folder / "Sheet_3_solutions.md") - backend = FakeBackend(PAIRED_SPEC) - - rows = sweep(root, tmp_path, backend=backend) - - assert [(row.source, row.set, row.outcome) for row in rows] == [ - ("worksheets/Sheet_3.md", "worksheets", "built") - ] - assert len(backend.calls) == 1 - # The solutions file was frozen as the draft's second source, so the four - # part solutions came out of it. - draft = corpus.package.draft_of(tmp_path / "work" / "worksheets" / "Sheet_3.md") - frozen = json.loads(draft.read_text()) - assert [one["source"] for one in frozen["sources"]] == [ - "Sheet_3.md", - "Sheet_3_solutions.md", - ] - assert [ - key - for key, written in frozen["fields"].items() - if written.get("source") == 2 and not key.endswith(".ignore") - ] == [ - "q1.p1.solution", - "q1.p2.solution", - "q2.p1.solution", - "q2.p2.solution", - ] - - -def test_solutions_with_no_questions_beside_them_are_skipped(tmp_path): - root = tmp_path / "corpus" - folder = root / "worksheets" - folder.mkdir(parents=True) - shutil.copy( - FIXTURES / "paired_solutions.md", folder / "Tutorial_2_Solutions.md" - ) - backend = FakeBackend(PAIRED_SPEC) - - rows = sweep(root, tmp_path, backend=backend) - - # `skipped` is an outcome the command exits 0 on, as a drawing's row is. - assert [(row.source, row.outcome, row.reason) for row in rows] == [ - ("worksheets/Tutorial_2_Solutions.md", "skipped", "solutions without questions") - ] - # Nothing was frozen or called on its account. - assert len(backend.calls) == 0 - assert not (tmp_path / "work" / "worksheets").exists() diff --git a/tests/test_package.py b/tests/test_package.py index 64ed67f..94b2478 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -98,22 +98,6 @@ 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_second_source_is_frozen_into_the_same_draft(tmp_path): - folder = tmp_path / "sheets" - folder.mkdir() - for name in ("paired.md", "paired_solutions.md"): - shutil.copy(FIXTURES / name, folder / name) - - written = package.source_add(folder / "paired.md", folder / "paired_solutions.md") - - assert written == folder / "paired.draft.json" - frozen = json.loads(written.read_text())["sources"] - assert [one["source"] for one in frozen] == ["paired.md", "paired_solutions.md"] - # The second source's blocks carry its number, which is how a command and a - # spec-writing prompt address them. - assert all(one["id"].startswith("2/") for one in frozen[1]["blocks"]) - - def test_the_warnings_a_build_says_are_returned_rather_than_printed(tmp_path): # The same sheet without its solutions: in2lambda builds it and warns about # each unanswered part as it goes. Those are the messages the validate line diff --git a/tests/test_pair.py b/tests/test_pair.py deleted file mode 100644 index a7ed312..0000000 --- a/tests/test_pair.py +++ /dev/null @@ -1,86 +0,0 @@ -"""Reading a solutions document's questions document off the file names.""" - -from pathlib import Path - -import pytest - -from in2lambda_agent import pair - - -@pytest.mark.parametrize( - "name, stem", - [ - ("Worksheet_1_solutions.pdf", "Worksheet_1"), - ("Tutorial_2_Solutions.pdf", "Tutorial_2"), - ("sheet-solutions.md", "sheet"), - ("Sheet 1 Solutions.docx", "Sheet 1"), - # No separator before `solutions`, so the word is part of a longer one. - ("resolutions.pdf", None), - # Nothing before the separator, so there is no stem to pair with. - ("Solutions.pdf", None), - ("Worksheet_1.pdf", None), - ], -) -def test_a_solutions_document_is_read_off_its_name(name, stem): - assert pair.questions_stem(Path("sheets") / name) == stem - - -def test_the_questions_document_is_the_one_beside_it(tmp_path): - (tmp_path / "Worksheet_1.pdf").write_bytes(b"%PDF") - solutions = tmp_path / "Worksheet_1_solutions.pdf" - solutions.write_bytes(b"%PDF") - - assert pair.questions_beside(solutions) == tmp_path / "Worksheet_1.pdf" - assert pair.solutions_beside(tmp_path / "Worksheet_1.pdf") == solutions - - -def test_a_file_of_another_suffix_is_not_the_questions_document(tmp_path): - # The pairing is within one suffix: a tex sheet does not answer to the - # solutions someone scanned. - (tmp_path / "Worksheet_1.tex").write_text("\\begin{document}\\end{document}") - solutions = tmp_path / "Worksheet_1_solutions.pdf" - solutions.write_bytes(b"%PDF") - - assert pair.questions_beside(solutions) is None - assert pair.solutions_beside(tmp_path / "Worksheet_1.tex") is None - - -def test_a_document_with_no_solutions_beside_it_is_run_alone(tmp_path): - questions = tmp_path / "Worksheet_1.md" - questions.write_text("# Sheet\n") - - assert pair.of(questions) == (questions, None) - - -def test_naming_the_solutions_document_runs_the_questions_document(tmp_path): - questions = tmp_path / "Worksheet_1.md" - questions.write_text("# Sheet\n") - solutions = tmp_path / "Worksheet_1_solutions.md" - solutions.write_text("# Solutions\n") - - assert pair.of(solutions) == (questions, solutions) - assert pair.of(questions) == (questions, solutions) - - -def test_a_folder_that_is_not_there_holds_no_companion(tmp_path): - # The pairing is the first thing a run does, so a mistyped path reaches it - # before anything has read the source. It leaves the complaining to - # in2lambda rather than raising an OSError of its own here. - absent = tmp_path / "nope" - - assert pair.questions_beside(absent / "Worksheet_1_solutions.pdf") is None - assert pair.solutions_beside(absent / "Worksheet_1.pdf") is None - assert pair.of(absent / "Worksheet_1.pdf") == (absent / "Worksheet_1.pdf", None) - with pytest.raises(pair.SolutionsWithoutQuestions): - pair.of(absent / "Worksheet_1_solutions.pdf") - - -def test_solutions_with_no_questions_say_what_is_missing(tmp_path): - solutions = tmp_path / "Tutorial_2_Solutions.pdf" - solutions.write_bytes(b"%PDF") - - with pytest.raises( - pair.SolutionsWithoutQuestions, - match="solutions without questions: nothing named Tutorial_2.pdf", - ): - pair.of(solutions) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index dc8fc32..f0d20bc 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -9,10 +9,10 @@ import in2lambda.draft import pytest -from conftest import PNG, FakeBackend, FakeMathpix +from conftest import FakeBackend, FakeMathpix from in2lambda.validation.pdf import missing_tools -from in2lambda_agent import package, pair, pipeline +from in2lambda_agent import package, pipeline from in2lambda_agent.cli import main from in2lambda_agent.model import ModelUnavailable from in2lambda_agent.package import SpecRejected @@ -23,7 +23,6 @@ FIXTURES = Path(__file__).parent / "fixtures" SOURCE = FIXTURES / "sheet.md" SPEC = (FIXTURES / "sheet-spec.yaml").read_text() -PAIRED_SPEC = (FIXTURES / "paired-spec.yaml").read_text() TEX_SPEC = (FIXTURES / "tex-sheet-spec.yaml").read_text() FAULTY_SPEC = (FIXTURES / "faulty-spec.yaml").read_text() @@ -146,60 +145,6 @@ def figures(tmp_path): return folder -@pytest.fixture -def paired(tmp_path): - """A sheet whose solutions are written as a file of their own beside it.""" - folder = tmp_path / "paired" - folder.mkdir() - for name in ("paired.md", "paired_solutions.md"): - shutil.copy(FIXTURES / name, folder / name) - return folder - - -@pytest.fixture -def marked(tmp_path): - """A pair whose solutions file writes a marker above each group of solutions. - - A solutions document written to stand on its own repeats the question it is - answering, so the `question` selector matches in the second source as well - as the first. `marked_solutions.md` leaves question 1's part (b) - unanswered, so the marker `Q2.` is what sends the solutions under it to - question 2 rather than to question 1's remaining part. - """ - folder = tmp_path / "marked" - folder.mkdir() - shutil.copy(FIXTURES / "paired.md", folder / "marked.md") - shutil.copy(FIXTURES / "marked_solutions.md", folder / "marked_solutions.md") - return folder - - -class PairedMathpix: - """A Mathpix client answering with the fixture each PDF is named after. - - Each conversion holds a figure, as a scanned sheet does, and both call it - `media/plot.png`: the pair of names that must not become one file when the - solutions are copied beside the questions. - """ - - def __init__(self): - self.calls: list[Path] = [] - - def convert(self, pdf: Path, media_dir: Path) -> str: - self.calls.append(Path(pdf)) - media_dir.mkdir(parents=True, exist_ok=True) - (media_dir / "plot.png").write_bytes(PNG) - lines = (FIXTURES / f"{Path(pdf).stem}.md").read_text().splitlines() - # Inside the first paragraph, which is a field of the draft: an image of - # a block the spec assigns to nothing is a coverage error, not a figure. - first = next( - number - for number, line in enumerate(lines) - if line and not line.startswith("#") - ) - lines[first] += " ![a plot](media/plot.png)" - return "\n".join(lines) + "\n" - - def test_one_model_call_writes_the_sets_spec_and_the_run_builds(sheets, tmp_path): backend = FakeBackend(SPEC) @@ -1596,197 +1541,6 @@ def test_a_source_beside_its_figures_builds_with_the_images_in_media( assert "media/ball.png" in zipfile.ZipFile(result.zip_path).namelist() -def test_a_sheet_and_its_solutions_are_frozen_into_one_draft(paired, tmp_path): - result = pipeline.run( - paired / "paired.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=FakeBackend(PAIRED_SPEC), - ) - frozen = json.loads(result.draft.read_text()) - - assert result.draft == drafted(paired, "paired.md") - assert [one["source"] for one in frozen["sources"]] == [ - "paired.md", - "paired_solutions.md", - ] - # Every solution was quoted out of the second source, and every question's - # own text out of the first. - assert { - key: field.get("source", 1) - for key, field in frozen["fields"].items() - if not key.endswith(".ignore") - } == { - "q1.text": 1, - "q1.p1.text": 1, - "q1.p2.text": 1, - "q2.text": 1, - "q2.p1.text": 1, - "q2.p2.text": 1, - "q1.p1.solution": 2, - "q1.p2.solution": 2, - "q2.p1.solution": 2, - "q2.p2.solution": 2, - } - assert result.zip_path is not None and result.zip_path.exists() - - -def test_the_zip_quotes_the_part_solutions_from_the_solutions_file(paired, tmp_path): - result = pipeline.run( - paired / "paired.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=FakeBackend(PAIRED_SPEC), - ) - - question = json.loads( - zipfile.ZipFile(result.zip_path).read("question_000_Question_1.json") - ) - assert [part["workedSolution"]["content"] for part in question["parts"]] == [ - "$\\omega = v / r$", - "$E = \\tfrac{3}{4} m v^2$", - ] - - -def test_a_marker_in_the_solutions_file_writes_no_question(marked, tmp_path): - result = pipeline.run( - marked / "marked.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=FakeBackend(PAIRED_SPEC), - ) - fields = json.loads(result.draft.read_text())["fields"] - - # `Q1.` and `Q2.` match the `question` selector in the second source. The - # draft holds the two questions the first source states, and neither marker - # is a field. - assert sorted(key for key in fields if key.endswith(".text")) == [ - "q1.p1.text", - "q1.p2.text", - "q1.text", - "q2.p1.text", - "q2.p2.text", - "q2.text", - ] - # Each marker is ignored, so its text reaches no question of the set. - assert fields["2/b3.ignore"]["value"] is True - assert fields["2/b5.ignore"]["value"] is True - # Each marker sends the solutions below it to its own question, so question - # 2's solutions answer question 2's parts. Question 1's part (b) is - # unanswered, which is a warning the build runs after. - assert { - key: field["source"] for key, field in fields.items() if ".solution" in key - } == { - "q1.p1.solution": 2, - "q2.p1.solution": 2, - "q2.p2.solution": 2, - } - assert result.zip_path is not None and result.zip_path.exists() - - -def test_the_spec_prompt_shows_both_sources(paired, tmp_path): - backend = FakeBackend(PAIRED_SPEC) - - pipeline.run( - paired / "paired.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=backend, - ) - (_, prompt) = backend.calls[0] - - assert "whose blocks are `2/b1` onwards" in prompt - assert "Source 1: paired.md" in prompt - assert "Source 2: paired_solutions.md" in prompt - assert "2/b3 5 1(a)" in prompt - - -def test_naming_the_solutions_file_runs_the_questions_file(paired, tmp_path): - result = pipeline.run( - paired / "paired_solutions.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=FakeBackend(PAIRED_SPEC), - ) - freeze = next(stage for stage in result.stages if stage.name == "freeze") - - assert result.draft == drafted(paired, "paired.md") - assert freeze.message == ( - f"{drafted(paired, 'paired.md')}, with paired_solutions.md as source 2" - ) - # The record is the questions file's, so the two names are one run. - (line,) = (paired / RECORD_NAME).read_text().splitlines() - assert json.loads(line)["source"] == str(paired / "paired.md") - assert result.zip_path.exists() - - -def test_solutions_with_no_questions_beside_them_stop_the_run(tmp_path): - folder = tmp_path / "lone" - folder.mkdir() - shutil.copy(FIXTURES / "paired_solutions.md", folder / "paired_solutions.md") - - with pytest.raises( - pair.SolutionsWithoutQuestions, - match="solutions without questions: nothing named paired.md", - ): - pipeline.run( - folder / "paired_solutions.md", - out_dir=tmp_path / "out", - settings=Settings(), - backend=FakeBackend(PAIRED_SPEC), - ) - - -def test_a_lone_solutions_file_exits_one_saying_so(tmp_path, capsys): - folder = tmp_path / "lone" - folder.mkdir() - shutil.copy(FIXTURES / "paired_solutions.md", folder / "paired_solutions.md") - - code = main( - ["run", str(folder / "paired_solutions.md"), "--out", str(tmp_path / "out")] - ) - printed = capsys.readouterr() - - assert code == 1 - assert "solutions without questions" in printed.err - assert printed.out == "" - - -def test_a_pair_of_pdfs_is_converted_and_frozen_into_one_draft(tmp_path): - for name in ("paired.pdf", "paired_solutions.pdf"): - (tmp_path / name).write_bytes(f"%PDF-1.4 {name}".encode()) - client = PairedMathpix() - - result = pipeline.run( - tmp_path / "paired.pdf", - out_dir=tmp_path / "out", - settings=Settings(), - cache_dir=tmp_path / "cache", - mathpix=client, - backend=FakeBackend(PAIRED_SPEC), - ) - frozen = json.loads(result.draft.read_text()) - - assert [one.name for one in client.calls] == [ - "paired.pdf", - "paired_solutions.pdf", - ] - # Each PDF has a cache entry of its own, and a draft holds the documents of - # one directory, so the solutions markdown is copied beside the questions. - assert [one["source"] for one in frozen["sources"]] == [ - "source.md", - "paired_solutions.md", - ] - # The images came with it, and under a name of their own: both conversions - # call their figure media/plot.png, and the set holds each of them. - assert result.zip_path.exists() - assert [ - one - for one in zipfile.ZipFile(result.zip_path).namelist() - if one.startswith("media/") - ] == ["media/plot.png", "media/question_000_Question_1_0001.png"] - - def test_a_pdf_with_a_figure_builds_with_the_image_in_media(pdf, tmp_path): # As the OCR leaves a converted PDF: the image under the cache entry's # media/, and the markdown referring to it by that folder and its name. From e897910ec2a4ee73a2b61eb7e02142a413f7cf18 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Mon, 21 Sep 2026 01:23:06 +0100 Subject: [PATCH 2/2] implement: Relock in2lambda and offer field set and part solution to the fix loop (t26) --- README.md | 12 ++ in2lambda_agent/cli.py | 12 +- in2lambda_agent/corpus.py | 24 ++- in2lambda_agent/package.py | 22 ++- in2lambda_agent/pair.py | 129 +++++++++++++++ in2lambda_agent/pipeline.py | 138 ++++++++++++++-- in2lambda_agent/spec.py | 30 +++- tests/fixtures/marked_solutions.md | 13 ++ tests/fixtures/paired-spec.yaml | 10 ++ tests/fixtures/paired.md | 17 ++ tests/fixtures/paired_solutions.md | 11 ++ tests/test_corpus.py | 59 ++++++- tests/test_package.py | 16 ++ tests/test_pair.py | 86 ++++++++++ tests/test_pipeline.py | 250 ++++++++++++++++++++++++++++- 15 files changed, 795 insertions(+), 34 deletions(-) create mode 100644 in2lambda_agent/pair.py create mode 100644 tests/fixtures/marked_solutions.md create mode 100644 tests/fixtures/paired-spec.yaml create mode 100644 tests/fixtures/paired.md create mode 100644 tests/fixtures/paired_solutions.md create mode 100644 tests/test_pair.py diff --git a/README.md b/README.md index 4762694..23b8ca2 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,14 @@ agent keeps the markdown and the images under the PDF's hash in `--cache` (defau `--fresh-ocr` converts the PDF again and restarts the run from the new markdown. `--out` defaults to `./out`, where in2lambda writes the set's JSON folder and its zip. +A sheet whose solutions are a document of their own is one run. The solutions file is +the one beside `SOURCE` whose name is the sheet's with `_solutions`, `-solutions` or +` Solutions` after it, in any case, and whose suffix is the same: `Worksheet_1.pdf` +and `Worksheet_1_solutions.pdf`. Both are frozen into the one draft, the questions +first and the solutions second, and the run is named after the questions file whether +you name that file or the solutions one. A solutions file with no questions file +beside it stops the run, which exits 1 saying `solutions without questions`. + The run writes a spec — the YAML selectors naming which blocks of the source are questions, parts and solutions — in one model call, and saves it as `in2lambda-spec.yaml` beside `SOURCE`. A folder of sheets is one document set and @@ -253,6 +261,10 @@ all of it. `--suffix` is repeatable and defaults to `tex`, `md` and `docx`; `--s pdf` runs the PDFs too, which needs Mathpix credentials and one call per PDF. Every run is review mode `none`. +A sheet and the solutions file beside it are one run and one row, named after the +questions file. A solutions file with no questions file beside it is a `skipped` row +with the reason `solutions without questions`. + The sweep never writes to the corpus. It copies each set's folder into `--work` (default `./.in2lambda-agent/corpus`), empties that copy first, and runs the documents there. It keeps the sets' specs in `--specs` (default `./corpus-specs`) in a tree diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 139d35b..7664e72 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -11,6 +11,7 @@ from in2lambda_agent.model import ModelUnavailable, choose_backend from in2lambda_agent.ocr import ocr_pdf from in2lambda_agent.package import CommandRefused, SpecRejected +from in2lambda_agent.pair import SolutionsWithoutQuestions from in2lambda_agent.review import ReviewError from in2lambda_agent.settings import load_settings from in2lambda_agent.spec import BadSpec @@ -77,7 +78,12 @@ def build_parser() -> argparse.ArgumentParser: subcommands = parser.add_subparsers(dest="command", required=True) run = subcommands.add_parser("run", help="Convert SOURCE into a set.") - run.add_argument("source", type=Path, help="The question file to convert.") + run.add_argument( + "source", + type=Path, + help="The question file to convert. A solutions file beside it, named " + "after it, is frozen with it.", + ) run.add_argument( "--spec", type=Path, @@ -353,10 +359,12 @@ def main(argv: Optional[Sequence[str]] = None) -> int: SpecRejected, ReviewError, CommandRefused, + SolutionsWithoutQuestions, ) as error: # Missing credentials among them: the message names the variables, or # the login to run, or what a spec says that a spec cannot say, or the - # question a review command names that is not under review. + # question a review command names that is not under review, or the + # questions file a solutions file was run without. print(f"in2lambda-agent: {error}", file=sys.stderr) return 1 diff --git a/in2lambda_agent/corpus.py b/in2lambda_agent/corpus.py index 95fb16e..494d19c 100644 --- a/in2lambda_agent/corpus.py +++ b/in2lambda_agent/corpus.py @@ -6,6 +6,9 @@ is one run of the pipeline per document and one row of a table per run, written where two runs can be diffed against each other. +A questions document and the solutions document beside it are one run and one +row, which is the questions document's. + Nothing here writes into the corpus. A run leaves a draft beside its source, and a spec and a record beside that, so each set's folder is copied into a work directory and run there, and the specs are kept in a tree of their own @@ -21,7 +24,7 @@ from pathlib import Path from typing import Optional, Sequence -from in2lambda_agent import package, pipeline +from in2lambda_agent import package, pair, pipeline from in2lambda_agent.model import Backend, ModelUnavailable from in2lambda_agent.package import SpecRejected from in2lambda_agent.settings import Settings @@ -57,7 +60,8 @@ class Row: outcome: `built`, `build refused` where the checks came clean and in2lambda still would not write the set out, `faulted` for a draft the checks still fault and no zip, `skipped` for a file that is not - a document, `no spec` for a replay with nothing saved to replay, + a document and for a solutions document with no questions document + beside it, `no spec` for a replay with nothing saved to replay, `no model`, `spec rejected`, `bad spec`, or `error: `. reason: What the run had to say for itself, in the words of whatever said it: the refusal, the first error the checks were still finding, @@ -421,6 +425,22 @@ def sweep( print(f"{row.outcome:<20} {row.source}") rows.append(row) continue + # A solutions document is frozen into the run of the questions document + # it answers, so the pair is one row, which is the questions document's. + # One with no questions document beside it has no questions to attach + # its solutions to, and is a row of its own saying so. + if pair.questions_stem(document) is not None: + if pair.questions_beside(document) is not None: + continue + row = Row( + source=relative.as_posix(), + set=relative.parent.as_posix(), + outcome="skipped", + reason="solutions without questions", + ) + print(f"{row.outcome:<20} {row.source}") + rows.append(row) + continue # Staging is per set and the row is per document, so the guard is here # rather than in `run_one`: what a copy raises — an unreadable folder, a # file that goes while it is being read — is a row for every sheet of diff --git a/in2lambda_agent/package.py b/in2lambda_agent/package.py index 43f4e9a..3fcd69f 100644 --- a/in2lambda_agent/package.py +++ b/in2lambda_agent/package.py @@ -168,25 +168,29 @@ class Report: findings: list[Finding] = field(default_factory=list) -def source_add(source: Path) -> Path: - """Freezes a source document and returns the draft written beside it. +def source_add(source: Path, *more: Path) -> Path: + """Freezes one or more source documents into the draft beside the first. Always from the beginning: the agent's run owns the draft it writes, so a second run over the same file is a second run and not a continuation of - the first one's fields. One document: a draft can hold a second source — - the solutions written separately — and nothing the agent does asks for one. + the first one's fields. Args: - source: The markdown, tex or docx file to freeze. + source: The markdown, tex or docx file to freeze. The draft is named + after it, and its blocks are `b1` onwards. + more: Further documents to freeze into the same draft, in the order + they are to be numbered: a sheet's solutions written as a file of + their own. The blocks of the second source are `2/b1` onwards. Returns: The `FILE.draft.json` that was written, which every command below is given. Raises: - SourceError: pandoc or panflute is missing, or the file cannot be read. + SourceError: pandoc or panflute is missing, the files are not all in + one directory, or a file cannot be read. """ - return in2lambda.source.add([str(source)], True) + return in2lambda.source.add([str(source), *(str(one) for one in more)], True) def source_show(draft: Path) -> str: @@ -196,7 +200,9 @@ def source_show(draft: Path) -> str: draft: The draft file. Returns: - What the model is shown to write a spec from. + What the model is shown to write a spec from. A draft of two sources + heads each with `Source N: NAME`, and the ids of the second source's + blocks carry its number: `2/b1`. Raises: SourceError: there is no draft there, or its source has moved on. diff --git a/in2lambda_agent/pair.py b/in2lambda_agent/pair.py new file mode 100644 index 0000000..1e5c25a --- /dev/null +++ b/in2lambda_agent/pair.py @@ -0,0 +1,129 @@ +"""Which document holds the solutions to which, going by the file names. + +A folder of worksheets often writes the questions and the solutions as separate +documents: `Worksheet_1.pdf` beside `Worksheet_1_solutions.pdf`. A solutions +document alone holds no questions for its solutions to answer, so the run +freezes the two into one draft, the questions first and the solutions second, +and is named after the questions document. + +The pairing is by name. A solutions document is one whose stem ends in +`solutions` after a space, an underscore or a hyphen, in any case. Its questions +document is the file beside it whose stem is the stem before that ending and +whose suffix is the same. +""" + +import re +from pathlib import Path +from typing import Optional + +SOLUTIONS = re.compile(r"^(?P.+?)[ _-]solutions$", re.IGNORECASE) +"""A solutions document's stem, and the questions document's stem within it. + +The separator is required, so `resolutions.pdf` is not a solutions document and +`Solutions.pdf` names no questions document. +""" + + +class SolutionsWithoutQuestions(ValueError): + """A solutions document has no questions document beside it.""" + + +def questions_stem(document: Path) -> Optional[str]: + """The stem of the questions document a solutions document answers. + + Args: + document: Any file. + + Returns: + The stem before the `solutions` ending, or None where the name has no + such ending. + """ + found = SOLUTIONS.match(Path(document).stem) + return found.group("stem") if found else None + + +def questions_beside(solutions: Path) -> Optional[Path]: + """The questions document of a solutions document, in the same folder. + + Args: + solutions: A solutions document, as `questions_stem` reads one. + + Returns: + The file with that stem and the same suffix, or None where the folder + holds no such file. The suffix is compared without regard to case. + """ + stem = questions_stem(solutions) + if stem is None: + return None + solutions = Path(solutions) + return next( + ( + path + for path in _files_in(solutions.parent) + if path.stem == stem and path.suffix.lower() == solutions.suffix.lower() + ), + None, + ) + + +def solutions_beside(questions: Path) -> Optional[Path]: + """The solutions document of a questions document, in the same folder. + + Args: + questions: A document that is not itself a solutions document. + + Returns: + The first file, in name order, whose `questions_stem` is this + document's stem and whose suffix is the same, or None where the folder + holds no such file. + """ + questions = Path(questions) + return next( + ( + path + for path in _files_in(questions.parent) + if path.suffix.lower() == questions.suffix.lower() + and questions_stem(path) == questions.stem + ), + None, + ) + + +def _files_in(folder: Path) -> list[Path]: + """The files of a folder, in name order, and none where there is no folder. + + The pairing is the first thing a run does, before anything has read the + source, so it is where a mistyped path arrives first. It has nothing to say + about one: a source that is not there is in2lambda's to complain about, in + the words it uses for every file it cannot read. + """ + if not folder.is_dir(): + return [] + return sorted(path for path in folder.iterdir() if path.is_file()) + + +def of(source: Path) -> tuple[Path, Optional[Path]]: + """The two documents a run freezes, whichever of them the user named. + + Args: + source: The document the user named, the questions or the solutions. + + Returns: + The questions document, and the solutions document to freeze after it, + or None where the folder holds no solutions document for it. + + Raises: + SolutionsWithoutQuestions: `source` is a solutions document and the + folder holds no questions document for it. + """ + source = Path(source) + stem = questions_stem(source) + if stem is None: + return source, solutions_beside(source) + questions = questions_beside(source) + if questions is None: + raise SolutionsWithoutQuestions( + f"solutions without questions: nothing named {stem}{source.suffix} " + f"beside {source.name}" + ) + return questions, source diff --git a/in2lambda_agent/pipeline.py b/in2lambda_agent/pipeline.py index 564e517..fe721d1 100644 --- a/in2lambda_agent/pipeline.py +++ b/in2lambda_agent/pipeline.py @@ -26,15 +26,17 @@ """ import random +import re +import shutil from dataclasses import dataclass, field from pathlib import Path from typing import Callable, Optional -from in2lambda_agent import package +from in2lambda_agent import package, pair from in2lambda_agent.fix import RoundResult, fix_round, summary, unrepaired from in2lambda_agent.mathpix import MathpixClient from in2lambda_agent.model import Backend, ModelUnavailable, Usage, choose_backend -from in2lambda_agent.ocr import ocr_pdf +from in2lambda_agent.ocr import MEDIA_NAME, ocr_pdf from in2lambda_agent.review import RECORD, Question, Review, choose from in2lambda_agent.settings import Settings from in2lambda_agent.spec import RECORD_NAME, record_run, spec_path, write_spec @@ -42,6 +44,11 @@ # Where the OCR of each PDF is kept, under the directory the user ran from. DEFAULT_CACHE_DIR = Path(".in2lambda-agent") +# A markdown image whose file is in the OCR's media folder, as far as the folder +# name: `![a plot](media/plot.png)`. What a second source's images are renamed +# by when they are copied beside the first source's. +MEDIA_REFERENCE = re.compile(rf"(!\[[^\]]*\]\(){MEDIA_NAME}/") + REVIEW_MODES = ("none", "sample", "per-question") @@ -120,7 +127,8 @@ def run( """Drives in2lambda over one source file. Args: - source: The question file to convert. + source: The question file to convert, or the solutions file beside it, + which runs the questions file it answers. out_dir: Where in2lambda writes the set's JSON folder and zip. settings: The environment the run has available. spec: The set's spec file, when it is not the one beside the source. @@ -152,6 +160,8 @@ def run( BadSpec: If what the model answers with is not a spec. SpecRejected: If in2lambda will not run the spec. SourceError: If in2lambda cannot freeze or check the source. + SolutionsWithoutQuestions: If `source` is a solutions file and no + questions file is beside it. """ # A relative --out means the directory the user ran from, whatever in2lambda # does with the working directory along the way. @@ -159,6 +169,11 @@ def run( out_dir = Path(out_dir).resolve() cache_dir = Path(cache_dir).resolve() + # A sheet whose solutions are written as a file of their own is one run and + # one draft, named after the questions file. So the run is the questions + # file's from here on, whichever of the two the user named. + source, solutions = pair.of(source) + # The set is the folder the user's file is in, so this is settled before # OCR moves a PDF's markdown off into the cache. saved = spec_path(source, spec) @@ -166,19 +181,30 @@ def run( result = RunResult(on_stage=on_stage) # The rest of the pipeline reads markdown, so a PDF becomes markdown first. - if source.suffix.lower() == ".pdf": - client = mathpix or MathpixClient.from_settings(settings) - ocr = ocr_pdf(source, cache_dir=cache_dir, client=client, fresh=fresh_ocr) - frozen = ocr.markdown - # A fresh pass is a restart: every stage below reads the new markdown. - message = ( - f"fresh pass, restarting from {frozen}" - if ocr.fresh - else f"cached {frozen}" + frozen, _, message = _markdown( + source, cache_dir=cache_dir, settings=settings, mathpix=mathpix, fresh=fresh_ocr + ) + frozen_solutions = None + if solutions is not None: + frozen_solutions, solutions_media, said = _markdown( + solutions, + cache_dir=cache_dir, + settings=settings, + mathpix=mathpix, + fresh=fresh_ocr, ) - else: - frozen = source - message = f"not needed for {source.name}" + message += f"; {said}" + # in2lambda freezes into one draft the documents of one directory, and + # the OCR of each PDF is cached in an entry named after its own hash. So + # the solutions markdown, and the images it refers to, are copied beside + # the questions markdown. + if frozen_solutions.parent != frozen.parent: + frozen_solutions = _copy_beside( + frozen_solutions, + solutions_media, + name=solutions.stem, + into=frozen.parent, + ) result.add_stage("ocr", message) # One pass, or two where a saved spec leaves something for the checks to @@ -186,8 +212,15 @@ def run( reused = saved.is_file() report = package.Report(clean=False, errors=[]) while True: - draft = result.draft = package.source_add(frozen) - result.add_stage("freeze", str(draft)) + draft = result.draft = package.source_add( + frozen, *([frozen_solutions] if frozen_solutions is not None else []) + ) + result.add_stage( + "freeze", + f"{draft}, with {solutions.name} as source 2" + if solutions is not None + else str(draft), + ) # What the set's spec said before this pass wrote over it, where it # said anything: a rewrite in2lambda then refuses puts it back. @@ -202,6 +235,7 @@ def run( package.source_show(draft), backend, report if report.errors else None, + sources=2 if frozen_solutions is not None else 1, ) if saved.is_file(): replaced = saved.read_text(encoding="utf-8") @@ -464,6 +498,76 @@ def resume( return result +def _markdown( + document: Path, + *, + cache_dir: Path, + settings: Settings, + mathpix: Optional[MathpixClient], + fresh: bool, +) -> tuple[Path, Optional[Path], str]: + """The markdown a document is frozen from, and the OCR stage's line for it. + + Args: + document: The file the user named, or the solutions file beside it. + cache_dir: Where the OCR of each PDF is kept. + settings: The environment the run has available. + mathpix: The client to convert with, built from the settings if absent. + fresh: Convert a PDF again even if it is already cached. + + Returns: + The markdown, which is the document itself where it is not a PDF, the + folder holding the images it refers to where OCR wrote any, and what + the OCR stage says about it. + + Raises: + MathpixError: If the PDF cannot be converted. + """ + if document.suffix.lower() != ".pdf": + return document, None, f"not needed for {document.name}" + client = mathpix or MathpixClient.from_settings(settings) + ocr = ocr_pdf(document, cache_dir=cache_dir, client=client, fresh=fresh) + # A fresh pass is a restart: every stage below reads the new markdown. + if ocr.fresh: + return ocr.markdown, ocr.media, f"fresh pass, restarting from {ocr.markdown}" + return ocr.markdown, ocr.media, f"cached {ocr.markdown}" + + +def _copy_beside( + markdown: Path, media: Optional[Path], *, name: str, into: Path +) -> Path: + """Copies a second source, and the images it refers to, beside the first. + + The images cannot come across under the name their folder has, because each + PDF's OCR calls its own folder `media` and two sheets can each hold a + `plot.png`: the second copy would be the first one gone. So they arrive in a + folder named after their document, and the references in the copy are + rewritten to it. in2lambda resolves a reference from the folder the draft is + in, which is the folder copied into, so any name does. + + Args: + markdown: The markdown to copy. + media: The folder its images are in, where the document has any. + name: What the copies are named after: the solutions document's stem. + into: The folder the first source's markdown is in. + + Returns: + The copy, which is what the draft freezes as its second source. + """ + folder = f"{name}-{MEDIA_NAME}" + copied = into / f"{name}.md" + copied.write_text( + MEDIA_REFERENCE.sub( + lambda found: f"{found.group(1)}{folder}/", + markdown.read_text(encoding="utf-8"), + ), + encoding="utf-8", + ) + if media is not None and media.is_dir(): + shutil.copytree(media, into / folder, dirs_exist_ok=True) + return copied + + def _fix_rounds( draft: Path, report: package.Report, diff --git a/in2lambda_agent/spec.py b/in2lambda_agent/spec.py index 07fcd07..2269fd2 100644 --- a/in2lambda_agent/spec.py +++ b/in2lambda_agent/spec.py @@ -73,6 +73,20 @@ * A block indented under a list item is inside it, not beside it: such a question and its parts are one block, and there is nothing to select. +A draft holds two documents where the solutions are written as a file of their +own. The questions file is the first source, with block ids `b1` onwards, and +the solutions file is the second, with ids `2/b1` onwards. The same selectors +run over both. In the solutions file, every block `part` or `solution` matches is +a solution, and the solutions answer the questions of the first source in order: +each question's parts, or the question itself where it has none. A block +`question` matches there is a marker — the `Q2.` or `## Question 2` written above +the solutions to the second question. A marker is no question of its own, and its +text reaches no field. The first marker in the file assigns the solutions after +it to the first question, the second marker to the second question, so a +`question` selector that matches the marker above one question's solutions and +not the marker above another's assigns every solution after it to the wrong +question. `layout` describes the questions file alone. + The layout says which question or part a solution answers: PartsSepSol every solution together at the end, in part order: each @@ -120,7 +134,10 @@ def spec_path(source: Path, spec: Optional[Path] = None) -> Path: def write_spec( - shown: str, backend: Backend, report: Optional[Report] = None + shown: str, + backend: Backend, + report: Optional[Report] = None, + sources: int = 1, ) -> tuple[str, Reply]: """Writes a spec for a source, in one model call with no tools. @@ -129,6 +146,8 @@ def write_spec( backend: The backend to call, already known to be available. report: What the checks found about the draft a previous spec made, where this is the rewrite that follows a dirty validate. + sources: How many documents the draft holds: 2 where the solutions are + a file of their own, which the prompt then says before the source. Returns: The spec, and the reply it came in. @@ -137,7 +156,14 @@ def write_spec( BadSpec: the reply is not YAML, is not a mapping, or names no layout or one that is not a layout. """ - prompt = f"Here is the source, one line each with its block id:\n\n{shown}\n" + prompt = "" + if sources > 1: + prompt = ( + "The draft holds two documents: the questions file, whose blocks " + "are `b1` onwards, and its solutions file, whose blocks are `2/b1` " + "onwards. Every solution is in the second.\n\n" + ) + prompt += f"Here is the source, one line each with its block id:\n\n{shown}\n" if report is not None: prompt += ( "\nA previous spec for this set left the draft with this to answer " diff --git a/tests/fixtures/marked_solutions.md b/tests/fixtures/marked_solutions.md new file mode 100644 index 0000000..4b9549e --- /dev/null +++ b/tests/fixtures/marked_solutions.md @@ -0,0 +1,13 @@ +# Tutorial Sheet 3: answers + +## Solutions + +Q1. + +(a) $\omega = v / r$ + +Q2. + +(a) $T = 2\pi\sqrt{m/k}$ + +(b) $v = A\sqrt{k/m}$ diff --git a/tests/fixtures/paired-spec.yaml b/tests/fixtures/paired-spec.yaml new file mode 100644 index 0000000..0dae92a --- /dev/null +++ b/tests/fixtures/paired-spec.yaml @@ -0,0 +1,10 @@ +# The spec the fake backend answers with for a sheet whose solutions are a file +# of their own. The same selectors run over both sources: the questions file +# holds no `## Solutions` heading, so nothing in it is a solution, and every +# solution comes from the second source. +ignore: Header +question: Para text~'^[A-Z]' +part: ListItem +solution: after Header text=Solutions, Para +strip: ['^\([a-z]\) ', '^\d+\([a-z]\) '] +layout: PartsSepSol diff --git a/tests/fixtures/paired.md b/tests/fixtures/paired.md new file mode 100644 index 0000000..7cf326b --- /dev/null +++ b/tests/fixtures/paired.md @@ -0,0 +1,17 @@ +# Tutorial Sheet 3 + +## Question 1 + +A cylinder of radius $r$ rolls along the ground without slipping. + +(a) Find its angular velocity at speed $v$. + +(b) Find its kinetic energy. + +## Question 2 + +A spring of stiffness $k$ carries a mass $m$. + +(a) Find the period of small oscillations. + +(b) Find the greatest speed of the mass in an oscillation of amplitude $A$. diff --git a/tests/fixtures/paired_solutions.md b/tests/fixtures/paired_solutions.md new file mode 100644 index 0000000..02f3d19 --- /dev/null +++ b/tests/fixtures/paired_solutions.md @@ -0,0 +1,11 @@ +# Tutorial Sheet 3: answers + +## Solutions + +1(a) $\omega = v / r$ + +1(b) $E = \tfrac{3}{4} m v^2$ + +2(a) $T = 2\pi\sqrt{m/k}$ + +2(b) $v = A\sqrt{k/m}$ diff --git a/tests/test_corpus.py b/tests/test_corpus.py index 70a1376..38d2f46 100644 --- a/tests/test_corpus.py +++ b/tests/test_corpus.py @@ -1,12 +1,13 @@ """The sweep over a corpus: one row per document, and a replay with no model in it.""" +import json import shutil from dataclasses import asdict from pathlib import Path import pytest from conftest import FakeBackend -from test_pipeline import FAULTY_SPEC, FIXES, SPEC, TEX_SPEC +from test_pipeline import FAULTY_SPEC, FIXES, PAIRED_SPEC, SPEC, TEX_SPEC from in2lambda_agent import corpus, pipeline from in2lambda_agent.settings import Settings @@ -459,3 +460,59 @@ def without_clocks(row): for key, value in asdict(row).items() if key not in ("model_seconds", "wall_seconds") } + + +def test_a_sheet_and_its_solutions_file_are_one_row(tmp_path): + # UCL_MechEng writes each worksheet as two documents: Worksheet_1.pdf and + # Worksheet_1_solutions.pdf beside it. + root = tmp_path / "corpus" + folder = root / "worksheets" + folder.mkdir(parents=True) + shutil.copy(FIXTURES / "paired.md", folder / "Sheet_3.md") + shutil.copy(FIXTURES / "paired_solutions.md", folder / "Sheet_3_solutions.md") + backend = FakeBackend(PAIRED_SPEC) + + rows = sweep(root, tmp_path, backend=backend) + + assert [(row.source, row.set, row.outcome) for row in rows] == [ + ("worksheets/Sheet_3.md", "worksheets", "built") + ] + assert len(backend.calls) == 1 + # The solutions file was frozen as the draft's second source, so the four + # part solutions came out of it. + draft = corpus.package.draft_of(tmp_path / "work" / "worksheets" / "Sheet_3.md") + frozen = json.loads(draft.read_text()) + assert [one["source"] for one in frozen["sources"]] == [ + "Sheet_3.md", + "Sheet_3_solutions.md", + ] + assert [ + key + for key, written in frozen["fields"].items() + if written.get("source") == 2 and not key.endswith(".ignore") + ] == [ + "q1.p1.solution", + "q1.p2.solution", + "q2.p1.solution", + "q2.p2.solution", + ] + + +def test_solutions_with_no_questions_beside_them_are_skipped(tmp_path): + root = tmp_path / "corpus" + folder = root / "worksheets" + folder.mkdir(parents=True) + shutil.copy( + FIXTURES / "paired_solutions.md", folder / "Tutorial_2_Solutions.md" + ) + backend = FakeBackend(PAIRED_SPEC) + + rows = sweep(root, tmp_path, backend=backend) + + # `skipped` is an outcome the command exits 0 on, as a drawing's row is. + assert [(row.source, row.outcome, row.reason) for row in rows] == [ + ("worksheets/Tutorial_2_Solutions.md", "skipped", "solutions without questions") + ] + # Nothing was frozen or called on its account. + assert len(backend.calls) == 0 + assert not (tmp_path / "work" / "worksheets").exists() diff --git a/tests/test_package.py b/tests/test_package.py index 94b2478..64ed67f 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -98,6 +98,22 @@ 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_second_source_is_frozen_into_the_same_draft(tmp_path): + folder = tmp_path / "sheets" + folder.mkdir() + for name in ("paired.md", "paired_solutions.md"): + shutil.copy(FIXTURES / name, folder / name) + + written = package.source_add(folder / "paired.md", folder / "paired_solutions.md") + + assert written == folder / "paired.draft.json" + frozen = json.loads(written.read_text())["sources"] + assert [one["source"] for one in frozen] == ["paired.md", "paired_solutions.md"] + # The second source's blocks carry its number, which is how a command and a + # spec-writing prompt address them. + assert all(one["id"].startswith("2/") for one in frozen[1]["blocks"]) + + def test_the_warnings_a_build_says_are_returned_rather_than_printed(tmp_path): # The same sheet without its solutions: in2lambda builds it and warns about # each unanswered part as it goes. Those are the messages the validate line diff --git a/tests/test_pair.py b/tests/test_pair.py new file mode 100644 index 0000000..a7ed312 --- /dev/null +++ b/tests/test_pair.py @@ -0,0 +1,86 @@ +"""Reading a solutions document's questions document off the file names.""" + +from pathlib import Path + +import pytest + +from in2lambda_agent import pair + + +@pytest.mark.parametrize( + "name, stem", + [ + ("Worksheet_1_solutions.pdf", "Worksheet_1"), + ("Tutorial_2_Solutions.pdf", "Tutorial_2"), + ("sheet-solutions.md", "sheet"), + ("Sheet 1 Solutions.docx", "Sheet 1"), + # No separator before `solutions`, so the word is part of a longer one. + ("resolutions.pdf", None), + # Nothing before the separator, so there is no stem to pair with. + ("Solutions.pdf", None), + ("Worksheet_1.pdf", None), + ], +) +def test_a_solutions_document_is_read_off_its_name(name, stem): + assert pair.questions_stem(Path("sheets") / name) == stem + + +def test_the_questions_document_is_the_one_beside_it(tmp_path): + (tmp_path / "Worksheet_1.pdf").write_bytes(b"%PDF") + solutions = tmp_path / "Worksheet_1_solutions.pdf" + solutions.write_bytes(b"%PDF") + + assert pair.questions_beside(solutions) == tmp_path / "Worksheet_1.pdf" + assert pair.solutions_beside(tmp_path / "Worksheet_1.pdf") == solutions + + +def test_a_file_of_another_suffix_is_not_the_questions_document(tmp_path): + # The pairing is within one suffix: a tex sheet does not answer to the + # solutions someone scanned. + (tmp_path / "Worksheet_1.tex").write_text("\\begin{document}\\end{document}") + solutions = tmp_path / "Worksheet_1_solutions.pdf" + solutions.write_bytes(b"%PDF") + + assert pair.questions_beside(solutions) is None + assert pair.solutions_beside(tmp_path / "Worksheet_1.tex") is None + + +def test_a_document_with_no_solutions_beside_it_is_run_alone(tmp_path): + questions = tmp_path / "Worksheet_1.md" + questions.write_text("# Sheet\n") + + assert pair.of(questions) == (questions, None) + + +def test_naming_the_solutions_document_runs_the_questions_document(tmp_path): + questions = tmp_path / "Worksheet_1.md" + questions.write_text("# Sheet\n") + solutions = tmp_path / "Worksheet_1_solutions.md" + solutions.write_text("# Solutions\n") + + assert pair.of(solutions) == (questions, solutions) + assert pair.of(questions) == (questions, solutions) + + +def test_a_folder_that_is_not_there_holds_no_companion(tmp_path): + # The pairing is the first thing a run does, so a mistyped path reaches it + # before anything has read the source. It leaves the complaining to + # in2lambda rather than raising an OSError of its own here. + absent = tmp_path / "nope" + + assert pair.questions_beside(absent / "Worksheet_1_solutions.pdf") is None + assert pair.solutions_beside(absent / "Worksheet_1.pdf") is None + assert pair.of(absent / "Worksheet_1.pdf") == (absent / "Worksheet_1.pdf", None) + with pytest.raises(pair.SolutionsWithoutQuestions): + pair.of(absent / "Worksheet_1_solutions.pdf") + + +def test_solutions_with_no_questions_say_what_is_missing(tmp_path): + solutions = tmp_path / "Tutorial_2_Solutions.pdf" + solutions.write_bytes(b"%PDF") + + with pytest.raises( + pair.SolutionsWithoutQuestions, + match="solutions without questions: nothing named Tutorial_2.pdf", + ): + pair.of(solutions) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index f0d20bc..dc8fc32 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -9,10 +9,10 @@ import in2lambda.draft import pytest -from conftest import FakeBackend, FakeMathpix +from conftest import PNG, FakeBackend, FakeMathpix from in2lambda.validation.pdf import missing_tools -from in2lambda_agent import package, pipeline +from in2lambda_agent import package, pair, pipeline from in2lambda_agent.cli import main from in2lambda_agent.model import ModelUnavailable from in2lambda_agent.package import SpecRejected @@ -23,6 +23,7 @@ FIXTURES = Path(__file__).parent / "fixtures" SOURCE = FIXTURES / "sheet.md" SPEC = (FIXTURES / "sheet-spec.yaml").read_text() +PAIRED_SPEC = (FIXTURES / "paired-spec.yaml").read_text() TEX_SPEC = (FIXTURES / "tex-sheet-spec.yaml").read_text() FAULTY_SPEC = (FIXTURES / "faulty-spec.yaml").read_text() @@ -145,6 +146,60 @@ def figures(tmp_path): return folder +@pytest.fixture +def paired(tmp_path): + """A sheet whose solutions are written as a file of their own beside it.""" + folder = tmp_path / "paired" + folder.mkdir() + for name in ("paired.md", "paired_solutions.md"): + shutil.copy(FIXTURES / name, folder / name) + return folder + + +@pytest.fixture +def marked(tmp_path): + """A pair whose solutions file writes a marker above each group of solutions. + + A solutions document written to stand on its own repeats the question it is + answering, so the `question` selector matches in the second source as well + as the first. `marked_solutions.md` leaves question 1's part (b) + unanswered, so the marker `Q2.` is what sends the solutions under it to + question 2 rather than to question 1's remaining part. + """ + folder = tmp_path / "marked" + folder.mkdir() + shutil.copy(FIXTURES / "paired.md", folder / "marked.md") + shutil.copy(FIXTURES / "marked_solutions.md", folder / "marked_solutions.md") + return folder + + +class PairedMathpix: + """A Mathpix client answering with the fixture each PDF is named after. + + Each conversion holds a figure, as a scanned sheet does, and both call it + `media/plot.png`: the pair of names that must not become one file when the + solutions are copied beside the questions. + """ + + def __init__(self): + self.calls: list[Path] = [] + + def convert(self, pdf: Path, media_dir: Path) -> str: + self.calls.append(Path(pdf)) + media_dir.mkdir(parents=True, exist_ok=True) + (media_dir / "plot.png").write_bytes(PNG) + lines = (FIXTURES / f"{Path(pdf).stem}.md").read_text().splitlines() + # Inside the first paragraph, which is a field of the draft: an image of + # a block the spec assigns to nothing is a coverage error, not a figure. + first = next( + number + for number, line in enumerate(lines) + if line and not line.startswith("#") + ) + lines[first] += " ![a plot](media/plot.png)" + return "\n".join(lines) + "\n" + + def test_one_model_call_writes_the_sets_spec_and_the_run_builds(sheets, tmp_path): backend = FakeBackend(SPEC) @@ -1541,6 +1596,197 @@ def test_a_source_beside_its_figures_builds_with_the_images_in_media( assert "media/ball.png" in zipfile.ZipFile(result.zip_path).namelist() +def test_a_sheet_and_its_solutions_are_frozen_into_one_draft(paired, tmp_path): + result = pipeline.run( + paired / "paired.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(PAIRED_SPEC), + ) + frozen = json.loads(result.draft.read_text()) + + assert result.draft == drafted(paired, "paired.md") + assert [one["source"] for one in frozen["sources"]] == [ + "paired.md", + "paired_solutions.md", + ] + # Every solution was quoted out of the second source, and every question's + # own text out of the first. + assert { + key: field.get("source", 1) + for key, field in frozen["fields"].items() + if not key.endswith(".ignore") + } == { + "q1.text": 1, + "q1.p1.text": 1, + "q1.p2.text": 1, + "q2.text": 1, + "q2.p1.text": 1, + "q2.p2.text": 1, + "q1.p1.solution": 2, + "q1.p2.solution": 2, + "q2.p1.solution": 2, + "q2.p2.solution": 2, + } + assert result.zip_path is not None and result.zip_path.exists() + + +def test_the_zip_quotes_the_part_solutions_from_the_solutions_file(paired, tmp_path): + result = pipeline.run( + paired / "paired.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(PAIRED_SPEC), + ) + + question = json.loads( + zipfile.ZipFile(result.zip_path).read("question_000_Question_1.json") + ) + assert [part["workedSolution"]["content"] for part in question["parts"]] == [ + "$\\omega = v / r$", + "$E = \\tfrac{3}{4} m v^2$", + ] + + +def test_a_marker_in_the_solutions_file_writes_no_question(marked, tmp_path): + result = pipeline.run( + marked / "marked.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(PAIRED_SPEC), + ) + fields = json.loads(result.draft.read_text())["fields"] + + # `Q1.` and `Q2.` match the `question` selector in the second source. The + # draft holds the two questions the first source states, and neither marker + # is a field. + assert sorted(key for key in fields if key.endswith(".text")) == [ + "q1.p1.text", + "q1.p2.text", + "q1.text", + "q2.p1.text", + "q2.p2.text", + "q2.text", + ] + # Each marker is ignored, so its text reaches no question of the set. + assert fields["2/b3.ignore"]["value"] is True + assert fields["2/b5.ignore"]["value"] is True + # Each marker sends the solutions below it to its own question, so question + # 2's solutions answer question 2's parts. Question 1's part (b) is + # unanswered, which is a warning the build runs after. + assert { + key: field["source"] for key, field in fields.items() if ".solution" in key + } == { + "q1.p1.solution": 2, + "q2.p1.solution": 2, + "q2.p2.solution": 2, + } + assert result.zip_path is not None and result.zip_path.exists() + + +def test_the_spec_prompt_shows_both_sources(paired, tmp_path): + backend = FakeBackend(PAIRED_SPEC) + + pipeline.run( + paired / "paired.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=backend, + ) + (_, prompt) = backend.calls[0] + + assert "whose blocks are `2/b1` onwards" in prompt + assert "Source 1: paired.md" in prompt + assert "Source 2: paired_solutions.md" in prompt + assert "2/b3 5 1(a)" in prompt + + +def test_naming_the_solutions_file_runs_the_questions_file(paired, tmp_path): + result = pipeline.run( + paired / "paired_solutions.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(PAIRED_SPEC), + ) + freeze = next(stage for stage in result.stages if stage.name == "freeze") + + assert result.draft == drafted(paired, "paired.md") + assert freeze.message == ( + f"{drafted(paired, 'paired.md')}, with paired_solutions.md as source 2" + ) + # The record is the questions file's, so the two names are one run. + (line,) = (paired / RECORD_NAME).read_text().splitlines() + assert json.loads(line)["source"] == str(paired / "paired.md") + assert result.zip_path.exists() + + +def test_solutions_with_no_questions_beside_them_stop_the_run(tmp_path): + folder = tmp_path / "lone" + folder.mkdir() + shutil.copy(FIXTURES / "paired_solutions.md", folder / "paired_solutions.md") + + with pytest.raises( + pair.SolutionsWithoutQuestions, + match="solutions without questions: nothing named paired.md", + ): + pipeline.run( + folder / "paired_solutions.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(PAIRED_SPEC), + ) + + +def test_a_lone_solutions_file_exits_one_saying_so(tmp_path, capsys): + folder = tmp_path / "lone" + folder.mkdir() + shutil.copy(FIXTURES / "paired_solutions.md", folder / "paired_solutions.md") + + code = main( + ["run", str(folder / "paired_solutions.md"), "--out", str(tmp_path / "out")] + ) + printed = capsys.readouterr() + + assert code == 1 + assert "solutions without questions" in printed.err + assert printed.out == "" + + +def test_a_pair_of_pdfs_is_converted_and_frozen_into_one_draft(tmp_path): + for name in ("paired.pdf", "paired_solutions.pdf"): + (tmp_path / name).write_bytes(f"%PDF-1.4 {name}".encode()) + client = PairedMathpix() + + result = pipeline.run( + tmp_path / "paired.pdf", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + mathpix=client, + backend=FakeBackend(PAIRED_SPEC), + ) + frozen = json.loads(result.draft.read_text()) + + assert [one.name for one in client.calls] == [ + "paired.pdf", + "paired_solutions.pdf", + ] + # Each PDF has a cache entry of its own, and a draft holds the documents of + # one directory, so the solutions markdown is copied beside the questions. + assert [one["source"] for one in frozen["sources"]] == [ + "source.md", + "paired_solutions.md", + ] + # The images came with it, and under a name of their own: both conversions + # call their figure media/plot.png, and the set holds each of them. + assert result.zip_path.exists() + assert [ + one + for one in zipfile.ZipFile(result.zip_path).namelist() + if one.startswith("media/") + ] == ["media/plot.png", "media/question_000_Question_1_0001.png"] + + def test_a_pdf_with_a_figure_builds_with_the_image_in_media(pdf, tmp_path): # As the OCR leaves a converted PDF: the image under the cache entry's # media/, and the markdown referring to it by that folder and its name.