diff --git a/.gitignore b/.gitignore index 497518a..8d879b1 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,9 @@ dist/ corpus-specs/ # 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 +# they are run from. +.in2lambda-agent/ # The gate's CI corpus: xelatex compiles this PDF from ci-corpus/tex/sheet-1.tex # in the workflow, so the repository holds the tex source and not the output. ci-corpus/pdf/*.pdf diff --git a/README.md b/README.md index 0d31e93..74e46a2 100644 --- a/README.md +++ b/README.md @@ -56,12 +56,13 @@ poetry run in2lambda-agent run sheet.pdf In full: ```sh -poetry run in2lambda-agent run SOURCE [--spec FILE] [--review none|sample|per-question] [--rounds N] [--sample N] [--cache DIR] [--fresh-ocr] [--out DIR] +poetry run in2lambda-agent run SOURCE [--spec FILE] [--review none|sample|per-question] [--rounds N] [--tries N] [--sample N] [--cache DIR] [--fresh-ocr] [--out DIR] ``` `SOURCE` is a PDF, markdown, tex or docx file. Mathpix converts a PDF first, and the agent keeps the markdown and the images under the PDF's hash in `--cache` (default -`./.in2lambda-agent`), so a second run over the same PDF makes no Mathpix call. +`./.in2lambda-agent`), so a second run over the same PDF makes no Mathpix call. The +cache also holds the copy of the set's other sheet each spec is run over. `--fresh-ocr` converts the PDF again and restarts the run from the new markdown. `--out` defaults to `./out`, where in2lambda writes the set's JSON folder and its zip. @@ -74,25 +75,46 @@ you name that file or the solutions one. A solutions file with no questions file beside it stops the run, which exits 1 saying `solutions without questions`. The run writes a spec — the YAML selectors naming which blocks of the source are -questions, parts and solutions — in one model call, and saves it as -`in2lambda-spec.yaml` beside `SOURCE`. A folder of sheets is one document set and -shares one spec, so the second sheet in that folder runs with no model call. `--spec` -keeps the set's spec elsewhere; the agent reads that file if it exists and writes it if -it does not. A run appends a line to `in2lambda-agent-runs.jsonl` beside the spec. The -line records the layout, the blocks and the fields of the spec run, the tokens and the -seconds of the model calls, and the commands of each fixing round. A run that stops for -a review appends its line at the last approval. +questions, parts and solutions — and saves it as `in2lambda-spec.yaml` beside `SOURCE`. +A folder of sheets is one document set and shares one spec, so the second sheet in that +folder runs with no model call. `--spec` keeps the set's spec elsewhere; the agent reads +that file if it exists and writes it if it does not. + +Every sheet of the set reuses that spec, so the run writes it in up to `--tries` model +calls, three by default, and keeps the best of them. Each call after the first reads the +spec before it, what running that spec covered, the errors the checks found, and the +blocks that spec left in no field in another document of the folder. The run keeps the +spec that left the fewest blocks unassigned and the fewest errors, and makes no further +call once a spec leaves neither. + +Each spec is run over a copy of that other document, kept in `--cache`, so the run +writes nothing beside the set's own sheets: the draft an earlier run left beside a +sheet, and the fields and the commands in it, stay as that run wrote them. The sheet's +own solutions file is not another document, since it is in this run's draft already. A +PDF beside a PDF source is passed over, because converting it takes a Mathpix call. The +record's `second` names the document the specs were run over, or names the one the run +passed over and says why, and a `set` line says the same. + +A run appends a line to `in2lambda-agent-runs.jsonl` beside the spec. The line records +the layout, the blocks and the fields of the spec run, the tokens and the seconds of the +model calls, one `iterations` entry per spec the run wrote, and the commands of each +fixing round. A run that stops for a review appends its line at the last approval. Each stage prints a line: ``` ocr fresh pass, restarting from /home/me/sheets/.in2lambda-agent/9f2c…/source.md freeze /home/me/sheets/.in2lambda-agent/9f2c…/source.draft.json -spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s -coverage PartsSepSol: 14 blocks, 9 fields at layer 1, 4 ignored, b13 unassigned -validate b13 (lines 21-21) is in no field and not marked ignore. -fix round 1: 1 command (question solution q2), 2604 tokens, 4.1s +spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s (try 1 of 3) +coverage PartsSepSol: 14 blocks, 8 fields at layer 1, 4 ignored, b12, b13 unassigned +validate b12 (lines 19-19) is in no field and not marked ignore.; b13 (lines 21-21) is in no field and not marked ignore. +set sheet-2.md: PartsSepSol: 11 blocks, 7 fields at layer 1, 3 ignored, b9 unassigned +freeze /home/me/sheets/.in2lambda-agent/9f2c…/source.draft.json +spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 2410 tokens, 7.1s (try 2 of 3) +coverage PartsSepSol: 14 blocks, 10 fields at layer 1, 4 ignored, none unassigned validate nothing to report +set sheet-2.md: PartsSepSol: 11 blocks, 8 fields at layer 1, 3 ignored, none unassigned +spec kept try 2 of 3 review not asked for (mode none) build /home/me/sheets/out/set.zip ``` @@ -103,6 +125,7 @@ build /home/me/sheets/out/set.zip | `freeze` | the draft in2lambda wrote from the source | | `spec` | the spec file, with the backend and the tokens where the model wrote it | | `coverage` | the layout, the blocks, the fields per layer, and the blocks in no field | +| `set` | what the spec covered of the set's other document, or why none was run over | | `validate` | what the checks found, or `nothing to report` | | `fix` | the round's number, the commands the model ran, and the tokens | | `render` | the question PDFs a reviewer reads, or why there are none | @@ -134,8 +157,9 @@ that tries to write one ends the run, and the last line names the field and what checks say about it, for a person or a later command to quote the source range into. Where the run reused a saved spec and the checks fault the draft, the run writes the spec again with -the report in the prompt, if `--rounds` is 1 or more: a spec that covers the whole set -repairs every sheet in it. That rewrite is not one of the rounds. +what that spec covered in the prompt, if `--rounds` is 1 or more: a spec that covers the +whole set repairs every sheet in it. That rewrite takes `--tries` calls like any other +spec, and is not one of the rounds. ### Exit codes @@ -251,7 +275,7 @@ 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] [--results FILE] [--work DIR] [--specs DIR] [--cache DIR] +poetry run in2lambda-agent corpus ROOT [PATH ...] [--suffix S] [--replay] [--rounds N] [--tries N] [--results FILE] [--work DIR] [--specs DIR] [--cache DIR] ``` `ROOT` is the corpus directory and each `PATH` a folder under it to run, defaulting to diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 66a177f..95a3ef5 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -6,8 +6,9 @@ writes and every message the stage prints. [README.md](../README.md) gives the commands and their options. The agent makes three kinds of model call and no others: one writes the set's spec, -one rewrites a saved spec the checks fault, and one answers a validation report. -in2lambda performs every other step and every write. +one rewrites a saved spec the checks fault, and one answers a validation report. A run +makes the spec call up to `--tries` times, three by default, and saves one of the specs +it wrote. in2lambda performs every other step and every write. ## The stages @@ -17,18 +18,28 @@ then the message: ``` ocr fresh pass, restarting from /home/me/sheets/.in2lambda-agent/9f2c…/source.md freeze /home/me/sheets/.in2lambda-agent/9f2c…/source.draft.json -spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s -coverage PartsSepSol: 14 blocks, 9 fields at layer 1, 4 ignored, b13 unassigned -validate b13 (lines 21-21) is in no field and not marked ignore. -fix round 1: 1 command (question solution q2), 2604 tokens, 4.1s +spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s (try 1 of 3) +coverage PartsSepSol: 14 blocks, 8 fields at layer 1, 4 ignored, b12, b13 unassigned +validate b12 (lines 19-19) is in no field and not marked ignore.; b13 (lines 21-21) is in no field and not marked ignore. +set sheet-2.md: PartsSepSol: 11 blocks, 7 fields at layer 1, 3 ignored, b9 unassigned +freeze /home/me/sheets/.in2lambda-agent/9f2c…/source.draft.json +spec wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 2410 tokens, 7.1s (try 2 of 3) +coverage PartsSepSol: 14 blocks, 10 fields at layer 1, 4 ignored, none unassigned validate nothing to report +set sheet-2.md: PartsSepSol: 11 blocks, 8 fields at layer 1, 3 ignored, none unassigned +spec kept try 2 of 3 review not asked for (mode none) build /home/me/sheets/out/set.zip ``` -There are nine stage names: `ocr`, `freeze`, `spec`, `coverage`, `validate`, `fix`, -`render`, `review` and `build`. A run prints `validate` once per check and `fix` once -per fixing round, so those two names repeat. +There are ten stage names: `ocr`, `freeze`, `spec`, `coverage`, `validate`, `set`, +`fix`, `render`, `review` and `build`. The spec loop prints `freeze`, `spec`, +`coverage`, `validate` and `set` once per try, and a run prints `validate` once per +check and `fix` once per fixing round, so those names repeat. Each line is printed as +the run makes it, so a `--tries 3` run prints seven lines before its second model +call: `ocr`, the five lines of the first try, and the `freeze` of the second. The run +above made two of its three tries, because the second spec left no block unassigned +and no error behind. | Stage | in2lambda function | What the stage writes | | --- | --- | --- | @@ -36,6 +47,7 @@ per fixing round, so those two names repeat. | `freeze` | `in2lambda.source.add` | `SOURCE.draft.json`, beside the frozen source | | `spec` | `in2lambda.source.show`, for the model's prompt | `in2lambda-spec.yaml`, beside `SOURCE` or at `--spec` | | `coverage` | `in2lambda.draft.execute` with `in2lambda.draft.spec_command` | the layer 1 fields of the draft | +| `set` | `in2lambda.source.add`, then `in2lambda.draft.execute` with `in2lambda.draft.spec_command` | the draft of the copy in `CACHE/second/`, and nothing beside the set's own sheets | | `validate` | `in2lambda.draft.report.validate` | the report inside the draft | | `fix` | `in2lambda.source.show`, then `in2lambda.draft.execute` once per command | the fields and the log of the draft | | `render` | `in2lambda.draft.export.render` | `OUT/render/question_000_Question_1.pdf`, one PDF per question | @@ -75,15 +87,20 @@ run wrote. ### `spec` -The stage prints one of two messages: +The stage prints one of three messages: * `reused /home/me/sheets/in2lambda-spec.yaml` — the spec file exists, and the stage makes no model call. A run that reuses a spec the checks then fault prints this stage a second time in its `wrote` form. -* `wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s` — the - model wrote the spec. The backend is `anthropic`, `openrouter` or `agent-sdk`. The - token count is the call's input and output tokens added together, and the time is - the wall time of the call to one decimal place. +* `wrote /home/me/sheets/in2lambda-spec.yaml via anthropic, 1883 tokens, 6.4s (try 1 of + 3)` — the model wrote the spec. The backend is `anthropic`, `openrouter` or + `agent-sdk`. The token count is the call's input and output tokens added together, + and the time is the wall time of the call to one decimal place. The try number counts + from 1 to `--tries`. +* `kept try 2 of 3` — the loop wrote more than one spec, and this names the try saved + for the set: the one that left the fewest blocks unassigned and the fewest errors, + over this source and over the set's other document. The loop writes one spec where + the first leaves neither, and prints no `kept` line. in2lambda refuses a spec it cannot run, and the run raises `SpecRejected`. A spec this run wrote is deleted before that refusal reaches the user, and a spec this run wrote @@ -105,6 +122,33 @@ stage prints `no fields` where the spec wrote none. `4 ignored` is the number of blocks the spec's `ignore` selector matched. The unassigned blocks are listed by id, and the stage prints `none unassigned` where every block reached a field. +### `set` + +The stage runs the try's spec over another document of the set, so that a spec is +judged on the set it is saved for rather than on this one sheet. The document is the +first other document of the source's folder, by name, whose suffix is the source's. The +run copies it under `CACHE/second/` and freezes that copy, so the draft an earlier run +left beside that sheet stays as that run wrote it. The stage prints one of four +messages: + +* `sheet-2.md: PartsSepSol: 11 blocks, 7 fields at layer 1, 3 ignored, b9 unassigned` — + the spec ran over the other document, in the `coverage` line's own form. The blocks + it left in no field there count toward the try's score, beside the blocks and the + errors this source left. +* `sheet-2.md: in2lambda refused the spec: ERROR` — in2lambda ran the spec over this + source and refused it over the other document. The try scores as leaving every block + of that document in no field. The next try is run over the document again. +* `sheet-2.md cannot be read: ERROR` — in2lambda refuses the document itself, a Word + lock file beside a docx among them. The run continues, judges its tries on this + source alone, and runs no later try over the document. +* `sheet-2.pdf passed over: converting it takes an OCR call, and the spec loop makes no + call but the model's` — the run passed the document over before running a spec over + it. A PDF beside a PDF source is passed over for the reason the message gives, and a + document the run cannot copy is passed over saying so. + +A folder holding one sheet prints no `set` line. The sheet's own solutions file is no +other document of the set: it is a second source of this run's own draft already. + ### `validate` `in2lambda.draft.report.validate` checks the draft and writes its report into the @@ -213,8 +257,8 @@ Each backend limits a call differently: | Call | What it is given | What it may write | | --- | --- | --- | -| Spec | the spec system prompt, and the frozen source as `in2lambda.source.show` prints it | `in2lambda-spec.yaml`, and nothing else | -| Spec rewrite | the same, with the errors of the last report appended to the prompt | `in2lambda-spec.yaml`, and nothing else | +| Spec | the spec system prompt, the frozen source as `in2lambda.source.show` prints it, and, from the second call on, the spec before it, that spec's coverage line, the errors the report holds and the blocks that spec left in no field in the set's other document | `in2lambda-spec.yaml`, and nothing else | +| Spec rewrite | the same, with the saved spec and what running it covered as the first call's try 0 | `in2lambda-spec.yaml`, and nothing else | | Fixing round | the fixing system prompt, the frozen source, every finding of the report, and a reviewer's note where there is one | the eight draft commands, and nothing else | The spec call has no tools. Its reply is the YAML of a spec, past a code fence where @@ -222,9 +266,16 @@ the model wrote one. The agent refuses a reply that is not YAML, a reply that is mapping, and a reply naming a `layout` outside `PartsSepSol`, `PartsOneSol`, `PartSolPartSol` and `PartPartSolSol`. -The spec rewrite is the same call with the report's errors in the prompt. It runs once -per run, before any fixing round, where the run reused a saved spec and the checks -fault the draft. It writes layer 1 fields, and it is not one of the `--rounds`. +A run makes the spec call up to `--tries` times, three by default. Each call after the +first is asked for a spec that leaves fewer blocks unassigned and fewer errors behind +than the one before it, over this source and over the set's other document. The run +makes no further call once a spec leaves no block unassigned and no error behind, and +saves the try that left the fewest of both. + +The spec rewrite is the same loop, with the saved spec and what running it covered as +try 0. It runs where the run reused a saved spec and the checks fault the draft, before +any fixing round, and takes `--tries` calls like any other spec. It writes layer 1 +fields, and it is not one of the `--rounds`. The fixing round's tools are the eight in2lambda draft commands: `mark ignore`, `question add`, `part add`, `question solution`, `part solution`, `field replace`, diff --git a/in2lambda_agent/cli.py b/in2lambda_agent/cli.py index f83bd6b..97a469f 100644 --- a/in2lambda_agent/cli.py +++ b/in2lambda_agent/cli.py @@ -41,6 +41,27 @@ def sample_count(given: str) -> int: return count +def try_count(given: str) -> int: + """How many specs the agent may write, which is at least one. + + Args: + given: What was typed after `--tries`. + + Returns: + The count. + + Raises: + ArgumentTypeError: it is below one. A run that may write no spec has + none to run, and a set with no saved spec has nothing to reuse. + """ + count = int(given) + if count < 1: + raise argparse.ArgumentTypeError( + f"a run writes at least one spec, not {count}" + ) + return count + + def reviewer_name(given: Optional[str]) -> str: """Who the draft's log records an edit as being by. @@ -102,6 +123,12 @@ def build_parser() -> argparse.ArgumentParser: default=3, help="How many times the agent may try to fix validation errors.", ) + run.add_argument( + "--tries", + type=try_count, + default=3, + help="How many specs the agent may write before keeping the best.", + ) run.add_argument( "--sample", type=sample_count, @@ -188,6 +215,12 @@ def build_parser() -> argparse.ArgumentParser: 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( "--results", type=Path, @@ -297,6 +330,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: specs=args.specs, replay=args.replay, rounds=args.rounds, + tries=args.tries, cache=args.cache, settings=load_settings(), ) @@ -390,6 +424,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: spec=args.spec, review=args.review, rounds=args.rounds, + tries=args.tries, sample=args.sample, cache_dir=args.cache, fresh_ocr=args.fresh_ocr, diff --git a/in2lambda_agent/corpus.py b/in2lambda_agent/corpus.py index 881036b..1155edb 100644 --- a/in2lambda_agent/corpus.py +++ b/in2lambda_agent/corpus.py @@ -26,7 +26,7 @@ from in2lambda_agent import package, pair, pipeline from in2lambda_agent.model import Backend, ModelUnavailable -from in2lambda_agent.package import SpecRejected +from in2lambda_agent.package import SpecRejected, is_document from in2lambda_agent.settings import Settings from in2lambda_agent.spec import RECORD_NAME, SPEC_NAME, BadSpec @@ -141,19 +141,6 @@ def call(self, system: str, prompt: str, tools: Sequence = ()) -> None: raise ModelUnavailable(self.unavailable()) -def is_document(path: Path) -> bool: - """Whether a file is a document of its own rather than input to one. - - A tex file with no `\\begin{document}` is a fragment: a TikZ source under a - `figures/` folder, or a preamble a sheet inputs. Frozen and built it is a set - of one question made of a drawing, which is not what the corpus holds it for. - The other suffixes have no such marker, and every file of them is a document. - """ - if path.suffix.lower() != ".tex": - return True - return r"\begin{document}" in path.read_text(encoding="utf-8", errors="replace") - - def documents( root: Path, paths: Sequence[Path] = (), suffixes: Sequence[str] = DEFAULT_SUFFIXES ) -> list[Path]: @@ -252,6 +239,7 @@ def run_one( spec: Path, settings: Settings, rounds: int = 3, + tries: int = 3, replay: bool = False, cache: Path = pipeline.DEFAULT_CACHE_DIR, backend: Optional[Backend] = None, @@ -269,6 +257,7 @@ def run_one( if it is not. settings: The environment the run has available. rounds: The round limit, ignored in a replay, which can run none. + tries: How many specs the run may write before keeping the best. replay: Run the saved spec and nothing else, making no model call. cache: Where the OCR of each PDF is kept. backend: The backend to write a spec with, chosen from the settings if @@ -291,6 +280,7 @@ def run_one( # report: the row then says what the saved spec left rather than # that a call could not be made. rounds=0 if replay else rounds, + tries=tries, cache_dir=cache, backend=NoModel() if replay else backend, ) @@ -369,6 +359,7 @@ def sweep( specs: Path = DEFAULT_SPEC_DIR, replay: bool = False, rounds: int = 3, + tries: int = 3, cache: Path = pipeline.DEFAULT_CACHE_DIR, settings: Optional[Settings] = None, backend: Optional[Backend] = None, @@ -384,6 +375,7 @@ def sweep( specs: The tree the sets' specs are kept in, mirroring the corpus. replay: Run the saved specs and nothing else, making no model call. rounds: The round limit each run is given. + tries: How many specs each run may write before keeping the best. cache: Where the OCR of each PDF is kept, so that a sweep pointed at a cache another run filled converts nothing. settings: The environment the runs have available. @@ -480,6 +472,7 @@ def sweep( spec=spec, settings=settings, rounds=rounds, + tries=tries, replay=replay, cache=cache, backend=backend, diff --git a/in2lambda_agent/package.py b/in2lambda_agent/package.py index b1af232..7842d36 100644 --- a/in2lambda_agent/package.py +++ b/in2lambda_agent/package.py @@ -169,6 +169,39 @@ class Report: findings: list[Finding] = field(default_factory=list) +def is_document(path: Path) -> bool: + """Whether a file is a document of its own rather than input to one. + + A tex file with no `\\begin{document}` is a fragment: a TikZ source under a + `figures/` folder, or a preamble a sheet inputs. Frozen and built it is a set + of one question made of a drawing, which is not what the corpus holds it for. + The other suffixes have no such marker, and every file of them is a document. + """ + if path.suffix.lower() != ".tex": + return True + return r"\begin{document}" in path.read_text(encoding="utf-8", errors="replace") + + +def said(report: Report) -> str: + """The validate line of a report nothing stops: the warnings, or nothing.""" + if not report.warnings: + return "nothing to report" + return "; ".join(report.warnings) + " — warnings, building" + + +def froze(draft: Path, solutions: Optional[str] = None) -> str: + """The freeze line of a draft: the file, and what else was frozen into it. + + Args: + draft: The draft that was written. + solutions: The name of the solutions document, where the draft holds + one as its second source. + """ + if not solutions: + return str(draft) + return f"{draft}, with {solutions} as source 2" + + def source_add(source: Path, *more: Path) -> Path: """Freezes one or more source documents into the draft beside the first. @@ -237,9 +270,7 @@ def spec_run(draft: Path, spec: Path) -> Coverage: raise SpecRejected(str(error)) from None found = _frozen(draft) - coverage = Coverage( - layout=layout, blocks=sum(len(one["blocks"]) for one in found["sources"]) - ) + coverage = Coverage(layout=layout, blocks=blocks(draft)) for key, written in found["fields"].items(): if key.endswith(".ignore"): coverage.ignored += 1 @@ -252,6 +283,22 @@ def spec_run(draft: Path, spec: Path) -> Coverage: return coverage +def blocks(draft: Path) -> int: + """How many blocks a draft's frozen source has. + + A spec run reports this count in its coverage. in2lambda runs no spec it + refuses, so a caller reporting how many blocks a refused spec left in no + field counts every block of the source. + + Args: + draft: The draft file. + + Returns: + The count. + """ + return sum(len(one["blocks"]) for one in _frozen(draft)["sources"]) + + def _frozen(draft: Path) -> dict[str, Any]: """A draft read off disk, as in2lambda writes one.""" return json.loads(Path(draft).read_text()) diff --git a/in2lambda_agent/pipeline.py b/in2lambda_agent/pipeline.py index 5440952..74c499e 100644 --- a/in2lambda_agent/pipeline.py +++ b/in2lambda_agent/pipeline.py @@ -1,8 +1,8 @@ """The run: source in, Lambda Feedback zip out. The stages are the design spec's pipeline. The agent acts at three of them — the -OCR pass, the one model call that writes the set's spec, and the rounds that -answer what the checks found — and in2lambda does the rest: freezing the source, +OCR pass, the model calls that write the set's spec, and the rounds that answer +what the checks found — and in2lambda does the rest: freezing the source, running the spec over it, checking the draft and writing the zip. A spec that covers its source is layer 1 and builds with nothing more asked of @@ -13,10 +13,11 @@ rather than using the limit up: a finding no range of the source answers — a part whose solution is not on the sheet — is reported, not invented, and the next round would be the same prompt over the same report. A spec saved from an -earlier sheet gets one rewrite before -any of that, since a spec that covers the set is worth more than a field +earlier sheet is written again before any of that where the checks fault the +draft it filled, since a spec that covers the set is worth more than a field repaired in one sheet of it; that rewrite is layer 1, and is not one of the -rounds. +rounds. `spec.iterate_spec` is what writes a spec, in up to `tries` calls, and +what the run goes on with is the one of them that covered the set best. A run asked for a review stops once the checks are quiet: it renders the questions the reviewer is to see, leaves a record of them in the cache, and @@ -39,11 +40,23 @@ from in2lambda_agent.ocr import MEDIA_NAME, cached, ocr_pdf from in2lambda_agent.review import RECORD, Question, Review, choose from in2lambda_agent.settings import Settings -from in2lambda_agent.spec import RECORD_NAME, record_run, spec_path, write_spec +from in2lambda_agent.spec import ( + RECORD_NAME, + Previous, + Second, + SpecTry, + iterate_spec, + record_run, + spec_path, +) # Where the OCR of each PDF is kept, under the directory the user ran from. DEFAULT_CACHE_DIR = Path(".in2lambda-agent") +# Where the copy of the set's other document, which each candidate spec is run +# over, is kept under the cache directory. +SECOND_NAME = "second" + # A markdown image whose file is in the OCR's media folder, as far as the folder # name: `![a plot](media/plot.png)`. What a second source's images are renamed # by when they are copied beside the first source's. @@ -85,6 +98,8 @@ class RunResult: zip_path: Optional[Path] = None coverage: Optional[package.Coverage] = None usage: Usage = field(default_factory=Usage) + tries: list[SpecTry] = field(default_factory=list) + second: Optional[Second] = None rounds: list[RoundResult] = field(default_factory=list) review: Optional[Review] = None draft: Optional[Path] = None @@ -116,6 +131,7 @@ def run( spec: Optional[Path] = None, review: str = "none", rounds: int = 3, + tries: int = 3, sample: int = 3, cache_dir: Path = DEFAULT_CACHE_DIR, fresh_ocr: bool = False, @@ -135,6 +151,8 @@ def run( review: One of REVIEW_MODES. rounds: The round limit, N in the design spec: how many model calls may answer what the checks found before the run stops without a zip. + tries: How many specs may be written before the best of them is saved + for the set. sample: How many questions a review in sample mode shows. cache_dir: Where the OCR of each PDF is kept, and where a review that is waiting to be answered is left. @@ -208,75 +226,62 @@ def run( ) result.add_stage("ocr", message) - # One pass, or two where a saved spec leaves something for the checks to - # find: the second writes the spec again with the report in the prompt. + # The saved spec's own pass, where the set has one. A spec the checks fault + # is written again, and what that pass covered is what the first call is + # asked to improve on. reused = saved.is_file() - report = package.Report(clean=False, errors=[]) - while True: + previous = None + if reused: draft = result.draft = package.source_add( frozen, *([frozen_solutions] if frozen_solutions is not None else []) ) result.add_stage( "freeze", - f"{draft}, with {solutions.name} as source 2" - if solutions is not None - else str(draft), + package.froze(draft, solutions.name if solutions is not None else None), ) - - # What the set's spec said before this pass wrote over it, where it - # said anything: a rewrite in2lambda then refuses puts it back. - replaced = None - if reused: - result.add_stage("spec", f"reused {saved}") - else: - backend = backend or choose_backend(settings) - if (reason := backend.unavailable()) is not None: - raise ModelUnavailable(reason) - text, reply = write_spec( - package.source_show(draft), - backend, - report if report.errors else None, - sources=2 if frozen_solutions is not None else 1, + result.add_stage("spec", f"reused {saved}") + result.coverage = package.spec_run(draft, saved) + result.add_stage("coverage", str(result.coverage)) + report = package.validate(draft) + if report.clean or rounds < 1: + result.add_stage( + "validate", + package.said(report) if report.clean else "; ".join(report.errors), ) - if saved.is_file(): - replaced = saved.read_text(encoding="utf-8") - saved.write_text(text, encoding="utf-8") - result.usage.input_tokens += reply.usage.input_tokens - result.usage.output_tokens += reply.usage.output_tokens - result.usage.seconds += reply.usage.seconds - tokens = reply.usage.input_tokens + reply.usage.output_tokens + else: result.add_stage( - "spec", - f"wrote {saved} via {reply.backend}, {tokens} tokens, " - f"{reply.usage.seconds:.1f}s", + "validate", + "; ".join(report.errors) + " — writing the set's spec again", + ) + previous = Previous( + text=saved.read_text(encoding="utf-8"), + coverage=result.coverage, + report=report, ) - - try: - result.coverage = package.spec_run(draft, saved) - except package.SpecRejected: - # A spec is only kept once in2lambda has run it. One it refuses, - # left beside the sources, is read by every later run over the set - # — which then makes no call, and fails in the same place, until - # someone deletes the file by hand. - if not reused: - if replaced is None: - saved.unlink() - else: - saved.write_text(replaced, encoding="utf-8") - raise - result.add_stage("coverage", str(result.coverage)) - - report = package.validate(draft) - if report.clean: - result.add_stage("validate", _said(report)) - break - errors = "; ".join(report.errors) - if reused and rounds >= 1: - result.add_stage("validate", f"{errors} — writing the set's spec again") reused = False - continue - result.add_stage("validate", errors) - break + + if not reused: + backend = backend or choose_backend(settings) + if (reason := backend.unavailable()) is not None: + raise ModelUnavailable(reason) + result.second = _second(source, cache_dir, solutions) + draft, coverage, report, result.tries = iterate_spec( + frozen, + saved, + backend, + tries=tries, + on_stage=result.add_stage, + second=result.second, + previous=previous, + solutions=frozen_solutions, + solutions_name=solutions.name if solutions is not None else "", + ) + result.draft = draft + result.coverage = coverage + for one in result.tries: + result.usage.input_tokens += one.usage.input_tokens + result.usage.output_tokens += one.usage.output_tokens + result.usage.seconds += one.usage.seconds # Layers 3 and 4, a round at a time. Reached only with a spec this run # wrote, so the backend is the one that wrote it. @@ -315,6 +320,8 @@ def run( reused=reused, coverage=result.coverage, usage=result.usage, + tries=result.tries, + second=result.second, rounds=result.rounds, ) infos = package.questions(draft) @@ -343,6 +350,8 @@ def run( reused=reused, coverage=result.coverage, usage=result.usage, + tries=result.tries, + second=result.second, rounds=result.rounds, ) return result @@ -398,6 +407,8 @@ def resume( result = RunResult( coverage=waiting.coverage, usage=waiting.usage, + tries=waiting.tries, + second=waiting.second, rounds=waiting.rounds, review=waiting, draft=draft, @@ -418,7 +429,8 @@ def resume( waiting.errors = report.errors result.clean = report.clean result.add_stage( - "validate", _said(report) if report.clean else "; ".join(report.errors) + "validate", + package.said(report) if report.clean else "; ".join(report.errors), ) if report.clean: _build(draft, Path(waiting.out_dir), result, report.warnings) @@ -432,6 +444,8 @@ def resume( reused=waiting.reused, coverage=waiting.coverage, usage=waiting.usage, + tries=waiting.tries, + second=waiting.second, rounds=waiting.rounds, review=waiting.to_json(), ) @@ -481,7 +495,8 @@ def resume( result.add_stage("review", f"{by} edited {field}") report = package.validate(draft) result.add_stage( - "validate", _said(report) if report.clean else "; ".join(report.errors) + "validate", + package.said(report) if report.clean else "; ".join(report.errors), ) edited = field.split(".")[0] relisted = [edited] if any( @@ -633,7 +648,7 @@ def _fix_rounds( RoundResult(number, reply.calls, reply.usage, len(report.errors)) ) if report.clean: - result.add_stage("validate", _said(report)) + result.add_stage("validate", package.said(report)) else: errors = "; ".join(report.errors) # A finding the round answered by writing a field rather than by @@ -705,6 +720,63 @@ def _build(draft: Path, out_dir: Path, result: RunResult, said: list[str]) -> No result.add_stage("build", str(result.zip_path)) +def _second( + source: Path, cache_dir: Path, solutions: Optional[Path] = None +) -> Optional[Second]: + """Another document of the set, which each candidate spec is also run over. + + The spec is saved for the whole folder, so one that covers the sheet in hand + and covers no other sheet of the set is not the spec to save. + + Each spec is run over a copy under `cache_dir`, never over the document in + the folder: running a spec over a sheet freezes it, and freezing writes that + sheet's draft again from the source, which deletes the fields a fixing round + or a reviewer wrote there and the log of the commands that wrote them. A tex + file that inputs files beside it does not find them beside the copy; where + in2lambda refuses it for that, the spec loop reports it as a document + in2lambda cannot read and judges the specs on this source. + + A PDF sibling is passed over rather than copied: converting it takes an OCR + call, and the spec loop makes no call but the model's. + + Args: + source: The file the user asked to convert, whose folder is the set. + cache_dir: Where the copy each spec is run over is written. + solutions: The solutions document of this sheet, which is in this run's + own draft and so is no other document of the set. + + Returns: + The first other document of the folder, by name, as the copy to run the + specs over or as the document the run passed over, and None where the + folder holds no other document. + """ + source = Path(source).resolve() + solutions = None if solutions is None else Path(solutions).resolve() + for path in sorted(source.parent.glob(f"*{source.suffix}")): + if path in (source, solutions) or not path.is_file(): + continue + if not package.is_document(path): + continue + if source.suffix.lower() == ".pdf": + return Second( + path.name, + passed_over="converting it takes an OCR call, and the spec " + "loop makes no call but the model's", + ) + copy = Path(cache_dir) / SECOND_NAME / path.name + try: + copy.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(path, copy) + except OSError as error: + # A sheet the OS will not let the run read, or a cache directory it + # will not let the run write. The other document is evidence about a + # spec and not the source the run converts, so the run goes on and + # says what it passed over, as it does for one in2lambda cannot read. + return Second(path.name, passed_over=f"copying it failed: {error}") + return Second(path.name, path=copy) + return None + + def _stuck(field: str, report: package.Report) -> str: """One field the rounds cannot repair, with what the report says about it.""" said = [one.message for one in report.findings if one.field == field] @@ -713,13 +785,6 @@ def _stuck(field: str, report: package.Report) -> str: return f"{field}: {'; '.join(said)}" -def _said(report: package.Report) -> str: - """The validate line of a report nothing stops: the warnings, or nothing.""" - if not report.warnings: - return "nothing to report" - return "; ".join(report.warnings) + " — warnings, building" - - def _render(draft: Path, out_dir: Path) -> tuple[dict[str, Path], str]: """Renders the draft's questions, or says why there are no pages to show.""" try: diff --git a/in2lambda_agent/review.py b/in2lambda_agent/review.py index 26ab6af..89a2b88 100644 --- a/in2lambda_agent/review.py +++ b/in2lambda_agent/review.py @@ -21,6 +21,7 @@ from in2lambda_agent.fix import RoundResult from in2lambda_agent.model import ToolCall, Usage from in2lambda_agent.package import Coverage, QuestionInfo +from in2lambda_agent.spec import Second, SpecTry RECORD = "review.json" """What the pending review is called, in the run's cache directory.""" @@ -72,6 +73,9 @@ class Review: rejections: Every rejection, in the order they were made. edits: Every field the reviewer changed by hand, and who they were. usage: What the run's model calls have cost so far. + tries: What each spec the run wrote covered and cost, for the run record. + second: The other document of the set the specs were run over, or the + one the run passed over, for the run record. rounds: What each fixing round has done so far, the reviewer's among them. """ @@ -90,6 +94,8 @@ class Review: rejections: list[dict[str, Any]] = field(default_factory=list) edits: list[dict[str, Any]] = field(default_factory=list) usage: Usage = field(default_factory=Usage) + tries: list[SpecTry] = field(default_factory=list) + second: Optional[Second] = None rounds: list[RoundResult] = field(default_factory=list) @property @@ -152,6 +158,11 @@ def save(self, path: Path) -> None: Path(path).parent.mkdir(parents=True, exist_ok=True) state = { **asdict(self), + "tries": [_try_json(one) for one in self.tries], + # `asdict` writes the copy the specs were run over as a Path, which + # json refuses, and the copy is of no use to the command that + # answers the review. + "second": None if self.second is None else self.second.to_json(), "rounds": [_round_json(one) for one in self.rounds], } Path(path).write_text(json.dumps(state, indent=2), encoding="utf-8") @@ -194,6 +205,12 @@ def load(cls, path: Path) -> "Review": ), "questions": [Question(**one) for one in state["questions"]], "usage": Usage(**state["usage"]), + "tries": [_try_from(one) for one in state["tries"]], + "second": ( + None + if state["second"] is None + else Second.from_json(state["second"]) + ), "rounds": [_round_from(one) for one in state["rounds"]], } ) @@ -243,6 +260,30 @@ def _lines(ranges: Sequence[Sequence[int]]) -> str: return ", ".join(f"{start}-{end}" for start, end in ranges) +def _try_json(one: SpecTry) -> dict[str, Any]: + """One spec the run wrote, as the record keeps it between the two commands.""" + return { + "number": one.number, + "usage": asdict(one.usage), + "unassigned": one.unassigned, + "errors": one.errors, + "second": one.second, + "chosen": one.chosen, + } + + +def _try_from(saved: dict[str, Any]) -> SpecTry: + """One spec the run wrote, back out of the record for the run's own record.""" + return SpecTry( + number=saved["number"], + usage=Usage(**saved["usage"]), + unassigned=saved["unassigned"], + errors=saved["errors"], + second=saved["second"], + chosen=saved["chosen"], + ) + + def _round_json(one: RoundResult) -> dict[str, Any]: """One fixing round, as the record keeps it between the two commands.""" return { diff --git a/in2lambda_agent/spec.py b/in2lambda_agent/spec.py index 2269fd2..adf9bc7 100644 --- a/in2lambda_agent/spec.py +++ b/in2lambda_agent/spec.py @@ -1,4 +1,4 @@ -"""Layer 1: the YAML spec, written once per document set by one model call. +"""Layer 1: the YAML spec, written once per document set and iterated into shape. A spec is selectors over the frozen source saying which blocks are questions, which are parts and which are solutions, what to strip off the front of a field @@ -10,14 +10,22 @@ shares, because a document set is a folder of sheets written the same way: the next one runs the saved spec with no model call. `--spec` names another file, which is read if it is there and written if it is not. + +A spec is the one piece of model output every sheet of a set reuses, so it is +written against what running it covers rather than blind. `iterate_spec` writes +one, runs it over this source and over another document of the set, reads the +coverage and the validation report back to the next call, and saves the spec +that left the fewest blocks unassigned and the fewest errors behind. """ import json +from dataclasses import dataclass, field from pathlib import Path -from typing import Optional, Sequence +from typing import Callable, Optional, Sequence import yaml +from in2lambda_agent import package from in2lambda_agent.fix import RoundResult from in2lambda_agent.model import Backend, Reply, Usage from in2lambda_agent.package import Coverage, Report @@ -109,7 +117,13 @@ Every block of the source must end up in a field or be ignored: a block left over is reported, and the run stops. Write selectors that account for all of -them.\ +them. + +You may be shown the spec you last wrote, what running it covered and what the +checks found in the draft it filled, and asked for a better one. The spec is +saved for the whole set, so a document of the set other than this one is shown +as well where the folder holds one. Change the selectors that left blocks over +and keep the ones that did not.\ """ @@ -117,6 +131,96 @@ class BadSpec(ValueError): """What the model answered with is not a spec.""" +@dataclass +class SpecTry: + """One spec the agent wrote, and what running it made of the set. + + Attributes: + number: 0 for the saved spec a rewrite starts from, then 1 for the first + call, 2 for the second. + usage: What the call cost, all zeroes for try 0. + unassigned: How many blocks the spec left in no field and not ignored. + errors: How many errors the checks then found in the draft it filled. + second: How many blocks the spec left in no field in another document + of the set, or None where the run ran no spec over another document + — the record's `second` says why. A spec in2lambda refuses over + that document wrote no field there, so it left every block of it in + no field. + chosen: Whether this is the spec the run saved and went on with. + """ + + number: int + usage: Usage = field(default_factory=Usage) + unassigned: int = 0 + errors: int = 0 + second: Optional[int] = None + chosen: bool = False + + @property + def score(self) -> int: + """What the tries are ranked by, the lowest winning. + + A block in no field is an error of the report as well as a line of the + coverage, so it counts twice. That is the same double for every try and + does not change the order they come in. + """ + return self.unassigned + self.errors + (self.second or 0) + + +@dataclass +class Second: + """Another document of the set, which each candidate spec is also run over. + + Attributes: + name: The file name of that document, which the `set` stage line and the + run record name. + path: The copy of that document each spec is run over, or None where + in2lambda can run no spec over the document. + passed_over: Why in2lambda ran no spec over the document: the document + is a PDF, the run cannot copy it, in2lambda cannot read the copy, + or in2lambda refused the spec the run kept. None where in2lambda + ran that spec over the document. A spec in2lambda refused there and + the run then threw away is that try's alone and is not here. + """ + + name: str + path: Optional[Path] = None + passed_over: Optional[str] = None + + def to_json(self) -> dict[str, Optional[str]]: + """The document as the run record and a waiting review keep it.""" + return {"name": self.name, "passed_over": self.passed_over} + + @staticmethod + def from_json(saved: dict) -> "Second": + """The document back out of a waiting review, for the run record. + + The copy the specs were run over is not kept: the review is answered + after the spec loop has ended, and no spec is run again. + """ + return Second(name=saved["name"], passed_over=saved["passed_over"]) + + +@dataclass +class Previous: + """A spec that has been run, as the call revising it is shown it. + + Attributes: + text: The spec itself. + coverage: What running it made of this source. + report: What the checks found in the draft it filled. + second: What running it made of another document of the set, or None + where the folder holds no other document. + second_name: That document's file name. + """ + + text: str + coverage: Optional[Coverage] = None + report: Optional[Report] = None + second: Optional[Coverage] = None + second_name: str = "" + + def spec_path(source: Path, spec: Optional[Path] = None) -> Path: """Where this source's set keeps its spec. @@ -136,7 +240,7 @@ def spec_path(source: Path, spec: Optional[Path] = None) -> Path: def write_spec( shown: str, backend: Backend, - report: Optional[Report] = None, + previous: Optional[Previous] = None, sources: int = 1, ) -> tuple[str, Reply]: """Writes a spec for a source, in one model call with no tools. @@ -144,8 +248,8 @@ def write_spec( Args: shown: The numbered source with block ids, as `source show` prints it. backend: The backend to call, already known to be available. - report: What the checks found about the draft a previous spec made, - where this is the rewrite that follows a dirty validate. + previous: The spec run before this call and what running it covered, + where this call is a revision of that spec. sources: How many documents the draft holds: 2 where the solutions are a file of their own, which the prompt then says before the source. @@ -164,17 +268,245 @@ def write_spec( "onwards. Every solution is in the second.\n\n" ) prompt += f"Here is the source, one line each with its block id:\n\n{shown}\n" - if report is not None: - prompt += ( - "\nA previous spec for this set left the draft with this to answer " - "for. Write a spec that does not:\n\n" + "\n".join(report.errors) + "\n" - ) + if previous is not None: + prompt += _revision(previous) reply = backend.call(SYSTEM, prompt) text = _unfenced(reply.text) _check(text) return text, reply +def _revision(previous: Previous) -> str: + """The last spec and what running it covered, as the next call is shown them.""" + said = [f"\nYour last spec for this set was:\n\n{previous.text}"] + if previous.coverage is not None: + said.append(f"\nRunning it over this source covered:\n\n{previous.coverage}\n") + if previous.report is not None and previous.report.errors: + said.append( + "\nThe checks then found:\n\n" + "\n".join(previous.report.errors) + "\n" + ) + if previous.second is not None: + left = ", ".join(previous.second.unassigned) or "no blocks" + said.append( + f"\nRunning it over {previous.second_name}, another document of this " + f"set, left {left} in no field.\n" + ) + said.append( + "\nWrite a spec that leaves fewer blocks unassigned and fewer errors " + "behind, over this source and over the rest of the set.\n" + ) + return "".join(said) + + +def iterate_spec( + frozen: Path, + saved: Path, + backend: Backend, + *, + tries: int, + on_stage: Callable[[str, str], None], + second: Optional[Second] = None, + previous: Optional[Previous] = None, + solutions: Optional[Path] = None, + solutions_name: str = "", +) -> tuple[Path, Coverage, Report, list[SpecTry]]: + """Writes the set's spec up to `tries` times and saves the best of them. + + Each call after the first is shown the spec before it, the coverage line, + the errors the checks found and the blocks the spec left over in another + document of the set. The loop stops at a spec that leaves no block + unassigned and no error behind, since a further call has nothing to improve. + + Args: + frozen: The markdown, tex or docx file each spec is run over. + saved: The set's spec file, which every try writes and the chosen spec + is left in. + backend: The backend to call, already known to be available. + tries: How many specs may be written. + on_stage: Called with the `(stage, message)` of each line as the loop + makes it, so that the caller prints a line while the loop is still + running. It is a function of two strings rather than + `RunResult.add_stage` itself, because `pipeline` imports `spec`. + second: Another document of the set, run to say whether a spec covers + the set rather than this one sheet of it, or one the run passed + over, which the `set` line and the record name. A document + in2lambda cannot read becomes a document passed over. A document + in2lambda reads and refuses the spec over stays, and the next try + is run over that document as well. + previous: The saved spec and what running it covered, where this loop + is the rewrite of a spec the checks faulted. The saved spec is run + over the other document first, so that it is recorded as try 0 and + the first call reads what it left there; the spec it names is being + replaced, so it is not one of the tries chosen from. + solutions: The markdown of the solutions document, frozen into the same + draft as the second source, where the sheet has one. + solutions_name: That document's file name, which the freeze line names. + + Returns: + The draft the chosen spec filled, what that spec covered, what the + checks found in the draft, and what each try did. Each line the loop + prints went to `on_stage` as the loop made it. + + Raises: + BadSpec: what the model answered with is not a spec. + SpecRejected: in2lambda will not run a spec this loop wrote. + SourceError: in2lambda cannot freeze or check this source. + + Every error leaving this function puts the spec the set had before the loop + back, since the spec of a try the loop never chose is not one to save. + """ + made: list[SpecTry] = [] + if second is not None and second.passed_over is not None: + on_stage("set", f"{second.name} passed over: {second.passed_over}") + if previous is not None: + # The saved spec is still the file on disk, so running it over the other + # document says what it left there. Try 0 records that, and the first + # call is asked to improve on the set rather than on this sheet alone. + over_second, left_over = _over_second(second, saved, on_stage) + previous.second = over_second + previous.second_name = second.name if over_second is not None else "" + made.append( + SpecTry( + number=0, + unassigned=len(previous.coverage.unassigned), + errors=len(previous.report.errors), + second=left_over, + ) + ) + # What the set's spec said before this loop wrote over it. Every try writes + # the file, and no try has been chosen until the loop ends, so a loop that + # raises puts the old spec back: a try's spec left beside the sources is + # read by every later run over the set, which then makes no call, until + # someone deletes the file by hand. + replaced = saved.read_text(encoding="utf-8") if saved.is_file() else None + + # Each try's spec, and what the other document was left as after that try + # ran over it: the record is to say what became of the other document under + # the spec the loop kept, not under a later try it threw away. + best: Optional[tuple[SpecTry, str, Optional[str]]] = None + more = [solutions] if solutions is not None else [] + try: + for number in range(1, tries + 1): + draft = package.source_add(frozen, *more) + on_stage("freeze", package.froze(draft, solutions_name)) + text, reply = write_spec( + package.source_show(draft), + backend, + previous, + sources=2 if solutions is not None else 1, + ) + saved.write_text(text, encoding="utf-8") + tokens = reply.usage.input_tokens + reply.usage.output_tokens + on_stage( + "spec", + f"wrote {saved} via {reply.backend}, {tokens} tokens, " + f"{reply.usage.seconds:.1f}s (try {number} of {tries})", + ) + coverage, report = _run(draft, saved, on_stage) + over_second, left_over = _over_second(second, saved, on_stage) + one = SpecTry( + number=number, + usage=reply.usage, + unassigned=len(coverage.unassigned), + errors=len(report.errors), + second=left_over, + ) + made.append(one) + if best is None or one.score < best[0].score: + best = (one, text, second.passed_over if second is not None else None) + if one.score == 0: + break + previous = Previous( + text=text, + coverage=coverage, + report=report, + second=over_second, + second_name=second.name if over_second is not None else "", + ) + except Exception: + if replaced is None: + saved.unlink(missing_ok=True) + else: + saved.write_text(replaced, encoding="utf-8") + raise + + chosen, text, passed_over = best + chosen.chosen = True + if second is not None and second.path is not None: + # A document in2lambda could not read is passed over for good, whatever + # try found that out. A spec it refused there is that try's alone: the + # record is to say what became of the document under the spec the loop + # kept. + second.passed_over = passed_over + if len([one for one in made if one.number]) > 1: + on_stage("spec", f"kept try {chosen.number} of {tries}") + if chosen.number != made[-1].number: + # A later try covered the set less well, so the chosen spec is written + # and run again: the draft the run goes on with is the one that spec + # filled, not the one the last try left. + saved.write_text(text, encoding="utf-8") + draft = package.source_add(frozen, *more) + on_stage("freeze", package.froze(draft, solutions_name)) + coverage, report = _run(draft, saved, on_stage) + return draft, coverage, report, made + + +def _over_second( + second: Optional[Second], saved: Path, on_stage: Callable[[str, str], None] +) -> tuple[Optional[Coverage], Optional[int]]: + """Runs the spec now in `saved` over the set's other document. + + Returns: + The coverage the spec made of the other document, and how many blocks + it left in no field there. Both None where the run has no other + document to run a spec over. Where in2lambda refuses the spec over the + other document, there is no coverage to show the next call, and the + count is every block of that document. + """ + if second is None or second.path is None: + return None, None + try: + draft = package.source_add(second.path) + except package.SourceError as error: + # A folder holds files that are not documents — a Word lock file beside + # a docx — and in2lambda refuses them. in2lambda also refuses a tex + # file that inputs the figures beside it, since the copy in the cache + # has no figures beside it. The other document is evidence about a + # spec and not the source the run converts, so the run continues and + # judges the tries on this source. The next try runs over no other + # document, and the record names the document the run passed over. + second.passed_over = str(error) + second.path = None + on_stage("set", f"{second.name} cannot be read: {error}") + return None, None + try: + coverage = package.spec_run(draft, saved) + except package.SpecRejected as error: + # in2lambda runs this spec over this source and refuses it over the + # other document. The next spec the run writes may run over both + # documents, so the copy stays and the next try is run over the other + # document as well. + second.passed_over = f"in2lambda refused the spec: {error}" + on_stage("set", f"{second.name}: {second.passed_over}") + return None, package.blocks(draft) + second.passed_over = None + on_stage("set", f"{second.name}: {coverage}") + return coverage, len(coverage.unassigned) + + +def _run( + draft: Path, saved: Path, on_stage: Callable[[str, str], None] +) -> tuple[Coverage, Report]: + """Runs one spec over one draft and reports the two lines it prints.""" + coverage = package.spec_run(draft, saved) + on_stage("coverage", str(coverage)) + report = package.validate(draft) + on_stage( + "validate", package.said(report) if report.clean else "; ".join(report.errors) + ) + return coverage, report + + def record_run( path: Path, source: Path, @@ -182,6 +514,8 @@ def record_run( reused: bool, coverage: Coverage, usage: Usage, + tries: Sequence[SpecTry] = (), + second: Optional[Second] = None, rounds: Sequence[RoundResult] = (), review: Optional[dict] = None, ) -> None: @@ -198,6 +532,13 @@ def record_run( reused: Whether the spec was the saved one rather than a new call. coverage: What the spec run made of the source. usage: What the run's model calls cost, all zeroes where there were none. + tries: What each spec the run wrote covered and cost, in order, and + empty where the run reused the set's saved spec. + second: The other document of the set each spec was run over, and why + in2lambda ran no spec over the document where it ran none. None + where the folder holds no other document, and where the run reused + the saved spec and ran no loop, which `reused` on the same line + says. rounds: What each round of fixing did, in order, and empty where the draft came clean out of the spec alone. review: What a reviewer made of the set, as `Review.to_json` says it, @@ -216,6 +557,20 @@ def record_run( "input_tokens": usage.input_tokens, "output_tokens": usage.output_tokens, "seconds": round(usage.seconds, 3), + "iterations": [ + { + "try": one.number, + "input_tokens": one.usage.input_tokens, + "output_tokens": one.usage.output_tokens, + "seconds": round(one.usage.seconds, 3), + "unassigned": one.unassigned, + "errors": one.errors, + "second": one.second, + "chosen": one.chosen, + } + for one in tries + ], + "second": None if second is None else second.to_json(), "rounds": [ { "round": one.number, diff --git a/tests/test_cli.py b/tests/test_cli.py index 77805be..ae52898 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,6 +25,7 @@ def test_defaults(): assert args.cache == Path(".in2lambda-agent") assert args.fresh_ocr is False assert args.sample == 3 + assert args.tries == 3 def test_every_option(): @@ -38,6 +39,8 @@ def test_every_option(): "per-question", "--rounds", "5", + "--tries", + "2", "--sample", "2", "--cache", @@ -55,6 +58,7 @@ def test_every_option(): assert args.cache == Path("cached") assert args.fresh_ocr is True assert args.sample == 2 + assert args.tries == 2 @pytest.mark.parametrize("mode", ["none", "sample", "per-question"]) @@ -88,6 +92,7 @@ def test_corpus_defaults(): 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") @@ -108,6 +113,8 @@ def test_corpus_every_option(): "--replay", "--rounds", "1", + "--tries", + "1", "--results", "sweep.csv", "--work", @@ -123,6 +130,7 @@ def test_corpus_every_option(): 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") @@ -408,6 +416,40 @@ def record(source, **given): assert called["source"] == Path("sheet.md") +def test_how_many_specs_may_be_written_reaches_the_run(monkeypatch, tmp_path): + given = {} + + def record(source, **passed): + given.update(passed) + return pipeline.RunResult(zip_path=tmp_path / "set.zip") + + monkeypatch.setattr(pipeline, "run", record) + + assert main(["run", "sheet.md", "--tries", "5"]) == 0 + 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): + build_parser().parse_args(["run", "s.md", "--tries", count]) + + assert "at least one spec" in capsys.readouterr().err + + def test_a_verdict_is_required(): with pytest.raises(SystemExit): build_parser().parse_args(["review"]) diff --git a/tests/test_corpus.py b/tests/test_corpus.py index 2124397..1e9c9b4 100644 --- a/tests/test_corpus.py +++ b/tests/test_corpus.py @@ -180,7 +180,7 @@ def test_the_rounds_a_document_took_are_counted_by_layer(tmp_path): root = tmp_path / "corpus" make_set(root, "faulty", ["faulty.md"]) - (row,) = sweep(root, tmp_path, backend=FakeBackend(FAULTY_SPEC, FIXES)) + (row,) = sweep(root, tmp_path, tries=1, backend=FakeBackend(FAULTY_SPEC, FIXES)) # The spec's own fields, then the three the round quoted out of the source, # one of which it went on to edit. diff --git a/tests/test_docs.py b/tests/test_docs.py index 467318a..5273475 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -34,21 +34,34 @@ def _options(parser: argparse.ArgumentParser) -> set[str]: def _stage_names() -> set[str]: - """Every name the pipeline adds a stage under. + """Every name a stage is reported under. - `add_stage` is the one place a stage is recorded: it constructs the - `StageResult` and reports it to whoever asked to be told. + A run records a stage in one of two ways: `RESULT.add_stage(name, message)`, + or, in the spec loop, the `on_stage(name, message)` callback the loop is + given and `pipeline.run` answers with `add_stage`. So this reads both files + and both forms: an `add_stage` attribute call, and a call of the bare name + `on_stage`, which is the parameter rather than `RunResult.on_stage`. + + A stage name that is not a string constant is one this test cannot read, so + it fails naming the file and the line. """ - source = (ROOT / "in2lambda_agent" / "pipeline.py").read_text(encoding="utf-8") - return { - node.args[0].value - for node in ast.walk(ast.parse(source)) - if isinstance(node, ast.Call) - and isinstance(node.func, ast.Attribute) - and node.func.attr == "add_stage" - and node.args - and isinstance(node.args[0], ast.Constant) - } + names = set() + for module in ("pipeline.py", "spec.py"): + path = ROOT / "in2lambda_agent" / module + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if not isinstance(node, ast.Call) or not node.args: + continue + records = ( + isinstance(node.func, ast.Attribute) and node.func.attr == "add_stage" + ) or (isinstance(node.func, ast.Name) and node.func.id == "on_stage") + if not records: + continue + assert isinstance(node.args[0], ast.Constant), ( + f"{module} line {node.lineno}: the stage name is not a string, " + "so this test cannot tell which stage it is" + ) + names.add(node.args[0].value) + return names def test_readme_names_every_option(): @@ -58,8 +71,10 @@ def test_readme_names_every_option(): def test_how_it_works_names_every_stage(): names = _stage_names() - assert len(names) == 9 - missing = [one for one in sorted(names) if f"`{one}`" not in HOW_IT_WORKS] + assert len(names) == 10 + # The stage's own section, rather than the name anywhere on the page: a + # column of the corpus table shares a name with a stage. + missing = [one for one in sorted(names) if f"### `{one}`" not in HOW_IT_WORKS] assert not missing diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 3b22aca..4355f5d 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -1,6 +1,7 @@ """The end-to-end run: a markdown or PDF source in, a Lambda Feedback zip out.""" import json +import os import random import shutil import warnings @@ -17,10 +18,10 @@ from in2lambda_agent.cli import main from in2lambda_agent.model import ModelUnavailable from in2lambda_agent.ocr import ocr_pdf -from in2lambda_agent.package import SpecRejected +from in2lambda_agent.package import SourceError, SpecRejected from in2lambda_agent.review import ReviewError from in2lambda_agent.settings import Settings -from in2lambda_agent.spec import RECORD_NAME, SPEC_NAME +from in2lambda_agent.spec import RECORD_NAME, SPEC_NAME, BadSpec FIXTURES = Path(__file__).parent / "fixtures" SOURCE = FIXTURES / "sheet.md" @@ -34,6 +35,9 @@ # it answers, and then the brace the OCR dropped out of that solution put back. # The three quotations are layer 3; the replacement is the layer 4 edit, which # in2lambda marks on the field rather than moving where it came from. +# +# A run scripted with these passes `tries=1`: the spec loop stops at one spec, +# and the replies after it are the round's commands rather than another spec. FIXES = [ ("split_block", {"block": "b7", "at": 14}), ("question_add", {"text": "b7a"}), @@ -65,6 +69,48 @@ line for line in SPEC.splitlines() if not line.startswith("part:") ) +# And with no `solution` selector, which leaves the four solution paragraphs in +# no field: fewer blocks over than PARTLESS_SPEC leaves, so the spec loop keeps +# this one of the two. +SOLUTIONLESS_SPEC = ( + "\n".join(line for line in SPEC.splitlines() if not line.startswith("solution:")) + + "\n" +) + +# A spec whose question selector is keyed to the wording of the first sheet — +# "A ball", "A block" — and so covers it completely while leaving the second +# sheet's stem, "A car brakes...", in no field. +FIRST_SHEET_SPEC = SPEC.replace("text~'^[A-Z]'", "text~'^A b'") + +# And one whose layout reads the solutions as a run of parts followed by a run +# of solutions, which covers sheet-2.md completely. It is the spec the two +# tests below have in2lambda refuse over the other sheet. +SECOND_SHEET_SPEC = SPEC.replace("PartsSepSol", "PartPartSolSol") + + +def refused_over_the_other_sheet(monkeypatch, text): + """Has in2lambda refuse one of the loop's specs over the set's other sheet. + + in2lambda reports a block two of a layout's fields would hold rather than + refusing the spec over the document holding it, so no spec these fixtures + can hold draws a refusal out of it over one sheet and not the other. It + still refuses a spec it cannot read there, and a run still has to score the + try that drew the refusal, so the refusal is put in here. + + Args: + monkeypatch: The test's own. + text: The spec of the try to refuse, as the loop writes it to the file. + """ + ran = package.spec_run + + def refusing(draft, spec): + # The copy of the other sheet, which the run freezes under its cache. + if "second" in Path(draft).parts and Path(spec).read_text() == text: + raise SpecRejected("the spec cannot be read over this document") + return ran(draft, spec) + + monkeypatch.setattr(package, "spec_run", refusing) + def drafted(folder, name): """The draft a run over one sheet of a folder left, which is named after it.""" @@ -403,6 +449,100 @@ def test_a_rewrite_in2lambda_will_not_run_leaves_the_saved_spec_alone( assert (sheets / SPEC_NAME).read_text() == PARTLESS_SPEC +def test_a_try_the_loop_never_chose_is_not_left_beside_the_sources(sheets, tmp_path): + # The first call answers with a spec that runs but covers little, the second + # with something that is not a spec at all. The set keeps the spec it had: + # try 1 was written to the file, and no try was ever chosen. + (sheets / SPEC_NAME).write_text(PARTLESS_SPEC) + backend = FakeBackend(SOLUTIONLESS_SPEC, "I would rather not.") + + with pytest.raises(BadSpec): + pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=backend, + ) + + assert len(backend.calls) == 2 + assert (sheets / SPEC_NAME).read_text() == PARTLESS_SPEC + + +def test_a_document_of_the_set_in2lambda_cannot_read_is_passed_over( + sheets, tmp_path, monkeypatch +): + # A folder holds files that are not documents — a Word lock file beside a + # docx — and in2lambda refuses them. The run reports the file and converts + # the source it was asked for. + freeze = package.source_add + + def refuse(source): + if Path(source).name == "sheet-2.md": + raise SourceError("pandoc could not read sheet-2.md") + return freeze(source) + + monkeypatch.setattr(package, "source_add", refuse) + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=FakeBackend(SPEC), + ) + (over_set,) = [stage for stage in result.stages if stage.name == "set"] + + assert over_set.message == ( + "sheet-2.md cannot be read: pandoc could not read sheet-2.md" + ) + assert result.zip_path is not None and result.zip_path.exists() + # The try is then judged on this source alone, and the record says which + # document the run passed over and why, rather than reading as a set of one. + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + (one,) = record["iterations"] + assert one["second"] is None and one["chosen"] is True + assert record["second"] == { + "name": "sheet-2.md", + "passed_over": "pandoc could not read sheet-2.md", + } + + +def test_a_sheet_of_the_set_that_has_a_draft_of_its_own_is_left_alone( + sheets, tmp_path +): + # A run over sheet-2.md left the draft beside it, holding that sheet's + # fields and the log of the commands that wrote them. Freezing sheet-2.md + # to try a spec over it would write the draft again from the source and + # delete both, so the loop runs each spec over a copy in the cache and the + # draft beside the sheet is still the one that run wrote. + elsewhere = tmp_path / "other-spec.yaml" + elsewhere.write_text(SPEC) + second = package.source_add(sheets / "sheet-2.md") + package.spec_run(second, elsewhere) + before = second.read_text() + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + backend=FakeBackend(SPEC), + ) + (over_set,) = [stage for stage in result.stages if stage.name == "set"] + + assert second.read_text() == before + assert (tmp_path / "cache" / "second" / "sheet-2.md").is_file() + assert result.zip_path is not None and result.zip_path.exists() + # The sheet is evidence about the spec all the same: the copy is run over, + # the line names the sheet, and the try carries what it left there. + assert over_set.message.startswith("sheet-2.md: ") + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + (one,) = record["iterations"] + assert one["second"] == 0 + assert record["second"] == {"name": "sheet-2.md", "passed_over": None} + + def test_a_saved_spec_the_checks_fault_is_written_again_once(sheets, tmp_path): (sheets / SPEC_NAME).write_text(PARTLESS_SPEC) backend = FakeBackend(SPEC) @@ -436,6 +576,7 @@ def test_a_fresh_spec_the_checks_fault_stops_the_run_with_no_zip(sheets, tmp_pat out_dir=tmp_path / "out", settings=Settings(), rounds=0, + tries=1, backend=backend, ) stages = {stage.name: stage.message for stage in result.stages} @@ -452,6 +593,346 @@ def test_a_fresh_spec_the_checks_fault_stops_the_run_with_no_zip(sheets, tmp_pat assert not (tmp_path / "out").exists() +def test_the_spec_is_written_again_against_what_running_the_last_one_covered( + sheets, tmp_path +): + # Three specs, none of them clean: the second leaves four blocks over where + # the first and third leave six, so it is the one the set keeps. + backend = FakeBackend(PARTLESS_SPEC, SOLUTIONLESS_SPEC, PARTLESS_SPEC) + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + rounds=0, + backend=backend, + ) + kept = [stage for stage in result.stages if stage.name == "spec"][-1] + fields = json.loads(drafted(sheets, "sheet.md").read_text())["fields"] + + assert len(backend.calls) == 3 + # What the first spec covered is in the second call's prompt, by the block + # ids the coverage line names and the sentences the checks wrote. + assert "b4, b5, b8, b9, b13, b14 unassigned" in backend.calls[1][1] + assert "b4 (lines 7-7) is in no field" in backend.calls[1][1] + assert "sheet-2.md, another document of this set" in backend.calls[1][1] + assert kept.message == "kept try 2 of 3" + # The spec beside the sources, the coverage the run carries and the draft on + # disk are all the second try's, which the third try wrote over and the loop + # ran again. + assert (sheets / SPEC_NAME).read_text() == SOLUTIONLESS_SPEC + assert result.coverage.unassigned == ["b11", "b12", "b13", "b14"] + assert "q1.p1.text" in fields + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + iterations = json.loads(line)["iterations"] + assert [one["try"] for one in iterations] == [1, 2, 3] + assert all(one["input_tokens"] > 0 and one["seconds"] > 0 for one in iterations) + assert [one["unassigned"] for one in iterations] == [6, 4, 6] + assert [one["chosen"] for one in iterations] == [False, True, False] + + +def test_on_stage_sees_each_line_of_the_spec_loop_as_it_is_made(sheets, tmp_path): + # The page that shows a run sends each line to the browser as the run adds + # it, so the spec loop reports a line when it makes it rather than every + # line at the end. What the backend has been told when it is called is what + # says which of the two happened. + watched: list[tuple[str, str]] = [] + seen: list[list[str]] = [] + + class Watching(FakeBackend): + def call(self, system, prompt, tools=(), images=()): + seen.append([name for name, _ in watched]) + return super().call(system, prompt, tools, images) + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + rounds=0, + tries=2, + backend=Watching(PARTLESS_SPEC, SPEC), + on_stage=lambda stage: watched.append((stage.name, stage.message)), + ) + + # The first call is made once the source is frozen, and the second once the + # first spec has been run over this sheet and over the other one. + assert seen[0] == ["ocr", "freeze"] + assert seen[1] == [ + "ocr", + "freeze", + "spec", + "coverage", + "validate", + "set", + "freeze", + ] + assert watched == [(stage.name, stage.message) for stage in result.stages] + + +def test_a_rewrite_reads_the_other_sheet_of_a_set_whose_sheets_all_have_drafts( + sheets, tmp_path +): + # The sweep's third sheet: every other sheet of the set has been run and has + # a draft beside it, and the saved spec faults this one. The rewrite is the + # call this ticket added try 0 for, so try 0 is run over the other sheet as + # well and the first call reads what the saved spec left there. + elsewhere = tmp_path / "other-spec.yaml" + elsewhere.write_text(SPEC) + second = package.source_add(sheets / "sheet-2.md") + package.spec_run(second, elsewhere) + before = second.read_text() + (sheets / SPEC_NAME).write_text(PARTLESS_SPEC) + backend = FakeBackend(FIRST_SHEET_SPEC, SPEC) + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + tries=2, + backend=backend, + ) + + # The saved spec leaves b4, b5 and b8 of sheet-2.md in no field, and the + # first call is asked for a spec that does not. + assert "another document of this set, left b4, b5, b8" in backend.calls[0][1] + assert (sheets / SPEC_NAME).read_text() == SPEC + assert second.read_text() == before + assert result.zip_path is not None and result.zip_path.exists() + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + # Try 0 is the saved spec, try 1 covers this sheet alone and try 2 the set. + assert [one["try"] for one in record["iterations"]] == [0, 1, 2] + assert [one["second"] for one in record["iterations"]] == [3, 5, 0] + assert record["second"] == {"name": "sheet-2.md", "passed_over": None} + + +def test_a_spec_in2lambda_refuses_over_the_other_sheet_is_scored_on_that( + sheets, tmp_path, monkeypatch +): + # The first spec covers this sheet, and in2lambda refuses it over the other + # sheet. in2lambda reads the other sheet, so the copy of it stays and the + # next try is run over it as well, and the refused try scores as leaving + # every block of it in no field. + refused_over_the_other_sheet(monkeypatch, SECOND_SHEET_SPEC) + backend = FakeBackend(SECOND_SHEET_SPEC, SPEC) + + result = pipeline.run( + sheets / "sheet-2.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + tries=2, + backend=backend, + ) + refused, ran = [stage for stage in result.stages if stage.name == "set"] + + assert refused.message.startswith("sheet.md: in2lambda refused the spec: ") + assert ran.message.startswith("sheet.md: ") + assert (sheets / SPEC_NAME).read_text() == SPEC + assert result.zip_path is not None and result.zip_path.exists() + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + # The refused try covers this sheet completely, so the other sheet is what + # carried the loop on to the try that covers both sheets. The refused spec + # wrote no field in sheet.md, so it left all 14 of its blocks in no field. + assert [one["unassigned"] for one in record["iterations"]] == [0, 0] + assert [one["second"] for one in record["iterations"]] == [14, 0] + assert [one["chosen"] for one in record["iterations"]] == [False, True] + # The spec the run kept ran over the document, so the record does not say + # the document was passed over. + assert record["second"] == {"name": "sheet.md", "passed_over": None} + + +def test_a_refusal_that_went_with_a_try_the_run_threw_away_is_not_recorded( + sheets, tmp_path, monkeypatch +): + # The second spec covers this sheet and in2lambda refuses it over the other + # sheet, so the run keeps the first, which ran over both. What the record + # says became of the other sheet is what the kept spec made of it, not what + # the try after it did. + refused_over_the_other_sheet(monkeypatch, SECOND_SHEET_SPEC) + backend = FakeBackend(SOLUTIONLESS_SPEC, SECOND_SHEET_SPEC) + + result = pipeline.run( + sheets / "sheet-2.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + rounds=0, + tries=2, + backend=backend, + ) + ran, refused = [stage for stage in result.stages if stage.name == "set"] + kept = [stage for stage in result.stages if stage.name == "spec"][-1] + + # The stage lines are the log of the loop, so both tries are still in them. + assert ran.message.startswith("sheet.md: PartsSepSol") + assert refused.message.startswith("sheet.md: in2lambda refused the spec: ") + assert kept.message == "kept try 1 of 2" + assert (sheets / SPEC_NAME).read_text() == SOLUTIONLESS_SPEC + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + assert [one["chosen"] for one in record["iterations"]] == [True, False] + assert record["second"] == {"name": "sheet.md", "passed_over": None} + + +def test_the_sheets_own_solutions_file_is_no_other_document_of_the_set( + paired, tmp_path +): + # The solutions file is the second source of this run's own draft, so it is + # not another document for a spec to be judged over, and the folder holds + # nothing else. + result = pipeline.run( + paired / "paired.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + backend=FakeBackend(PAIRED_SPEC), + ) + + assert not [stage for stage in result.stages if stage.name == "set"] + assert not (tmp_path / "cache" / "second").exists() + (line,) = (paired / RECORD_NAME).read_text().splitlines() + assert json.loads(line)["second"] is None + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root may read any file") +def test_a_document_of_the_set_the_run_cannot_copy_is_passed_over(sheets, tmp_path): + # The copy is what keeps the spec loop off the set's own drafts, so a sheet + # the OS will not let the run copy is one to pass over, as one in2lambda + # cannot read is, rather than one that ends the run with no spec written. + (sheets / "sheet-2.md").chmod(0o000) + try: + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + backend=FakeBackend(SPEC), + ) + finally: + (sheets / "sheet-2.md").chmod(0o644) + (over_set,) = [stage for stage in result.stages if stage.name == "set"] + + assert over_set.message.startswith("sheet-2.md passed over: copying it failed: ") + assert result.zip_path is not None and result.zip_path.exists() + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + assert record["second"]["name"] == "sheet-2.md" + assert record["second"]["passed_over"].startswith("copying it failed: ") + (one,) = record["iterations"] + assert one["second"] is None + + +def test_a_pdf_beside_a_pdf_source_is_passed_over_and_the_record_says_why( + pdf, tmp_path +): + (tmp_path / "sheet-2.pdf").write_bytes(b"%PDF-1.4 the next sheet") + + result = pipeline.run( + pdf, + out_dir=tmp_path / "out", + settings=Settings(), + cache_dir=tmp_path / "cache", + mathpix=FakeMathpix(markdown=SOURCE.read_text()), + backend=FakeBackend(SPEC), + ) + (over_set,) = [stage for stage in result.stages if stage.name == "set"] + + assert over_set.message == ( + "sheet-2.pdf passed over: converting it takes an OCR call, and the " + "spec loop makes no call but the model's" + ) + (line,) = (tmp_path / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + assert record["second"]["name"] == "sheet-2.pdf" + assert record["second"]["passed_over"].startswith("converting it takes") + (one,) = record["iterations"] + assert one["second"] is None + + +def test_a_saved_spec_the_checks_fault_is_the_try_the_rewrite_improves_on( + sheets, tmp_path +): + (sheets / SPEC_NAME).write_text(PARTLESS_SPEC) + backend = FakeBackend(SPEC) + + pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + backend=backend, + ) + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + saved, written = json.loads(line)["iterations"] + + # The saved spec is try 0: it is what the call was asked to improve on, and + # it cost no call of its own. + assert (saved["try"], saved["unassigned"], saved["chosen"]) == (0, 6, False) + assert saved["input_tokens"] == saved["output_tokens"] == 0 + assert (written["try"], written["unassigned"], written["chosen"]) == (1, 0, True) + assert written["output_tokens"] > 0 + + +def test_a_spec_that_covers_this_sheet_alone_does_not_stop_the_loop(sheets, tmp_path): + # The first spec covers sheet.md completely and leaves sheet-2.md's stem in + # no field. The spec is saved for the whole set, so that is not a spec to + # stop at: the second call is made, and covers both. + backend = FakeBackend(FIRST_SHEET_SPEC, SPEC) + + result = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + tries=2, + backend=backend, + ) + over_set = [stage for stage in result.stages if stage.name == "set"] + + assert len(backend.calls) == 2 + assert over_set[0].message.startswith("sheet-2.md: ") + # Nothing of the second sheet is covered: its stem is in no field, and its + # parts and solutions have no question to belong to. + assert "b3, b4, b5, b7, b8 unassigned" in over_set[0].message + assert (sheets / SPEC_NAME).read_text() == SPEC + assert result.zip_path is not None and result.zip_path.exists() + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + iterations = json.loads(line)["iterations"] + assert [one["unassigned"] for one in iterations] == [0, 0] + assert [one["second"] for one in iterations] == [5, 0] + assert [one["chosen"] for one in iterations] == [False, True] + + +def test_a_sheet_with_no_other_document_beside_it_is_judged_on_its_own( + faulty, tmp_path +): + backend = FakeBackend(FAULTY_SPEC, FIXES) + + result = pipeline.run( + faulty / "faulty.md", + out_dir=tmp_path / "out", + settings=Settings(), + tries=1, + backend=backend, + ) + + assert not [stage for stage in result.stages if stage.name == "set"] + (line,) = (faulty / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + (one,) = record["iterations"] + assert one["second"] is None and one["chosen"] is True + # Null here is the folder holding no other document, which is what a reader + # of the record can tell it from a document the run passed over by. + assert record["second"] is None + + def test_the_rounds_fix_what_the_checks_found_and_the_run_builds(faulty, tmp_path): backend = FakeBackend(FAULTY_SPEC, FIXES) @@ -459,6 +940,7 @@ def test_the_rounds_fix_what_the_checks_found_and_the_run_builds(faulty, tmp_pat faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) fixed = next(stage for stage in result.stages if stage.name == "fix") @@ -493,6 +975,7 @@ def test_each_round_answers_what_the_one_before_it_left(faulty, tmp_path): faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) second = backend.calls[2][1] @@ -511,6 +994,7 @@ def test_every_fix_is_in_the_drafts_log_with_the_layer_it_wrote(faulty, tmp_path faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=FakeBackend(FAULTY_SPEC, FIXES), ) log = package.command_log(drafted(faulty, "faulty.md")) @@ -546,6 +1030,7 @@ def test_the_record_says_what_each_round_cost(faulty, tmp_path): faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=FakeBackend(FAULTY_SPEC, FIXES), ) @@ -569,6 +1054,7 @@ def test_a_command_in2lambda_refuses_is_answered_rather_than_ending_the_run( faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) refused = result.rounds[0].commands[0] @@ -591,6 +1077,7 @@ def test_a_part_whose_solution_is_not_on_the_sheet_is_reported_not_written( unsolved / "faulty-unsolved.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) checked = [stage for stage in result.stages if stage.name == "validate"][-1] @@ -628,6 +1115,7 @@ def test_a_solution_the_model_types_out_is_refused_and_the_finding_stays( unsolved / "faulty-unsolved.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) typed = result.rounds[0].commands[0] @@ -735,6 +1223,7 @@ def test_a_problem_the_set_checks_find_reaches_the_next_round(faulty, tmp_path): out_dir=tmp_path / "out", settings=Settings(), rounds=2, + tries=1, backend=backend, ) _, second_round = backend.calls[2] @@ -760,6 +1249,7 @@ def test_a_round_that_answers_nothing_ends_the_run_with_what_it_left( faulty / "faulty.md", out_dir=tmp_path / "out", settings=Settings(), + tries=1, backend=backend, ) last = result.stages[-1] @@ -791,6 +1281,7 @@ def test_a_field_the_round_tried_to_write_ends_the_run_naming_it(faulty, tmp_pat out_dir=tmp_path / "out", settings=Settings(), rounds=3, + tries=1, backend=backend, ) last = result.stages[-1] @@ -817,6 +1308,7 @@ def test_a_run_still_making_progress_stops_at_the_limit_with_no_zip(faulty, tmp_ out_dir=tmp_path / "out", settings=Settings(), rounds=1, + tries=1, backend=backend, ) last = result.stages[-1] @@ -844,6 +1336,8 @@ def test_the_rounds_running_out_prints_its_stages_and_exits_one( str(faulty / "faulty.md"), "--rounds", "1", + "--tries", + "1", "--out", str(tmp_path / "out"), ] @@ -911,6 +1405,9 @@ def test_the_stages_run_in_order(sheets, tmp_path): "spec", "coverage", "validate", + # What the spec covered of the set's other sheet, which is the last + # thing the choice between two specs is made on. + "set", "review", "build", ] @@ -993,6 +1490,7 @@ def test_a_review_of_a_question_a_literal_wrote_lists_the_lines_it_has( review="sample", cache_dir=tmp_path / "cache", rng=random.Random(0), + tries=1, backend=FakeBackend(FAULTY_SPEC, TYPED_FIXES), ) stages = {stage.name: stage.message for stage in result.stages} @@ -1090,6 +1588,36 @@ def test_approving_every_question_builds_the_set_and_records_the_review( assert recorded["rejections"] == [] and recorded["edits"] == [] +def test_the_line_an_approval_writes_names_the_other_document_of_the_set( + sheets, tmp_path +): + # The spec loop ran before the review, so the line the last approval writes + # says which document of the set the specs were run over, as the line a run + # without a review writes does. + waiting = pipeline.run( + sheets / "sheet.md", + out_dir=tmp_path / "out", + settings=Settings(), + review="sample", + cache_dir=tmp_path / "cache", + rng=random.Random(0), + backend=FakeBackend(SPEC), + ) + + for question in list(waiting.review.questions): + pipeline.resume( + tmp_path / "cache", + verdict="approve", + key=question.key, + settings=Settings(), + ) + + (line,) = (sheets / RECORD_NAME).read_text().splitlines() + record = json.loads(line) + assert record["second"] == {"name": "sheet-2.md", "passed_over": None} + assert [one["second"] for one in record["iterations"]] == [0] + + def test_a_review_is_answered_from_wherever_the_reviewer_is( sheets, tmp_path, monkeypatch ): @@ -1364,7 +1892,11 @@ def test_the_set_is_written_where_the_run_was_told_to(sheets, tmp_path, monkeypa # The zip the build stage names is the one on disk, in the given directory. assert result.zip_path == tmp_path / "out" / "set.zip" assert result.zip_path.exists() - assert list(working.iterdir()) == [] + # The cache is the one thing the run writes under the working directory, + # since `--cache` defaults to a folder there, and it holds the copy of the + # set's other sheet each spec was run over. + assert [one.name for one in working.iterdir()] == [".in2lambda-agent"] + assert (working / ".in2lambda-agent" / "second" / "sheet-2.md").is_file() def test_a_relative_out_dir_is_resolved_against_the_working_directory( diff --git a/tests/test_review.py b/tests/test_review.py index 0762b37..1a55ee8 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -2,6 +2,7 @@ import random import re +from pathlib import Path import pytest @@ -9,6 +10,7 @@ from in2lambda_agent.model import ToolCall, Usage from in2lambda_agent.package import Coverage, QuestionInfo from in2lambda_agent.review import Question, Review, ReviewError, choose +from in2lambda_agent.spec import Second, SpecTry def infos(*layers): @@ -75,6 +77,10 @@ def test_a_sample_is_the_same_sample_twice_from_the_same_seed(): def test_the_record_goes_to_json_and_comes_back(tmp_path): saved = review( usage=Usage(input_tokens=120, output_tokens=40, seconds=1.5), + tries=[ + SpecTry(0, Usage(), unassigned=2, errors=2), + SpecTry(1, Usage(input_tokens=120, output_tokens=40), second=0, chosen=True), + ], rounds=[ RoundResult(1, [ToolCall("part_add", {"question": "q2"}, "wrote")], Usage(), 0) ], @@ -89,6 +95,32 @@ def test_the_record_goes_to_json_and_comes_back(tmp_path): # The layers keep their numbers, which is how every other reader has them. assert read.coverage.fields == {1: 10} assert read.rounds[0].commands[0].name == "part_add" + # The iterations too, so that the record the last approval writes says what + # each spec the run wrote covered and cost. + assert [one.number for one in read.tries] == [0, 1] + assert read.tries[1].usage.input_tokens == 120 and read.tries[1].chosen is True + + +def test_the_other_document_of_the_set_goes_to_json_and_comes_back(tmp_path): + # A review with no other document of the set records None, which the record + # the last approval writes reads as a folder of one sheet. + assert Review.load(_written(review(), tmp_path)).second is None + + over = review(second=Second("sheet-2.md", path=Path("cache/second/sheet-2.md"))) + passed = review(second=Second("sheet-2.md", passed_over="an OCR call")) + + # The copy each spec was run over is not kept: the approval writes the + # record and runs no spec. + assert Review.load(_written(over, tmp_path)).second == Second("sheet-2.md") + assert Review.load(_written(passed, tmp_path)).second == Second( + "sheet-2.md", passed_over="an OCR call" + ) + + +def _written(waiting, tmp_path): + """The file a review was saved to, for a test that reads it back.""" + waiting.save(tmp_path / "review.json") + return tmp_path / "review.json" def test_no_review_waiting_says_what_writes_one(tmp_path): diff --git a/tests/test_spec.py b/tests/test_spec.py index 381b6b0..44405d3 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -11,6 +11,9 @@ from in2lambda_agent.spec import ( SPEC_NAME, BadSpec, + Previous, + Second, + SpecTry, record_run, spec_path, write_spec, @@ -52,14 +55,40 @@ def test_the_prompt_carries_the_numbered_source_as_it_was_shown(): assert reply.usage.output_tokens > 0 -def test_a_rewrite_carries_what_the_checks_found(): +def test_a_revision_carries_the_last_spec_and_what_running_it_covered(): backend = FakeBackend(SPEC) - report = Report(clean=False, errors=["b4 (lines 7-7) is in no field."]) + previous = Previous( + text="question: Para\nlayout: PartsSepSol\n", + coverage=Coverage( + layout="PartsSepSol", blocks=14, fields={1: 9}, ignored=4, + unassigned=["b4"], + ), + report=Report(clean=False, errors=["b4 (lines 7-7) is in no field."]), + second=Coverage(layout="PartsSepSol", blocks=9, unassigned=["b5", "b6"]), + second_name="sheet-2.md", + ) - write_spec("b1 1 # Sheet", backend, report) + write_spec("b1 1 # Sheet", backend, previous) (_, prompt), = backend.calls + assert "question: Para\nlayout: PartsSepSol\n" in prompt + assert "b4 unassigned" in prompt assert "b4 (lines 7-7) is in no field." in prompt + assert "over sheet-2.md, another document of this set, left b5, b6" in prompt + + +def test_a_revision_of_a_spec_that_covered_the_set_says_so(): + backend = FakeBackend(SPEC) + previous = Previous( + text="question: Para\nlayout: PartsSepSol\n", + second=Coverage(layout="PartsSepSol", blocks=9), + second_name="sheet-2.md", + ) + + write_spec("b1 1 # Sheet", backend, previous) + + (_, prompt), = backend.calls + assert "left no blocks in no field" in prompt def test_a_spec_in_a_code_fence_is_unwrapped(): @@ -131,3 +160,80 @@ def test_each_run_appends_one_line_saying_what_the_spec_covered(tmp_path): assert first["unassigned"] == ["b13"] assert (first["input_tokens"], first["output_tokens"]) == (900, 80) assert second["reused"] is True and second["output_tokens"] == 0 + # A run that reused the set's spec wrote none, so it iterated over nothing. + assert first["iterations"] == [] and second["iterations"] == [] + + +def test_the_record_says_what_each_spec_the_run_wrote_covered_and_cost(tmp_path): + record = tmp_path / "runs.jsonl" + coverage = Coverage(layout="PartsSepSol", blocks=14, fields={1: 10}) + + record_run( + record, + Path("sheet.md"), + reused=False, + coverage=coverage, + usage=Usage(input_tokens=900, output_tokens=80, seconds=2.5), + tries=[ + SpecTry(number=0, unassigned=2, errors=2), + SpecTry( + number=1, + usage=Usage(input_tokens=900, output_tokens=80, seconds=2.5), + unassigned=0, + errors=0, + second=1, + ), + SpecTry( + number=2, + usage=Usage(input_tokens=950, output_tokens=70, seconds=2.0), + chosen=True, + ), + ], + ) + + (line,) = record.read_text().splitlines() + saved, first, second = json.loads(line)["iterations"] + + # The saved spec the rewrite started from, which cost no call of its own. + assert saved == { + "try": 0, + "input_tokens": 0, + "output_tokens": 0, + "seconds": 0.0, + "unassigned": 2, + "errors": 2, + "second": None, + "chosen": False, + } + assert (first["input_tokens"], first["output_tokens"]) == (900, 80) + assert (first["second"], first["chosen"]) == (1, False) + assert (second["try"], second["seconds"], second["chosen"]) == (2, 2.0, True) + + +def test_the_record_names_the_other_document_of_the_set(tmp_path): + record = tmp_path / "runs.jsonl" + coverage = Coverage(layout="PartsSepSol", blocks=14, fields={1: 10}) + + for second in ( + Second("sheet-2.md", path=tmp_path / "second" / "sheet-2.md"), + Second("sheet-2.pdf", passed_over="converting it takes an OCR call"), + None, + ): + record_run( + record, + Path("sheet.md"), + reused=False, + coverage=coverage, + usage=Usage(), + second=second, + ) + + over, passed, alone = [json.loads(line) for line in record.read_text().splitlines()] + + assert over["second"] == {"name": "sheet-2.md", "passed_over": None} + assert passed["second"] == { + "name": "sheet-2.pdf", + "passed_over": "converting it takes an OCR call", + } + # Null is the folder holding no other document, and nothing else. + assert alone["second"] is None