From 7607cfbc93c5638d32016b43211cb7d7559f8f95 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 02:01:40 +0100 Subject: [PATCH 1/7] implement: Add a `convert` command to the command line (t38) --- README.md | 86 ++++++++++++--- in2lambda_agent/cli.py | 124 ++++++++++++++++++++- in2lambda_agent/routes.py | 25 +++++ tests/test_cli.py | 220 +++++++++++++++++++++++++++++++++++++- tests/test_pipeline.py | 22 ++-- tests/test_routes.py | 81 +++++++++++++- 6 files changed, 527 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index e9cc057..5d4b84f 100644 --- a/README.md +++ b/README.md @@ -4,14 +4,21 @@ Turns a PDF, docx, tex or md file into a validated Lambda Feedback set. [in2lambda](https://github.com/lambda-feedback/in2lambda) performs every deterministic step and every write; this agent performs the OCR, the model calls and the loop control. -One model call per document set writes a spec of selectors, which is layer 1, and -in2lambda runs that spec over the frozen source. A draft the checks fault returns to -the model as a fixing round, which repairs the draft one field at a time at layers 3 -and 4, until the checks report no error or the round limit is reached. - -[docs/how-it-works.md](docs/how-it-works.md) describes a run stage by stage: the line -each stage prints, the file each stage writes, and what each of the three model calls -is given and may write. +`convert` reads the document by two routes and compares them. Route A is one model call +that returns the set as JSON. Route B is a Lua filter, written by one model call per +folder, that pandoc runs with no further call. A field the two routes read the same way +is taken as it stands; a field they read differently goes to a small adjudicating call; +a field that call cannot settle is flagged for a person to read. Every field of either +route must be a quote of the document. [docs/plan.md](docs/plan.md) describes each step +and what it detects. + +`run --route spec` is the earlier route. One model call per document set writes a spec +of selectors, which is layer 1, and in2lambda runs that spec over the frozen source. A +draft the checks fault returns to the model as a fixing round, which repairs the draft +one field at a time at layers 3 and 4, until the checks report no error or the round +limit is reached. [docs/how-it-works.md](docs/how-it-works.md) describes that route +stage by stage: the line each stage prints, the file each stage writes, and what each of +the three model calls is given and may write. ## Install @@ -47,16 +54,71 @@ installed and `claude login` run. A run over a set whose spec is saved makes no call, and needs no key at all. A stage that needs a variable names that variable and the run exits 1. -## Run +## Convert + +```sh +poetry run in2lambda-agent convert sheet.pdf +``` + +In full: + +```sh +poetry run in2lambda-agent convert DOCUMENT [--solutions FILE] [--filter FILE | --write-filter] [--out DIR] [--cache DIR] +``` + +`DOCUMENT` is a PDF, markdown, tex or docx file. Mathpix converts a PDF first and the +agent keeps the markdown and the images under the PDF's hash in `--cache` (default +`./.in2lambda-agent`), so a second conversion of the same PDF makes no Mathpix call. +pandoc converts a tex or docx file. `--out` defaults to `./out`, where in2lambda writes +the set's JSON folder and its zip. + +`--solutions` names the document holding the solutions. Without it the agent takes the +file beside `DOCUMENT` whose name is the document'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`. Route A reads both documents in one call, and route B reads +each under its own role. + +`--filter` names the Lua filter route B runs, which is the file `--write-filter` wrote +for another sheet of the same set. `--write-filter` writes one for this document with a +model call and keeps it at `OUT/filter.lua`. The two options together are refused: a +conversion runs one filter. With neither option route A converts the document alone, no +field is compared, and the counts line says so. + +The command prints one line per flagged field, the counts of the comparison, and the +zip: + +``` +flag q2.p1.worked_solution: a stray minus sign inside or beside a display maths; Mathpix reads a separator line as one +flag q4.p2.content: two readings of the source + A: Find the drag force on the plate. + B: Find the drag force on the plate, in newtons. +fields 60 fields, agreed 54, defaulted 4, adjudicated 2, flagged 2 +build /home/me/out/sheet.zip +``` + +A flag names the field, the reason, and each route's text where both routes filled the +field. `fields` counts the fields the two routes agreed on, the fields one route alone +filled, the fields the adjudicating call settled, and the fields flagged. A filter run +that fails adds a `route B failed` line naming pandoc's message, and the set is route +A's reading alone. + +`convert` exits 1 where Mathpix, the model or pandoc failed, and 0 otherwise. A flagged +field does not change the exit code: the zip is written whatever the flags say, and a +person reads the flags after it. + +## Run: the spec route + +`run DOCUMENT` converts the document as `convert` does and takes the same options. +`--route spec` runs the earlier route instead, which the rest of this section describes. ```sh -poetry run in2lambda-agent run sheet.pdf +poetry run in2lambda-agent run sheet.pdf --route spec ``` In full: ```sh -poetry run in2lambda-agent run SOURCE [--spec FILE] [--review none|sample|per-question] [--rounds N] [--tries N] [--sample N] [--cache DIR] [--fresh-ocr] [--out DIR] +poetry run in2lambda-agent run SOURCE --route spec [--spec FILE] [--review none|sample|per-question] [--rounds N] [--tries N] [--sample N] [--cache DIR] [--fresh-ocr] [--out DIR] ``` `SOURCE` is a PDF, markdown, tex or docx file. Mathpix converts a PDF first, and the @@ -164,7 +226,7 @@ spec, and is not one of the rounds. ### Exit codes -`run` and `review` exit 0 where they wrote a zip, and where a review is still waiting +`run --route spec` and `review` exit 0 where they wrote a zip, and where a review is still waiting for a verdict. They exit 1 where the checks still fault the draft, where in2lambda refused the build, and where a review has no question left to answer and no zip was written. A missing credential, an unavailable backend, a reply that is not a spec, a diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 07cb389..dab06f4 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -2,12 +2,13 @@ import argparse import getpass +import subprocess import sys import tempfile from pathlib import Path from typing import Optional, Sequence -from in2lambda_agent import compare, corpus, gate, pipeline +from in2lambda_agent import compare, corpus, gate, pair, pipeline, routes from in2lambda_agent.mathpix import MathpixClient, MathpixError from in2lambda_agent.model import ModelError, ModelUnavailable, choose_backend from in2lambda_agent.ocr import ocr_pdf @@ -85,12 +86,41 @@ def reviewer_name(given: Optional[str]) -> str: return "reviewer" +def _conversion_options(parser: argparse.ArgumentParser) -> None: + """Adds the options of a conversion, which `convert` and `run` both take. + + Args: + parser: The subcommand's parser. + """ + parser.add_argument( + "--solutions", + type=Path, + default=None, + help="The solutions document. Default: the file beside the document " + "whose name is the document's with `_solutions` after it.", + ) + filter_ = parser.add_mutually_exclusive_group() + filter_.add_argument( + "--filter", + type=Path, + default=None, + help="The Lua filter route B runs. Without one, route A converts the " + "document alone and no field is compared.", + ) + filter_.add_argument( + "--write-filter", + action="store_true", + help="Write route B's filter for this document with a model call, and " + "keep it at `OUT/filter.lua`.", + ) + + def build_parser() -> argparse.ArgumentParser: """The command line as the design spec describes it. Returns: - A parser with the `run`, `review`, `corpus`, `gate`, `compare` and - `ui` subcommands. + A parser with the `convert`, `run`, `review`, `corpus`, `gate`, + `compare` and `ui` subcommands. """ parser = argparse.ArgumentParser( prog="in2lambda-agent", @@ -98,13 +128,45 @@ def build_parser() -> argparse.ArgumentParser: ) subcommands = parser.add_subparsers(dest="command", required=True) - run = subcommands.add_parser("run", help="Convert SOURCE into a set.") + convert = subcommands.add_parser( + "convert", help="Convert DOCUMENT into a set through both routes." + ) + convert.add_argument( + "document", + type=Path, + help="The question file to convert: a PDF, markdown, tex or docx file.", + ) + _conversion_options(convert) + convert.add_argument( + "--out", + type=Path, + default=Path("out"), + help="Where to write the set's JSON folder and zip.", + ) + convert.add_argument( + "--cache", + type=Path, + default=pipeline.DEFAULT_CACHE_DIR, + help="Where the OCR of each PDF is kept.", + ) + + run = subcommands.add_parser( + "run", help="Convert SOURCE into a set; `convert` under the default route." + ) 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( + "--route", + choices=("direct", "spec"), + default="direct", + help="Which route converts the document: `direct` is the `convert` " + "command, and `spec` writes a spec of selectors and runs it.", + ) + _conversion_options(run) run.add_argument( "--spec", type=Path, @@ -308,6 +370,57 @@ def build_parser() -> argparse.ArgumentParser: return parser +def convert_command(args: argparse.Namespace) -> int: + """Converts one document through both routes and prints the report. + + Args: + args: The parsed arguments of `convert`, or of `run` under the direct + route, which takes the same options. + + Returns: + 0 where the zip was written, and 1 where a conversion step failed. A + flagged field does not change the code: the flags are what a person + reads after the build, and no check blocks the write. + """ + # `run SOURCE` converts the same document, under the other name. + document = Path(getattr(args, "document", None) or args.source) + solutions = args.solutions or pair.solutions_beside(document) + out_dir = Path(args.out) + settings = load_settings() + backend = choose_backend(settings) + lua = args.filter + try: + if args.write_filter: + out_dir.mkdir(parents=True, exist_ok=True) + lua = out_dir / "filter.lua" + lua.write_text( + routes.write_filter(document, solutions, backend)[0], encoding="utf-8" + ) + result = routes.convert( + document, + solutions, + out_dir=out_dir, + cache_dir=args.cache, + backend=backend, + settings=settings, + lua=lua, + name=document.stem, + ) + except (MathpixError, ModelUnavailable, ModelError) as error: + print(f"in2lambda-agent: {error}", file=sys.stderr) + return 1 + except subprocess.CalledProcessError as error: + # pandoc read the document, or ran the filter, and refused. Its own + # message names the line; the exit status alone names nothing. + stderr = (error.stderr or b"").decode("utf-8", "replace").strip() + print(f"in2lambda-agent: {stderr or error}", file=sys.stderr) + return 1 + + for line in result.report(): + print(line) + return 0 if result.zip_path else 1 + + def main(argv: Optional[Sequence[str]] = None) -> int: """Runs the command. @@ -319,6 +432,9 @@ def main(argv: Optional[Sequence[str]] = None) -> int: """ args = build_parser().parse_args(argv) + if args.command == "convert" or (args.command == "run" and args.route == "direct"): + return convert_command(args) + if args.command == "corpus": rows = corpus.sweep( args.root, diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 91626ae..730ccee 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -388,6 +388,31 @@ class Converted: adjudicated: int = 0 route_b_error: Optional[str] = None + def report(self) -> list[str]: + """One line per flag, a line of the counts, and the zip. + + A flag prints each route's text where both routes filled the field, because the + reader decides between the two. A field one route filled prints the reason + alone. + """ + lines = [] + for one in self.flags: + lines.append(f"flag {one.field}: {one.reason}") + if one.a and one.b: + lines += [f" A: {_squash(one.a)}", f" B: {_squash(one.b)}"] + counts = _counted( + [self.fields, self.agreed, self.defaulted, self.adjudicated, len(self.flags)] + ) + if not self.fields and self.route_b_error is None: + # No filter was given, so no field was compared and every field is route A's. + counts += " (route B did not run)" + lines.append(f"fields {counts}") + if self.route_b_error: + lines.append(f"route B failed: {self.route_b_error}") + if self.zip_path: + lines.append(f"build {self.zip_path}") + return lines + _UNDERLINE = Path(__file__).parent / "underline.lua" diff --git a/tests/test_cli.py b/tests/test_cli.py index ae52898..fed0187 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,16 +1,17 @@ """The command line the design spec describes.""" import getpass +import subprocess import sys from pathlib import Path from types import SimpleNamespace import pytest -from conftest import FakeMathpix +from conftest import FakeBackend, FakeMathpix -from in2lambda_agent import cli, compare, corpus, gate, pipeline +from in2lambda_agent import cli, compare, corpus, gate, pipeline, routes from in2lambda_agent.cli import build_parser, main, reviewer_name -from in2lambda_agent.model import Usage +from in2lambda_agent.model import ModelUnavailable, Usage from in2lambda_agent.settings import Settings @@ -81,6 +82,215 @@ def test_a_sample_of_no_questions_is_refused(count, capsys): assert "at least one question" in capsys.readouterr().err +# --- convert --------------------------------------------------------------- + + +def test_convert_defaults(): + args = build_parser().parse_args(["convert", "sheet.pdf"]) + + assert args.command == "convert" + assert args.document == Path("sheet.pdf") + assert args.solutions is None + assert args.filter is None + assert args.write_filter is False + assert args.out == Path("out") + assert args.cache == Path(".in2lambda-agent") + + +def test_convert_every_option(): + args = build_parser().parse_args( + [ + "convert", + "sheet.pdf", + "--solutions", + "sheet_solutions.pdf", + "--filter", + "set.lua", + "--out", + "somewhere", + "--cache", + "cached", + ] + ) + + assert args.document == Path("sheet.pdf") + assert args.solutions == Path("sheet_solutions.pdf") + assert args.filter == Path("set.lua") + assert args.out == Path("somewhere") + assert args.cache == Path("cached") + + written = build_parser().parse_args(["convert", "sheet.pdf", "--write-filter"]) + assert written.write_filter is True + assert written.filter is None + + +@pytest.mark.parametrize("command", ["convert", "run"]) +def test_a_filter_and_a_written_filter_together_are_refused(command): + # One run has one route B filter: either the file named or the file written. + with pytest.raises(SystemExit): + build_parser().parse_args( + [command, "sheet.md", "--filter", "set.lua", "--write-filter"] + ) + + +def test_run_converts_through_both_routes_unless_the_spec_route_is_asked_for(): + assert build_parser().parse_args(["run", "sheet.md"]).route == "direct" + assert build_parser().parse_args(["run", "s.md", "--route", "spec"]).route == "spec" + assert build_parser().parse_args(["run", "s.md", "--solutions", "s2.md"]).solutions + + +def converted(zip_path=None, **counts): + """What a monkeypatched `routes.convert` answers with.""" + return routes.Converted( + set=None, zip_path=zip_path, flags=counts.pop("flags", []), reply=[], **counts + ) + + +def records(given, result): + """A stand-in for `routes.convert` that records what it was given.""" + + def record(document, solutions=None, **passed): + given.update(passed, document=document, solutions=solutions) + return result + + return record + + +@pytest.fixture +def backend(monkeypatch): + """A backend the convert branch takes without reading the environment.""" + fake = FakeBackend() + monkeypatch.setattr(cli, "choose_backend", lambda settings: fake) + return fake + + +def test_convert_hands_the_document_and_the_options_to_the_route( + tmp_path, backend, monkeypatch, capsys +): + given = {} + zip_path = tmp_path / "out" / "sheet.zip" + monkeypatch.setattr( + routes, + "convert", + records(given, converted(zip_path, fields=10, agreed=9, defaulted=1)), + ) + + code = main( + [ + "convert", + str(tmp_path / "sheet.md"), + "--solutions", + str(tmp_path / "sol.md"), + "--filter", + str(tmp_path / "set.lua"), + "--out", + str(tmp_path / "out"), + "--cache", + str(tmp_path / "cache"), + ] + ) + + assert code == 0 + assert given["document"] == tmp_path / "sheet.md" + assert given["solutions"] == tmp_path / "sol.md" + assert given["lua"] == tmp_path / "set.lua" + assert given["out_dir"] == tmp_path / "out" + assert given["cache_dir"] == tmp_path / "cache" + assert given["backend"] is backend + assert given["name"] == "sheet" + out = capsys.readouterr().out + assert "10 fields, agreed 9, defaulted 1, adjudicated 0, flagged 0" in out + assert f"build {zip_path}" in out + + +def test_convert_takes_the_solutions_document_beside_the_document( + tmp_path, backend, monkeypatch +): + (tmp_path / "sheet.md").write_text("x") + (tmp_path / "sheet_solutions.md").write_text("x") + given = {} + monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) + + assert main(["convert", str(tmp_path / "sheet.md"), "--out", str(tmp_path)]) == 0 + assert given["solutions"] == tmp_path / "sheet_solutions.md" + + +def test_run_without_a_route_converts_the_document(tmp_path, backend, monkeypatch): + given = {} + monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) + monkeypatch.setattr( + pipeline, "run", lambda *a, **k: pytest.fail("the spec route ran") + ) + + assert main(["run", str(tmp_path / "sheet.md"), "--out", str(tmp_path)]) == 0 + assert given["document"] == tmp_path / "sheet.md" + + +def test_the_written_filter_is_kept_in_the_out_directory( + tmp_path, backend, monkeypatch +): + given = {} + monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) + monkeypatch.setattr( + routes, "write_filter", lambda document, solutions, backend: ("-- lua", None) + ) + + code = main( + [ + "convert", + str(tmp_path / "sheet.md"), + "--write-filter", + "--out", + str(tmp_path / "out"), + ] + ) + + assert code == 0 + assert (tmp_path / "out" / "filter.lua").read_text() == "-- lua" + assert given["lua"] == tmp_path / "out" / "filter.lua" + + +def test_a_flagged_field_does_not_stop_the_build(tmp_path, backend, monkeypatch, capsys): + # The zip is written whatever the flags say; a person reads the flags after it. + flag = routes.Flag("q2.p1.worked_solution", "a", "", routes.STRAY_MINUS) + monkeypatch.setattr( + routes, "convert", records({}, converted(tmp_path / "s.zip", flags=[flag])) + ) + + code = main(["convert", str(tmp_path / "sheet.md"), "--out", str(tmp_path)]) + + assert code == 0 + assert "flag q2.p1.worked_solution:" in capsys.readouterr().out + + +def test_convert_without_a_backend_says_what_to_set(tmp_path, backend, monkeypatch, capsys): + def unavailable(*args, **kwargs): + raise ModelUnavailable("set ANTHROPIC_API_KEY, or run `claude login`") + + monkeypatch.setattr(routes, "convert", unavailable) + + code = main(["convert", str(tmp_path / "sheet.md"), "--out", str(tmp_path)]) + printed = capsys.readouterr() + + assert code == 1 + assert "claude login" in printed.err + assert printed.out == "" + + +def test_convert_reports_what_pandoc_said(tmp_path, backend, monkeypatch, capsys): + def refuses(*args, **kwargs): + raise subprocess.CalledProcessError( + 43, "pandoc", stderr=b"Error at line 3 column 1\n" + ) + + monkeypatch.setattr(routes, "convert", refuses) + + code = main(["convert", str(tmp_path / "sheet.tex"), "--out", str(tmp_path)]) + + assert code == 1 + assert "Error at line 3 column 1" in capsys.readouterr().err + + def test_corpus_defaults(): args = build_parser().parse_args(["corpus", "ExampleContents"]) @@ -412,7 +622,7 @@ def record(source, **given): monkeypatch.setattr(pipeline, "run", record) - assert main(["run", "sheet.md"]) == 0 + assert main(["run", "sheet.md", "--route", "spec"]) == 0 assert called["source"] == Path("sheet.md") @@ -425,7 +635,7 @@ def record(source, **passed): monkeypatch.setattr(pipeline, "run", record) - assert main(["run", "sheet.md", "--tries", "5"]) == 0 + assert main(["run", "sheet.md", "--route", "spec", "--tries", "5"]) == 0 assert given["tries"] == 5 diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 22ac5f1..65ffbf1 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -23,6 +23,10 @@ from in2lambda_agent.settings import Settings from in2lambda_agent.spec import RECORD_NAME, SPEC_NAME, BadSpec +# The command line of the spec route, which these tests cover. `run` without +# `--route spec` converts the document through the two routes of routes.py. +RUN = ["run", "--route", "spec"] + FIXTURES = Path(__file__).parent / "fixtures" SOURCE = FIXTURES / "sheet.md" SPEC = (FIXTURES / "sheet-spec.yaml").read_text() @@ -1414,7 +1418,7 @@ def test_the_rounds_running_out_prints_its_stages_and_exits_one( code = main( [ - "run", + *RUN, str(faulty / "faulty.md"), "--rounds", "1", @@ -2077,7 +2081,7 @@ def test_a_pdf_without_credentials_exits_one_naming_the_variables( monkeypatch.delenv("MATHPIX_APP_ID", raising=False) monkeypatch.delenv("MATHPIX_API_KEY", raising=False) - code = main(["run", str(pdf), "--out", str(tmp_path / "out")]) + code = main([*RUN, str(pdf), "--out", str(tmp_path / "out")]) printed = capsys.readouterr() assert code == 1 @@ -2088,7 +2092,7 @@ def test_a_pdf_without_credentials_exits_one_naming_the_variables( def test_the_command_exits_zero_and_prints_a_line_per_stage(sheets, tmp_path, capsys): (sheets / SPEC_NAME).write_text(SPEC) - code = main(["run", str(sheets / "sheet.md"), "--out", str(tmp_path / "out")]) + code = main([*RUN, str(sheets / "sheet.md"), "--out", str(tmp_path / "out")]) printed = capsys.readouterr().out.splitlines() assert code == 0 @@ -2109,7 +2113,7 @@ def test_a_review_run_and_its_approvals_exit_zero_and_build(sheets, tmp_path, ca where = ["--cache", str(tmp_path / "cache")] out = ["--out", str(tmp_path / "out")] - stopped = main(["run", str(sheets / "sheet.md"), "--review", "sample", *where, *out]) + stopped = main([*RUN, str(sheets / "sheet.md"), "--review", "sample", *where, *out]) printed = capsys.readouterr().out # Waiting for a reviewer is not a failure, and nothing is built yet. @@ -2131,7 +2135,7 @@ def test_approving_a_draft_the_checks_fault_exits_one_saying_what_they_found( where = ["--cache", str(tmp_path / "cache")] out = ["--out", str(tmp_path / "out")] - main(["run", str(sheets / "sheet.md"), "--review", "sample", *where, *out]) + main([*RUN, str(sheets / "sheet.md"), "--review", "sample", *where, *out]) main(["review", "edit", EMPTIES["field"], EMPTIES["old"], EMPTIES["new"], *where]) capsys.readouterr() @@ -2156,7 +2160,7 @@ def test_the_default_out_is_the_working_directorys_out(sheets, tmp_path, monkeyp (sheets / SPEC_NAME).write_text(SPEC) monkeypatch.chdir(tmp_path) - assert main(["run", str(sheets / "sheet.md")]) == 0 + assert main([*RUN, str(sheets / "sheet.md")]) == 0 assert (tmp_path / "out" / "set.zip").exists() @@ -2166,7 +2170,7 @@ def test_a_run_the_checks_fault_prints_its_stages_and_exits_one( (sheets / SPEC_NAME).write_text(PARTLESS_SPEC) code = main( - ["run", str(sheets / "sheet.md"), "--rounds", "0", "--out", str(tmp_path)] + [*RUN, str(sheets / "sheet.md"), "--rounds", "0", "--out", str(tmp_path)] ) printed = capsys.readouterr() @@ -2182,7 +2186,7 @@ def test_a_run_with_no_backend_exits_one_naming_what_to_do( pipeline, "choose_backend", lambda settings: FakeBackend(reason="run claude login") ) - code = main(["run", str(sheets / "sheet.md"), "--out", str(tmp_path / "out")]) + code = main([*RUN, str(sheets / "sheet.md"), "--out", str(tmp_path / "out")]) printed = capsys.readouterr() assert code == 1 @@ -2236,7 +2240,7 @@ def test_a_refused_build_prints_its_stages_and_exits_one( ), ) - code = main(["run", str(figures / "figure.md"), "--out", str(tmp_path / "out")]) + code = main([*RUN, str(figures / "figure.md"), "--out", str(tmp_path / "out")]) printed = capsys.readouterr() assert code == 1 diff --git a/tests/test_routes.py b/tests/test_routes.py index 1e8ff60..2fa4a58 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -21,7 +21,7 @@ from conftest import FakeBackend import in2lambda_agent.routes as routes -from in2lambda_agent import pair +from in2lambda_agent import cli, pair from in2lambda_agent.settings import Settings ME2 = Path(__file__).parent / "fixtures" / "me2" @@ -33,6 +33,10 @@ "/Users/peterbjohnson/code/lambdafeedback/in2lambda-agent/ExampleContents/" "PHYS40002-Mechanics/problem_sheets_and_figures" ) +ME2_TARGET = Path( + "/Users/peterbjohnson/code/lambdafeedback/in2lambda-agent/ExampleContents/targets/" + "ME2_Fluids_introduction" +) FILTER = Path(__file__).parent / "fixtures" / "ps1-filter.lua" live = pytest.mark.skipif(not os.environ.get("IN2LAMBDA_LIVE"), reason="calls Mathpix and a model") @@ -415,6 +419,58 @@ def test_a_sheet_whose_filter_run_fails_keeps_its_route_a_reply(tmp_path): assert (result.fields, result.agreed, result.flags) == (0, 0, []) +# --- the report of one document ------------------------------------------------------------- + + +def test_the_report_of_a_document_gives_the_flags_the_counts_and_the_zip(tmp_path): + result = routes.Converted( + set=None, zip_path=tmp_path / "sheet.zip", reply=[], + flags=[routes.Flag("q2.p2.content", "Find the drag.", "Find the drag, in N.", "two readings")], + fields=10, agreed=8, defaulted=1, adjudicated=1, + ) + assert result.report() == [ + "flag q2.p2.content: two readings", + " A: Find the drag.", + " B: Find the drag, in N.", + "fields 10 fields, agreed 8, defaulted 1, adjudicated 1, flagged 1", + f"build {tmp_path / 'sheet.zip'}", + ] + + +def test_a_flag_only_one_route_filled_shows_the_one_text(tmp_path): + result = routes.Converted( + set=None, zip_path=tmp_path / "sheet.zip", reply=[], + flags=[routes.Flag("q1.p1.worked_solution", "$$\n-x\n$$", "", routes.STRAY_MINUS)], + fields=10, agreed=10, + ) + assert result.report() == [ + f"flag q1.p1.worked_solution: {routes.STRAY_MINUS}", + "fields 10 fields, agreed 10, defaulted 0, adjudicated 0, flagged 1", + f"build {tmp_path / 'sheet.zip'}", + ] + + +def test_the_report_says_where_route_b_did_not_run(tmp_path): + # No filter, so no field was compared and every field is route A's. + result = routes.Converted(set=None, zip_path=tmp_path / "sheet.zip", flags=[], reply=[]) + assert result.report() == [ + "fields 0 fields, agreed 0, defaulted 0, adjudicated 0, flagged 0 (route B did not run)", + f"build {tmp_path / 'sheet.zip'}", + ] + + +def test_the_report_says_what_the_filter_run_failed_with(tmp_path): + result = routes.Converted( + set=None, zip_path=tmp_path / "sheet.zip", flags=[], reply=[], + route_b_error="Error running filter set.lua: attempt to index a nil value", + ) + assert result.report() == [ + "fields 0 fields, agreed 0, defaulted 0, adjudicated 0, flagged 0", + "route B failed: Error running filter set.lua: attempt to index a nil value", + f"build {tmp_path / 'sheet.zip'}", + ] + + # --- live ------------------------------------------------------------------------------- @@ -451,3 +507,26 @@ def test_the_me2_pair_converts_with_no_flag(tmp_path): ("q3.p1.worked_solution", routes.STRAY_MINUS), ] assert [q.title for q in result.set.questions] == [q["title"] for q in exported()] + + +@live +@pytest.mark.skipif(not ME2_TARGET.is_dir(), reason="private corpus") +def test_the_me2_pair_converts_from_the_command_line(tmp_path, capsys): + # The ticket's run: the command a reader types, the report it prints, the zip. + (pdf,) = [p for p in ME2_TARGET.glob("*.pdf") if "solutions" not in p.name] + (solutions,) = ME2_TARGET.glob("*solutions.pdf") + + code = cli.main( + ["convert", str(pdf), "--solutions", str(solutions), "--out", str(tmp_path / "out")] + ) + printed = capsys.readouterr().out + print("\n" + printed) + + assert code == 0 + # The printed solutions PDF holds separator lines that Mathpix reads as minus signs, + # so the worked solutions of Friction on a plate and Towing a submarine are flagged. + assert [line.split(":")[0] for line in printed.splitlines() if line.startswith("flag")] == [ + "flag q2.p1.worked_solution", + "flag q3.p1.worked_solution", + ] + assert printed.splitlines()[-1].startswith(f"build {tmp_path / 'out'}") From 1c683ac96a44f26a9697912a8eec529bb3ec4a18 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 02:25:48 +0100 Subject: [PATCH 2/7] implement: Add a `convert` command to the command line (t38) --- README.md | 4 ++++ in2lambda_agent/cli.py | 47 ++++++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/README.md b/README.md index 5d4b84f..43f0f3a 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,10 @@ person reads the flags after it. `run DOCUMENT` converts the document as `convert` does and takes the same options. `--route spec` runs the earlier route instead, which the rest of this section describes. +The two routes' options do not mix: `--review` without `--route spec`, or +`--write-filter` with it, is refused naming the route the option belongs to, because a +run that read it and ignored it would be a spec written twice or a review never stopped +for. ```sh poetry run in2lambda-agent run sheet.pdf --route spec diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index dab06f4..204db29 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -370,6 +370,47 @@ def build_parser() -> argparse.ArgumentParser: return parser +# Each route's own options. `run` takes both sets, because argparse cannot know +# the route until it has parsed the line, so the run refuses an option of the +# route it is not taking rather than reading it and throwing it away. +_SPEC_ROUTE_OPTIONS = { + "spec": "--spec", + "review": "--review", + "rounds": "--rounds", + "tries": "--tries", + "sample": "--sample", + "fresh_ocr": "--fresh-ocr", +} +_DIRECT_ROUTE_OPTIONS = { + "solutions": "--solutions", + "filter": "--filter", + "write_filter": "--write-filter", +} + + +def misplaced_option(args: argparse.Namespace) -> Optional[str]: + """What is wrong where `run` was given an option of the other route. + + Args: + args: The parsed arguments of `run`. + + Returns: + What to print, naming the option and the route it belongs to, or None + where every option given belongs to the route the run is taking. + """ + if args.route == "direct": + options, route, fix = _SPEC_ROUTE_OPTIONS, "spec", "add --route spec" + else: + options, route, fix = _DIRECT_ROUTE_OPTIONS, "direct", "drop --route spec" + # An option counts as given where it is not the parser's default, which is + # read back from the parser rather than repeated here. + defaults = build_parser().parse_args(["run", str(args.source)]) + for dest, name in options.items(): + if getattr(args, dest) != getattr(defaults, dest): + return f"{name} is an option of the {route} route; {fix}" + return None + + def convert_command(args: argparse.Namespace) -> int: """Converts one document through both routes and prints the report. @@ -432,6 +473,12 @@ def main(argv: Optional[Sequence[str]] = None) -> int: """ args = build_parser().parse_args(argv) + if args.command == "run": + wrong = misplaced_option(args) + if wrong: + print(f"in2lambda-agent: {wrong}", file=sys.stderr) + return 1 + if args.command == "convert" or (args.command == "run" and args.route == "direct"): return convert_command(args) diff --git a/tests/test_cli.py b/tests/test_cli.py index fed0187..6862e9d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -291,6 +291,45 @@ def refuses(*args, **kwargs): assert "Error at line 3 column 1" in capsys.readouterr().err +@pytest.mark.parametrize( + "given, message", + [ + ( + ["--review", "per-question"], + "--review is an option of the spec route; add --route spec", + ), + ( + ["--spec", "set.yaml"], + "--spec is an option of the spec route; add --route spec", + ), + ( + ["--route", "spec", "--write-filter"], + "--write-filter is an option of the direct route; drop --route spec", + ), + ( + ["--route", "spec", "--solutions", "sol.md"], + "--solutions is an option of the direct route; drop --route spec", + ), + ], +) +def test_an_option_of_the_other_route_is_refused(given, message, monkeypatch, capsys): + # Under the route it does not belong to the option would be parsed and + # thrown away: a saved spec ignored and written again by two model calls, a + # review never stopped for, a filter never written. So the run says so, and + # says so before it has paid for anything. + monkeypatch.setattr( + cli, "choose_backend", lambda settings: pytest.fail("a model was asked for") + ) + monkeypatch.setattr( + pipeline, "run", lambda *a, **k: pytest.fail("the spec route ran") + ) + + code = main(["run", "sheet.md", *given]) + + assert code == 1 + assert capsys.readouterr().err.strip() == f"in2lambda-agent: {message}" + + def test_corpus_defaults(): args = build_parser().parse_args(["corpus", "ExampleContents"]) From 1c4dcc292136288b7f578dbee95acbdfaa49f643 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 02:57:46 +0100 Subject: [PATCH 3/7] implement: Add a `convert` command to the command line (t38) --- README.md | 16 ++++++++++++---- in2lambda_agent/cli.py | 5 +++++ in2lambda_agent/routes.py | 12 +++++++----- tests/test_cli.py | 31 ++++++++++++++++++++++++++++++- tests/test_routes.py | 12 ++++++++---- 5 files changed, 62 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 43f0f3a..ec7d24d 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,13 @@ the set's JSON folder and its zip. file beside `DOCUMENT` whose name is the document'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`. Route A reads both documents in one call, and route B reads -each under its own role. +each under its own role. Where the two names share nothing, as they do where the +platform has put the time of the download in each, the run finds no solutions document +and says so: + +``` +solutions none found beside ME2_Fluids_2024-03-11.pdf; pass --solutions FILE +``` `--filter` names the Lua filter route B runs, which is the file `--write-filter` wrote for another sheet of the same set. `--write-filter` writes one for this document with a @@ -98,9 +104,11 @@ build /home/me/out/sheet.zip A flag names the field, the reason, and each route's text where both routes filled the field. `fields` counts the fields the two routes agreed on, the fields one route alone -filled, the fields the adjudicating call settled, and the fields flagged. A filter run -that fails adds a `route B failed` line naming pandoc's message, and the set is route -A's reading alone. +filled, the fields the adjudicating call settled, and the fields flagged. With no +filter there is no comparison to count, so the line is `60 fields, route B did not run`: +the fields are route A's, and each one is flagged or is route A's word for it. A filter +run that fails adds a `route B failed` line naming pandoc's message, and the set is +route A's reading alone. `convert` exits 1 where Mathpix, the model or pandoc failed, and 0 otherwise. A flagged field does not change the exit code: the zip is written whatever the flags say, and a diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 204db29..0b240e3 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -426,6 +426,11 @@ def convert_command(args: argparse.Namespace) -> int: # `run SOURCE` converts the same document, under the other name. document = Path(getattr(args, "document", None) or args.source) solutions = args.solutions or pair.solutions_beside(document) + if solutions is None and pair.questions_stem(document) is None: + # A pair whose two documents do not share a stem, which is what the platform + # writes where it puts a timestamp in each name, is not a sheet with no + # solutions; a run that said nothing about it would read as one. + print(f"solutions none found beside {document}; pass --solutions FILE") out_dir = Path(args.out) settings = load_settings() backend = choose_backend(settings) diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 730ccee..490693f 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -400,12 +400,14 @@ def report(self) -> list[str]: lines.append(f"flag {one.field}: {one.reason}") if one.a and one.b: lines += [f" A: {_squash(one.a)}", f" B: {_squash(one.b)}"] - counts = _counted( - [self.fields, self.agreed, self.defaulted, self.adjudicated, len(self.flags)] - ) if not self.fields and self.route_b_error is None: - # No filter was given, so no field was compared and every field is route A's. - counts += " (route B did not run)" + # No filter was given, so no field was compared: every field is route A's, + # and the counts of a comparison that did not happen say nothing. + counts = f"{len(fields(normalise(self.reply)))} fields, route B did not run" + else: + counts = _counted( + [self.fields, self.agreed, self.defaulted, self.adjudicated, len(self.flags)] + ) lines.append(f"fields {counts}") if self.route_b_error: lines.append(f"route B failed: {self.route_b_error}") diff --git a/tests/test_cli.py b/tests/test_cli.py index 6862e9d..01d1015 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -215,6 +215,34 @@ def test_convert_takes_the_solutions_document_beside_the_document( assert given["solutions"] == tmp_path / "sheet_solutions.md" +def test_convert_says_where_it_found_no_solutions_document( + tmp_path, backend, monkeypatch, capsys +): + # A pair named without a shared stem is not a sheet with no solutions, and a run + # that said nothing about it would read as one. + monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) + + assert main(["convert", str(tmp_path / "sheet.pdf"), "--out", str(tmp_path)]) == 0 + assert ( + f"solutions none found beside {tmp_path / 'sheet.pdf'}; pass --solutions FILE" + in capsys.readouterr().out + ) + + +@pytest.mark.parametrize("named, found", [("sheet.pdf", "sheet_solutions.pdf"), ("sheet_solutions.pdf", None)]) +def test_convert_says_nothing_where_there_is_nothing_to_pass( + named, found, tmp_path, backend, monkeypatch, capsys +): + # A document converted with its solutions, and a solutions document converted on + # its own, are both what the reader asked for. + if found: + (tmp_path / found).write_text("x") + monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) + + assert main(["convert", str(tmp_path / named), "--out", str(tmp_path)]) == 0 + assert "solutions" not in capsys.readouterr().out + + def test_run_without_a_route_converts_the_document(tmp_path, backend, monkeypatch): given = {} monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) @@ -274,7 +302,8 @@ def unavailable(*args, **kwargs): assert code == 1 assert "claude login" in printed.err - assert printed.out == "" + # Nothing was converted, so there is no report. + assert "fields" not in printed.out and "build" not in printed.out def test_convert_reports_what_pandoc_said(tmp_path, backend, monkeypatch, capsys): diff --git a/tests/test_routes.py b/tests/test_routes.py index 2fa4a58..ab239e6 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -450,11 +450,15 @@ def test_a_flag_only_one_route_filled_shows_the_one_text(tmp_path): ] -def test_the_report_says_where_route_b_did_not_run(tmp_path): - # No filter, so no field was compared and every field is route A's. - result = routes.Converted(set=None, zip_path=tmp_path / "sheet.zip", flags=[], reply=[]) +def test_the_report_counts_route_as_fields_where_route_b_did_not_run(tmp_path): + # No filter, so no field was compared and every field is route A's. The counts of + # the comparison are left out rather than printed as zero. + result = routes.Converted( + set=None, zip_path=tmp_path / "sheet.zip", flags=[], fields=0, + reply=[{"title": "Ball", "main_text": "", "parts": [{"content": "Find h."}]}], + ) assert result.report() == [ - "fields 0 fields, agreed 0, defaulted 0, adjudicated 0, flagged 0 (route B did not run)", + "fields 5 fields, route B did not run", f"build {tmp_path / 'sheet.zip'}", ] From d53d1df547630db30c31d3d23cf64b7c69804b7e Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 03:21:21 +0100 Subject: [PATCH 4/7] implement: Add a `convert` command to the command line (t38) --- README.md | 15 +++++-------- in2lambda_agent/cli.py | 9 +++----- in2lambda_agent/routes.py | 12 ++++++----- tests/test_cli.py | 44 ++++++++++++++++++--------------------- tests/test_routes.py | 7 +++++-- 5 files changed, 40 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index ec7d24d..48c7211 100644 --- a/README.md +++ b/README.md @@ -76,13 +76,7 @@ the set's JSON folder and its zip. file beside `DOCUMENT` whose name is the document'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`. Route A reads both documents in one call, and route B reads -each under its own role. Where the two names share nothing, as they do where the -platform has put the time of the download in each, the run finds no solutions document -and says so: - -``` -solutions none found beside ME2_Fluids_2024-03-11.pdf; pass --solutions FILE -``` +each under its own role. `--filter` names the Lua filter route B runs, which is the file `--write-filter` wrote for another sheet of the same set. `--write-filter` writes one for this document with a @@ -107,10 +101,11 @@ field. `fields` counts the fields the two routes agreed on, the fields one route filled, the fields the adjudicating call settled, and the fields flagged. With no filter there is no comparison to count, so the line is `60 fields, route B did not run`: the fields are route A's, and each one is flagged or is route A's word for it. A filter -run that fails adds a `route B failed` line naming pandoc's message, and the set is -route A's reading alone. +run that fails counts route A's fields in the same way, and adds a `route B failed` line +naming pandoc's message; the set is route A's reading alone. -`convert` exits 1 where Mathpix, the model or pandoc failed, and 0 otherwise. A flagged +`convert` exits 1 where a named file is not there, and where Mathpix, the model or +pandoc failed, and 0 otherwise. A flagged field does not change the exit code: the zip is written whatever the flags say, and a person reads the flags after it. diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 0b240e3..62899ad 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -426,11 +426,6 @@ def convert_command(args: argparse.Namespace) -> int: # `run SOURCE` converts the same document, under the other name. document = Path(getattr(args, "document", None) or args.source) solutions = args.solutions or pair.solutions_beside(document) - if solutions is None and pair.questions_stem(document) is None: - # A pair whose two documents do not share a stem, which is what the platform - # writes where it puts a timestamp in each name, is not a sheet with no - # solutions; a run that said nothing about it would read as one. - print(f"solutions none found beside {document}; pass --solutions FILE") out_dir = Path(args.out) settings = load_settings() backend = choose_backend(settings) @@ -452,7 +447,9 @@ def convert_command(args: argparse.Namespace) -> int: lua=lua, name=document.stem, ) - except (MathpixError, ModelUnavailable, ModelError) as error: + except (MathpixError, ModelUnavailable, ModelError, OSError) as error: + # A document that is not there raises an OSError here, because this route reads + # the file itself and in2lambda never sees the name. print(f"in2lambda-agent: {error}", file=sys.stderr) return 1 except subprocess.CalledProcessError as error: diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 490693f..3212520 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -400,14 +400,16 @@ def report(self) -> list[str]: lines.append(f"flag {one.field}: {one.reason}") if one.a and one.b: lines += [f" A: {_squash(one.a)}", f" B: {_squash(one.b)}"] - if not self.fields and self.route_b_error is None: - # No filter was given, so no field was compared: every field is route A's, - # and the counts of a comparison that did not happen say nothing. - counts = f"{len(fields(normalise(self.reply)))} fields, route B did not run" - else: + if self.fields: counts = _counted( [self.fields, self.agreed, self.defaulted, self.adjudicated, len(self.flags)] ) + else: + # No filter was given, or the filter run failed: no field was compared, and + # every field is route A's. The counts of a comparison that did not happen + # say nothing, so the line counts route A's fields instead. + ran = "failed" if self.route_b_error else "did not run" + counts = f"{len(fields(normalise(self.reply)))} fields, route B {ran}" lines.append(f"fields {counts}") if self.route_b_error: lines.append(f"route B failed: {self.route_b_error}") diff --git a/tests/test_cli.py b/tests/test_cli.py index 01d1015..bd44af8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -215,32 +215,28 @@ def test_convert_takes_the_solutions_document_beside_the_document( assert given["solutions"] == tmp_path / "sheet_solutions.md" -def test_convert_says_where_it_found_no_solutions_document( - tmp_path, backend, monkeypatch, capsys -): - # A pair named without a shared stem is not a sheet with no solutions, and a run - # that said nothing about it would read as one. - monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) - - assert main(["convert", str(tmp_path / "sheet.pdf"), "--out", str(tmp_path)]) == 0 - assert ( - f"solutions none found beside {tmp_path / 'sheet.pdf'}; pass --solutions FILE" - in capsys.readouterr().out - ) +@pytest.mark.parametrize("missing", ["document", "solutions"]) +def test_convert_names_a_file_that_is_not_there(missing, tmp_path, backend, capsys): + # This route reads each file itself, so in2lambda never sees the name and never + # complains about it. + if missing == "solutions": + (tmp_path / "sheet.md").write_text("# Question 1\n") + code = main( + [ + "convert", + str(tmp_path / "sheet.md"), + "--solutions", + str(tmp_path / "sol.md"), + "--out", + str(tmp_path), + ] + ) + printed = capsys.readouterr() -@pytest.mark.parametrize("named, found", [("sheet.pdf", "sheet_solutions.pdf"), ("sheet_solutions.pdf", None)]) -def test_convert_says_nothing_where_there_is_nothing_to_pass( - named, found, tmp_path, backend, monkeypatch, capsys -): - # A document converted with its solutions, and a solutions document converted on - # its own, are both what the reader asked for. - if found: - (tmp_path / found).write_text("x") - monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) - - assert main(["convert", str(tmp_path / named), "--out", str(tmp_path)]) == 0 - assert "solutions" not in capsys.readouterr().out + assert code == 1 + assert printed.err.startswith("in2lambda-agent: ") + assert ("sol.md" if missing == "solutions" else "sheet.md") in printed.err def test_run_without_a_route_converts_the_document(tmp_path, backend, monkeypatch): diff --git a/tests/test_routes.py b/tests/test_routes.py index ab239e6..f15de3a 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -464,12 +464,15 @@ def test_the_report_counts_route_as_fields_where_route_b_did_not_run(tmp_path): def test_the_report_says_what_the_filter_run_failed_with(tmp_path): + # The set is route A's reading alone, so the line counts route A's fields, as it + # does where no filter was given at all. result = routes.Converted( - set=None, zip_path=tmp_path / "sheet.zip", flags=[], reply=[], + set=None, zip_path=tmp_path / "sheet.zip", flags=[], fields=0, + reply=[{"title": "Ball", "main_text": "", "parts": [{"content": "Find h."}]}], route_b_error="Error running filter set.lua: attempt to index a nil value", ) assert result.report() == [ - "fields 0 fields, agreed 0, defaulted 0, adjudicated 0, flagged 0", + "fields 5 fields, route B failed", "route B failed: Error running filter set.lua: attempt to index a nil value", f"build {tmp_path / 'sheet.zip'}", ] From 09c4511cea802757d59bcd62bd3ea66b2f56e04d Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 03:51:27 +0100 Subject: [PATCH 5/7] implement: Add a `convert` command to the command line (t38) --- README.md | 17 +++++++++++--- in2lambda_agent/cli.py | 8 +++++++ tests/test_cli.py | 52 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 48c7211..d285f62 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,17 @@ the set's JSON folder and its zip. file beside `DOCUMENT` whose name is the document'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`. Route A reads both documents in one call, and route B reads -each under its own role. +each under its own role. The `solutions` line of the report names the document the +conversion read. Where the two names share no stem, as they do where the platform has +put the time of the download in each, the agent finds no solutions document and the +line names none: + +``` +solutions none found beside ME2_Fluids_2024-03-11.pdf; pass --solutions FILE +``` + +A conversion that reads that line and goes on writes an empty answer and an empty +worked solution for every question. `--filter` names the Lua filter route B runs, which is the file `--write-filter` wrote for another sheet of the same set. `--write-filter` writes one for this document with a @@ -84,10 +94,11 @@ model call and keeps it at `OUT/filter.lua`. The two options together are refuse conversion runs one filter. With neither option route A converts the document alone, no field is compared, and the counts line says so. -The command prints one line per flagged field, the counts of the comparison, and the -zip: +The command prints the solutions document, one line per flagged field, the counts of +the comparison, and the zip: ``` +solutions /home/me/sheets/sheet_solutions.pdf flag q2.p1.worked_solution: a stray minus sign inside or beside a display maths; Mathpix reads a separator line as one flag q4.p2.content: two readings of the source A: Find the drag force on the plate. diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 62899ad..185a408 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -426,6 +426,14 @@ def convert_command(args: argparse.Namespace) -> int: # `run SOURCE` converts the same document, under the other name. document = Path(getattr(args, "document", None) or args.source) solutions = args.solutions or pair.solutions_beside(document) + if solutions is not None: + print(f"solutions {solutions}") + elif pair.questions_stem(document) is None: + # A pair whose two names share no stem, which is what the platform writes where + # it puts the time of the download in each name, is a sheet whose solutions the + # run did not find. A run that said nothing would read as a sheet with none, + # and the set it writes holds an empty answer for every question. + print(f"solutions none found beside {document}; pass --solutions FILE") out_dir = Path(args.out) settings = load_settings() backend = choose_backend(settings) diff --git a/tests/test_cli.py b/tests/test_cli.py index bd44af8..fe59707 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -215,6 +215,58 @@ def test_convert_takes_the_solutions_document_beside_the_document( assert given["solutions"] == tmp_path / "sheet_solutions.md" +@pytest.mark.parametrize("named", [False, True]) +def test_convert_names_the_solutions_document_it_read( + named, tmp_path, backend, monkeypatch, capsys +): + # The document `--solutions` names and the document found beside the questions are + # reported the same way: the reader sees which file the answers came from. + solutions = tmp_path / "sheet_solutions.md" + solutions.write_text("x") + (tmp_path / "sheet.md").write_text("x") + monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) + + code = main( + [ + "convert", + str(tmp_path / "sheet.md"), + "--out", + str(tmp_path), + *(["--solutions", str(solutions)] if named else []), + ] + ) + + assert code == 0 + assert f"solutions {solutions}" in capsys.readouterr().out + + +def test_convert_says_where_it_found_no_solutions_document( + tmp_path, backend, monkeypatch, capsys +): + # A pair whose two names share no stem is a sheet whose solutions the run did not + # find. A run that said nothing would read as a sheet with none. + monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) + + assert main(["convert", str(tmp_path / "sheet.pdf"), "--out", str(tmp_path)]) == 0 + assert ( + f"solutions none found beside {tmp_path / 'sheet.pdf'}; pass --solutions FILE" + in capsys.readouterr().out + ) + + +def test_convert_says_nothing_of_the_solutions_of_a_solutions_document( + tmp_path, backend, monkeypatch, capsys +): + # A solutions document converted on its own is what the reader asked for, and there + # is no file for `--solutions` to name. + monkeypatch.setattr(routes, "convert", records({}, converted(tmp_path / "s.zip"))) + + code = main(["convert", str(tmp_path / "sheet_solutions.md"), "--out", str(tmp_path)]) + + assert code == 0 + assert "solutions" not in capsys.readouterr().out + + @pytest.mark.parametrize("missing", ["document", "solutions"]) def test_convert_names_a_file_that_is_not_there(missing, tmp_path, backend, capsys): # This route reads each file itself, so in2lambda never sees the name and never From 313b96bed9f3688af52c24d9d3552f91cfe23422 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 04:18:16 +0100 Subject: [PATCH 6/7] implement: Add a `convert` command to the command line (t38) --- README.md | 7 +++++-- in2lambda_agent/cli.py | 10 +++++++++- tests/test_cli.py | 45 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d285f62..abb25b6 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,11 @@ the set's JSON folder and its zip. `--solutions` names the document holding the solutions. Without it the agent takes the file beside `DOCUMENT` whose name is the document'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`. Route A reads both documents in one call, and route B reads -each under its own role. The `solutions` line of the report names the document the +`Worksheet_1_solutions.pdf`. Naming the solutions document converts the pair too, and +the set is named after the questions document whichever of the two you name; a solutions +document with no questions document beside it converts on its own. Route A reads both +documents in one call, and route B reads each under its own role. The `solutions` line +of the report names the document the conversion read. Where the two names share no stem, as they do where the platform has put the time of the download in each, the agent finds no solutions document and the line names none: diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 185a408..29e4da4 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -425,7 +425,15 @@ def convert_command(args: argparse.Namespace) -> int: """ # `run SOURCE` converts the same document, under the other name. document = Path(getattr(args, "document", None) or args.source) - solutions = args.solutions or pair.solutions_beside(document) + if args.solutions is not None: + # The user named the two documents, so the folder is not asked. + solutions = args.solutions + else: + # Either half of a pair may be named, so the pairing goes both ways: name the + # solutions document and the questions document beside it is what converts, and + # the set is named after it. A solutions document with none beside it comes back + # as the document itself, and converts on its own. + document, solutions = pair.of(document) if solutions is not None: print(f"solutions {solutions}") elif pair.questions_stem(document) is None: diff --git a/tests/test_cli.py b/tests/test_cli.py index fe59707..59e2aae 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -215,6 +215,51 @@ def test_convert_takes_the_solutions_document_beside_the_document( assert given["solutions"] == tmp_path / "sheet_solutions.md" +@pytest.mark.parametrize("command", ["convert", "run"]) +def test_convert_named_by_its_solutions_document_converts_the_pair( + command, tmp_path, backend, monkeypatch, capsys +): + # Naming either half of a pair converts the pair, and the set is named after the + # questions document, as the spec route has always done. + (tmp_path / "Worksheet_1.md").write_text("x") + (tmp_path / "Worksheet_1_solutions.md").write_text("x") + given = {} + monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) + + code = main( + [command, str(tmp_path / "Worksheet_1_solutions.md"), "--out", str(tmp_path)] + ) + + assert code == 0 + assert given["document"] == tmp_path / "Worksheet_1.md" + assert given["solutions"] == tmp_path / "Worksheet_1_solutions.md" + assert given["name"] == "Worksheet_1" + assert f"solutions {tmp_path / 'Worksheet_1_solutions.md'}" in capsys.readouterr().out + + +def test_convert_with_solutions_named_does_not_pair(tmp_path, backend, monkeypatch): + # `--solutions` means what the user says, not what the folder holds. + (tmp_path / "Worksheet_1.md").write_text("x") + (tmp_path / "Worksheet_1_solutions.md").write_text("x") + given = {} + monkeypatch.setattr(routes, "convert", records(given, converted(tmp_path / "s.zip"))) + + code = main( + [ + "convert", + str(tmp_path / "Worksheet_1_solutions.md"), + "--solutions", + str(tmp_path / "other.md"), + "--out", + str(tmp_path), + ] + ) + + assert code == 0 + assert given["document"] == tmp_path / "Worksheet_1_solutions.md" + assert given["solutions"] == tmp_path / "other.md" + + @pytest.mark.parametrize("named", [False, True]) def test_convert_names_the_solutions_document_it_read( named, tmp_path, backend, monkeypatch, capsys From b843f2c2da8c7fe861b1466a7a33a18c016d68ae Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 04:40:43 +0100 Subject: [PATCH 7/7] implement: Add a `convert` command to the command line (t38) --- in2lambda_agent/cli.py | 6 ++++-- in2lambda_agent/routes.py | 23 +++++++++++++++++++++-- tests/test_cli.py | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 29e4da4..5df4d4b 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -463,9 +463,11 @@ def convert_command(args: argparse.Namespace) -> int: lua=lua, name=document.stem, ) - except (MathpixError, ModelUnavailable, ModelError, OSError) as error: + except (MathpixError, ModelUnavailable, ModelError, OSError, routes.BadReply) as error: # A document that is not there raises an OSError here, because this route reads - # the file itself and in2lambda never sees the name. + # the file itself and in2lambda never sees the name. A reply that is not a JSON + # list of questions raises BadReply, as a reply that is not a spec raises + # BadSpec on the other route. print(f"in2lambda-agent: {error}", file=sys.stderr) return 1 except subprocess.CalledProcessError as error: diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 3212520..581f2ce 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -221,8 +221,27 @@ def _prompt(markdown: str, solutions: Optional[str]) -> str: ) -def _json(text: str) -> Any: - return json.loads(re.sub(r"^```(json)?\s*|\s*```$", "", text.strip())) +class BadReply(ValueError): + """What the model answered with is not a JSON list.""" + + +def _json(text: str) -> list: + """The JSON list a call was asked for. + + Raises: + BadReply: the text is not JSON, or is JSON that is not a list. A model that + answers with a sentence, and an answer cut short at the output-token + limit, both arrive here; `fields` and `to_set` read a list, and neither + reports the text they were given instead. + """ + stripped = re.sub(r"^```(json)?\s*|\s*```$", "", text.strip()) + try: + answered = json.loads(stripped) + except json.JSONDecodeError as error: + raise BadReply(f"The reply is not JSON: {error}.") from None + if not isinstance(answered, list): + raise BadReply(f"A reply is a JSON list, which {_squash(stripped)[:60]!r} is not.") + return answered def direct(markdown: str, solutions: Optional[str], backend: Backend) -> tuple[Reply_, Reply]: diff --git a/tests/test_cli.py b/tests/test_cli.py index 59e2aae..a1a3aad 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -413,6 +413,26 @@ def refuses(*args, **kwargs): assert "Error at line 3 column 1" in capsys.readouterr().err +@pytest.mark.parametrize( + "answer", ["I cannot convert this sheet.", '{"title": "A ball", "parts": []}'] +) +def test_convert_reports_a_reply_that_is_not_a_list_of_questions( + answer, tmp_path, monkeypatch, capsys +): + # Route A is asked for a JSON list. A model that answers with a sentence, and an + # answer cut short at the output-token limit, both arrive as text no step below + # route A reads. The run names the fault, as it does for pandoc and for Mathpix. + (tmp_path / "sheet.md").write_text("# Question 1\n\nFind the height.\n") + monkeypatch.setattr(cli, "choose_backend", lambda settings: FakeBackend(answer)) + + code = main(["convert", str(tmp_path / "sheet.md"), "--out", str(tmp_path / "out")]) + printed = capsys.readouterr() + + assert code == 1 + assert printed.err.startswith("in2lambda-agent: ") + assert "fields" not in printed.out and "build" not in printed.out + + @pytest.mark.parametrize( "given, message", [