From 098e625825bfc21ac0e6026ef18ac947b74935d0 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 05:32:44 +0100 Subject: [PATCH 1/4] implement: Compare each target document with the set Lambda Feedback exported from it (t40) --- .gitignore | 4 + README.md | 57 ++++++++ in2lambda_agent/cli.py | 60 +++++++- in2lambda_agent/pair.py | 50 +++++++ in2lambda_agent/targets.py | 277 +++++++++++++++++++++++++++++++++++ poetry.lock | 6 +- pyproject.toml | 6 +- tests/test_cli.py | 88 +++++++++++- tests/test_pair.py | 52 +++++++ tests/test_targets.py | 287 +++++++++++++++++++++++++++++++++++++ 10 files changed, 878 insertions(+), 9 deletions(-) create mode 100644 in2lambda_agent/targets.py create mode 100644 tests/test_targets.py diff --git a/.gitignore b/.gitignore index 8d879b1..e25d806 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,10 @@ dist/ # gate-baseline.json beside them records those documents' file names and the # absolute path of the corpus on one machine. corpus-specs/ +# The default `--filters`: a target's filter is written from a private +# document's structure, and a line of its differs.txt quotes that document's +# text. +targets/ # in2lambda's KaTeX converter writes this into the working directory on import. log # The default `--cache`, which a run and the tests write into the directory diff --git a/README.md b/README.md index 43f0f3a..06204c0 100644 --- a/README.md +++ b/README.md @@ -389,6 +389,63 @@ sweep, and a set whose folder cannot be copied is a row for each of its document the directory `run` caches into, so a sweep over PDFs that `run` has already converted makes no Mathpix call and needs no Mathpix credentials. +## Targets + +A target is a folder holding one set: a questions document, a solutions document where +the set has one, and the folder Lambda Feedback exported for that set, named +`set_`. The export is what the conversion is trying to reproduce, so a target is +the one place the agent can be told right from wrong rather than merely flagged: + +```sh +poetry run in2lambda-agent targets ExampleContents/targets +``` + +In full: + +```sh +poetry run in2lambda-agent targets ROOT [PATH ...] [--filters DIR] [--out DIR] [--cache DIR] +``` + +`ROOT` is the directory the targets are under and each `PATH` a folder under it to run, +defaulting to all of them. A target is found by the `set_*` folder it holds, either +directly under `ROOT` or grouped by course a folder down: +`targets/ME2_Fluids_introduction/` and `targets/EART40013_Mathematical_Methods_II/CW1/` +are both targets. The folder's two documents are read by role rather than by name: the +one whose name ends in `_solutions` is the solutions document, and the other is the +questions document, which is how a sheet pairs with a solutions document the platform +printed months later under a name of its own. A folder holding two of either is +reported and not run. + +Each target is converted, and the zip it wrote is compared question by question with +the export. The run prints one line per difference and one line of counts per target: + +``` +differs ME2_Fluids_introduction: Question 2 "", part (a), text: the agent says … and the export says … +known ME2_Fluids_introduction: Question 1 "", main text: the agent says … and the export says … +ME2_Fluids_introduction: 4 differ, 3 known, 1 new, 2 flagged +``` + +A difference you have read and accepted goes into `differs.txt` beside that target's +filter, one line as the run printed it — without the `differs NAME: ` the line begins +with — and the ticket that would close it after a `#`: + +``` +Question 1 "", main text: the agent says '…' and the export says '…' # t42 Mathpix reads the piston sketch's caption twice +``` + +An accepted line is reported as `known` from then on. The command exits 0 when every +target ran and reported nothing new, and 1 otherwise, so it is a check as well as a +report. + +`--filters` (default `./targets`) is the tree of saved filters, mirroring the targets: +target `A/B` keeps its filter at `targets/A/B/filter.lua` and its accepted differences +at `targets/A/B/differs.txt`. The filter is written by one model call the first time a +target runs and read every time after that, so a target costs one call ever. A target +whose questions document is a PDF has no filter: pandoc cannot read one, so route B +cannot run and route A converts the pages' markdown alone. `--out` +(default `./out`) is where each target's set is written, under the target's own name, +and `--cache` (default `./.in2lambda-agent`) is where the OCR of each PDF is kept. + ## Gate Nothing merges without a replay over real documents. `gate` reruns the saved specs diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 204db29..80a6dbd 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import Optional, Sequence -from in2lambda_agent import compare, corpus, gate, pair, pipeline, routes +from in2lambda_agent import compare, corpus, gate, pair, pipeline, routes, targets from in2lambda_agent.mathpix import MathpixClient, MathpixError from in2lambda_agent.model import ModelError, ModelUnavailable, choose_backend from in2lambda_agent.ocr import ocr_pdf @@ -119,8 +119,8 @@ def build_parser() -> argparse.ArgumentParser: """The command line as the design spec describes it. Returns: - A parser with the `convert`, `run`, `review`, `corpus`, `gate`, - `compare` and `ui` subcommands. + A parser with the `convert`, `run`, `review`, `corpus`, `targets`, + `gate`, `compare` and `ui` subcommands. """ parser = argparse.ArgumentParser( prog="in2lambda-agent", @@ -311,6 +311,40 @@ def build_parser() -> argparse.ArgumentParser: "another run filled converts nothing.", ) + against_export = subcommands.add_parser( + "targets", + help="Convert each target under ROOT and compare it with its export.", + ) + against_export.add_argument( + "root", type=Path, help="The directory the targets are under." + ) + against_export.add_argument( + "paths", + nargs="*", + type=Path, + help="Folders under ROOT to run, defaulting to all of them.", + ) + against_export.add_argument( + "--filters", + type=Path, + default=targets.DEFAULT_FILTER_DIR, + help="The tree each target's filter is kept in, mirroring the targets, " + f"with the differences the maintainer accepted in {targets.DIFFERS_NAME} " + "beside it.", + ) + against_export.add_argument( + "--out", + type=Path, + default=Path("out"), + help="Where to write each target's set, under the target's own name.", + ) + against_export.add_argument( + "--cache", + type=Path, + default=pipeline.DEFAULT_CACHE_DIR, + help="Where the OCR of each PDF is kept.", + ) + check = subcommands.add_parser( "gate", help="Replay the corpus the baseline names and check it against it." ) @@ -504,6 +538,26 @@ def main(argv: Optional[Sequence[str]] = None) -> int: succeeded = {"built", "skipped"} return 0 if rows and all(row.outcome in succeeded for row in rows) else 1 + if args.command == "targets": + results = targets.run( + args.root, + paths=args.paths, + filters=args.filters, + out_dir=args.out, + cache_dir=args.cache, + settings=load_settings(), + ) + new = sum(len(one.new) for one in results) + failed = [one for one in results if one.error] + print( + f"{len(results)} target{'' if len(results) == 1 else 's'}, " + f"{new} new difference{'' if new == 1 else 's'}" + + (f", {len(failed)} did not run" if failed else "") + ) + # A root with no target under it is a mistyped path rather than a clean + # run, so an empty run fails like a new difference does. + return 0 if results and not new and not failed else 1 + if args.command == "gate": baseline = gate.read_baseline(args.baseline) # The directory is printed and is not deleted, so that the drafts of a diff --git a/in2lambda_agent/pair.py b/in2lambda_agent/pair.py index a8bf72e..de054db 100644 --- a/in2lambda_agent/pair.py +++ b/in2lambda_agent/pair.py @@ -13,6 +13,10 @@ `pairs_in` reads a whole folder that way: every sheet in it with its solutions document, which is what a run over a folder converts. + +`target_documents` reads the other kind of folder, the one holding a single set. +There is nothing to pair there, so the roles alone decide: the document whose +name ends in `solutions` answers the other one, whatever either is called. """ import re @@ -134,6 +138,52 @@ def pairs_in(folder: Path) -> list[tuple[Path, Optional[Path]]]: return [(path, solutions_beside(path)) for path in sheets.values()] +def target_documents(folder: Path) -> tuple[Path, Optional[Path]]: + """The two documents of a target folder, paired by role rather than by name. + + A target folder holds one set, so there is nothing to pair: the document + whose name ends in `solutions` or `sol` is the solutions document and the + other one is the questions document, whatever the two are called. The sets + are often exported months apart from the sheet, so their names share only + the course. + + Args: + folder: A target's folder: its two documents, and the folder Lambda + Feedback exported from them. + + Returns: + The questions document, and the solutions document or None where the + folder holds none. Subfolders are not looked into. + + Raises: + ValueError: Where the folder holds no questions document, or more than + one document of either role, naming what it holds. Which of two + sheets is the target's is not this function's to guess. + """ + folder = Path(folder) + documents = [one for one in _files_in(folder) if one.suffix.lower() in DOCUMENTS] + by_role = { + "questions": [one for one in documents if questions_stem(one) is None], + "solutions": [one for one in documents if questions_stem(one) is not None], + } + for role, held in by_role.items(): + if len(held) > 1: + named = ", ".join(one.name for one in held) + raise ValueError( + f"{folder} holds {len(held)} {role} documents: {named}. A target " + "folder holds one set: one questions document, and a solutions " + "document whose name ends in `_solutions`." + ) + if not by_role["questions"]: + held = ", ".join(one.name for one in _files_in(folder)) or "nothing" + raise ValueError( + f"{folder} holds no questions document, only {held}. A target " + f"folder holds one file whose suffix is one of {' '.join(DOCUMENTS)} " + "and whose name does not end in `_solutions`." + ) + return by_role["questions"][0], (by_role["solutions"] or [None])[0] + + def of(source: Path) -> tuple[Path, Optional[Path]]: """The two documents a run freezes, whichever of them the user named. diff --git a/in2lambda_agent/targets.py b/in2lambda_agent/targets.py new file mode 100644 index 0000000..1d49b78 --- /dev/null +++ b/in2lambda_agent/targets.py @@ -0,0 +1,277 @@ +"""Each target against the set Lambda Feedback exported from it. + +A target is a folder holding one set: one questions document, a solutions +document where the set has one, and the folder the platform exported for that +set, named `set_`. The export is what the conversion is trying to +reproduce, so a target is the one place the agent can be marked right or wrong +rather than merely flagged, and `find` looks for that `set_*` folder to find +one. + +Each target converts through `routes.convert` under a filter of its own, and +`in2lambda.compare.differences` reports every place the zip and the export say +something else. A difference the maintainer has read and accepted is a line of +a `differs.txt` beside that target's filter, which `in2lambda.compare.known` +reads: those are reported as known, and what is left is new. A run with a new +difference is a run that changed what the agent makes of a document nobody +looked at again. + +The filters live in a tree of their own mirroring the targets, so that nothing +is written into the corpus and the filter of a target is read again rather than +paid for a second time. +""" + +from dataclasses import dataclass, field +from pathlib import Path +from typing import Optional, Sequence + +from in2lambda.api.set import Set +from in2lambda.compare import differences, known + +from in2lambda_agent import pair, routes +from in2lambda_agent.model import Backend, choose_backend +from in2lambda_agent.settings import Settings, load_settings + +DEFAULT_FILTER_DIR = Path("targets") +"""The tree the targets' filters are kept in, mirroring the targets themselves.""" + +FILTER_NAME = "filter.lua" +"""A target's saved filter, under its own folder of the tree: read if it is +there, written by one model call if it is not.""" + +DIFFERS_NAME = "differs.txt" +"""The differences the maintainer has accepted, beside the target's filter: one +line as `differences` words it, with the ticket that would close it after `#`.""" + +EXPORT_PREFIX = "set_" +"""What Lambda Feedback names an exported set's folder with. The rest of the +name is the set's own, which is what the conversion is built under so that the +two zips are comparable file by file.""" + + +@dataclass +class Target: + """One folder holding one set: its documents, and what to compare with. + + Attributes: + name: The folder, relative to the root, which is what a line of the + report and the folder of its filter are named after. + export: The `set_*` folder Lambda Feedback exported. + questions: The questions document. + solutions: The solutions document, or None where there is none. + error: What is wrong with the folder, where a target cannot be read at + all: two exports in it, or no document to convert. A target holding + one is reported and not run, and the targets after it still run. + """ + + name: str + export: Path + questions: Optional[Path] = None + solutions: Optional[Path] = None + error: Optional[str] = None + + +@dataclass +class Result: + """What one target's comparison found. + + Attributes: + name: The target's. + differences: Every line the comparison reported. + known: Those of them the target's `differs.txt` holds. + new: Those it does not, which is what a run is read for. + flags: How many fields the conversion flagged for a person. + tokens: What its model calls cost. + error: What stopped the target, and nothing else filled. + """ + + name: str + differences: list[str] = field(default_factory=list) + known: list[str] = field(default_factory=list) + new: list[str] = field(default_factory=list) + flags: int = 0 + tokens: int = 0 + error: Optional[str] = None + + def report(self) -> list[str]: + """The new differences, the accepted ones, and the counts. + + The new lines come first, because they are what a reader is looking + for; the known ones are printed too, so that a line the maintainer + accepted and the run no longer reports can be seen to have gone. + """ + if self.error: + return [f"error {self.name}: {self.error}"] + return ( + [f"differs {self.name}: {line}" for line in self.new] + + [f"known {self.name}: {line}" for line in self.known] + + [ + f"{self.name}: {len(self.differences)} differ, " + f"{len(self.known)} known, {len(self.new)} new, " + f"{self.flags} flagged" + ] + ) + + +def find(root: Path, paths: Sequence[Path] = ()) -> list[Target]: + """Every target under a root, by the export folder each one holds. + + Args: + root: The directory the targets are under, directly or grouped by + course. + paths: Folders under it to look in, relative to it; all of it if empty. + + Returns: + One target per folder holding a `set_*` folder, in name order. A folder + that cannot be read as a target — two exports in it, or no document — + is a target carrying an `error` rather than an exception, so that one + bad folder does not stop the run. + """ + root = Path(root) + exports: dict[Path, set[Path]] = {} + for where in [root / one for one in paths] or [root]: + for path in where.rglob(f"{EXPORT_PREFIX}*"): + if path.is_dir(): + exports.setdefault(path.parent, set()).add(path) + + found = [] + for folder in sorted(exports, key=lambda one: one.relative_to(root).as_posix()): + held = sorted(exports[folder]) + name = folder.relative_to(root).as_posix() + if len(held) > 1: + named = ", ".join(one.name for one in held) + found.append( + Target( + name=name, + export=held[0], + error=f"{folder} holds {len(held)} exported sets: {named}. " + "A target folder holds one.", + ) + ) + continue + try: + questions, solutions = pair.target_documents(folder) + except ValueError as problem: + found.append(Target(name=name, export=held[0], error=str(problem))) + continue + found.append( + Target( + name=name, export=held[0], questions=questions, solutions=solutions + ) + ) + return found + + +def run_one( + target: Target, + *, + filters: Path = DEFAULT_FILTER_DIR, + out_dir: Path = Path("out"), + cache_dir: Path = Path(".in2lambda-agent"), + settings: Optional[Settings] = None, + backend: Optional[Backend] = None, +) -> Result: + """Converts one target and compares what came out with its export. + + Args: + target: The target, as `find` read it. + filters: The tree of filters and accepted differences. + out_dir: Where each target's set is written, under its own name. + cache_dir: Where the OCR of each PDF is kept. + settings: The environment the run has available. + backend: The backend to write a filter with, chosen from the settings + if absent. + + Returns: + The target's result. Nothing a target raises leaves this function: what + stopped it is its result's `error` and the targets after it still run. + """ + if target.error: + return Result(name=target.name, error=target.error) + settings = settings or load_settings() + backend = backend or choose_backend(settings) + saved = Path(filters) / target.name + # Pandoc reads neither a PDF nor the markdown an OCR made of one back into + # the document's structure, so route B cannot run over a scanned target: + # it converts through route A alone, and no filter is written for it. + lua = None if target.questions.suffix.lower() == ".pdf" else saved / FILTER_NAME + try: + if lua is not None and not lua.is_file(): + saved.mkdir(parents=True, exist_ok=True) + lua.write_text( + routes.write_filter(target.questions, target.solutions, backend)[0], + encoding="utf-8", + ) + converted = routes.convert( + target.questions, + target.solutions, + out_dir=Path(out_dir) / target.name, + cache_dir=cache_dir, + backend=backend, + settings=settings, + lua=lua, + # The export's own name, so that the zip holds the files the export + # holds and the two are compared file by file. + name=target.export.name[len(EXPORT_PREFIX) :], + ) + except Exception as problem: + # A missing credential, a model call that did not finish, a document + # pandoc refused: all of them are this target's line, and the run goes + # on to the next target. + return Result(name=target.name, error=" ".join(str(problem).split())) + + found = differences( + Set.from_json(str(converted.zip_path)), + Set.from_json(str(target.export)), + left_name="the agent", + right_name="the export", + ) + accepted = known(saved / DIFFERS_NAME) + return Result( + name=target.name, + differences=found, + known=[line for line in found if line in accepted], + new=[line for line in found if line not in accepted], + flags=len(converted.flags), + tokens=converted.tokens, + ) + + +def run( + root: Path, + *, + paths: Sequence[Path] = (), + filters: Path = DEFAULT_FILTER_DIR, + out_dir: Path = Path("out"), + cache_dir: Path = Path(".in2lambda-agent"), + settings: Optional[Settings] = None, + backend: Optional[Backend] = None, +) -> list[Result]: + """Runs every target under a root, printing each one's report as it finishes. + + Args: + root: The directory the targets are under. + paths: Folders under it to run, relative to it; all of it if empty. + filters: The tree of filters and accepted differences. + out_dir: Where each target's set is written, under its own name. + cache_dir: Where the OCR of each PDF is kept. + settings: The environment the runs have available. + backend: The backend to write the filters with. + + Returns: + One result per target, in the order they ran. + """ + settings = settings if settings is not None else load_settings() + results = [] + for target in find(root, paths): + result = run_one( + target, + filters=filters, + out_dir=out_dir, + cache_dir=cache_dir, + settings=settings, + backend=backend, + ) + for line in result.report(): + print(line) + results.append(result) + return results diff --git a/poetry.lock b/poetry.lock index d6d0e00..bb7695f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -531,8 +531,8 @@ convert = ["panflute (>=2.3.1,<3.0.0)", "pyyaml (>=6.0,<7.0)"] [package.source] type = "git" url = "https://github.com/lambda-feedback/in2lambda.git" -reference = "dev" -resolved_reference = "a76603dc3d21847c687caf558255941aa966a5fa" +reference = "0f899a64d61f77556658fb0c33622d7554a8cfcb" +resolved_reference = "0f899a64d61f77556658fb0c33622d7554a8cfcb" [[package]] name = "iniconfig" @@ -1721,4 +1721,4 @@ ui = ["starlette", "uvicorn"] [metadata] lock-version = "2.1" python-versions = "^3.10" -content-hash = "2e0eeb0a8521e18d1264f4ab2ad0acca837875852eeea30f1d5543c1c07f08a7" +content-hash = "ae14de7279e16e6cab61007ac0ae124713cb2a2207786f347cb9f74e2a5071ca" diff --git a/pyproject.toml b/pyproject.toml index ebaf7d7..44c0d55 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,9 +9,11 @@ packages = [{ include = "in2lambda_agent" }] [tool.poetry.dependencies] python = "^3.10" -# From git until a release carries the API the agent needs. The convert extra +# From git until a release carries the API the agent needs, and at a commit +# rather than the branch, so that what a target's comparison reports changes +# when this line does and not when someone pushes to dev. The convert extra # pulls in panflute, which in2lambda.main.runner needs to read a document. -in2lambda = { git = "https://github.com/lambda-feedback/in2lambda.git", branch = "dev", extras = [ +in2lambda = { git = "https://github.com/lambda-feedback/in2lambda.git", rev = "0f899a64d61f77556658fb0c33622d7554a8cfcb", extras = [ "convert", ] } python-dotenv = "^1.0" diff --git a/tests/test_cli.py b/tests/test_cli.py index 6862e9d..93290f8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,7 +9,7 @@ import pytest from conftest import FakeBackend, FakeMathpix -from in2lambda_agent import cli, compare, corpus, gate, pipeline, routes +from in2lambda_agent import cli, compare, corpus, gate, pipeline, routes, targets from in2lambda_agent.cli import build_parser, main, reviewer_name from in2lambda_agent.model import ModelUnavailable, Usage from in2lambda_agent.settings import Settings @@ -386,6 +386,92 @@ def test_corpus_every_option(): assert args.cache == Path("cached") +def test_targets_defaults(): + args = build_parser().parse_args(["targets", "ExampleContents/targets"]) + + assert args.command == "targets" + assert args.root == Path("ExampleContents/targets") + assert args.paths == [] + assert args.filters == Path("targets") + assert args.out == Path("out") + assert args.cache == Path(".in2lambda-agent") + + +def test_targets_every_option(): + args = build_parser().parse_args( + [ + "targets", + "ExampleContents/targets", + "EART40013_Mathematical_Methods_II", + "--filters", + "saved", + "--out", + "built", + "--cache", + "cached", + ] + ) + + assert args.paths == [Path("EART40013_Mathematical_Methods_II")] + assert args.filters == Path("saved") + assert args.out == Path("built") + assert args.cache == Path("cached") + + +def test_a_targets_run_prints_its_report_and_passes_with_no_new_difference( + monkeypatch, capsys +): + given = {} + + def record(root, **kwargs): + given.update(root=root, **kwargs) + return [ + targets.Result( + name="ME2", differences=["Question 1 \"\": a"], + known=["Question 1 \"\": a"], flags=2, + ) + ] + + monkeypatch.setattr(targets, "run", record) + + code = main(["targets", "ExampleContents/targets", "--cache", "cached"]) + + assert code == 0 + assert given["root"] == Path("ExampleContents/targets") + assert given["cache_dir"] == Path("cached") + # `run` printed the target's own report as it went; this is the total. + assert capsys.readouterr().out == "1 target, 0 new differences\n" + + +@pytest.mark.parametrize( + "result, total", + [ + ( + targets.Result(name="ME2", differences=["a"], new=["a"]), + "1 target, 1 new difference\n", + ), + ( + targets.Result(name="ME2", error="set MATHPIX_APP_ID"), + "1 target, 0 new differences, 1 did not run\n", + ), + ], +) +def test_a_new_difference_or_a_target_that_failed_fails_the_run( + monkeypatch, result, total, capsys +): + monkeypatch.setattr(targets, "run", lambda *args, **kwargs: [result]) + + assert main(["targets", "ExampleContents/targets"]) == 1 + assert capsys.readouterr().out == total + + +def test_a_run_over_no_target_at_all_fails(monkeypatch): + # A root with no `set_*` folder under it is a mistyped path, not a clean run. + monkeypatch.setattr(targets, "run", lambda *args, **kwargs: []) + + assert main(["targets", "ExampleContents/targets"]) == 1 + + def test_the_corpus_cache_is_handed_to_the_sweep(monkeypatch): given = {} diff --git a/tests/test_pair.py b/tests/test_pair.py index 7740867..3318294 100644 --- a/tests/test_pair.py +++ b/tests/test_pair.py @@ -111,6 +111,58 @@ def test_a_folders_solutions_file_is_not_a_sheet_of_its_own(tmp_path): assert pair.pairs_in(tmp_path) == [] +def test_a_targets_two_documents_are_read_by_role_not_by_name(tmp_path): + # The ME2 target: the solutions document was printed from Lambda Feedback + # months after the sheet, so the two names share nothing but the course. + questions = tmp_path / "mech50010_fluid_mechanics_S1_20251210163022 (2).pdf" + solutions = tmp_path / "mech50010_fluid_mechanics_S1_20260921091254_solutions.pdf" + for document in (questions, solutions): + document.write_bytes(b"%PDF") + (tmp_path / "set_Introduction").mkdir() + + assert pair.target_documents(tmp_path) == (questions, solutions) + # And by stem, which is what a folder of sheets is paired by, they are two + # sheets and neither answers the other. + assert pair.pairs_in(tmp_path) == [(questions, None)] + + +def test_a_targets_documents_may_share_a_stem(tmp_path): + # EART40013's CW2: the same pair, named the way a folder of sheets names it. + questions = tmp_path / "EART40013_S2_20260202164327.tex" + solutions = tmp_path / "EART40013_S2_20260202164327_solutions.tex" + for document in (questions, solutions): + document.write_text("x") + + assert pair.target_documents(tmp_path) == (questions, solutions) + + +def test_a_target_with_no_solutions_document_has_none(tmp_path): + questions = tmp_path / "CW1.pdf" + questions.write_bytes(b"%PDF") + + assert pair.target_documents(tmp_path) == (questions, None) + + +@pytest.mark.parametrize( + "names, complaint", + [ + (["Sheet_1.tex", "Sheet_2.tex"], "2 questions documents: Sheet_1.tex, Sheet_2.tex"), + ( + ["Sheet_1.tex", "Sheet_1_solutions.tex", "Sheet_2_sol.tex"], + "2 solutions documents: Sheet_1_solutions.tex, Sheet_2_sol.tex", + ), + (["notes.png"], "no questions document"), + ], +) +def test_a_target_folder_that_holds_more_than_one_set_is_refused(tmp_path, names, complaint): + for name in names: + (tmp_path / name).write_text("x") + + with pytest.raises(ValueError) as refused: + pair.target_documents(tmp_path) + assert complaint in str(refused.value) + + def test_solutions_with_no_questions_run_alone(tmp_path): # The markers above the solutions are this document's questions, so the # document converts with no second file. diff --git a/tests/test_targets.py b/tests/test_targets.py new file mode 100644 index 0000000..8c8b245 --- /dev/null +++ b/tests/test_targets.py @@ -0,0 +1,287 @@ +"""Each target against the set Lambda Feedback exported from it. + +Written before the module, over the ME2 fixtures: the direct-route reply of +2026-09-21 built into a zip stands in for a conversion, and the export beside it +is what the comparison holds it against. The live test runs the three real +targets and is skipped without the private corpus. +""" + +import json +import os +import shutil +from pathlib import Path + +import pytest + +from conftest import FakeBackend + +from in2lambda_agent import gate, routes, targets + +ME2 = Path(__file__).parent / "fixtures" / "me2" +REPLY = json.loads((ME2 / "direct.json").read_text()) +TARGETS = Path( + "/Users/peterbjohnson/code/lambdafeedback/in2lambda-agent/ExampleContents/targets" +) + +live = pytest.mark.skipif( + not os.environ.get("IN2LAMBDA_LIVE"), reason="calls Mathpix and a model" +) + + +def make_target(root, name, *, questions="sheet.md", solutions="sheet_solutions.md"): + """A target folder: the ME2 documents, and the ME2 export under its own name.""" + folder = Path(root) / name + folder.mkdir(parents=True) + (folder / questions).write_text((ME2 / "questions.md").read_text()) + if solutions: + (folder / solutions).write_text((ME2 / "solutions.md").read_text()) + shutil.copytree(ME2 / "export", folder / "set_Introduction") + return folder + + +def fake_convert(monkeypatch, flags=(), error=None): + """Stands in for a conversion: builds the fixture reply, records the call.""" + calls = [] + + def convert(document, solutions=None, **options): + calls.append({"document": document, "solutions": solutions, **options}) + if error is not None: + raise error + built = routes.to_set(REPLY, name=options["name"]) + return routes.Converted( + set=built, + zip_path=routes.build(built, options["out_dir"]), + flags=list(flags), + reply=REPLY, + tokens=1200, + ) + + monkeypatch.setattr(targets.routes, "convert", convert) + return calls + + +# --- finding the targets ---------------------------------------------------------------- + + +def test_a_target_is_found_by_its_export_folder_at_any_depth(tmp_path): + make_target(tmp_path, "ME2_Fluids_introduction") + make_target(tmp_path, "EART40013_Mathematical_Methods_II/CW1") + make_target(tmp_path, "EART40013_Mathematical_Methods_II/CW2") + # A folder of sheets with no export is not a target. + (tmp_path / "PHYS40002").mkdir() + (tmp_path / "PHYS40002" / "Sheet_1.tex").write_text("x") + + found = targets.find(tmp_path) + + assert [one.name for one in found] == [ + "EART40013_Mathematical_Methods_II/CW1", + "EART40013_Mathematical_Methods_II/CW2", + "ME2_Fluids_introduction", + ] + assert found[2].questions == tmp_path / "ME2_Fluids_introduction" / "sheet.md" + assert found[2].solutions == tmp_path / "ME2_Fluids_introduction" / "sheet_solutions.md" + assert found[2].export == tmp_path / "ME2_Fluids_introduction" / "set_Introduction" + assert [one.error for one in found] == [None, None, None] + + +def test_a_named_path_narrows_the_run_to_what_is_under_it(tmp_path): + make_target(tmp_path, "ME2_Fluids_introduction") + make_target(tmp_path, "EART40013_Mathematical_Methods_II/CW1") + + found = targets.find(tmp_path, [Path("EART40013_Mathematical_Methods_II")]) + + assert [one.name for one in found] == ["EART40013_Mathematical_Methods_II/CW1"] + + +def test_a_target_whose_documents_cannot_be_read_is_an_error_not_a_raise(tmp_path): + folder = tmp_path / "ME2_Fluids_introduction" + (folder / "set_Introduction").mkdir(parents=True) + make_target(tmp_path, "CW1") + + found = targets.find(tmp_path) + + assert [one.name for one in found] == ["CW1", "ME2_Fluids_introduction"] + assert found[0].error is None + assert "no questions document" in found[1].error + + +def test_a_folder_holding_two_exports_is_one_target_and_an_error(tmp_path): + folder = make_target(tmp_path, "ME2_Fluids_introduction") + (folder / "set_Second_half").mkdir() + + (found,) = targets.find(tmp_path) + + assert found.name == "ME2_Fluids_introduction" + assert "set_Introduction" in found.error and "set_Second_half" in found.error + + +# --- one target ------------------------------------------------------------------------- + + +def test_the_comparison_reports_every_difference_from_the_export(tmp_path, monkeypatch): + calls = fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "ME2") + (target,) = targets.find(tmp_path / "corpus") + + result = targets.run_one( + target, + filters=tmp_path / "filters", + out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), + ) + + # The reply was converted under the export's own name, so the zip the + # comparison reads is the set Lambda Feedback would have exported. + assert calls[0]["name"] == "Introduction" + assert calls[0]["document"] == target.questions + assert calls[0]["solutions"] == target.solutions + # The fixture reply is the model's reading of the same pages, so the two + # sets differ in wording over most of the questions. + assert len(result.differences) == 21 + assert result.new == result.differences + assert result.known == [] + assert result.error is None + assert result.tokens == 1200 + + +def test_a_difference_the_maintainer_accepted_is_known_and_not_new(tmp_path, monkeypatch): + fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "ME2") + filters = tmp_path / "filters" + (target,) = targets.find(tmp_path / "corpus") + first = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), + ) + + # Every difference accepted, and one line more that the comparison no + # longer reports, which is neither known nor new. + accepted = filters / "ME2" / targets.DIFFERS_NAME + accepted.write_text( + "".join(f"{line} # t40\n" for line in first.differences) + + "Question 9 \"\": the agent says 'gone' and the export says 'went' # t41\n" + ) + again = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), + ) + + assert again.new == [] + assert again.known == first.differences + + +def test_the_filter_is_written_once_and_read_after_that(tmp_path, monkeypatch): + calls = fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "ME2") + (target,) = targets.find(tmp_path / "corpus") + filters = tmp_path / "filters" + backend = FakeBackend("function Pandoc(doc) end") + + targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=backend, + ) + lua = filters / "ME2" / targets.FILTER_NAME + assert lua.read_text() == "function Pandoc(doc) end" + assert calls[0]["lua"] == lua + assert len(backend.calls) == 1 + + # The second run reads the saved filter, so a target costs one call ever. + targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=backend, + ) + assert len(backend.calls) == 1 + assert calls[1]["lua"] == lua + + +def test_a_scanned_target_converts_with_no_filter(tmp_path, monkeypatch): + # Pandoc cannot read a PDF, so there is no structure to write a filter from + # and no call to make: route A converts the pages' markdown alone. + calls = fake_convert(monkeypatch) + make_target( + tmp_path / "corpus", "ME2", + questions="sheet.pdf", solutions="sheet_solutions.pdf", + ) + (target,) = targets.find(tmp_path / "corpus") + backend = FakeBackend() + + targets.run_one( + target, filters=tmp_path / "filters", out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=backend, + ) + + assert calls[0]["lua"] is None + assert backend.calls == [] + assert not (tmp_path / "filters" / "ME2").exists() + + +def test_a_target_that_failed_is_a_line_of_its_own_and_the_next_one_runs(tmp_path, monkeypatch): + from in2lambda_agent.mathpix import MathpixError + + fake_convert(monkeypatch, error=MathpixError("set MATHPIX_APP_ID")) + make_target(tmp_path / "corpus", "ME2") + make_target(tmp_path / "corpus", "CW1") + + results = targets.run( + tmp_path / "corpus", filters=tmp_path / "filters", out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend("-- a", "-- b"), + ) + + assert [one.name for one in results] == ["CW1", "ME2"] + assert [one.error for one in results] == ["set MATHPIX_APP_ID"] * 2 + assert [one.new for one in results] == [[], []] + + +# --- the report --------------------------------------------------------------------------- + + +def test_the_report_names_each_difference_and_counts_them(): + result = targets.Result( + name="ME2", + differences=["Question 1 \"\": a", "Question 2 \"\": b"], + known=["Question 1 \"\": a"], + new=["Question 2 \"\": b"], + flags=2, + ) + + assert result.report() == [ + 'differs ME2: Question 2 "": b', + 'known ME2: Question 1 "": a', + "ME2: 2 differ, 1 known, 1 new, 2 flagged", + ] + + +def test_the_report_of_a_target_that_failed_says_what_stopped_it(): + result = targets.Result(name="CW1", error="set MATHPIX_APP_ID") + + assert result.report() == ["error CW1: set MATHPIX_APP_ID"] + + +# --- live ----------------------------------------------------------------------------------- + + +@live +@pytest.mark.skipif(not TARGETS.is_dir(), reason="private corpus") +def test_every_target_reports_its_known_differences_and_no_other(tmp_path, capsys): + # The ticket's run: the three targets, their saved filters, their accepted + # differences, and no difference beside them. + results = targets.run( + TARGETS, + filters=targets.DEFAULT_FILTER_DIR, + out_dir=tmp_path / "out", + # The cache the gate shares between worktrees: a second OCR pass of the + # same PDF is paid for again and reads it a little differently, which + # is a difference in the run and not in the agent. + cache_dir=gate.DEFAULT_CACHE_DIR, + ) + print("\n" + capsys.readouterr().out) + + assert [one.name for one in results] == [ + "EART40013_Mathematical_Methods_II/CW1", + "EART40013_Mathematical_Methods_II/CW2", + "ME2_Fluids_introduction", + ] + assert [one.error for one in results] == [None, None, None] + assert [one.new for one in results] == [[], [], []] From 3248c5591ebcc9f0ea7e0116becc2dc770384da6 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 05:59:32 +0100 Subject: [PATCH 2/4] implement: Compare each target document with the set Lambda Feedback exported from it (t40) --- in2lambda_agent/targets.py | 47 ++++++++++++++++++++++++++------------ tests/test_targets.py | 35 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/in2lambda_agent/targets.py b/in2lambda_agent/targets.py index 1d49b78..3a13a5b 100644 --- a/in2lambda_agent/targets.py +++ b/in2lambda_agent/targets.py @@ -59,8 +59,9 @@ class Target: questions: The questions document. solutions: The solutions document, or None where there is none. error: What is wrong with the folder, where a target cannot be read at - all: two exports in it, or no document to convert. A target holding - one is reported and not run, and the targets after it still run. + all: two exports in it, no document to convert, or the root passed + being the target itself. A target holding one is reported and not + run, and the targets after it still run. """ name: str @@ -122,9 +123,10 @@ def find(root: Path, paths: Sequence[Path] = ()) -> list[Target]: Returns: One target per folder holding a `set_*` folder, in name order. A folder - that cannot be read as a target — two exports in it, or no document — - is a target carrying an `error` rather than an exception, so that one - bad folder does not stop the run. + that cannot be read as a target — two exports in it, or no document, or + the root itself, which has no name to be kept under — is a target + carrying an `error` rather than an exception, so that one bad folder + does not stop the run. """ root = Path(root) exports: dict[Path, set[Path]] = {} @@ -137,6 +139,23 @@ def find(root: Path, paths: Sequence[Path] = ()) -> list[Target]: for folder in sorted(exports, key=lambda one: one.relative_to(root).as_posix()): held = sorted(exports[folder]) name = folder.relative_to(root).as_posix() + if name == ".": + # The root is the target itself, so the path a target is named by + # is `.` and everything built from it collapses onto the root of + # the filter tree: the target's saved filter would be missed and + # paid for again, and its accepted differences read from a file + # that is not there. Refused, rather than run for a wrong answer. + found.append( + Target( + name=folder.resolve().name, + export=held[0], + error=f"{root} is a target itself, not a directory targets " + "are under, and a target is named by its path from that " + f"directory. Run `in2lambda-agent targets {root.parent} " + f"{folder.resolve().name}` instead.", + ) + ) + continue if len(held) > 1: named = ", ".join(one.name for one in held) found.append( @@ -213,19 +232,19 @@ def run_one( # holds and the two are compared file by file. name=target.export.name[len(EXPORT_PREFIX) :], ) + found = differences( + Set.from_json(str(converted.zip_path)), + Set.from_json(str(target.export)), + left_name="the agent", + right_name="the export", + ) + accepted = known(saved / DIFFERS_NAME) except Exception as problem: # A missing credential, a model call that did not finish, a document - # pandoc refused: all of them are this target's line, and the run goes - # on to the next target. + # pandoc refused, an export half-copied into the corpus: all of them + # are this target's line, and the run goes on to the next target. return Result(name=target.name, error=" ".join(str(problem).split())) - found = differences( - Set.from_json(str(converted.zip_path)), - Set.from_json(str(target.export)), - left_name="the agent", - right_name="the export", - ) - accepted = known(saved / DIFFERS_NAME) return Result( name=target.name, differences=found, diff --git a/tests/test_targets.py b/tests/test_targets.py index 8c8b245..908d951 100644 --- a/tests/test_targets.py +++ b/tests/test_targets.py @@ -105,6 +105,19 @@ def test_a_target_whose_documents_cannot_be_read_is_an_error_not_a_raise(tmp_pat assert "no questions document" in found[1].error +def test_a_root_that_is_the_target_itself_is_refused_naming_the_root_to_pass(tmp_path): + folder = make_target(tmp_path, "EART40013_Mathematical_Methods_II/CW2") + + (found,) = targets.find(folder) + + # Its name from that root is `.`, and a filter and a differs.txt kept under + # that name are not the ones the target has: it is refused rather than + # converted against a filter tree it would write over the top of. + assert found.name == "CW2" + assert "is a target itself" in found.error + assert f"targets {folder.parent} CW2" in found.error + + def test_a_folder_holding_two_exports_is_one_target_and_an_error(tmp_path): folder = make_target(tmp_path, "ME2_Fluids_introduction") (folder / "set_Second_half").mkdir() @@ -217,6 +230,28 @@ def test_a_scanned_target_converts_with_no_filter(tmp_path, monkeypatch): assert not (tmp_path / "filters" / "ME2").exists() +def test_a_target_whose_export_cannot_be_read_is_an_error_and_the_next_one_runs( + tmp_path, monkeypatch +): + # Half of an export copied into the corpus: the folder is there, so the + # target is found and its conversion is paid for, and the comparison is + # what fails. It is this target's line like any other. + fake_convert(monkeypatch) + half = make_target(tmp_path / "corpus", "ME2") + shutil.rmtree(half / "set_Introduction") + (half / "set_Introduction").mkdir() + make_target(tmp_path / "corpus", "CW1") + + results = targets.run( + tmp_path / "corpus", filters=tmp_path / "filters", out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend("-- a", "-- b"), + ) + + assert [one.name for one in results] == ["CW1", "ME2"] + assert results[0].error is None and results[0].new + assert results[1].error + + def test_a_target_that_failed_is_a_line_of_its_own_and_the_next_one_runs(tmp_path, monkeypatch): from in2lambda_agent.mathpix import MathpixError From dcd8ea0ac504097bb9bd96666e127ba76a6d4243 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 07:07:51 +0100 Subject: [PATCH 3/4] implement: Compare each target document with the set Lambda Feedback exported from it (t40) --- .gitignore | 3 +- README.md | 39 +++++++---- in2lambda_agent/cli.py | 13 +++- in2lambda_agent/routes.py | 22 ++++-- in2lambda_agent/targets.py | 131 ++++++++++++++++++++++++++++++------ tests/test_cli.py | 4 ++ tests/test_routes.py | 21 ++++++ tests/test_targets.py | 133 +++++++++++++++++++++++++++++++++---- 8 files changed, 311 insertions(+), 55 deletions(-) diff --git a/.gitignore b/.gitignore index e25d806..c3ff88d 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,7 @@ dist/ # absolute path of the corpus on one machine. corpus-specs/ # The default `--filters`: a target's filter is written from a private -# document's structure, and a line of its differs.txt quotes that document's -# text. +# document's structure, and its reply.json holds that document's text. targets/ # in2lambda's KaTeX converter writes this into the working directory on import. log diff --git a/README.md b/README.md index e43ab17..86617da 100644 --- a/README.md +++ b/README.md @@ -423,7 +423,7 @@ poetry run in2lambda-agent targets ExampleContents/targets In full: ```sh -poetry run in2lambda-agent targets ROOT [PATH ...] [--filters DIR] [--out DIR] [--cache DIR] +poetry run in2lambda-agent targets ROOT [PATH ...] [--filters DIR] [--out DIR] [--cache DIR] [--fresh] ``` `ROOT` is the directory the targets are under and each `PATH` a folder under it to run, @@ -442,29 +442,40 @@ the export. The run prints one line per difference and one line of counts per ta ``` differs ME2_Fluids_introduction: Question 2 "", part (a), text: the agent says … and the export says … known ME2_Fluids_introduction: Question 1 "", main text: the agent says … and the export says … +agrees ME2_Fluids_introduction: q3.p2.worked_solution now agrees, remove the line ME2_Fluids_introduction: 4 differ, 3 known, 1 new, 2 flagged ``` A difference you have read and accepted goes into `differs.txt` beside that target's -filter, one line as the run printed it — without the `differs NAME: ` the line begins -with — and the ticket that would close it after a `#`: +filter. A line of that file names the field the difference is in, and states after a `#` +why the field differs: ``` -Question 1 "", main text: the agent says '…' and the export says '…' # t42 Mathpix reads the piston sketch's caption twice +q1.main_text # the export keeps the spacing the platform wrote around display maths +q2.p1.worked_solution # Mathpix reads the separator line under the working as a minus sign ``` -An accepted line is reported as `known` from then on. The command exits 0 when every -target ran and reported nothing new, and 1 otherwise, so it is a check as well as a -report. +The file records a field rather than a sentence because the report quotes a model's +wording. Route A reads the document on every run, and a model writes the same field +differently each time it is asked. A difference in a field the file names is reported as +`known` whatever its wording; a difference in any other field is reported as `differs` +and is new; a field the file names that no longer differs is reported as `agrees`, which +is a line to delete, and does not fail the run. The command exits 0 when every target +ran and reported no new difference, and 1 otherwise, so a target set is a check as well +as a report. `--filters` (default `./targets`) is the tree of saved filters, mirroring the targets: -target `A/B` keeps its filter at `targets/A/B/filter.lua` and its accepted differences -at `targets/A/B/differs.txt`. The filter is written by one model call the first time a -target runs and read every time after that, so a target costs one call ever. A target -whose questions document is a PDF has no filter: pandoc cannot read one, so route B -cannot run and route A converts the pages' markdown alone. `--out` -(default `./out`) is where each target's set is written, under the target's own name, -and `--cache` (default `./.in2lambda-agent`) is where the OCR of each PDF is kept. +target `A/B` keeps its filter at `targets/A/B/filter.lua`, route A's reply at +`targets/A/B/reply.json` and its accepted fields at `targets/A/B/differs.txt`. The first +run over a target makes one model call for the filter and one for route A's reply, and +writes both. Every run after that reads the two files and makes neither call, so the +second run over a target compares the set the first run compared. `--fresh` reads the +documents again and writes a new reply, which changes the wording of the report and the +number of fields flagged. A target whose questions document is a PDF has no filter: +pandoc cannot read a PDF, so route B does not run and route A converts the pages' +markdown alone. `--out` (default `./out`) is where each target's set is written, under +the target's own name, and `--cache` (default `./.in2lambda-agent`) is where the OCR of +each PDF is kept. ## Gate diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 9003041..e477b36 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -328,9 +328,15 @@ def build_parser() -> argparse.ArgumentParser: "--filters", type=Path, default=targets.DEFAULT_FILTER_DIR, - help="The tree each target's filter is kept in, mirroring the targets, " - f"with the differences the maintainer accepted in {targets.DIFFERS_NAME} " - "beside it.", + help="The tree each target's filter and saved reply are kept in, " + "mirroring the targets, with the fields the maintainer accepts a " + f"difference in written in {targets.DIFFERS_NAME} beside them.", + ) + against_export.add_argument( + "--fresh", + action="store_true", + help="Read each document again rather than converting the saved reply, " + "which is how a target is given a new reading of its pages.", ) against_export.add_argument( "--out", @@ -566,6 +572,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: out_dir=args.out, cache_dir=args.cache, settings=load_settings(), + fresh=args.fresh, ) new = sum(len(one.new) for one in results) failed = [one for one in results if one.error] diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 63d6d79..78bfb74 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -399,6 +399,10 @@ class Converted: zip_path: Optional[Path] flags: list[Flag] reply: Reply_ + # Route A's reply, before reconciling. A caller that needs the same answer + # twice saves this reply and passes it back as `convert`'s `route_a`; a + # second call to the model returns different wording. + route_a: Reply_ = field(default_factory=list) tokens: int = 0 # The counts of the reconciliation, zero where route B did not run. fields: int = 0 @@ -494,6 +498,7 @@ def convert( lua: Optional[Path] = None, name: str = "set", on_stage: Optional[Callable[[str, str], None]] = None, + route_a: Optional[Reply_] = None, ) -> Converted: """Route A, route B where a filter is given, reconcile, verify, write. @@ -502,6 +507,10 @@ def convert( result and `route_b_error` holds pandoc's message, so that one sheet of a folder does not stop the other eight. + `route_a`, where it is given, is a reply from an earlier run of this document, kept + by a caller who needs the same answer twice: route A is not called, and the result's + `route_a` is what was given. + `on_stage`, where it is given, is called with a name and a message as each step finishes - `ocr`, `route A`, `route B`, `fields`, `build` - so that a caller watching a run shows each line as the step ends rather than the report at the end of it. @@ -518,9 +527,14 @@ def said(stage: str, message: str) -> None: solutions_md = markdown_of(solutions, cache_dir, settings)[0] if solutions else None said("ocr", "; ".join(read)) source = markdown + ("\n" + solutions_md if solutions_md else "") - reply, usage = direct(markdown, solutions_md, backend) - tokens = usage.usage.input_tokens + usage.usage.output_tokens - said("route A", f"{tokens} tokens") + if route_a is None: + route_a, usage = direct(markdown, solutions_md, backend) + tokens = usage.usage.input_tokens + usage.usage.output_tokens + said("route A", f"{tokens} tokens") + else: + tokens = 0 + said("route A", "the reply given, no call made") + reply = route_a counts, error = (0, 0, 0, 0), None flags = [Flag(k, fields(reply)[k], "", "not a quote of the source") for k in not_verbatim(reply, source)] if lua is None: @@ -547,7 +561,7 @@ def said(stage: str, message: str) -> None: flags.append(Flag(k, fields(reply)[k], "", STRAY_MINUS)) result = Converted( set=to_set(reply, name=name, directory=images), zip_path=None, flags=flags, - reply=reply, tokens=tokens, + reply=reply, route_a=route_a, tokens=tokens, fields=counts[0], agreed=counts[1], defaulted=counts[2], adjudicated=counts[3], route_b_error=error, ) diff --git a/in2lambda_agent/targets.py b/in2lambda_agent/targets.py index 3a13a5b..4ae06e2 100644 --- a/in2lambda_agent/targets.py +++ b/in2lambda_agent/targets.py @@ -10,22 +10,31 @@ Each target converts through `routes.convert` under a filter of its own, and `in2lambda.compare.differences` reports every place the zip and the export say something else. A difference the maintainer has read and accepted is a line of -a `differs.txt` beside that target's filter, which `in2lambda.compare.known` -reads: those are reported as known, and what is left is new. A run with a new -difference is a run that changed what the agent makes of a document nobody -looked at again. - -The filters live in a tree of their own mirroring the targets, so that nothing -is written into the corpus and the filter of a target is read again rather than -paid for a second time. +a `differs.txt` beside that target's filter: the field it is in, `field_key`, +and the reason after a `#`. A run reports the differences in accepted fields as +known and the rest as new, and a run with a new difference is a run that +changed what the agent makes of a document nobody looked at again. + +A `differs.txt` line names a field rather than a sentence because the report +quotes a model's wording. Route A reads the document on every run, and a model +writes the same field differently each time it is asked. Route A's reply is +saved beside the filter and read back for the same reason, so that a second run +over a target compares the set the first run compared. `fresh` reads the +documents again and writes a new reply. + +The filters, the replies and the accepted fields are kept in a tree of their +own mirroring the targets, so that no file is written into the corpus and a +target's saved files are found again by the target's name. """ +import json +import re from dataclasses import dataclass, field from pathlib import Path from typing import Optional, Sequence from in2lambda.api.set import Set -from in2lambda.compare import differences, known +from in2lambda.compare import differences from in2lambda_agent import pair, routes from in2lambda_agent.model import Backend, choose_backend @@ -40,7 +49,11 @@ DIFFERS_NAME = "differs.txt" """The differences the maintainer has accepted, beside the target's filter: one -line as `differences` words it, with the ticket that would close it after `#`.""" +field key a line, with the reason it differs written after a `#`.""" + +REPLY_NAME = "reply.json" +"""Route A's reply, beside the target's filter: read back by every run after +the first, so that a target is converted the same way twice.""" EXPORT_PREFIX = "set_" """What Lambda Feedback names an exported set's folder with. The rest of the @@ -78,10 +91,13 @@ class Result: Attributes: name: The target's. differences: Every line the comparison reported. - known: Those of them the target's `differs.txt` holds. - new: Those it does not, which is what a run is read for. + known: Those of them in a field the target's `differs.txt` accepts. + new: Those in a field it does not, which is what a run is read for. + agreed: The keys it accepts that nothing differs in any more, which are + lines to take out of it. flags: How many fields the conversion flagged for a person. - tokens: What its model calls cost. + tokens: What its model calls cost, nothing where the saved reply was + read back. error: What stopped the target, and nothing else filled. """ @@ -89,22 +105,26 @@ class Result: differences: list[str] = field(default_factory=list) known: list[str] = field(default_factory=list) new: list[str] = field(default_factory=list) + agreed: list[str] = field(default_factory=list) flags: int = 0 tokens: int = 0 error: Optional[str] = None def report(self) -> list[str]: - """The new differences, the accepted ones, and the counts. + """The new differences, then the accepted ones, then the counts. - The new lines come first, because they are what a reader is looking - for; the known ones are printed too, so that a line the maintainer - accepted and the run no longer reports can be seen to have gone. + A known difference is printed as well as a new one, so that the + maintainer reads what a field the `differs.txt` accepts says now. """ if self.error: return [f"error {self.name}: {self.error}"] return ( [f"differs {self.name}: {line}" for line in self.new] + [f"known {self.name}: {line}" for line in self.known] + + [ + f"agrees {self.name}: {key} now agrees, remove the line" + for key in self.agreed + ] + [ f"{self.name}: {len(self.differences)} differ, " f"{len(self.known)} known, {len(self.new)} new, " @@ -113,6 +133,56 @@ def report(self) -> list[str]: ) +_LOCATION = re.compile( + r'Question (\d+) "[^"]*"(?:, part \(([a-z]+)\))?(?:, ([a-z ]+))?: ' +) +"""How `in2lambda.compare.differences` names where a difference is.""" + + +def field_key(line: str) -> str: + """The field a difference is in, as a `differs.txt` names it. + + A difference names its location in words and quotes what each set says + there: `Question 2 "", part (a), worked solution: the agent says ...`. The + quotation is a model's wording of that run and changes between runs. The + location does not change, so a `differs.txt` records the location. + + Args: + line: A line as `differences` words it. + + Returns: + The question, the part and the field as a key: `q2.p1.worked_solution`, + or `q2.p1` and `q2` where the difference is a whole part or question + one side wrote and the other did not. + """ + number, part, name = _LOCATION.match(line).groups() + key = f"q{number}" + if part is not None: + key += f".p{ord(part[0]) - ord('a') + 1}" + if name is not None: + key += f".{name.replace(' ', '_')}" + return key + + +def accepted(path: Path) -> list[str]: + """The field keys a target's `differs.txt` accepts. + + Args: + path: The file, which a target that differs from its export nowhere + does not have. + + Returns: + One key a line, in the order the file writes them, with everything from + a `#` on dropped: that is where the reason a field differs is written, + and a line that is all reason names no field. + """ + path = Path(path) + if not path.is_file(): + return [] + lines = (line.split("#")[0].strip() for line in path.read_text().splitlines()) + return [line for line in lines if line] + + def find(root: Path, paths: Sequence[Path] = ()) -> list[Target]: """Every target under a root, by the export folder each one holds. @@ -188,6 +258,7 @@ def run_one( cache_dir: Path = Path(".in2lambda-agent"), settings: Optional[Settings] = None, backend: Optional[Backend] = None, + fresh: bool = False, ) -> Result: """Converts one target and compares what came out with its export. @@ -199,6 +270,8 @@ def run_one( settings: The environment the run has available. backend: The backend to write a filter with, chosen from the settings if absent. + fresh: Read the document again rather than converting the reply saved + beside the filter, which is how a target is given a new reading. Returns: The target's result. Nothing a target raises leaves this function: what @@ -209,6 +282,12 @@ def run_one( settings = settings or load_settings() backend = backend or choose_backend(settings) saved = Path(filters) / target.name + reply = saved / REPLY_NAME + route_a = ( + json.loads(reply.read_text(encoding="utf-8")) + if reply.is_file() and not fresh + else None + ) # Pandoc reads neither a PDF nor the markdown an OCR made of one back into # the document's structure, so route B cannot run over a scanned target: # it converts through route A alone, and no filter is written for it. @@ -231,25 +310,33 @@ def run_one( # The export's own name, so that the zip holds the files the export # holds and the two are compared file by file. name=target.export.name[len(EXPORT_PREFIX) :], + route_a=route_a, ) + if route_a is None: + # The reply is written before the comparison, so that a comparison + # the export's files break does not discard the model's answer. + saved.mkdir(parents=True, exist_ok=True) + reply.write_text(json.dumps(converted.route_a, indent=2), encoding="utf-8") found = differences( Set.from_json(str(converted.zip_path)), Set.from_json(str(target.export)), left_name="the agent", right_name="the export", ) - accepted = known(saved / DIFFERS_NAME) + accepts = accepted(saved / DIFFERS_NAME) except Exception as problem: # A missing credential, a model call that did not finish, a document # pandoc refused, an export half-copied into the corpus: all of them # are this target's line, and the run goes on to the next target. return Result(name=target.name, error=" ".join(str(problem).split())) + differing = {field_key(line) for line in found} return Result( name=target.name, differences=found, - known=[line for line in found if line in accepted], - new=[line for line in found if line not in accepted], + known=[line for line in found if field_key(line) in accepts], + new=[line for line in found if field_key(line) not in accepts], + agreed=[key for key in accepts if key not in differing], flags=len(converted.flags), tokens=converted.tokens, ) @@ -264,6 +351,7 @@ def run( cache_dir: Path = Path(".in2lambda-agent"), settings: Optional[Settings] = None, backend: Optional[Backend] = None, + fresh: bool = False, ) -> list[Result]: """Runs every target under a root, printing each one's report as it finishes. @@ -275,6 +363,8 @@ def run( cache_dir: Where the OCR of each PDF is kept. settings: The environment the runs have available. backend: The backend to write the filters with. + fresh: Read every document again rather than converting the saved + replies. Returns: One result per target, in the order they ran. @@ -289,6 +379,7 @@ def run( cache_dir=cache_dir, settings=settings, backend=backend, + fresh=fresh, ) for line in result.report(): print(line) diff --git a/tests/test_cli.py b/tests/test_cli.py index b05197b..2dcee6e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -537,6 +537,7 @@ def test_targets_defaults(): assert args.filters == Path("targets") assert args.out == Path("out") assert args.cache == Path(".in2lambda-agent") + assert args.fresh is False def test_targets_every_option(): @@ -551,6 +552,7 @@ def test_targets_every_option(): "built", "--cache", "cached", + "--fresh", ] ) @@ -558,6 +560,7 @@ def test_targets_every_option(): assert args.filters == Path("saved") assert args.out == Path("built") assert args.cache == Path("cached") + assert args.fresh is True def test_a_targets_run_prints_its_report_and_passes_with_no_new_difference( @@ -581,6 +584,7 @@ def record(root, **kwargs): assert code == 0 assert given["root"] == Path("ExampleContents/targets") assert given["cache_dir"] == Path("cached") + assert given["fresh"] is False # `run` printed the target's own report as it went; this is the total. assert capsys.readouterr().out == "1 target, 0 new differences\n" diff --git a/tests/test_routes.py b/tests/test_routes.py index 97db383..9356b32 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -140,6 +140,27 @@ def test_convert_reports_the_stray_minus_as_a_flag(tmp_path): ("q2.p1.worked_solution", routes.STRAY_MINUS), ("q3.p1.worked_solution", routes.STRAY_MINUS), ] + # The reply is returned as route A answered it, for a caller to save. + assert result.route_a == REPLY + + +def test_a_reply_given_to_convert_is_route_as_and_no_call_is_made(tmp_path): + # What a targets run hands back from the reply it saved: the same reading of + # the document, so that two runs compare the same set with the export. + backend = FakeBackend() # No replies: a call would raise rather than answer. + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + route_a=REPLY, + ) + + assert backend.calls == [] + assert result.route_a == REPLY + assert result.tokens == 0 # --- tier 1: agreement ---------------------------------------------------------------- diff --git a/tests/test_targets.py b/tests/test_targets.py index 908d951..16f89cd 100644 --- a/tests/test_targets.py +++ b/tests/test_targets.py @@ -6,6 +6,7 @@ targets and is skipped without the private corpus. """ +import copy import json import os import shutil @@ -39,20 +40,27 @@ def make_target(root, name, *, questions="sheet.md", solutions="sheet_solutions. return folder -def fake_convert(monkeypatch, flags=(), error=None): - """Stands in for a conversion: builds the fixture reply, records the call.""" +def fake_convert(monkeypatch, flags=(), error=None, reply=None): + """Stands in for a conversion: builds the fixture reply, records the call. + + A saved reply handed back is what route A answered, as `convert` uses it, so + a second run over a target reads the first run's reply where it has one. + """ calls = [] + reply = REPLY if reply is None else reply def convert(document, solutions=None, **options): calls.append({"document": document, "solutions": solutions, **options}) if error is not None: raise error - built = routes.to_set(REPLY, name=options["name"]) + answered = options.get("route_a") or reply + built = routes.to_set(answered, name=options["name"]) return routes.Converted( set=built, zip_path=routes.build(built, options["out_dir"]), flags=list(flags), - reply=REPLY, + reply=answered, + route_a=answered, tokens=1200, ) @@ -158,7 +166,34 @@ def test_the_comparison_reports_every_difference_from_the_export(tmp_path, monke assert result.tokens == 1200 -def test_a_difference_the_maintainer_accepted_is_known_and_not_new(tmp_path, monkeypatch): +@pytest.mark.parametrize( + "line, key", + [ + ('Question 1 "", main text: the agent says \'a\'', "q1.main_text"), + ('Question 12 "", part (c), worked solution: the agent', "q12.p3.worked_solution"), + ('Question 2 "Towing a submarine", part (a), text: the', "q2.p1.text"), + ('Question 3 "": the export wrote this question and the agent did not', "q3"), + ('Question 3 "", part (b): the export wrote this part', "q3.p2"), + ], +) +def test_the_key_of_a_difference_is_the_field_it_names(line, key): + assert targets.field_key(line) == key + + +def test_a_differs_file_holds_a_key_a_line_and_a_reason_after_the_hash(tmp_path): + path = tmp_path / targets.DIFFERS_NAME + path.write_text( + "# Accepted 2026-09-23.\n" + "q1.main_text # the export keeps the platform's own spacing\n" + "\n" + "q2.p1.worked_solution\n" + ) + + assert targets.accepted(path) == ["q1.main_text", "q2.p1.worked_solution"] + assert targets.accepted(tmp_path / "nothing-here.txt") == [] + + +def test_a_key_the_maintainer_accepted_is_known_and_not_new(tmp_path, monkeypatch): fake_convert(monkeypatch) make_target(tmp_path / "corpus", "ME2") filters = tmp_path / "filters" @@ -168,12 +203,11 @@ def test_a_difference_the_maintainer_accepted_is_known_and_not_new(tmp_path, mon backend=FakeBackend("-- filter"), ) - # Every difference accepted, and one line more that the comparison no - # longer reports, which is neither known nor new. - accepted = filters / "ME2" / targets.DIFFERS_NAME - accepted.write_text( - "".join(f"{line} # t40\n" for line in first.differences) - + "Question 9 \"\": the agent says 'gone' and the export says 'went' # t41\n" + # Every field that differs accepted, and one key more that no field of this + # run differs in, which is neither known nor new. + (filters / "ME2" / targets.DIFFERS_NAME).write_text( + "".join(f"{targets.field_key(line)} # t40\n" for line in first.differences) + + "q9.main_text # t41 a question this sheet no longer has\n" ) again = targets.run_one( target, filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", @@ -182,6 +216,63 @@ def test_a_difference_the_maintainer_accepted_is_known_and_not_new(tmp_path, mon assert again.new == [] assert again.known == first.differences + assert again.agreed == ["q9.main_text"] + + +def test_an_accepted_field_stays_known_however_the_run_words_it(tmp_path, monkeypatch): + # What a differs.txt accepts is the field, not the sentence: route A is a + # model call, and a model does not word a field the same way twice. + fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "ME2") + filters = tmp_path / "filters" + (target,) = targets.find(tmp_path / "corpus") + first = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), + ) + (filters / "ME2" / targets.DIFFERS_NAME).write_text( + "".join(f"{targets.field_key(line)} # t40\n" for line in first.differences) + ) + + # The first question's main text, which differs from the export either way, + # read a sentence longer this time. + reworded = copy.deepcopy(REPLY) + reworded[0]["main_text"] += " Take $g$ as $9.81\\,\\mathrm{m/s^2}$." + fake_convert(monkeypatch, reply=reworded) + again = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), fresh=True, + ) + + assert again.differences != first.differences + assert again.new == [] + assert again.agreed == [] + + +def test_route_as_reply_is_saved_and_read_back_unless_a_fresh_one_is_asked_for( + tmp_path, monkeypatch +): + # A target's conversion is repeatable because route A's reply is: the model + # is called for it once, and a fresh call is a deliberate act. + calls = fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "ME2") + filters = tmp_path / "filters" + (target,) = targets.find(tmp_path / "corpus") + ran = dict( + filters=filters, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache", + backend=FakeBackend("-- filter"), + ) + + targets.run_one(target, **ran) + saved = filters / "ME2" / targets.REPLY_NAME + assert calls[0]["route_a"] is None + assert json.loads(saved.read_text()) == REPLY + + targets.run_one(target, **ran) + assert calls[1]["route_a"] == REPLY + + targets.run_one(target, **ran, fresh=True) + assert calls[2]["route_a"] is None def test_the_filter_is_written_once_and_read_after_that(tmp_path, monkeypatch): @@ -227,7 +318,7 @@ def test_a_scanned_target_converts_with_no_filter(tmp_path, monkeypatch): assert calls[0]["lua"] is None assert backend.calls == [] - assert not (tmp_path / "filters" / "ME2").exists() + assert not (tmp_path / "filters" / "ME2" / targets.FILTER_NAME).exists() def test_a_target_whose_export_cannot_be_read_is_an_error_and_the_next_one_runs( @@ -278,12 +369,14 @@ def test_the_report_names_each_difference_and_counts_them(): differences=["Question 1 \"\": a", "Question 2 \"\": b"], known=["Question 1 \"\": a"], new=["Question 2 \"\": b"], + agreed=["q3.p1.text"], flags=2, ) assert result.report() == [ 'differs ME2: Question 2 "": b', 'known ME2: Question 1 "": a', + "agrees ME2: q3.p1.text now agrees, remove the line", "ME2: 2 differ, 1 known, 1 new, 2 flagged", ] @@ -320,3 +413,19 @@ def test_every_target_reports_its_known_differences_and_no_other(tmp_path, capsy ] assert [one.error for one in results] == [None, None, None] assert [one.new for one in results] == [[], [], []] + + +@live +@pytest.mark.skipif(not TARGETS.is_dir(), reason="private corpus") +def test_the_same_run_twice_reports_the_same_fields(tmp_path, capsys): + # The saved reply is what makes the run above a check rather than a reading: + # route A is not called again, so the second run compares the same set. + ran = dict(filters=targets.DEFAULT_FILTER_DIR, cache_dir=gate.DEFAULT_CACHE_DIR) + first = targets.run(TARGETS, out_dir=tmp_path / "first", **ran) + again = targets.run(TARGETS, out_dir=tmp_path / "again", **ran) + print("\n" + capsys.readouterr().out) + + assert [one.new for one in again] == [[], [], []] + assert [len(one.differences) for one in again] == [ + len(one.differences) for one in first + ] From 0e249e032ea587a3c4d151e0d6e1d55aa1e98ea4 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 09:58:29 +0100 Subject: [PATCH 4/4] implement: Compare each target document with the set Lambda Feedback exported from it (t40) --- README.md | 14 +++++--- in2lambda_agent/targets.py | 70 ++++++++++++++++++++++--------------- tests/test_targets.py | 71 +++++++++++++++++++++++++++++++++++--- 3 files changed, 119 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 86617da..11b13b7 100644 --- a/README.md +++ b/README.md @@ -468,10 +468,16 @@ as a report. target `A/B` keeps its filter at `targets/A/B/filter.lua`, route A's reply at `targets/A/B/reply.json` and its accepted fields at `targets/A/B/differs.txt`. The first run over a target makes one model call for the filter and one for route A's reply, and -writes both. Every run after that reads the two files and makes neither call, so the -second run over a target compares the set the first run compared. `--fresh` reads the -documents again and writes a new reply, which changes the wording of the report and the -number of fields flagged. A target whose questions document is a PDF has no filter: +writes both. Every run after that reads the two files and makes neither of those two +calls, so the second run over a target differs from the export in the same fields as the +first. `--fresh` reads the documents again and writes a new reply, which changes the +wording of the report and the number of fields flagged. + +Those two are the only calls a saved target spares. A target with a filter runs route B +on every run, and a model adjudicates every field the two routes word differently. A +verdict can go the other way on a later run, so the wording of a difference and the +`flagged` count move from run to run while the fields `differs.txt` accepts stay +accepted. A target whose questions document is a PDF has no filter: pandoc cannot read a PDF, so route B does not run and route A converts the pages' markdown alone. `--out` (default `./out`) is where each target's set is written, under the target's own name, and `--cache` (default `./.in2lambda-agent`) is where the OCR of diff --git a/in2lambda_agent/targets.py b/in2lambda_agent/targets.py index 4ae06e2..c31520f 100644 --- a/in2lambda_agent/targets.py +++ b/in2lambda_agent/targets.py @@ -16,11 +16,17 @@ changed what the agent makes of a document nobody looked at again. A `differs.txt` line names a field rather than a sentence because the report -quotes a model's wording. Route A reads the document on every run, and a model -writes the same field differently each time it is asked. Route A's reply is -saved beside the filter and read back for the same reason, so that a second run -over a target compares the set the first run compared. `fresh` reads the -documents again and writes a new reply. +quotes a model's wording. A model writes the same field differently each time +it is asked. The filter and route A's reply are saved beside each other and +read back for the same reason: a second run over a target makes neither of the +two calls that read the document, so the two runs differ from the export in the +same fields. `fresh` reads the documents again and writes a new reply. + +Those are the only two calls a run saves. A target with a filter runs route B +on every run, and `routes.reconcile` has a model adjudicate every field the two +routes word differently. A verdict can go the other way on a later run, so the +wording of a difference and the number of fields flagged move between runs +while the fields `differs.txt` accepts stay accepted. The filters, the replies and the accepted fields are kept in a tree of their own mirroring the targets, so that no file is written into the corpus and a @@ -96,8 +102,9 @@ class Result: agreed: The keys it accepts that nothing differs in any more, which are lines to take out of it. flags: How many fields the conversion flagged for a person. - tokens: What its model calls cost, nothing where the saved reply was - read back. + tokens: What route A cost, and nothing where the saved reply was read + back. The adjudication a target with a filter pays for on every run + is not counted here. error: What stopped the target, and nothing else filled. """ @@ -153,9 +160,14 @@ def field_key(line: str) -> str: Returns: The question, the part and the field as a key: `q2.p1.worked_solution`, or `q2.p1` and `q2` where the difference is a whole part or question - one side wrote and the other did not. + one side wrote and the other did not. A line naming no location returns + the line itself, which no `differs.txt` holds, so a difference this + function cannot read is reported as new. """ - number, part, name = _LOCATION.match(line).groups() + match = _LOCATION.match(line) + if match is None: + return line + number, part, name = match.groups() key = f"q{number}" if part is not None: key += f".p{ord(part[0]) - ord('a') + 1}" @@ -283,16 +295,20 @@ def run_one( backend = backend or choose_backend(settings) saved = Path(filters) / target.name reply = saved / REPLY_NAME - route_a = ( - json.loads(reply.read_text(encoding="utf-8")) - if reply.is_file() and not fresh - else None - ) # Pandoc reads neither a PDF nor the markdown an OCR made of one back into # the document's structure, so route B cannot run over a scanned target: # it converts through route A alone, and no filter is written for it. lua = None if target.questions.suffix.lower() == ".pdf" else saved / FILTER_NAME + route_a = None try: + if reply.is_file() and not fresh: + try: + route_a = json.loads(reply.read_text(encoding="utf-8")) + except ValueError as problem: + # A run interrupted while writing the reply leaves part of a + # JSON document behind, and json.loads names a column of it and + # no file. The name of the file is what the maintainer needs. + raise ValueError(f"{reply}: {problem}; --fresh writes a new one") if lua is not None and not lua.is_file(): saved.mkdir(parents=True, exist_ok=True) lua.write_text( @@ -324,23 +340,23 @@ def run_one( right_name="the export", ) accepts = accepted(saved / DIFFERS_NAME) + keys = [field_key(line) for line in found] + return Result( + name=target.name, + differences=found, + known=[line for line, key in zip(found, keys) if key in accepts], + new=[line for line, key in zip(found, keys) if key not in accepts], + agreed=[key for key in accepts if key not in set(keys)], + flags=len(converted.flags), + tokens=converted.tokens, + ) except Exception as problem: # A missing credential, a model call that did not finish, a document - # pandoc refused, an export half-copied into the corpus: all of them - # are this target's line, and the run goes on to the next target. + # pandoc refused, an export half-copied into the corpus, a reply that + # is not JSON: all of them are this target's line, and the run goes on + # to the next target. return Result(name=target.name, error=" ".join(str(problem).split())) - differing = {field_key(line) for line in found} - return Result( - name=target.name, - differences=found, - known=[line for line in found if field_key(line) in accepts], - new=[line for line in found if field_key(line) not in accepts], - agreed=[key for key in accepts if key not in differing], - flags=len(converted.flags), - tokens=converted.tokens, - ) - def run( root: Path, diff --git a/tests/test_targets.py b/tests/test_targets.py index 16f89cd..5260623 100644 --- a/tests/test_targets.py +++ b/tests/test_targets.py @@ -174,6 +174,12 @@ def test_the_comparison_reports_every_difference_from_the_export(tmp_path, monke ('Question 2 "Towing a submarine", part (a), text: the', "q2.p1.text"), ('Question 3 "": the export wrote this question and the agent did not', "q3"), ('Question 3 "", part (b): the export wrote this part', "q3.p2"), + # A line naming no field at all: its key is the line, which no + # `differs.txt` holds, so the run reports it as new. + ( + "the export has 3 questions and the agent has 4", + "the export has 3 questions and the agent has 4", + ), ], ) def test_the_key_of_a_difference_is_the_field_it_names(line, key): @@ -343,6 +349,59 @@ def test_a_target_whose_export_cannot_be_read_is_an_error_and_the_next_one_runs( assert results[1].error +def test_a_difference_naming_no_field_is_new_and_does_not_stop_the_run( + tmp_path, monkeypatch +): + # A difference in2lambda words some other way - a count of the questions, + # say - names no field for a `differs.txt` to accept. The run reports it as + # new, like a difference in a field nobody has accepted. + fake_convert(monkeypatch) + odd = "the export has 3 questions and the agent has 4" + monkeypatch.setattr( + targets, "differences", lambda *args, **kwargs: [odd, 'Question 1 "": a'] + ) + make_target(tmp_path / "corpus", "ME2") + filters = tmp_path / "filters" + (target,) = targets.find(tmp_path / "corpus") + (filters / "ME2").mkdir(parents=True) + (filters / "ME2" / targets.DIFFERS_NAME).write_text("q1 # t40\n") + + result = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend("-- filter"), + ) + + assert result.error is None + assert result.new == [odd] + assert result.known == ['Question 1 "": a'] + + +def test_a_reply_that_is_not_json_is_a_line_of_its_own_and_the_next_one_runs( + tmp_path, monkeypatch +): + # A first run interrupted while writing the reply leaves half a JSON + # document behind. Reading it is this target's error, and the target after + # it still runs. + fake_convert(monkeypatch) + make_target(tmp_path / "corpus", "CW1") + make_target(tmp_path / "corpus", "ME2") + filters = tmp_path / "filters" + (filters / "CW1").mkdir(parents=True) + (filters / "CW1" / targets.REPLY_NAME).write_text( + json.dumps(REPLY, indent=2)[: len(json.dumps(REPLY, indent=2)) // 2] + ) + + results = targets.run( + tmp_path / "corpus", filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend("-- a", "-- b"), + ) + + assert [one.name for one in results] == ["CW1", "ME2"] + assert targets.REPLY_NAME in results[0].error + assert "--fresh" in results[0].error + assert results[1].error is None and results[1].new + + def test_a_target_that_failed_is_a_line_of_its_own_and_the_next_one_runs(tmp_path, monkeypatch): from in2lambda_agent.mathpix import MathpixError @@ -418,14 +477,16 @@ def test_every_target_reports_its_known_differences_and_no_other(tmp_path, capsy @live @pytest.mark.skipif(not TARGETS.is_dir(), reason="private corpus") def test_the_same_run_twice_reports_the_same_fields(tmp_path, capsys): - # The saved reply is what makes the run above a check rather than a reading: - # route A is not called again, so the second run compares the same set. + # The saved reply and filter are what make the run above a check rather than + # a reading: neither document is read again, so the fields the two runs + # differ from the export in are the same fields. The wording of a difference + # and the number of them is not compared: a target with a filter has the + # fields its two routes word differently adjudicated by a model call on + # every run, and a verdict can go the other way. ran = dict(filters=targets.DEFAULT_FILTER_DIR, cache_dir=gate.DEFAULT_CACHE_DIR) first = targets.run(TARGETS, out_dir=tmp_path / "first", **ran) again = targets.run(TARGETS, out_dir=tmp_path / "again", **ran) print("\n" + capsys.readouterr().out) + assert [one.new for one in first] == [[], [], []] assert [one.new for one in again] == [[], [], []] - assert [len(one.differences) for one in again] == [ - len(one.differences) for one in first - ]