From fa29ba17a9f4e905272742562450db35fb56a608 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 05:30:47 +0100 Subject: [PATCH 1/3] implement: Run the corpus sweep through the two-route conversion (t39) --- README.md | 63 ++++---- docs/how-it-works.md | 50 +++---- in2lambda_agent/cli.py | 72 +++------- in2lambda_agent/corpus.py | 4 + in2lambda_agent/routes.py | 32 +++-- in2lambda_agent/sweep.py | 292 ++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 54 ++----- tests/test_routes.py | 6 +- tests/test_sweep.py | 260 +++++++++++++++++++++++++++++++++ 9 files changed, 665 insertions(+), 168 deletions(-) create mode 100644 in2lambda_agent/sweep.py create mode 100644 tests/test_sweep.py diff --git a/README.md b/README.md index 43f0f3a..43e9f69 100644 --- a/README.md +++ b/README.md @@ -332,8 +332,8 @@ lists the differences `compare` found over three corpus documents and judges eac ## Corpus -The design spec's test plan is the agent run over a corpus of real documents, with one -row of a table recorded for each document: +The design spec's test plan is the two-route conversion run over a corpus of real +documents, with one row of a table recorded for each sheet: ```sh poetry run in2lambda-agent corpus ExampleContents --suffix tex --suffix md @@ -342,48 +342,38 @@ poetry run in2lambda-agent corpus ExampleContents --suffix tex --suffix md In full: ```sh -poetry run in2lambda-agent corpus ROOT [PATH ...] [--suffix S] [--replay] [--rounds N] [--tries N] [--results FILE] [--work DIR] [--specs DIR] [--cache DIR] +poetry run in2lambda-agent corpus ROOT [PATH ...] [--suffix S] [--results FILE] [--work DIR] [--cache DIR] ``` `ROOT` is the corpus directory and each `PATH` a folder under it to run, defaulting to all of it. `--suffix` is repeatable and defaults to `tex`, `md` and `docx`; `--suffix -pdf` runs the PDFs too, which needs Mathpix credentials and one call per PDF. Every run -is review mode `none`. - -A sheet and the solutions file beside it are one run and one row, named after the -questions file. A solutions file with no questions file beside it is converted on its -own, and is a row like any other document. - -The sweep never writes to the corpus. It copies each set's folder into `--work` -(default `./.in2lambda-agent/corpus`), empties that copy first, and runs the documents -there. The copy holds everything under the folder — the figures a sheet names among it — -less what an earlier run left there: a spec, a draft, an `.in2lambda-agent` directory. -It keeps three kinds of file in `--specs` (default `./corpus-specs`), in a tree -mirroring the corpus: the set's spec, each document's log of the commands its fixing -rounds ran, and the `in2lambda-agent-runs.jsonl` every run appends a line to. Set `A/B` -keeps its spec at `corpus-specs/A/B/in2lambda-spec.yaml` and the log of `A/B/sheet.tex` -at `corpus-specs/A/B/sheet.tex.commands.json`. A log entry holds the block ids, field -keys and line ranges its command named, and the wording a `field replace` or a typed -field spells out. It does not hold the draft's fields, which hold every field's -captured text. - -So the copies are throwaway and the specs and the logs are worth keeping. `--replay` -runs the set's spec, then the document's log, and nothing from the model, which turns -a document set into a deterministic test: a document a fixing round repaired replays -to the set the sweep built. - -`--results` (default `./results.csv`) holds one row per document, sorted by path, with -these columns: +pdf` runs the PDFs too, which needs Mathpix credentials and one call per PDF. + +A set is a folder holding at least one questions document. Each set converts as a +folder run of `convert` does: one model call writes the set's filter from its first +sheet, and each sheet of the set then runs through route A and that filter. A sheet and +the solutions file beside it are one conversion and one row, named after the questions +file. A folder of figures, and a folder holding a solutions file alone, are not sets and +have no row. + +The sweep never writes to the corpus. It writes each set's filter to +`WORK/SET/filter.lua` and each sheet's set folder and zip to `WORK/SET/SHEET/`, where +`--work` defaults to `./.in2lambda-agent/corpus`. + +`--results` (default `./results.csv`) holds one row per sheet, in path order, with these +twelve columns: ``` -source, set, outcome, reason, spec, layout, blocks, fields, layer1..layer4, edited, -unassigned, rounds, input_tokens, output_tokens, model_seconds, wall_seconds, -review, rejections +set, sheet, questions, parts, fields, agreed, adjudicated, flagged, not_verbatim, +tokens, seconds, reason ``` [docs/how-it-works.md](docs/how-it-works.md#the-corpus-table) names each column and -where its value comes from. One document that fails is one row and not the end of the -sweep, and a set whose folder cannot be copied is a row for each of its documents. +where its value comes from. `reason` is empty where the sheet ran through both routes +and built its set. One sheet whose conversion raises is one row, with `no set:` and the +error as its reason, and the sheets after it still run. A set whose filter call does not +finish converts every sheet of it through route A alone, and each of those rows reads +`no filter:` and the error. The command exits 1 where a sheet built no set. `--cache` is where the OCR of each PDF is kept. It defaults to `./.in2lambda-agent`, the directory `run` caches into, so a sweep over PDFs that `run` has already converted @@ -399,7 +389,8 @@ with what the baseline records: poetry run in2lambda-agent gate BASELINE [--record] [--cache DIR] [--work DIR] ``` -Every run is `corpus --replay`, so no model call is made. The command prints one line +Every run replays the spec route's saved specs and command logs, so no model call is +made. The command prints one line per folder, and exits 1 when a folder builds fewer documents than the baseline records or when a single document does worse than the baseline records it doing. The second check is what a baseline of no builds rests on: a corpus where every document faults diff --git a/docs/how-it-works.md b/docs/how-it-works.md index dcbcf77..9ba7760 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -163,9 +163,9 @@ the draft the earlier run's rounds left. The message has one form: ``` in2lambda refusing one of the commands ends the run, and the refusal names which -command it was: `command 3 of 5, question add: b7b is in a field already`. The corpus -sweep writes these files and `corpus --replay` reads them; a `run` is given one through -`pipeline.run`. +command it was: `command 3 of 5, question add: b7b is in a field already`. The spec +route's sweep, `corpus.sweep`, writes these files, and the gate's replay reads them; a +`run` is given one through `pipeline.run`. ### `set` @@ -412,35 +412,27 @@ run from another directory. ## The corpus table -`in2lambda-agent corpus` writes one row per document to `--results`. The columns are -these 21, in this order: +`in2lambda-agent corpus` writes one row per sheet to `--results`. The columns are these +12, in this order: | Column | Where it comes from | | --- | --- | -| `source` | the document, relative to the corpus root | -| `set` | the folder the document is in | -| `outcome` | `built`, `build refused`, `faulted`, `skipped`, `no spec`, `replay refused` where in2lambda would not run one of the saved commands, `no model`, `spec failed` and `fix failed` where the model call did not finish, `spec rejected`, `bad spec`, or `error: ` | -| `reason` | the build's refusal, the first error the checks still found, the warnings a build proceeded past, or what an exception said | -| `spec` | `wrote`, `reused`, or `rewritten` where the spec rewrite ran | -| `layout` | the coverage's layout | -| `blocks` | the coverage's block count, before any fixing round | -| `fields` | how many fields the finished draft holds | -| `layer1` | `package.layers`: how many fields the spec wrote | -| `layer2` | `package.layers`: how many a predicate wrote, so 0, because the agent's spec prompt asks for no predicate | -| `layer3` | `package.layers`: how many a round quoted out of the source | -| `layer4` | `package.layers`: how many a round typed out | -| `edited` | `package.layers`: how many fields carry the `edited` flag | -| `unassigned` | how many blocks the coverage left in no field, before any round | -| `rounds` | how many fixing rounds ran | -| `input_tokens` | what the run's model calls read | -| `output_tokens` | what they wrote | -| `model_seconds` | how long they took | -| `wall_seconds` | how long the whole document took | -| `review` | the review mode the run was given, which the sweep sets to `none` | -| `rejections` | how many questions a reviewer turned down, so 0 under mode `none` | - -`blocks` and `unassigned` report the spec run alone, so a `built` row can still name -blocks the spec left unassigned and a later round covered. +| `set` | the sheet's folder, relative to the corpus root | +| `sheet` | the questions document, relative to the corpus root; the solutions document beside it is read into the same row | +| `questions` | how many questions the conversion returned | +| `parts` | how many parts those questions hold | +| `fields` | `routes.fields`: how many text fields the questions and parts hold, the options of a multiple-choice part among them | +| `agreed` | how many fields the two routes returned the same text for, after `routes.fold` folds the whitespace and the notation that renders the same | +| `adjudicated` | how many fields the adjudication call decided | +| `flagged` | how many fields a person is asked to read | +| `not_verbatim` | how many of the flagged fields are not quotes of the source | +| `tokens` | what the sheet's model calls read and wrote, the direct call and the adjudication call; the set's filter call is counted on the set's first sheet | +| `seconds` | how long the sheet took, the filter call included on the set's first sheet | +| `reason` | empty where both routes ran and the sheet built its set; `no set: ` where the conversion raised, `route B failed: ` where the set is route A's alone, and `no filter: ` where the set's filter call did not finish | + +A field only one route filled is neither agreed nor adjudicated: the count of those is +`fields - agreed - adjudicated`. `fields` counts what the conversion returned, so a +sheet route B did not run on still reports its fields. ## Backends and settings diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index 204db29..23f9607 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, gate, pair, pipeline, routes, sweep from in2lambda_agent.mathpix import MathpixClient, MathpixError from in2lambda_agent.model import ModelError, ModelUnavailable, choose_backend from in2lambda_agent.ocr import ocr_pdf @@ -247,63 +247,38 @@ def build_parser() -> argparse.ArgumentParser: help="Where the run left the review.", ) - sweep = subcommands.add_parser( - "corpus", help="Run every document of a corpus and record what each did." + corpus_command = subcommands.add_parser( + "corpus", help="Convert every set of a corpus and record what each sheet did." ) - sweep.add_argument("root", type=Path, help="The corpus directory.") - sweep.add_argument( + corpus_command.add_argument("root", type=Path, help="The corpus directory.") + corpus_command.add_argument( "paths", nargs="*", type=Path, help="Folders under ROOT to run, defaulting to all of it.", ) - sweep.add_argument( + corpus_command.add_argument( "--suffix", action="append", dest="suffixes", metavar="SUFFIX", help="A file suffix to run, repeatable. Default: " - f"{', '.join(corpus.DEFAULT_SUFFIXES)}.", + f"{', '.join(sweep.DEFAULT_SUFFIXES)}.", ) - sweep.add_argument( - "--replay", - action="store_true", - help="Run the saved specs and the documents' saved logs, making no " - "model call.", - ) - sweep.add_argument( - "--rounds", - type=int, - default=3, - help="How many times the agent may try to fix validation errors.", - ) - sweep.add_argument( - "--tries", - type=try_count, - default=3, - help="How many specs the agent may write before keeping the best.", - ) - sweep.add_argument( + corpus_command.add_argument( "--results", type=Path, - default=corpus.DEFAULT_RESULTS, - help="Where to write the table, one row per document.", + default=sweep.DEFAULT_RESULTS, + help="Where to write the table, one row per sheet.", ) - sweep.add_argument( + corpus_command.add_argument( "--work", type=Path, - default=corpus.DEFAULT_WORK_DIR, - help="Where each set's folder is copied to be run; the corpus itself " - "is never written to.", - ) - sweep.add_argument( - "--specs", - type=Path, - default=corpus.DEFAULT_SPEC_DIR, - help="The tree the sets' specs are kept in, mirroring the corpus, with " - "each document's log of its fixing rounds beside its set's spec.", + default=sweep.DEFAULT_WORK_DIR, + help="Where each set's filter and each sheet's zip are written; the " + "corpus itself is never written to.", ) - sweep.add_argument( + corpus_command.add_argument( "--cache", type=Path, default=pipeline.DEFAULT_CACHE_DIR, @@ -483,26 +458,21 @@ def main(argv: Optional[Sequence[str]] = None) -> int: return convert_command(args) if args.command == "corpus": - rows = corpus.sweep( + rows = sweep.sweep( args.root, paths=args.paths, # Appended to, so the default cannot be the parser's: that would be # the default and whatever was named. - suffixes=args.suffixes or corpus.DEFAULT_SUFFIXES, + suffixes=args.suffixes or sweep.DEFAULT_SUFFIXES, results=args.results, work=args.work, - specs=args.specs, - replay=args.replay, - rounds=args.rounds, - tries=args.tries, cache=args.cache, settings=load_settings(), ) - print(f"{len(rows)} documents, written to {args.results}") - # A file that is not a document is not a document that failed, so a - # figure's tex source among the rows does not make the sweep one. - succeeded = {"built", "skipped"} - return 0 if rows and all(row.outcome in succeeded for row in rows) else 1 + print(f"{len(rows)} sheets, written to {args.results}") + # A sheet route B failed on built its set from route A, so the sheets + # that built no set are what the exit code reports. + return 0 if rows and all(row.built for row in rows) else 1 if args.command == "gate": baseline = gate.read_baseline(args.baseline) diff --git a/in2lambda_agent/corpus.py b/in2lambda_agent/corpus.py index db15e40..225a02a 100644 --- a/in2lambda_agent/corpus.py +++ b/in2lambda_agent/corpus.py @@ -1,5 +1,9 @@ """The test plan's sweep: the agent over a corpus, one row per document. +This is the spec route's sweep, and `gate.run` is its one caller: the gate +replays the saved specs, which the two-route conversion has nothing to replay. +The `corpus` command runs the two-route sweep of `sweep.py`. + The design spec asks, per document, what share of the fields each layer wrote, how many rounds the checks took to come clean or that they never did, what the model calls cost in tokens and time, and whether the set's spec was reused. That diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 730ccee..1975712 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -42,6 +42,8 @@ STRAY_MINUS = "a stray minus sign inside or beside a display maths; Mathpix reads a separator line as one" +NOT_VERBATIM = "not a quote of the source" + _FOLDS = ( ("\\left(", "("), ("\\right)", ")"), ("\\left[", "["), ("\\right]", "]"), ("\\mathrm{~", "\\mathrm{"), ("\\text {", "\\text{"), ("\\space", " "), @@ -270,12 +272,18 @@ def _source_lines(source: str, *texts: str, around: int = 2) -> str: return "\n".join(lines[n] for n in sorted(hits)) or "(not found)" -def adjudicate(a: Reply_, b: Reply_, keys: list[str], source: str, backend: Backend) -> dict[str, tuple[str, str]]: - """Tier 2: one small call over the disputed fields; a choice it may not make becomes 'person'.""" +def adjudicate( + a: Reply_, b: Reply_, keys: list[str], source: str, backend: Backend +) -> tuple[dict[str, tuple[str, str]], Optional[Reply]]: + """Tier 2: one small call over the disputed fields; a choice it may not make becomes 'person'. + + Returns the verdicts and the call's usage, which is None where no field was + disputed and no call was made. + """ fa, fb = fields(a), fields(b) keys = [k for k in keys if k in fa and k in fb] if not keys: - return {} + return {}, None shown = "\n\n".join( f"FIELD {k}\nA: {fa[k]}\nB: {fb[k]}\nSOURCE LINES:\n{_source_lines(source, fa[k], fb[k])}" for k in keys ) @@ -291,7 +299,7 @@ def adjudicate(a: Reply_, b: Reply_, keys: list[str], source: str, backend: Back verdicts[k] = ("text:" + v["text"], reason) else: verdicts[k] = ("person", reason or "the adjudicator's own words") - return verdicts + return verdicts, reply @dataclass @@ -309,6 +317,8 @@ class Reconciled: defaulted: int adjudicated: int flags: list[Flag] = field(default_factory=list) + # What the adjudication call read and wrote, and zero where it was not made. + tokens: int = 0 def _set_field(reply: Reply_, key: str, text: str) -> None: @@ -350,7 +360,9 @@ def reconcile(a: Reply_, b: Reply_, source: str, backend: Optional[Backend] = No _set_field(merged, k, fb[k]) for k in structural: result.flags.append(Flag(k, "present" if k in _structure(a) else "absent", "present" if k in _structure(b) else "absent", "one route did not find it")) - verdicts = adjudicate(a, b, wording, source, backend) if wording and backend is not None else {} + verdicts, usage = adjudicate(a, b, wording, source, backend) if wording and backend is not None else ({}, None) + if usage is not None: + result.tokens = usage.usage.input_tokens + usage.usage.output_tokens for k in wording: choice, reason = verdicts.get(k, ("person", "not adjudicated")) if choice == "B": @@ -361,7 +373,7 @@ def reconcile(a: Reply_, b: Reply_, source: str, backend: Optional[Backend] = No result.flags.append(Flag(k, fa[k], fb[k], reason)) for k in not_verbatim(merged, source): if not any(f.field == k for f in result.flags): - result.flags.append(Flag(k, fields(merged)[k], "", "not a quote of the source")) + result.flags.append(Flag(k, fields(merged)[k], "", NOT_VERBATIM)) return result @@ -464,8 +476,9 @@ def convert( solutions_md = markdown_of(solutions, cache_dir, settings)[0] if solutions else None 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 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)] + flags = [Flag(k, fields(reply)[k], "", NOT_VERBATIM) for k in not_verbatim(reply, source)] if lua is not None: try: other = run_filter(lua, document) @@ -477,6 +490,9 @@ def convert( else: reconciled = reconcile(reply, other, source, backend) reply, flags = reconciled.fields, reconciled.flags + # The adjudication call is the document's second call, so its tokens + # are the document's too. + tokens += reconciled.tokens counts = ( reconciled.agreed + reconciled.defaulted + reconciled.adjudicated, reconciled.agreed, reconciled.defaulted, reconciled.adjudicated, @@ -487,7 +503,7 @@ def convert( built = to_set(reply, name=name, directory=images) return Converted( set=built, zip_path=build(built, out_dir), flags=flags, reply=reply, - tokens=usage.usage.input_tokens + usage.usage.output_tokens, + tokens=tokens, fields=counts[0], agreed=counts[1], defaulted=counts[2], adjudicated=counts[3], route_b_error=error, ) diff --git a/in2lambda_agent/sweep.py b/in2lambda_agent/sweep.py new file mode 100644 index 0000000..a5f0139 --- /dev/null +++ b/in2lambda_agent/sweep.py @@ -0,0 +1,292 @@ +"""The two-route conversion over a corpus, one row of a table per sheet. + +A set is a folder holding at least one questions document. Each set is +converted as `routes.convert_folder` converts one: a model call writes a Lua +filter from the set's first sheet, and each sheet of the set then runs through +route A, the direct model call, and route B, that filter under pandoc. The +sweep runs the sheets itself rather than calling `convert_folder`, so that it +can time each sheet and write a row for a sheet whose conversion raised. + +The measures of docs/plan.md are the columns: how many fields the two routes +returned, how many they agreed on, how many the adjudication call decided, how +many are flagged for a person, how many are not quotes of the source, and what +the model calls read and wrote. + +Nothing here writes into the corpus. Each set's filter, and each sheet's set +folder and zip, are written under the work directory; the OCR of each PDF is +read from and written to the cache directory. +""" + +import csv +import time +from dataclasses import asdict, dataclass, fields +from pathlib import Path +from typing import Optional, Sequence + +from in2lambda_agent import pair, pipeline, routes +from in2lambda_agent.model import Backend, choose_backend +from in2lambda_agent.settings import Settings + +DEFAULT_SUFFIXES = ("tex", "md", "docx") +"""What a sweep runs over unless told otherwise: everything but the PDFs, which +need Mathpix credentials and a call each.""" + +DEFAULT_RESULTS = Path("results.csv") +"""The table, under the directory the user ran from.""" + +DEFAULT_WORK_DIR = Path(".in2lambda-agent/corpus") +"""Where each set's filter and each sheet's zip are written.""" + +NO_SET = "no set: " +"""What the reason of a sheet that built no set begins with.""" + + +@dataclass +class Row: + """One sheet's line of the table, in the order the columns are written. + + Attributes: + set: The sheet's folder, relative to the corpus root. + sheet: The questions document, relative to the corpus root. The + solutions document beside it is read into the same row. + questions: How many questions the conversion returned. + parts: How many parts those questions hold. + fields: How many text fields the questions and parts hold. + agreed: How many fields the two routes returned the same text for. + adjudicated: How many fields the adjudication call decided. A field + only one route filled is neither agreed nor adjudicated: the count + of those is `fields - agreed - adjudicated`. + flagged: How many fields a person is asked to read. + not_verbatim: How many of the flagged fields are not quotes of the + source. + tokens: What the sheet's model calls read and wrote. The first sheet of + a set carries the set's filter call as well as its own. + seconds: How long the sheet took, the filter call included on the first + sheet of a set. + reason: Empty where both routes ran and the sheet built its set. + `no set: ` where the conversion raised and the sheet built + nothing, `route B failed: ` where the set is + route A's alone, and `no filter: ` where the set's filter + call did not finish and no sheet of the set ran route B. + """ + + set: str + sheet: str + questions: int = 0 + parts: int = 0 + fields: int = 0 + agreed: int = 0 + adjudicated: int = 0 + flagged: int = 0 + not_verbatim: int = 0 + tokens: int = 0 + seconds: float = 0.0 + reason: str = "" + + @property + def built(self) -> bool: + """Whether the sheet wrote a set. A sheet route B failed on wrote one.""" + return not self.reason.startswith(NO_SET) + + +COLUMNS = tuple(one.name for one in fields(Row)) +"""The table's header, which is the row's own order.""" + + +def _one_line(text: str) -> str: + """A reason as one line of the table: a message that wraps stays one cell.""" + return " ".join(text.split()) + + +def _sheets( + folder: Path, suffixes: Sequence[str] = DEFAULT_SUFFIXES +) -> list[tuple[Path, Optional[Path]]]: + """The pairs of a folder a sweep of these suffixes converts. + + `pair.pairs_in` holds a sheet written both as `Sheet_1.tex` and as + `Sheet_1.pdf` once, under the suffix it prefers, which is the tex. A sweep + of the PDFs alone therefore has no such sheet to run. + """ + wanted = {"." + one.lower().lstrip(".") for one in suffixes} + return [ + (sheet, solutions) + for sheet, solutions in pair.pairs_in(folder) + if sheet.suffix.lower() in wanted + ] + + +def sets( + root: Path, paths: Sequence[Path] = (), suffixes: Sequence[str] = DEFAULT_SUFFIXES +) -> list[Path]: + """The folders of a corpus a sweep converts. + + Args: + root: The corpus directory. + paths: Folders under it to run, relative to it; all of it if empty. + suffixes: The file suffixes that are documents, with or without the dot. + + Returns: + Every folder holding at least one questions document of a wanted + suffix, the named folders themselves included, sorted by their path + relative to the root. A folder of figures, and a folder holding a + solutions document alone, are not sets and have no row. + """ + root = Path(root) + found = { + folder + for where in ([root / one for one in paths] or [root]) + for folder in [where, *(one for one in where.rglob("*") if one.is_dir())] + if _sheets(folder, suffixes) + } + return sorted(found, key=lambda folder: folder.relative_to(root).as_posix()) + + +def row_of( + set_name: str, sheet: str, converted: routes.Converted, seconds: float +) -> Row: + """The row of a sheet that converted. + + `converted.fields` counts the fields the two routes were compared over and + is zero where route B did not run, so the column is read off the reply + instead, which holds every field either route filled. + + Args: + set_name: The sheet's folder, relative to the corpus root. + sheet: The sheet, relative to the corpus root. + converted: What `routes.convert` returned. + seconds: How long the conversion took. + """ + return Row( + set=set_name, + sheet=sheet, + questions=len(converted.reply), + parts=sum(len(q.get("parts", [])) for q in converted.reply), + fields=len(routes.fields(converted.reply)), + agreed=converted.agreed, + adjudicated=converted.adjudicated, + flagged=len(converted.flags), + not_verbatim=len( + [one for one in converted.flags if one.reason == routes.NOT_VERBATIM] + ), + tokens=converted.tokens, + seconds=round(seconds, 3), + reason=( + f"route B failed: {_one_line(converted.route_b_error)}" + if converted.route_b_error + else "" + ), + ) + + +def write_results(rows: Sequence[Row], path: Path) -> None: + """Writes the table, header and all, over whatever was there before. + + Args: + rows: The rows, in the order they are to be written. + path: The file to write. + """ + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("w", newline="", encoding="utf-8") as out: + writer = csv.DictWriter(out, fieldnames=COLUMNS) + writer.writeheader() + for row in rows: + writer.writerow(asdict(row)) + + +def sweep( + root: Path, + *, + paths: Sequence[Path] = (), + suffixes: Sequence[str] = DEFAULT_SUFFIXES, + results: Path = DEFAULT_RESULTS, + work: Path = DEFAULT_WORK_DIR, + cache: Path = pipeline.DEFAULT_CACHE_DIR, + settings: Optional[Settings] = None, + backend: Optional[Backend] = None, +) -> list[Row]: + """Converts every set of a corpus, printing a line per sheet and writing the table. + + What a sheet raises is that sheet's row, and the sheets after it still run. + What a set's filter call raises is a row of every sheet of that set, each + converted through route A alone. + + Args: + root: The corpus directory. + paths: Folders under it to run, relative to it; all of it if empty. + suffixes: The file suffixes that are documents. + results: Where to write the table. + work: Where each set's filter and each sheet's zip are written. + cache: Where the OCR of each PDF is kept, so that a sweep pointed at a + cache another run filled converts no PDF again. + settings: The environment the runs have available. + backend: The backend the calls are made to, chosen from the settings if + absent. + + Returns: + One row per sheet, in the order they ran. + """ + root = Path(root).resolve() + work = Path(work).resolve() + settings = settings if settings is not None else Settings() + backend = backend if backend is not None else choose_backend(settings) + + rows = [] + for folder in sets(root, paths, suffixes): + set_name = folder.relative_to(root).as_posix() + pairs = _sheets(folder, suffixes) + into = work / set_name + into.mkdir(parents=True, exist_ok=True) + lua: Optional[Path] = None + no_filter = "" + filter_tokens = 0 + started = time.monotonic() + try: + source, usage = routes.write_filter(pairs[0][0], pairs[0][1], backend) + except Exception as problem: + no_filter = f"no filter: {_one_line(str(problem))}" + else: + lua = into / "filter.lua" + lua.write_text(source, encoding="utf-8") + filter_tokens = usage.usage.input_tokens + usage.usage.output_tokens + filter_seconds = time.monotonic() - started + + for number, (sheet, solutions) in enumerate(pairs): + name = sheet.relative_to(root).as_posix() + started = time.monotonic() + try: + converted = routes.convert( + sheet, + solutions, + out_dir=into / sheet.stem, + cache_dir=cache, + backend=backend, + settings=settings, + lua=lua, + name=sheet.stem, + ) + except Exception as problem: + row = Row( + set=set_name, + sheet=name, + seconds=round(time.monotonic() - started, 3), + reason=f"{NO_SET}{_one_line(str(problem))}", + ) + else: + row = row_of(set_name, name, converted, time.monotonic() - started) + if no_filter: + row.reason = no_filter + if number == 0: + # The filter is the set's and is written once, so the set's + # first sheet carries what writing it read, wrote and took. + row.tokens += filter_tokens + row.seconds = round(row.seconds + filter_seconds, 3) + print( + f"{'built' if row.built else 'no set':<8} {row.sheet}: " + f"{row.fields} fields, flagged {row.flagged}" + + (f" {row.reason}" if row.reason else "") + ) + rows.append(row) + + write_results(rows, results) + return rows diff --git a/tests/test_cli.py b/tests/test_cli.py index 6862e9d..7568ddd 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, gate, pipeline, routes, sweep from in2lambda_agent.cli import build_parser, main, reviewer_name from in2lambda_agent.model import ModelUnavailable, Usage from in2lambda_agent.settings import Settings @@ -339,12 +339,8 @@ def test_corpus_defaults(): # Resolved where it is used: appending to a list the parser holds would run # the default suffixes as well as the named ones. assert args.suffixes is None - assert args.replay is False - assert args.rounds == 3 - assert args.tries == 3 assert args.results == Path("results.csv") assert args.work == Path(".in2lambda-agent/corpus") - assert args.specs == Path("corpus-specs") assert args.cache == Path(".in2lambda-agent") @@ -359,17 +355,10 @@ def test_corpus_every_option(): "tex", "--suffix", "md", - "--replay", - "--rounds", - "1", - "--tries", - "1", "--results", "sweep.csv", "--work", "working", - "--specs", - "saved", "--cache", "cached", ] @@ -377,12 +366,8 @@ def test_corpus_every_option(): assert args.paths == [Path("Aero"), Path("MATE40002")] assert args.suffixes == ["tex", "md"] - assert args.replay is True - assert args.rounds == 1 - assert args.tries == 1 assert args.results == Path("sweep.csv") assert args.work == Path("working") - assert args.specs == Path("saved") assert args.cache == Path("cached") @@ -393,7 +378,7 @@ def record(*args, **kwargs): given.update(kwargs) return [] - monkeypatch.setattr(corpus, "sweep", record) + monkeypatch.setattr(sweep, "sweep", record) main(["corpus", "ExampleContents", "--cache", "cached"]) @@ -562,25 +547,23 @@ def test_compare_without_a_backend_says_what_to_set(tmp_path, pdf, monkeypatch, assert "claude login" in capsys.readouterr().err -def test_a_sweep_of_built_and_skipped_rows_is_a_sweep_that_worked(monkeypatch): - # A corpus folder with a figure's tex source in it has a skipped row in - # every sweep of it, and a file that is not a document is not a document - # that failed: the exit code is the documents' and nothing else's. +def test_a_sweep_is_a_sweep_that_worked_where_every_sheet_built_a_set(monkeypatch): + # A sheet route B failed on still built its set from route A, so the exit + # code reports the sheets that built nothing and nothing else. rows = [ - corpus.Row(source="tex/sheet.tex", set="tex", outcome="built"), - corpus.Row( - source="tex/figures/tunnel-potential.tex", - set="tex/figures", - outcome="skipped", - reason="no \\begin{document}", + sweep.Row(set="tex", sheet="tex/sheet.tex"), + sweep.Row( + set="tex", + sheet="tex/sheet-2.tex", + reason="route B failed: Error running filter", ), ] - monkeypatch.setattr(corpus, "sweep", lambda *args, **kwargs: rows) + monkeypatch.setattr(sweep, "sweep", lambda *args, **kwargs: rows) assert main(["corpus", "ExampleContents"]) == 0 rows.append( - corpus.Row(source="tex/sheet-2.tex", set="tex", outcome="build refused") + sweep.Row(set="tex", sheet="tex/sheet-3.tex", reason="no set: ModelError") ) assert main(["corpus", "ExampleContents"]) == 1 @@ -678,19 +661,6 @@ def record(source, **passed): assert given["tries"] == 5 -def test_how_many_specs_may_be_written_reaches_the_sweep(monkeypatch): - given = {} - - def record(root, **passed): - given.update(passed) - return [corpus.Row(source="sheets/sheet.md", set="sheets", outcome="built")] - - monkeypatch.setattr(corpus, "sweep", record) - - assert main(["corpus", "ExampleContents", "--tries", "5"]) == 0 - assert given["tries"] == 5 - - @pytest.mark.parametrize("count", ["0", "-1"]) def test_a_run_that_may_write_no_spec_is_refused(count, capsys): with pytest.raises(SystemExit): diff --git a/tests/test_routes.py b/tests/test_routes.py index 2fa4a58..0c18d14 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -215,8 +215,10 @@ def test_the_adjudicator_may_pick_one_side_and_its_pick_is_kept(): other = copy.deepcopy(REPLY) other[1]["parts"][1]["content"] = "Determine the drag force on the plate, in newtons." backend = FakeBackend(json.dumps([{"field": "q2.p2.content", "choice": "A", "reason": "B adds words the source lacks"}])) - verdicts = routes.adjudicate(REPLY, other, ["q2.p2.content"], QUESTIONS + "\n" + SOLUTIONS, backend) + verdicts, usage = routes.adjudicate(REPLY, other, ["q2.p2.content"], QUESTIONS + "\n" + SOLUTIONS, backend) assert verdicts == {"q2.p2.content": ("A", "B adds words the source lacks")} + # The call's usage, which the document's token count adds to the direct call's. + assert usage.usage.input_tokens > 0 ((_, prompt),) = backend.calls assert "Determine the drag force on the plate." in prompt and "in newtons" in prompt assert len(prompt) < 4000 # the disputed field and its source lines, not the document @@ -226,7 +228,7 @@ def test_the_adjudicators_own_words_are_refused_and_the_field_is_flagged(): other = copy.deepcopy(REPLY) other[1]["parts"][1]["content"] = "Determine the drag force on the plate, in newtons." backend = FakeBackend(json.dumps([{"field": "q2.p2.content", "choice": "text", "text": "Find the drag on the plate.", "reason": "shorter"}])) - verdicts = routes.adjudicate(REPLY, other, ["q2.p2.content"], QUESTIONS + "\n" + SOLUTIONS, backend) + verdicts, _ = routes.adjudicate(REPLY, other, ["q2.p2.content"], QUESTIONS + "\n" + SOLUTIONS, backend) assert verdicts["q2.p2.content"][0] == "person" diff --git a/tests/test_sweep.py b/tests/test_sweep.py new file mode 100644 index 0000000..83fb45e --- /dev/null +++ b/tests/test_sweep.py @@ -0,0 +1,260 @@ +"""The sweep of a corpus through the two-route conversion, one row per sheet. + +Written before the module. The fixture sets are the ones `test_routes` runs the +folder conversion over, and the replies are that test's, so a set of +`paired.md`, `paired_solutions.md` and `sheet.md` reports the counts recorded +there: 10 fields a sheet, 8 agreed and 1 adjudicated on `paired.md`. +""" + +import csv +import json +import os +import shutil +from pathlib import Path + +import pytest + +from conftest import FakeBackend +from test_routes import PAIRED_DIRECT, SHEET_DIRECT + +from in2lambda_agent import cli, routes, sweep +from in2lambda_agent.model import ModelError +from in2lambda_agent.settings import Settings + +FIXTURES = Path(__file__).parent / "fixtures" +SHEETS = ("paired.md", "paired_solutions.md", "sheet.md") +FILTER = (FIXTURES / "pair-filter.lua").read_text() +ADJUDICATION = json.dumps( + [{"field": "q2.main_text", "choice": "A", "reason": "B carries the parts too"}] +) +EXAMPLES = Path( + "/Users/peterbjohnson/code/lambdafeedback/in2lambda-agent/ExampleContents" +) +COURSES = ("MECH60014_Stress_analysis_3", "PHYS40002-Mechanics", "UCL_MechEng") + +live = pytest.mark.skipif( + not os.environ.get("IN2LAMBDA_LIVE"), reason="calls Mathpix and a model" +) +pandoc = pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") + + +def corpus(root: Path, *names: str) -> Path: + """Writes one set folder per name, each holding the three fixture sheets.""" + for name in names: + folder = root / name + folder.mkdir(parents=True) + for sheet in SHEETS: + shutil.copy(FIXTURES / sheet, folder / sheet) + return root + + +def replies(sets: int = 1) -> list[str]: + """The model's answers for one set: the filter, and each sheet's call.""" + return [FILTER, json.dumps(PAIRED_DIRECT), ADJUDICATION, json.dumps(SHEET_DIRECT)] * sets + + +def table(path: Path) -> tuple[list[str], list[dict]]: + """The header and the rows of a written table.""" + with Path(path).open(newline="", encoding="utf-8") as written: + reader = csv.DictReader(written) + return reader.fieldnames, list(reader) + + +# --- which folders are sets ------------------------------------------------------------- + + +def test_a_folder_with_a_sheet_in_it_is_a_set_and_one_without_is_not(tmp_path): + root = corpus(tmp_path / "corpus", "alpha", "beta/second") + (root / "solutions-only").mkdir() + shutil.copy(FIXTURES / "paired_solutions.md", root / "solutions-only") + (root / "alpha" / "figures").mkdir() + shutil.copy(FIXTURES / "ball.png", root / "alpha" / "figures") + + assert sweep.sets(root) == [root / "alpha", root / "beta" / "second"] + + +def test_only_the_named_paths_and_the_named_suffixes_run(tmp_path): + root = corpus(tmp_path / "corpus", "alpha", "beta") + + assert sweep.sets(root, paths=[Path("beta")]) == [root / "beta"] + # The fixture sheets are markdown, so a sweep of the tex files has no set. + assert sweep.sets(root, suffixes=["tex"]) == [] + + +# --- the row of one sheet --------------------------------------------------------------- + + +def test_the_row_counts_the_flags_and_the_fields_that_are_not_quotes(): + converted = routes.Converted( + set=None, + zip_path=Path("paired.zip"), + reply=PAIRED_DIRECT, + flags=[ + routes.Flag("q1.main_text", "a ball", "", routes.NOT_VERBATIM), + routes.Flag("q2.p1.content", "A", "B", "two readings"), + ], + tokens=500, + fields=10, + agreed=8, + defaulted=1, + adjudicated=1, + ) + + row = sweep.row_of("alpha", "alpha/paired.md", converted, 2.5) + + assert row.set == "alpha" and row.sheet == "alpha/paired.md" + assert (row.questions, row.parts, row.fields) == (2, 2, 10) + assert (row.agreed, row.adjudicated) == (8, 1) + assert (row.flagged, row.not_verbatim) == (2, 1) + assert (row.tokens, row.seconds) == (500, 2.5) + assert row.reason == "" and row.built + + +# --- the sweep --------------------------------------------------------------------------- + + +@pandoc +def test_a_sweep_writes_one_filter_a_set_and_one_row_a_sheet(tmp_path): + root = corpus(tmp_path / "corpus", "alpha", "beta") + before = sorted(path.relative_to(root).as_posix() for path in root.rglob("*")) + backend = FakeBackend(*replies(sets=2)) + + rows = sweep.sweep( + root, + results=tmp_path / "results.csv", + work=tmp_path / "work", + settings=Settings(), + backend=backend, + ) + + assert [(row.set, row.sheet) for row in rows] == [ + ("alpha", "alpha/paired.md"), + ("alpha", "alpha/sheet.md"), + ("beta", "beta/paired.md"), + ("beta", "beta/sheet.md"), + ] + assert [(row.questions, row.parts, row.fields) for row in rows] == [(2, 2, 10)] * 4 + assert [(row.agreed, row.adjudicated, row.flagged) for row in rows] == [ + (8, 1, 0), + (10, 0, 0), + (8, 1, 0), + (10, 0, 0), + ] + assert all(row.reason == "" and row.built for row in rows) + # One filter call a set, written under the set's own folder, and read by + # both sheets of that set. + for name in ("alpha", "beta"): + written_filter = (tmp_path / "work" / name / "filter.lua").read_text() + assert written_filter == FILTER.strip() + assert (tmp_path / "work" / "alpha" / "paired" / "paired.zip").is_file() + assert (tmp_path / "work" / "beta" / "sheet" / "sheet.zip").is_file() + # The filter call's tokens are the first sheet's, so each set's first row + # reports more tokens than its second. + assert rows[0].tokens > rows[1].tokens > 0 + header, written = table(tmp_path / "results.csv") + assert header == list(sweep.COLUMNS) + assert [one["sheet"] for one in written] == [row.sheet for row in rows] + assert written[0]["agreed"] == "8" and written[0]["not_verbatim"] == "0" + assert sorted(path.relative_to(root).as_posix() for path in root.rglob("*")) == before + + +@pandoc +def test_a_sheet_that_raises_is_a_row_and_the_next_sheet_still_runs(tmp_path): + root = corpus(tmp_path / "corpus", "alpha") + backend = FakeBackend( + FILTER, ModelError("the provider stopped the call"), json.dumps(SHEET_DIRECT) + ) + + rows = sweep.sweep( + root, + results=tmp_path / "results.csv", + work=tmp_path / "work", + settings=Settings(), + backend=backend, + ) + + assert [row.sheet for row in rows] == ["alpha/paired.md", "alpha/sheet.md"] + assert rows[0].reason == "no set: the provider stopped the call" + assert not rows[0].built + assert (rows[0].questions, rows[0].fields) == (0, 0) + assert rows[1].built and rows[1].fields == 10 + assert [one["sheet"] for one in table(tmp_path / "results.csv")[1]] == [ + "alpha/paired.md", + "alpha/sheet.md", + ] + + +@pandoc +def test_a_set_whose_filter_call_fails_converts_every_sheet_through_route_a(tmp_path): + root = corpus(tmp_path / "corpus", "alpha") + # No filter, so route B does not run and no field is adjudicated: the two + # direct calls are the whole of the set's calls. + backend = FakeBackend( + ModelError("the provider stopped the call"), + json.dumps(PAIRED_DIRECT), + json.dumps(SHEET_DIRECT), + ) + + rows = sweep.sweep( + root, + results=tmp_path / "results.csv", + work=tmp_path / "work", + settings=Settings(), + backend=backend, + ) + + assert [row.reason for row in rows] == [ + "no filter: the provider stopped the call" + ] * 2 + assert all(row.built and row.fields == 10 for row in rows) + assert [(row.agreed, row.adjudicated) for row in rows] == [(0, 0), (0, 0)] + + +@pandoc +def test_a_filter_pandoc_refuses_leaves_a_row_built_from_route_a(tmp_path): + root = corpus(tmp_path / "corpus", "alpha") + backend = FakeBackend( + "this is not a filter\n", json.dumps(PAIRED_DIRECT), json.dumps(SHEET_DIRECT) + ) + + rows = sweep.sweep( + root, + results=tmp_path / "results.csv", + work=tmp_path / "work", + settings=Settings(), + backend=backend, + ) + + assert all(row.built and row.fields == 10 for row in rows) + assert all(row.reason.startswith("route B failed: ") for row in rows) + + +# --- live ---------------------------------------------------------------------------------- + + +@live +@pytest.mark.skipif(not EXAMPLES.is_dir(), reason="private corpus") +def test_the_three_course_folders_sweep_from_the_command_line(tmp_path, capsys): + # The ticket's run: the three folders of ExampleContents the gate replays, + # PDFs included, reading the OCR the gate's cache holds. + results = tmp_path / "results.csv" + + code = cli.main( + [ + "corpus", + str(EXAMPLES), + *COURSES, + "--suffix", "tex", "--suffix", "md", "--suffix", "docx", "--suffix", "pdf", + "--results", str(results), + "--work", str(tmp_path / "work"), + "--cache", str(Path.home() / ".cache" / "in2lambda-agent"), + ] + ) + + printed = capsys.readouterr().out + print("\n" + printed) + print(results.read_text(encoding="utf-8")) + header, written = table(results) + assert header == list(sweep.COLUMNS) + assert {one["set"].split("/")[0] for one in written} == set(COURSES) + assert code in (0, 1) From aaeb97fb8beca89f61d34f2f28cc340679849f1d Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 05:53:54 +0100 Subject: [PATCH 2/3] implement: Run the corpus sweep through the two-route conversion (t39) --- docs/how-it-works.md | 5 ++--- tests/test_routes.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 9ba7760..914501f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -353,9 +353,8 @@ makes. `field replace` writes no layer. in2lambda leaves the field at the layer that wrote it, leaves it quoting the lines it was copied from, and sets the field's `edited` flag. So a -round that replaces a layer 1 field records `layer1=1, layer4=0, edited=1`, and the -corpus table's `layer1` to `layer4` columns count each field under the layer in2lambda -recorded. `package.questions` is the one reader that counts an edited field as layer 4, +round that replaces a layer 1 field records `layer1=1, layer4=0, edited=1`. +`package.questions` is the one reader that counts an edited field as layer 4, and it does so to sort the questions a `sample` review shows. ## Review modes diff --git a/tests/test_routes.py b/tests/test_routes.py index 287f70e..737b105 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -393,6 +393,30 @@ def test_a_folder_runs_one_filter_over_every_sheet_and_reports_each(tmp_path): assert paired.reply[0]["parts"][0]["worked_solution"].startswith("1(a) $\\omega") +@pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +def test_a_sheet_counts_the_tokens_of_its_direct_call_and_its_adjudication(tmp_path): + # What the corpus table's tokens column reports, and a sheet with a disputed + # field made two calls, not one. + direct = json.dumps(PAIRED_DIRECT) + adjudication = json.dumps([{"field": "q2.main_text", "choice": "A", "reason": "B carries the parts too"}]) + backend = FakeBackend(direct, adjudication) + + result = routes.convert( + FIXTURES / "paired.md", + solutions=FIXTURES / "paired_solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + lua=FIXTURES / "pair-filter.lua", + name="paired", + ) + + assert result.adjudicated == 1 + # What FakeBackend records as usage: each prompt it read and each reply it wrote. + assert len(backend.calls) == 2 + assert result.tokens == sum(len(prompt) for _, prompt in backend.calls) + len(direct) + len(adjudication) + + def test_a_folder_with_no_sheet_in_it_names_what_a_folder_run_converts(tmp_path): # A mistyped path and a folder holding solutions alone both pair to # nothing. Neither reaches in2lambda, so this is the only place that can From 25aa4bea21e302d94c02c3064ac2fd6960f4407f Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 11:48:22 +0100 Subject: [PATCH 3/3] implement: Run the corpus sweep through the two-route conversion (t39) --- README.md | 24 +++++--- docs/how-it-works.md | 2 +- in2lambda_agent/sweep.py | 62 ++++++++++++++------ tests/test_sweep.py | 118 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 175 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 30aced5..220e7e2 100644 --- a/README.md +++ b/README.md @@ -370,11 +370,12 @@ all of it. `--suffix` is repeatable and defaults to `tex`, `md` and `docx`; `--s pdf` runs the PDFs too, which needs Mathpix credentials and one call per PDF. A set is a folder holding at least one questions document. Each set converts as a -folder run of `convert` does: one model call writes the set's filter from its first -sheet, and each sheet of the set then runs through route A and that filter. A sheet and -the solutions file beside it are one conversion and one row, named after the questions -file. A folder of figures, and a folder holding a solutions file alone, are not sets and -have no row. +folder run of `convert` does: one model call writes the set's filter from the first +sheet of it pandoc can read, and each sheet of the set then runs through route A and +that filter. A sheet and the solutions file beside it are one conversion and one row, +named after the questions file. A folder of figures, a tex drawing with no +`\begin{document}` among them, and a folder holding a solutions file alone, are not +sets and have no row. The sweep never writes to the corpus. It writes each set's filter to `WORK/SET/filter.lua` and each sheet's set folder and zip to `WORK/SET/SHEET/`, where @@ -391,13 +392,18 @@ tokens, seconds, reason [docs/how-it-works.md](docs/how-it-works.md#the-corpus-table) names each column and where its value comes from. `reason` is empty where the sheet ran through both routes and built its set. One sheet whose conversion raises is one row, with `no set:` and the -error as its reason, and the sheets after it still run. A set whose filter call does not -finish converts every sheet of it through route A alone, and each of those rows reads -`no filter:` and the error. The command exits 1 where a sheet built no set. +error as its reason, and the sheets after it still run. A set with no filter — its +filter call did not finish, or every sheet of it is a PDF, which pandoc cannot read — +converts every sheet through route A alone, and each of those rows reads `no filter:` +and why. A PDF among sheets pandoc reads leaves the rest of the set on both routes: the +filter is written from one of those sheets, and the PDF alone fails route B. The command +exits 1 where a sheet built no set. `--cache` is where the OCR of each PDF is kept. It defaults to `./.in2lambda-agent`, the directory `run` caches into, so a sweep over PDFs that `run` has already converted -makes no Mathpix call and needs no Mathpix credentials. +makes no Mathpix call. It still reads `MATHPIX_APP_ID` and `MATHPIX_API_KEY`: `convert` +builds the Mathpix client before it asks the cache, and refuses a PDF where either +variable is unset, whether or not the cache holds that PDF. ## Gate diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 914501f..1383e0b 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -427,7 +427,7 @@ run from another directory. | `not_verbatim` | how many of the flagged fields are not quotes of the source | | `tokens` | what the sheet's model calls read and wrote, the direct call and the adjudication call; the set's filter call is counted on the set's first sheet | | `seconds` | how long the sheet took, the filter call included on the set's first sheet | -| `reason` | empty where both routes ran and the sheet built its set; `no set: ` where the conversion raised, `route B failed: ` where the set is route A's alone, and `no filter: ` where the set's filter call did not finish | +| `reason` | empty where both routes ran and the sheet built its set; `no set: ` where the conversion raised, `route B failed: ` where the set is route A's alone, and `no filter: ` where no sheet of the set ran route B, because its filter call did not finish or because pandoc reads no sheet of it | A field only one route filled is neither agreed nor adjudicated: the count of those is `fields - agreed - adjudicated`. `fields` counts what the conversion returned, so a diff --git a/in2lambda_agent/sweep.py b/in2lambda_agent/sweep.py index a5f0139..cca24b6 100644 --- a/in2lambda_agent/sweep.py +++ b/in2lambda_agent/sweep.py @@ -2,10 +2,11 @@ A set is a folder holding at least one questions document. Each set is converted as `routes.convert_folder` converts one: a model call writes a Lua -filter from the set's first sheet, and each sheet of the set then runs through -route A, the direct model call, and route B, that filter under pandoc. The -sweep runs the sheets itself rather than calling `convert_folder`, so that it -can time each sheet and write a row for a sheet whose conversion raised. +filter from the first sheet of the set pandoc can read, and each sheet of the +set then runs through route A, the direct model call, and route B, that filter +under pandoc. The sweep runs the sheets itself rather than calling +`convert_folder`, so that it can time each sheet and write a row for a sheet +whose conversion raised. The measures of docs/plan.md are the columns: how many fields the two routes returned, how many they agreed on, how many the adjudication call decided, how @@ -23,7 +24,7 @@ from pathlib import Path from typing import Optional, Sequence -from in2lambda_agent import pair, pipeline, routes +from in2lambda_agent import package, pair, pipeline, routes from in2lambda_agent.model import Backend, choose_backend from in2lambda_agent.settings import Settings @@ -40,6 +41,9 @@ NO_SET = "no set: " """What the reason of a sheet that built no set begins with.""" +NO_FILTER_PDF = "no filter: pandoc reads no sheet of this set" +"""The reason of a set with no filter because every sheet of it is a PDF.""" + @dataclass class Row: @@ -66,8 +70,9 @@ class Row: reason: Empty where both routes ran and the sheet built its set. `no set: ` where the conversion raised and the sheet built nothing, `route B failed: ` where the set is - route A's alone, and `no filter: ` where the set's filter - call did not finish and no sheet of the set ran route B. + route A's alone, and `no filter: ` where no sheet of the set + ran route B, because the filter call did not finish or because + pandoc reads no sheet of the set. """ set: str @@ -106,15 +111,33 @@ def _sheets( `pair.pairs_in` holds a sheet written both as `Sheet_1.tex` and as `Sheet_1.pdf` once, under the suffix it prefers, which is the tex. A sweep of the PDFs alone therefore has no such sheet to run. + + A tex file with no document body is a drawing or a preamble rather than a + sheet, as `package.is_document` reads one, and is left out: converting one + makes two model calls and returns a set of no questions. """ wanted = {"." + one.lower().lstrip(".") for one in suffixes} return [ (sheet, solutions) for sheet, solutions in pair.pairs_in(folder) - if sheet.suffix.lower() in wanted + if sheet.suffix.lower() in wanted and package.is_document(sheet) ] +def _filter_pair( + pairs: Sequence[tuple[Path, Optional[Path]]] +) -> Optional[tuple[Path, Optional[Path]]]: + """The pair a set's filter is written from: the first pandoc can read. + + `routes.write_filter` shows the call pandoc's tree of the document, and + pandoc cannot read a PDF. A set whose first sheet is a PDF would otherwise + have no filter at all, and lose route B on the sheets pandoc does read. + Where every sheet is a PDF there is nothing to write a filter from, and the + set converts through route A alone. + """ + return next((one for one in pairs if one[0].suffix.lower() != ".pdf"), None) + + def sets( root: Path, paths: Sequence[Path] = (), suffixes: Sequence[str] = DEFAULT_SUFFIXES ) -> list[Path]: @@ -208,8 +231,9 @@ def sweep( """Converts every set of a corpus, printing a line per sheet and writing the table. What a sheet raises is that sheet's row, and the sheets after it still run. - What a set's filter call raises is a row of every sheet of that set, each - converted through route A alone. + A set with no filter — its call raised, or pandoc reads no sheet of it — + converts every sheet of itself through route A alone, and says so in each + of its rows. Args: root: The corpus directory. @@ -240,15 +264,19 @@ def sweep( lua: Optional[Path] = None no_filter = "" filter_tokens = 0 + readable = _filter_pair(pairs) started = time.monotonic() - try: - source, usage = routes.write_filter(pairs[0][0], pairs[0][1], backend) - except Exception as problem: - no_filter = f"no filter: {_one_line(str(problem))}" + if readable is None: + no_filter = NO_FILTER_PDF else: - lua = into / "filter.lua" - lua.write_text(source, encoding="utf-8") - filter_tokens = usage.usage.input_tokens + usage.usage.output_tokens + try: + source, usage = routes.write_filter(readable[0], readable[1], backend) + except Exception as problem: + no_filter = f"no filter: {_one_line(str(problem))}" + else: + lua = into / "filter.lua" + lua.write_text(source, encoding="utf-8") + filter_tokens = usage.usage.input_tokens + usage.usage.output_tokens filter_seconds = time.monotonic() - started for number, (sheet, solutions) in enumerate(pairs): diff --git a/tests/test_sweep.py b/tests/test_sweep.py index 83fb45e..a81cf26 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -17,7 +17,7 @@ from conftest import FakeBackend from test_routes import PAIRED_DIRECT, SHEET_DIRECT -from in2lambda_agent import cli, routes, sweep +from in2lambda_agent import cli, ocr, routes, sweep from in2lambda_agent.model import ModelError from in2lambda_agent.settings import Settings @@ -37,6 +37,10 @@ ) pandoc = pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +CREDENTIALS = Settings(mathpix_app_id="id", mathpix_api_key="key") +"""Enough to convert a PDF the cache holds: `markdown_of` builds the client +before it asks the cache, and a client is refused without them.""" + def corpus(root: Path, *names: str) -> Path: """Writes one set folder per name, each holding the three fixture sheets.""" @@ -48,6 +52,18 @@ def corpus(root: Path, *names: str) -> Path: return root +def cached_pdf(folder: Path, name: str, cache: Path, markdown: Path) -> Path: + """A PDF sheet whose OCR the cache already holds, so Mathpix is not called.""" + pdf = folder / name + pdf.write_bytes(b"%PDF-1.4 " + name.encode()) + entry = cache / ocr._hash(pdf) + (entry / ocr.MEDIA_NAME).mkdir(parents=True) + (entry / ocr.SOURCE_NAME).write_text( + markdown.read_text(encoding="utf-8"), encoding="utf-8" + ) + return pdf + + def replies(sets: int = 1) -> list[str]: """The model's answers for one set: the filter, and each sheet's call.""" return [FILTER, json.dumps(PAIRED_DIRECT), ADJUDICATION, json.dumps(SHEET_DIRECT)] * sets @@ -73,6 +89,20 @@ def test_a_folder_with_a_sheet_in_it_is_a_set_and_one_without_is_not(tmp_path): assert sweep.sets(root) == [root / "alpha", root / "beta" / "second"] +def test_a_tex_file_with_no_document_body_is_not_a_sheet(tmp_path): + # A corpus folder of figures holds gnuplot and TikZ sources. Converting one + # buys a filter call, a direct call and a set of no questions. + root = tmp_path / "corpus" + (root / "sheets").mkdir(parents=True) + shutil.copy(FIXTURES / "tex-sheet.tex", root / "sheets") + (root / "figures").mkdir() + (root / "figures" / "plot.tex").write_text( + "\\begingroup\n\\draw (0,0) -- (1,1);\n\\endgroup\n" + ) + + assert sweep.sets(root, suffixes=["tex"]) == [root / "sheets"] + + def test_only_the_named_paths_and_the_named_suffixes_run(tmp_path): root = corpus(tmp_path / "corpus", "alpha", "beta") @@ -148,9 +178,6 @@ def test_a_sweep_writes_one_filter_a_set_and_one_row_a_sheet(tmp_path): assert written_filter == FILTER.strip() assert (tmp_path / "work" / "alpha" / "paired" / "paired.zip").is_file() assert (tmp_path / "work" / "beta" / "sheet" / "sheet.zip").is_file() - # The filter call's tokens are the first sheet's, so each set's first row - # reports more tokens than its second. - assert rows[0].tokens > rows[1].tokens > 0 header, written = table(tmp_path / "results.csv") assert header == list(sweep.COLUMNS) assert [one["sheet"] for one in written] == [row.sheet for row in rows] @@ -158,6 +185,34 @@ def test_a_sweep_writes_one_filter_a_set_and_one_row_a_sheet(tmp_path): assert sorted(path.relative_to(root).as_posix() for path in root.rglob("*")) == before +@pandoc +def test_the_set_filter_call_is_counted_on_its_first_sheet_and_nowhere_else(tmp_path, monkeypatch): + root = corpus(tmp_path / "corpus", "alpha") + backend = FakeBackend(*replies()) + # A clock of the test's own, so the seconds a row reports are known: the + # filter call takes 2, the first sheet 3 and the second 4. + ticks = iter([0, 2, 10, 13, 20, 24]) + monkeypatch.setattr(sweep.time, "monotonic", lambda: next(ticks)) + + rows = sweep.sweep( + root, + results=tmp_path / "results.csv", + work=tmp_path / "work", + settings=Settings(), + backend=backend, + ) + + # What FakeBackend records as usage: each prompt it read and each reply it + # wrote. The set's four calls are the filter, paired.md's direct call, the + # adjudication of the field the two routes read differently, and sheet.md's. + cost = [len(prompt) for _, prompt in backend.calls] + written = [FILTER, json.dumps(PAIRED_DIRECT), ADJUDICATION, json.dumps(SHEET_DIRECT)] + filter_call = cost[0] + len(written[0]) + assert rows[0].tokens == cost[1] + len(written[1]) + cost[2] + len(written[2]) + filter_call + assert rows[1].tokens == cost[3] + len(written[3]) + assert (rows[0].seconds, rows[1].seconds) == (5.0, 4.0) + + @pandoc def test_a_sheet_that_raises_is_a_row_and_the_next_sheet_still_runs(tmp_path): root = corpus(tmp_path / "corpus", "alpha") @@ -210,6 +265,61 @@ def test_a_set_whose_filter_call_fails_converts_every_sheet_through_route_a(tmp_ assert [(row.agreed, row.adjudicated) for row in rows] == [(0, 0), (0, 0)] +@pandoc +def test_the_filter_is_written_from_the_first_sheet_pandoc_can_read(tmp_path): + # pandoc cannot read a PDF, and one PDF at the head of a set would otherwise + # deny route B to every sheet of the set, the ones pandoc reads among them. + root = tmp_path / "corpus" + folder = root / "alpha" + folder.mkdir(parents=True) + for sheet in ("paired.md", "paired_solutions.md"): + shutil.copy(FIXTURES / sheet, folder / sheet) + cached_pdf(folder, "a_sheet.pdf", tmp_path / "cache", FIXTURES / "sheet.md") + backend = FakeBackend( + FILTER, json.dumps(SHEET_DIRECT), json.dumps(PAIRED_DIRECT), ADJUDICATION + ) + + rows = sweep.sweep( + root, + suffixes=["md", "pdf"], + results=tmp_path / "results.csv", + work=tmp_path / "work", + cache=tmp_path / "cache", + settings=CREDENTIALS, + backend=backend, + ) + + # The set's first call is the filter, and it was shown paired.md's blocks. + assert "Tutorial Sheet 3" in backend.calls[0][1] + assert [row.sheet for row in rows] == ["alpha/a_sheet.pdf", "alpha/paired.md"] + # The PDF fails route B on its own; the sheet pandoc reads has its filter. + assert rows[0].built and rows[0].reason.startswith("route B failed: ") + assert rows[1].reason == "" and (rows[1].agreed, rows[1].adjudicated) == (8, 1) + + +def test_a_set_of_pdfs_alone_makes_no_filter_call_and_runs_route_a(tmp_path): + root = tmp_path / "corpus" + folder = root / "alpha" + folder.mkdir(parents=True) + cached_pdf(folder, "only.pdf", tmp_path / "cache", FIXTURES / "sheet.md") + backend = FakeBackend(json.dumps(SHEET_DIRECT)) + + rows = sweep.sweep( + root, + suffixes=["pdf"], + results=tmp_path / "results.csv", + work=tmp_path / "work", + cache=tmp_path / "cache", + settings=CREDENTIALS, + backend=backend, + ) + + # The sheet's direct call and no filter call: pandoc has nothing to read. + assert len(backend.calls) == 1 + assert rows[0].built and rows[0].fields == 10 + assert rows[0].reason == sweep.NO_FILTER_PDF + + @pandoc def test_a_filter_pandoc_refuses_leaves_a_row_built_from_route_a(tmp_path): root = corpus(tmp_path / "corpus", "alpha")