From 3a4b3e0a8438458681264718e2ddf0b0fdf27c93 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 22:54:15 +0100 Subject: [PATCH] implement: Create response areas from each part's answer (t44) --- README.md | 53 +-- ci-corpus/filters/sheet/areas.json | 1 + docs/plan.md | 18 +- in2lambda_agent/response_areas.py | 356 +++++++++++++++++++ in2lambda_agent/routes.py | 68 +++- in2lambda_agent/targets.py | 106 ++++-- tests/conftest.py | 11 +- tests/test_gate.py | 4 +- tests/test_response_areas.py | 527 +++++++++++++++++++++++++++++ tests/test_routes.py | 9 +- tests/test_targets.py | 22 +- 11 files changed, 1093 insertions(+), 82 deletions(-) create mode 100644 ci-corpus/filters/sheet/areas.json create mode 100644 in2lambda_agent/response_areas.py create mode 100644 tests/test_response_areas.py diff --git a/README.md b/README.md index 4dcfaa1..7ef1ba9 100644 --- a/README.md +++ b/README.md @@ -99,12 +99,17 @@ flag q4.p2.content: two readings of the source A: Find the drag force on the plate. B: Find the drag force on the plate, in newtons. fields 60 fields, agreed 54, defaulted 4, adjudicated 2, flagged 2 +areas 13 for 12 parts build /home/me/out/sheet.zip ``` A flag names the field, the reason, and each route's text where both routes filled the field. `fields` counts the fields the two routes agreed on, the fields one route alone -filled, the fields the adjudicating call settled, and the fields flagged. With no +filled, the fields the adjudicating call settled, and the fields flagged. `areas` counts +the answer boxes written into the set and the parts holding them: one call per part, made +after the fields are settled, proposes the kind of box, the label before it and the +answer the platform marks against. A part that asks for a discussion gets no box, and a +proposal the platform could not mark is a flag on that part and no box. With no filter there is no comparison to count, so the line is `60 fields, route B did not run`: the fields are route A's, and each one is flagged or is route A's word for it. A filter run that fails counts route A's fields in the same way, and adds a `route B failed` line @@ -131,7 +136,7 @@ The page runs the `convert` command: the two routes, the reconciliation and the Name the solutions document, or leave that box empty for the document beside the source; name a Lua filter for route B, or tick Write filter for one model call that writes `filter.lua` into the out directory; then press Go. Each stage line — `ocr`, `route A`, -`route B`, `fields`, `build` — arrives on the page as the stage finishes. When the run +`route B`, `areas`, `fields`, `build` — arrives on the page as the stage finishes. When the run ends, the page shows each flagged field with the reason it is flagged and each route's reading of it, the counts of the reconciliation, the tokens, and links to the zip and to the filter where the run wrote one. @@ -258,9 +263,17 @@ the export. The run prints one line per difference and one line of counts per ta differs ME2_Fluids_introduction: Question 2 "", part (a), text: the agent says … and the export says … known ME2_Fluids_introduction: Question 1 "", main text: the agent says … and the export says … agrees ME2_Fluids_introduction: q3.p2.worked_solution now agrees, remove the line +areas ME2_Fluids_introduction: 11 of 12 match +miss ME2_Fluids_introduction: q4.p2[2]: wanted NUMERIC_UNITS '0.541 mm', made NUMERIC_UNITS '0.1 mm' ME2_Fluids_introduction: 4 differ, 3 known, 1 new, 2 flagged ``` +`areas` counts the export's answer boxes the agent made the same way: the same number of +boxes for the part, the same kind, and the same answer once units and symbols are +normalised. Every other box is a `miss` line naming the part, the box's place in it, and +what each side answers. A box is the export's box or it is wrong, so `differs.txt` does +not accept one. + A difference you have read and accepted goes into `differs.txt` beside that target's filter. A line of that file names the field the difference is in, and states after a `#` why the field differs: @@ -281,14 +294,15 @@ as a report. `--filters` (default `./targets`) is the tree of saved filters, mirroring the targets: target `A/B` keeps its filter at `targets/A/B/filter.lua`, route A's reply at -`targets/A/B/reply.json` and its accepted fields at `targets/A/B/differs.txt`. The first -run over a target makes one model call for the filter and one for route A's reply, and -writes both. Every run after that reads the two files and makes neither of those two -calls, so the second run over a target differs from the export in the same fields as the -first. `--fresh` reads the documents again and writes a new reply, which changes the -wording of the report and the number of fields flagged. - -Those two are the only calls a saved target spares. A target with a filter runs route B +`targets/A/B/reply.json`, the answer boxes proposed for its parts at +`targets/A/B/areas.json` and its accepted fields at `targets/A/B/differs.txt`. The first +run over a target makes one model call for the filter, one for route A's reply and one +for each part's answer boxes, and writes the three files. Every run after that reads them +and makes none of those calls, so the second run over a target differs from the export in +the same fields as the first. `--fresh` reads the documents again and writes a new reply +and new boxes, which changes the wording of the report and the number of fields flagged. + +Those are the only calls a saved target spares. A target with a filter runs route B on every run, and a model adjudicates every field the two routes word differently. A verdict can go the other way on a later run, so the wording of a difference and the `flagged` count move from run to run while the fields `differs.txt` accepts stay @@ -309,20 +323,21 @@ poetry run in2lambda-agent gate ROOT [PATH ...] --filters DIR [--cache DIR] [--w `gate` converts and compares each target as `targets` does, prints the same lines, and exits 0 where every target ran and reported no new difference. `gate` differs from -`targets` in one thing: a target whose `reply.json`, or whose `filter.lua`, is not saved -under `--filters` is reported as an error and is not converted. The two model calls that -read a document are therefore never made, and what the gate reports is a change to the -agent and not a model wording a field differently today. +`targets` in one thing: a target whose `reply.json`, `areas.json` or `filter.lua` is not +saved under `--filters` is reported as an error and is not converted. The model calls +that read a document are therefore never made, and what the gate reports is a change to +the agent and not a model wording a field differently today. ``` work /tmp/in2lambda-agent-gate-3f1a +areas sheet: 0 of 0 match sheet: 0 differ, 0 known, 0 new, 0 flagged 1 target, 0 new differences ``` The error names the file and the command that writes it. Run -`in2lambda-agent targets ROOT --filters DIR` over that target, read the reply and the -filter it saves, and commit them. +`in2lambda-agent targets ROOT --filters DIR` over that target, read the reply, the answer +boxes and the filter it saves, and commit them. `--cache` defaults to `~/.cache/in2lambda-agent`, which is outside every worktree, because the gate runs in a worktree of its own: a cache inside the branch's directory @@ -333,9 +348,9 @@ repository, so `git status` after a gate run reports no new file. The repository holds one target, `ci-corpus/targets/sheet`: two synthetic markdown documents, and the export `set_Sheet` that in2lambda's writer wrote from the saved -reply. `ci-corpus/filters/sheet` holds that target's filter and reply. The two routes -agree on every field of the sheet, so the run adjudicates nothing and reads no -credential. `.github/workflows/gate.yml` runs pytest and then +reply. `ci-corpus/filters/sheet` holds that target's filter, reply and answer boxes, +which are none: the export has no box for the run to make. The two routes agree on every +field of the sheet, so the run adjudicates nothing and reads no credential. `.github/workflows/gate.yml` runs pytest and then ```sh poetry run in2lambda-agent gate ci-corpus/targets --filters ci-corpus/filters diff --git a/ci-corpus/filters/sheet/areas.json b/ci-corpus/filters/sheet/areas.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/ci-corpus/filters/sheet/areas.json @@ -0,0 +1 @@ +{} diff --git a/docs/plan.md b/docs/plan.md index 0ee4cfe..286d9eb 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -92,10 +92,20 @@ beside a display maths. ## Response areas -The built set has no response areas: the answer boxes and their marking are not on the -sheet, and choosing them requires judgement. A model call per part, given the part's -content, options and final answer, can propose them, and the ME2 export gives 13 to -measure against. This is deferred. +The answer boxes and their marking are not on the sheet, so a call per part proposes +them. After the two routes have settled a part, the call receives the part's content, its +options and its final answer, and returns the boxes: the kind (`NUMERIC_UNITS`, +`MATH_SINGLE_LINE` or `MULTIPLE_CHOICE`), the label before the box, and the answer in the +form the platform marks against. The kind decides the evaluation function; the call does +not choose it. A part that asks for a discussion returns no box, and a proposal the +platform could not mark — a kind that is not one of the three, a multiple choice that is +not one true-or-false per option — is refused and flagged. + +A target scores its boxes against its export's: the same number of boxes for the part, +the same kind, and the same answer once units and symbols are normalised. The ME2 pair, +live, matched 11 of the export's 12 boxes. The miss is the second box of question 4 part +(b): the export answers `0.541 mm` and the agent answers `0.1 mm`, which is the lower end +of the range the worked solution derives. ## Cost diff --git a/in2lambda_agent/response_areas.py b/in2lambda_agent/response_areas.py new file mode 100644 index 0000000..b6b97c3 --- /dev/null +++ b/in2lambda_agent/response_areas.py @@ -0,0 +1,356 @@ +"""The answer boxes a part gets, proposed one part at a time and scored. + +A part of an imported set has no box to type an answer into until something +writes one. What kind of box it is, and what the platform marks against, is not +in the document: the sheet says "find the mass", and the platform wants +`0.106 kg` under `comparePhysicalQuantities`. So after the routes have settled a +part, one small call reads that part's statement, its options and its final +answer, and proposes its boxes (`propose`): a kind, the text before the box, and +the answer in the machine form the platform reads. A part that asks for a +discussion gets none. + +The kind decides the evaluation function (`KINDS`), which is never the model's to +choose, and a proposal the platform could not mark - a kind that is not one of +the three, a multiple choice that is not one true-or-false per option - is +refused with a reason rather than written (`_refusal`). `attach` does that for +every part of a reply and hands the refusals back for the caller to flag. + +`to_response_area` writes a proposal through in2lambda's `ResponseArea`. +`areas_of` reads the boxes back out of a written zip or an exported folder, in +the platform's own shape, and `score` holds one against the other: same count +per part, same kind, and the same answer once units and symbols are normalised +(`same_answer`). +""" + +from __future__ import annotations + +import json +import re +import zipfile +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Optional + +from in2lambda.api.response_area import ResponseArea + +from in2lambda_agent.model import Backend, Reply + +KINDS = { + "NUMERIC_UNITS": "comparePhysicalQuantities", + "MATH_SINGLE_LINE": "symbolicEqual", + "MULTIPLE_CHOICE": "arrayEqual", +} +"""The three kinds of box a set holds, and what marks each. A proposal names the +kind; the evaluation function is not a model's to choose.""" + +SYSTEM = ( + "You give a question part the answer boxes a learning platform marks it with. " + "Answer with JSON only, no prose, no code fence." +) + + +def _prompt(content: str, options: list[str], answer: str) -> str: + return ( + "Return a JSON array, one object per answer box the part needs, in the order a " + 'student fills them: {"kind": str, "pre_text": str, "answer": str}. ' + "kind is NUMERIC_UNITS for a number with units, MATH_SINGLE_LINE for an " + "expression in symbols, MULTIPLE_CHOICE where the part lists options. " + "pre_text is the short label shown before the box, as LaTeX between dollars - " + '"$m=$" - or an empty string. answer is the correct answer written as the ' + 'platform reads it: "0.106 kg" for a number with its units, ' + '"(pi/6)*rho*U**2*R**2" for an expression, with ** for powers and the names of ' + "Greek letters spelled out. For MULTIPLE_CHOICE, answer is instead a list of " + "true or false, one for each option in the order given, true for the correct " + "ones. Return one object per quantity the part asks for, and an empty array for " + "a part that asks for a discussion, a sketch or a proof rather than an answer to " + "type.\n\n" + f"PART:\n\n{content}\n\n" + + ( + "OPTIONS:\n\n" + "\n".join(f"{i}. {one}" for i, one in enumerate(options, 1)) + "\n\n" + if options + else "" + ) + + f"FINAL ANSWER:\n\n{answer}" + ) + + +@dataclass +class Attached: + """What a reply's parts were given. + + Attributes: + proposals: The boxes by part key, `q1.p1`, for a part that got any. + refused: Why a part's proposal was refused, by the same key, which the + caller flags for a person. + tokens: What the calls read and wrote. + """ + + proposals: dict[str, list[dict[str, Any]]] = field(default_factory=dict) + refused: dict[str, list[str]] = field(default_factory=dict) + tokens: int = 0 + + +def _parse(text: str) -> tuple[list, Optional[str]]: + """The JSON list a call was asked for, or why what came back is not one. + + The same shape as `routes._json`, except that a refused proposal is a flag on + one part rather than a raise that loses the set, so this answers with the + reason instead. + """ + stripped = re.sub(r"^```(json)?\s*|\s*```$", "", text.strip()) + try: + answered = json.loads(stripped) + except json.JSONDecodeError as error: + return [], f"the reply is not JSON: {error}" + if not isinstance(answered, list): + said = " ".join(stripped.split())[:60] + return [], f"a reply is a JSON list of response areas, which {said!r} is not" + return answered, None + + +def _refusal(proposal: Any, options: list[str]) -> Optional[str]: + """Why a proposal cannot be written, or None where it can. + + A box the platform cannot mark is worse than no box: the student types an + answer into it and is marked wrong whatever they type. + """ + if not isinstance(proposal, dict): + return f"a response area is an object, which {proposal!r} is not" + kind = proposal.get("kind") + if kind not in KINDS: + return f"{kind!r} is not one of {', '.join(KINDS)}" + answer = proposal.get("answer") + if kind == "MULTIPLE_CHOICE": + if ( + not isinstance(answer, list) + or len(answer) != len(options) + or not all(isinstance(one, bool) for one in answer) + or not any(answer) + ): + return ( + f"a multiple-choice answer is true or false for each of the " + f"{len(options)} options, with a true among them, which {answer!r} is not" + ) + elif not isinstance(answer, str) or not answer.strip(): + return f"an answer is text the platform marks, which {answer!r} is not" + return None + + +def propose( + content: str, options: list[str], answer: str, backend: Backend +) -> tuple[list[dict[str, Any]], list[str], Reply]: + """One call: the boxes proposed for one part, what was refused, and the call. + + Args: + content: The part's statement. + options: The choices of a multiple-choice part; empty otherwise. + answer: The part's final answer as the solutions document gives it. + backend: What to call. + + Returns: + The proposals that can be written, as `{"kind", "pre_text", "answer"}`; + the reason each refused proposal was refused, which is a flag for the + caller; and the call, for its usage. + """ + reply = backend.call(SYSTEM, _prompt(content, options, answer)) + answered, problem = _parse(reply.text) + if problem is not None: + return [], [problem], reply + proposals, refused = [], [] + for one in answered: + reason = _refusal(one, options) + if reason is not None: + refused.append(reason) + continue + proposals.append( + { + "kind": one["kind"], + "pre_text": str(one.get("pre_text") or ""), + "answer": one["answer"], + } + ) + return proposals, refused, reply + + +def attach(reply: list[dict[str, Any]], backend: Backend) -> Attached: + """A call for every part of a reply with something to answer. + + A part with no statement, no options and no answer is a part the document + left empty, and nothing can be proposed for it, so it is not asked about. + """ + attached = Attached() + for i, question in enumerate(reply, 1): + for j, part in enumerate(question.get("parts", []), 1): + content = part.get("content") or "" + options = list(part.get("options") or []) + answer = part.get("answer") or "" + if not (content.strip() or options or answer.strip()): + continue + proposals, refused, call = propose(content, options, answer, backend) + attached.tokens += call.usage.input_tokens + call.usage.output_tokens + key = f"q{i}.p{j}" + if proposals: + attached.proposals[key] = proposals + if refused: + attached.refused[key] = refused + return attached + + +def to_response_area(proposal: dict[str, Any], options: list[str]) -> ResponseArea: + """A proposal as in2lambda's `ResponseArea`, which writes the platform's JSON.""" + kind = proposal["kind"] + area = ResponseArea( + response_type=kind, + answer=proposal["answer"], + evaluation_function=KINDS[kind], + pre_text=proposal.get("pre_text", ""), + ) + if kind == "MULTIPLE_CHOICE": + area.config = { + "single": sum(bool(one) for one in area.answer) == 1, + "options": list(options), + "randomise": False, + } + return area + + +# --- what a set says --------------------------------------------------------------------- + + +def _questions(built: Path) -> list[dict[str, Any]]: + """Every question of a written zip or an exported folder, as the JSON has it.""" + built = Path(built) + if built.is_dir(): + texts = [one.read_text(encoding="utf-8") for one in sorted(built.glob("question_*.json"))] + else: + with zipfile.ZipFile(built) as archive: + texts = [ + archive.read(name).decode("utf-8") + for name in sorted(archive.namelist()) + if name.startswith("question_") + ] + return sorted((json.loads(one) for one in texts), key=lambda q: q.get("orderNumber", 0)) + + +def areas_of(built: Path) -> dict[str, list[tuple[str, Any]]]: + """The boxes of a set by part key, as the platform reads them. + + The zip and the export are read as JSON rather than through in2lambda's own + model, so what is scored is what Lambda Feedback will load. + + Args: + built: A zip the agent wrote, or a `set_*` folder the platform exported. + + Returns: + `{"q1.p1": [(kind, answer)]}`, each part's boxes in the order the + platform numbers them. A part with no box is left out. + """ + found: dict[str, list[tuple[str, Any]]] = {} + for i, question in enumerate(_questions(built), 1): + parts = sorted(question.get("parts", []), key=lambda p: p.get("orderNumber", 0)) + for j, part in enumerate(parts, 1): + boxes = sorted( + part.get("responseAreas", []), key=lambda a: a.get("orderNumber", 0) + ) + if boxes: + found[f"q{i}.p{j}"] = [ + ( + box["response"]["responseInput"]["responseType"], + box["response"]["responseInput"]["answer"], + ) + for box in boxes + ] + return found + + +_QUANTITY = re.compile(r"\s*([+-]?\d*\.?\d+(?:[eE][+-]?\d+)?)\s*(.*)", re.S) + + +def _quantity(text: str) -> Optional[tuple[float, str]]: + """A number with units as its value and its unit, or None where it has no number.""" + match = _QUANTITY.fullmatch(text) + if match is None: + return None + # `m^(-3)` and `m^-3` are the same unit; the platform writes either. + unit = re.sub(r"\^\(([^)]*)\)", r"^\1", match.group(2)) + return float(match.group(1)), "".join(unit.split()) + + +def _symbols(text: str) -> str: + """An expression as compared: `^` is `**`, and a bracket round one factor is not one.""" + folded = "".join(text.split()).replace("^", "**") + while True: + once = re.sub(r"\(([A-Za-z0-9_.*]+)\)", r"\1", folded) + if once == folded: + return folded + folded = once + + +def same_answer(kind: str, made: Any, wanted: Any) -> bool: + """Whether two answers of one kind are the same answer. + + A number is the same within half a percent, so that a rounded answer counts; + its unit is compared as written, so kilograms are not grams. An expression is + compared with its spacing and its brackets round single factors dropped. A + multiple choice is the same when the same options are true. + """ + if kind == "MULTIPLE_CHOICE": + return isinstance(made, list) and isinstance(wanted, list) and list(made) == list(wanted) + if not isinstance(made, str) or not isinstance(wanted, str): + return False + if kind == "NUMERIC_UNITS": + ours, theirs = _quantity(made), _quantity(wanted) + if ours is None or theirs is None: + return False + return ours[1] == theirs[1] and abs(ours[0] - theirs[0]) <= 0.005 * abs(theirs[0]) + return _symbols(made) == _symbols(wanted) + + +@dataclass +class Score: + """How many of an export's boxes the agent made, and which it did not. + + Attributes: + matches: The export's boxes the agent made the same way. + total: How many the export holds, which is what a run is scored out of. + misses: One line per box that differs, naming the part, the box's place + in it, and what each side says. + """ + + matches: int = 0 + total: int = 0 + misses: list[str] = field(default_factory=list) + + +def _said(box: Optional[tuple[str, Any]]) -> str: + return "nothing" if box is None else f"{box[0]} {box[1]!r}" + + +def _order(key: str) -> tuple[int, int]: + numbers = re.findall(r"\d+", key) + return int(numbers[0]), int(numbers[1]) + + +def score( + made: dict[str, list[tuple[str, Any]]], wanted: dict[str, list[tuple[str, Any]]] +) -> Score: + """The agent's boxes against an export's, part by part and place by place. + + A box matches when the part and the place in it are the same, the kind is the + same, and the answer is the same once normalised. A part given more boxes + than the export gives it misses on the extra ones, which is what stops a run + scoring by writing a box everywhere. + """ + scored = Score(total=sum(len(boxes) for boxes in wanted.values())) + for key in sorted(set(made) | set(wanted), key=_order): + ours, theirs = made.get(key, []), wanted.get(key, []) + for n in range(max(len(ours), len(theirs))): + box = ours[n] if n < len(ours) else None + want = theirs[n] if n < len(theirs) else None + if box is not None and want is not None and box[0] == want[0] and same_answer(want[0], box[1], want[1]): + scored.matches += 1 + else: + scored.misses.append( + f"{key}[{n + 1}]: wanted {_said(want)}, made {_said(box)}" + ) + return scored diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 3fc9076..890a118 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -9,7 +9,8 @@ for a person (`reconcile`). A field only one route filled is not a disagreement: the text of the route that filled it is taken, and no call is made. A minus sign inside or beside a display maths, which Mathpix reads from a separator line, is flagged too (`stray_minus`). -`to_set` and `build` write the result with in2lambda. +Each settled part is then given its answer boxes by a call of its own +(`response_areas.attach`). `to_set` and `build` write the result with in2lambda. `convert` converts one document. `convert_folder` converts a folder of them: it pairs each sheet with its solutions document, writes one filter from the first pair, and reports for @@ -32,7 +33,7 @@ from in2lambda.api.question import Question from in2lambda.api.set import Set -from in2lambda_agent import pair +from in2lambda_agent import pair, response_areas from in2lambda_agent.model import Backend, Reply, choose_backend from in2lambda_agent.settings import Settings, load_settings @@ -171,15 +172,27 @@ def disputed(a: Reply_, b: Reply_) -> list[str]: return found -def to_set(reply: Reply_, name: str = "set", directory: Optional[Path] = None) -> Set: - """The reply as in2lambda's Set. Images named in the texts are attached where they exist.""" +def to_set( + reply: Reply_, + name: str = "set", + directory: Optional[Path] = None, + areas: Optional[dict[str, list[dict[str, Any]]]] = None, +) -> Set: + """The reply as in2lambda's Set. Images named in the texts are attached where they exist. + + `areas`, where it is given, is the answer boxes proposed for each part by key, + as `response_areas.attach` returns them. + """ built = Set(_name=name) - for q in reply: + for i, q in enumerate(reply, 1): question = Question(title=q.get("title", ""), main_text=q.get("main_text", "")) - for p in q.get("parts", []): - question.parts.append( - Part(text=p.get("content", "") or "", worked_solution=p.get("worked_solution", "") or "", answer=p.get("answer", "") or "") - ) + for j, p in enumerate(q.get("parts", []), 1): + part = Part(text=p.get("content", "") or "", worked_solution=p.get("worked_solution", "") or "", answer=p.get("answer", "") or "") + part.response_areas = [ + response_areas.to_response_area(one, p.get("options") or []) + for one in (areas or {}).get(f"q{i}.p{j}", []) + ] + question.parts.append(part) if directory is not None: for text in [question.main_text] + [t for p in question.parts for t in (p.text, p.worked_solution, p.answer)]: for ref in _IMAGE.findall(text): @@ -415,6 +428,9 @@ class Converted: # twice saves this reply and passes it back as `convert`'s `route_a`; a # second call to the model returns different wording. route_a: Reply_ = field(default_factory=list) + # The answer boxes proposed for each part, by key, for the caller to save and + # hand back as `convert`'s `areas`. + areas: dict[str, list[dict[str, Any]]] = field(default_factory=dict) tokens: int = 0 # The counts of the reconciliation, zero where route B did not run. fields: int = 0 @@ -436,6 +452,8 @@ def report(self) -> list[str]: if one.a and one.b: lines += [f" A: {_squash(one.a)}", f" B: {_squash(one.b)}"] lines.append(f"fields {self.counted()}") + if self.areas: + lines.append(f"areas {_areas_counted(self.areas)}") if self.route_b_error: lines.append(f"route B failed: {self.route_b_error}") if self.zip_path: @@ -457,6 +475,11 @@ def counted(self) -> str: return f"{len(fields(normalise(self.reply)))} fields, route B {ran}" +def _areas_counted(areas: dict[str, list[dict[str, Any]]]) -> str: + """The `areas` line: how many boxes were proposed, over how many parts.""" + return f"{sum(len(one) for one in areas.values())} for {len(areas)} parts" + + _UNDERLINE = Path(__file__).parent / "underline.lua" @@ -511,6 +534,7 @@ def convert( name: str = "set", on_stage: Optional[Callable[[str, str], None]] = None, route_a: Optional[Reply_] = None, + areas: Optional[dict[str, list[dict[str, Any]]]] = None, ) -> Converted: """Route A, route B where a filter is given, reconcile, verify, write. @@ -523,9 +547,15 @@ def convert( by a caller who needs the same answer twice: route A is not called, and the result's `route_a` is what was given. + `areas` is the same for the answer boxes: where it is given, no part is asked about + and the boxes given are written, so `{}` is a document whose parts get none. Where it + is not, every part with something to answer is asked about (`response_areas.attach`) + and a refused proposal is a flag. + `on_stage`, where it is given, is called with a name and a message as each step - finishes - `ocr`, `route A`, `route B`, `fields`, `build` - so that a caller watching - a run shows each line as the step ends rather than the report at the end of it. + finishes - `ocr`, `route A`, `route B`, `areas`, `fields`, `build` - so that a caller + watching a run shows each line as the step ends rather than the report at the end of + it. """ settings = settings or load_settings() backend = backend or choose_backend(settings) @@ -574,9 +604,21 @@ def said(stage: str, message: str) -> None: for k in stray_minus(reply): if not any(f.field == k for f in flags): flags.append(Flag(k, fields(reply)[k], "", STRAY_MINUS)) + if areas is None: + attached = response_areas.attach(reply, backend) + areas = attached.proposals + tokens += attached.tokens + flags += [ + Flag(f"{key}.areas", "", "", reason) + for key, reasons in attached.refused.items() + for reason in reasons + ] + said("areas", f"{_areas_counted(areas)}, {attached.tokens} tokens") + else: + said("areas", "given, no call made") result = Converted( - set=to_set(reply, name=name, directory=images), zip_path=None, flags=flags, - reply=reply, route_a=route_a, tokens=tokens, + set=to_set(reply, name=name, directory=images, areas=areas), zip_path=None, flags=flags, + reply=reply, route_a=route_a, areas=areas, tokens=tokens, fields=counts[0], agreed=counts[1], defaulted=counts[2], adjudicated=counts[3], route_b_error=error, ) diff --git a/in2lambda_agent/targets.py b/in2lambda_agent/targets.py index 5297353..719e3d9 100644 --- a/in2lambda_agent/targets.py +++ b/in2lambda_agent/targets.py @@ -17,12 +17,17 @@ A `differs.txt` line names a field rather than a sentence because the report quotes a model's wording. A model writes the same field differently each time -it is asked. The filter and route A's reply are saved beside each other and -read back for the same reason: a second run over a target makes neither of the -two calls that read the document, so the two runs differ from the export in the -same fields. `fresh` reads the documents again and writes a new reply. +it is asked. The filter, route A's reply and the answer boxes proposed for the +parts are saved beside each other and read back for the same reason: a second +run over a target makes none of the calls that read the document, so the two +runs differ from the export in the same fields. `fresh` reads the documents +again and writes a new reply. -Those are the only two calls a run saves. A target with a filter runs route B +The answer boxes are scored rather than compared: `in2lambda.compare` does not +read them, and a box is either the export's box or it is wrong, so a run reports +how many of the export's boxes it made and names each one it did not. + +Those are the only calls a run saves. A target with a filter runs route B on every run, and `routes.reconcile` has a model adjudicate every field the two routes word differently. A verdict can go the other way on a later run, so the wording of a difference and the number of fields flagged move between runs @@ -42,7 +47,7 @@ from in2lambda.api.set import Set from in2lambda.compare import differences -from in2lambda_agent import pair, routes +from in2lambda_agent import pair, response_areas, routes from in2lambda_agent.model import Backend, choose_backend from in2lambda_agent.settings import Settings, load_settings @@ -61,6 +66,11 @@ """Route A's reply, beside the target's filter: read back by every run after the first, so that a target is converted the same way twice.""" +AREAS_NAME = "areas.json" +"""The answer boxes proposed for each part, beside the target's reply and read +back with it: a part is asked about once ever, so a second run writes the same +boxes into the set and scores the same against the export.""" + EXPORT_PREFIX = "set_" """What Lambda Feedback names an exported set's folder with. The rest of the name is the set's own, which is what the conversion is built under so that the @@ -101,10 +111,12 @@ class Result: new: Those in a field it does not, which is what a run is read for. agreed: The keys it accepts that nothing differs in any more, which are lines to take out of it. + areas: The answer boxes the conversion made against the export's, which + `in2lambda.compare` does not read. flags: How many fields the conversion flagged for a person. - tokens: What route A cost, and nothing where the saved reply was read - back. The adjudication a target with a filter pays for on every run - is not counted here. + tokens: What route A and the answer boxes cost, and nothing where both + were read back from what was saved. The adjudication a target with a + filter pays for on every run is not counted here. error: What stopped the target, and nothing else filled. """ @@ -113,15 +125,18 @@ class Result: known: list[str] = field(default_factory=list) new: list[str] = field(default_factory=list) agreed: list[str] = field(default_factory=list) + areas: Optional[response_areas.Score] = None flags: int = 0 tokens: int = 0 error: Optional[str] = None def report(self) -> list[str]: - """The new differences, then the accepted ones, then the counts. + """The new differences, then the accepted ones, then the areas, then the counts. A known difference is printed as well as a new one, so that the - maintainer reads what a field the `differs.txt` accepts says now. + maintainer reads what a field the `differs.txt` accepts says now. Every + answer box the conversion did not make is a line of its own: a box is + either the export's or it is wrong, and there is nothing to accept. """ if self.error: return [f"error {self.name}: {self.error}"] @@ -132,6 +147,15 @@ def report(self) -> list[str]: f"agrees {self.name}: {key} now agrees, remove the line" for key in self.agreed ] + + ( + [ + f"areas {self.name}: {self.areas.matches} of " + f"{self.areas.total} match" + ] + + [f"miss {self.name}: {line}" for line in self.areas.misses] + if self.areas is not None + else [] + ) + [ f"{self.name}: {len(self.differences)} differ, " f"{len(self.known)} known, {len(self.new)} new, " @@ -283,11 +307,12 @@ def run_one( settings: The environment the run has available. backend: The backend to write a filter with, chosen from the settings if absent. - fresh: Read the document again rather than converting the reply saved - beside the filter, which is how a target is given a new reading. - replay: Refuse a target whose filter or reply is not saved rather than - paying for one, so that the run reads what is committed and makes - no call that reads the document. + fresh: Read the document again rather than converting the reply and the + answer boxes saved beside the filter, which is how a target is given + a new reading. + replay: Refuse a target whose filter, reply or answer boxes are not + saved rather than paying for them, so that the run reads what is + committed and makes no call that reads the document. Returns: The target's result. Nothing a target raises leaves this function: what @@ -299,12 +324,13 @@ def run_one( backend = backend or choose_backend(settings) saved = Path(filters) / target.name reply = saved / REPLY_NAME + boxes = saved / AREAS_NAME # Pandoc reads neither a PDF nor the markdown an OCR made of one back into # the document's structure, so route B cannot run over a scanned target: # it converts through route A alone, and no filter is written for it. lua = None if target.questions.suffix.lower() == ".pdf" else saved / FILTER_NAME if replay: - absent = [one for one in (reply, lua) if one is not None and not one.is_file()] + absent = [one for one in (reply, boxes, lua) if one is not None and not one.is_file()] if absent: return Result( name=target.name, @@ -312,16 +338,21 @@ def run_one( f"reads a document. `in2lambda-agent targets ROOT --filters " f"{filters}` writes it.", ) - route_a = None + + def read_back(file: Path): + """What an earlier run saved there, or None for a run to write it again.""" + if fresh or not file.is_file(): + return None + try: + return json.loads(file.read_text(encoding="utf-8")) + except ValueError as problem: + # A run interrupted while writing the file leaves part of a JSON + # document behind, and json.loads names a column of it and no file. + # The name of the file is what the maintainer needs. + raise ValueError(f"{file}: {problem}; --fresh writes a new one") + try: - if reply.is_file() and not fresh: - try: - route_a = json.loads(reply.read_text(encoding="utf-8")) - except ValueError as problem: - # A run interrupted while writing the reply leaves part of a - # JSON document behind, and json.loads names a column of it and - # no file. The name of the file is what the maintainer needs. - raise ValueError(f"{reply}: {problem}; --fresh writes a new one") + route_a, areas = read_back(reply), read_back(boxes) if lua is not None and not lua.is_file(): saved.mkdir(parents=True, exist_ok=True) lua.write_text( @@ -340,12 +371,16 @@ def run_one( # holds and the two are compared file by file. name=target.export.name[len(EXPORT_PREFIX) :], route_a=route_a, + areas=areas, ) - if route_a is None: - # The reply is written before the comparison, so that a comparison - # the export's files break does not discard the model's answer. + if route_a is None or areas is None: + # What the model answered is written before the comparison, so that + # a comparison the export's files break does not discard it. saved.mkdir(parents=True, exist_ok=True) - reply.write_text(json.dumps(converted.route_a, indent=2), encoding="utf-8") + if route_a is None: + reply.write_text(json.dumps(converted.route_a, indent=2), encoding="utf-8") + if areas is None: + boxes.write_text(json.dumps(converted.areas, indent=2), encoding="utf-8") found = differences( Set.from_json(str(converted.zip_path)), Set.from_json(str(target.export)), @@ -360,6 +395,10 @@ def run_one( known=[line for line, key in zip(found, keys) if key in accepts], new=[line for line, key in zip(found, keys) if key not in accepts], agreed=[key for key in accepts if key not in set(keys)], + areas=response_areas.score( + response_areas.areas_of(converted.zip_path), + response_areas.areas_of(target.export), + ), flags=len(converted.flags), tokens=converted.tokens, ) @@ -393,10 +432,9 @@ def run( cache_dir: Where the OCR of each PDF is kept. settings: The environment the runs have available. backend: The backend to write the filters with. - fresh: Read every document again rather than converting the saved - replies. - replay: Refuse a target whose filter or reply is not saved rather than - paying for one. + fresh: Read every document again rather than converting what was saved. + replay: Refuse a target whose filter, reply or answer boxes are not + saved rather than paying for them. Returns: One result per target, in the order they ran. diff --git a/tests/conftest.py b/tests/conftest.py index e0e7ced..e676aef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,13 +20,20 @@ class FakeBackend: against whatever they were built over, exactly as a real backend's loop runs them. That is what scripts a fixing round without a model in it. A reply that is an exception is raised, which scripts a call that does not finish. + + `default` is what to answer with once the scripted replies run out: a test + of one call over a document that then asks about each of its parts scripts + the one reply it is about and gives `default="[]"` for the rest. Without it + a call past the end of the script raises, so a test that counts its calls + says so by leaving `default` alone. """ name = "fake" - def __init__(self, *replies, reason=None): + def __init__(self, *replies, reason=None, default=None): self.replies = list(replies) self.reason = reason + self.default = default self.calls: list[tuple[str, str]] = [] self.images: list[list[bytes]] = [] @@ -36,7 +43,7 @@ def unavailable(self): def call(self, system, prompt, tools=(), images=()): self.calls.append((system, prompt)) self.images.append(list(images)) - reply = self.replies.pop(0) + reply = self.replies.pop(0) if self.replies or self.default is None else self.default if isinstance(reply, Exception): raise reply made = [] diff --git a/tests/test_gate.py b/tests/test_gate.py index 2568a6f..80cebbe 100644 --- a/tests/test_gate.py +++ b/tests/test_gate.py @@ -20,12 +20,14 @@ CI_FILTERS = Path(__file__).parent.parent / "ci-corpus" / "filters" -def save(filters, name, *, reply=True, lua=True): +def save(filters, name, *, reply=True, areas=True, lua=True): """What a target must have saved for the gate to replay it.""" saved = Path(filters) / name saved.mkdir(parents=True, exist_ok=True) if reply: (saved / targets.REPLY_NAME).write_text(json.dumps(REPLY)) + if areas: + (saved / targets.AREAS_NAME).write_text("{}") if lua: (saved / targets.FILTER_NAME).write_text("-- filter") return saved diff --git a/tests/test_response_areas.py b/tests/test_response_areas.py new file mode 100644 index 0000000..07b39fa --- /dev/null +++ b/tests/test_response_areas.py @@ -0,0 +1,527 @@ +"""The answer boxes a part gets, and how they are scored against an export. + +Written before the module, over the ME2 fixtures: the export's own response +areas are read back from the platform's JSON, written into a set through +in2lambda, and scored against themselves, so the round trip is tested without a +model in it. What a model proposes is scripted with a fake backend, one reply a +part. The live test converts the ME2 pair and reports what it matched. +""" + +import json +import os +import zipfile +from pathlib import Path + +import pytest + +from conftest import FakeBackend + +from in2lambda_agent import gate, response_areas, routes, targets +from in2lambda_agent.settings import Settings + +ME2 = Path(__file__).parent / "fixtures" / "me2" +REPLY = json.loads((ME2 / "direct.json").read_text()) +EXPORT = ME2 / "export" +ME2_TARGET = Path( + "/Users/peterbjohnson/code/lambdafeedback/in2lambda-agent/ExampleContents/targets/" + "ME2_Fluids_introduction" +) +OUT = Path(__file__).parent.parent / "out" + +live = pytest.mark.skipif( + not os.environ.get("IN2LAMBDA_LIVE"), reason="calls Mathpix and a model" +) + + +def proposals_of(found): + """The areas of an export as proposals a part's call could have made.""" + return { + key: [{"kind": kind, "pre_text": "", "answer": answer} for kind, answer in boxes] + for key, boxes in found.items() + } + + +# --- one part's call -------------------------------------------------------------------- + + +def test_a_part_with_a_number_for_an_answer_is_a_numeric_box(): + backend = FakeBackend( + json.dumps([{"kind": "NUMERIC_UNITS", "pre_text": "$m=$", "answer": "0.106 kg"}]) + ) + + proposed, refused, reply = response_areas.propose( + "What mass does the scale read?", [], "$m = 0.106\\,\\mathrm{kg}$", backend + ) + + assert refused == [] + assert proposed == [ + {"kind": "NUMERIC_UNITS", "pre_text": "$m=$", "answer": "0.106 kg"} + ] + area = response_areas.to_response_area(proposed[0], []) + assert area.response_type == "NUMERIC_UNITS" + assert area.evaluation_function == "comparePhysicalQuantities" + assert area.answer == "0.106 kg" + assert area.pre_text == "$m=$" + assert area.config is None + # The prompt carries the part, so that the box is proposed for what it asks. + assert "What mass does the scale read?" in backend.calls[0][1] + + +def test_a_part_with_a_symbolic_answer_is_a_maths_box(): + backend = FakeBackend( + json.dumps( + [ + { + "kind": "MATH_SINGLE_LINE", + "pre_text": "$F=$", + "answer": "(pi/6)*rho*U**2*R**2", + } + ] + ) + ) + + proposed, refused, _ = response_areas.propose( + "Find the drag force.", [], "$F = \\pi \\rho U^2 R^2 / 6$", backend + ) + + assert refused == [] + area = response_areas.to_response_area(proposed[0], []) + assert area.response_type == "MATH_SINGLE_LINE" + assert area.evaluation_function == "symbolicEqual" + assert area.answer == "(pi/6)*rho*U**2*R**2" + + +def test_a_multiple_choice_part_is_one_true_or_false_per_option(): + options = ["Yes", "No"] + backend = FakeBackend( + json.dumps([{"kind": "MULTIPLE_CHOICE", "pre_text": "", "answer": [True, False]}]) + ) + + proposed, refused, _ = response_areas.propose( + "Is the continuum assumption valid?", options, "Yes", backend + ) + + assert refused == [] + area = response_areas.to_response_area(proposed[0], options) + assert area.response_type == "MULTIPLE_CHOICE" + assert area.evaluation_function == "arrayEqual" + assert area.answer == [True, False] + assert area.config == {"single": True, "options": options, "randomise": False} + + +def test_a_part_with_more_than_one_correct_option_is_not_a_single_choice(): + options = ["A", "B", "C"] + backend = FakeBackend( + json.dumps( + [{"kind": "MULTIPLE_CHOICE", "pre_text": "", "answer": [True, False, True]}] + ) + ) + + proposed, _, _ = response_areas.propose("Which hold?", options, "A and C", backend) + + assert response_areas.to_response_area(proposed[0], options).config["single"] is False + + +def test_a_part_that_asks_for_a_discussion_gets_no_box(): + backend = FakeBackend("[]") + + proposed, refused, _ = response_areas.propose( + "Comment on what this means for the flow.", [], "", backend + ) + + assert (proposed, refused) == ([], []) + + +def test_a_part_may_be_given_more_than_one_box(): + backend = FakeBackend( + json.dumps( + [ + {"kind": "NUMERIC_UNITS", "pre_text": "$\\ell_0=$", "answer": "3.58e-7 m"}, + {"kind": "NUMERIC_UNITS", "pre_text": "$d=$", "answer": "0.541 mm"}, + ] + ) + ) + + proposed, refused, _ = response_areas.propose("Find both.", [], "", backend) + + assert [one["pre_text"] for one in proposed] == ["$\\ell_0=$", "$d=$"] + assert refused == [] + + +@pytest.mark.parametrize( + "answered,says", + [ + ('[{"kind": "FREE_TEXT", "answer": "anything"}]', "FREE_TEXT"), + # One boolean per option, and this part has two. + ('[{"kind": "MULTIPLE_CHOICE", "answer": [true, false, false]}]', "[True, False, False]"), + # Nothing is correct, so nothing can be marked correct. + ('[{"kind": "MULTIPLE_CHOICE", "answer": [false, false]}]', "[False, False]"), + ('[{"kind": "NUMERIC_UNITS", "answer": 0.106}]', "0.106"), + ('[{"kind": "NUMERIC_UNITS", "answer": ""}]', "''"), + ("The part asks for a discussion.", "not JSON"), + ('{"kind": "NUMERIC_UNITS", "answer": "1 m"}', "JSON list"), + ], +) +def test_a_proposal_the_platform_could_not_mark_is_refused_with_the_reason(answered, says): + proposed, refused, _ = response_areas.propose( + "Is it valid?", ["Yes", "No"], "Yes", FakeBackend(answered) + ) + + assert proposed == [] + assert len(refused) == 1 + assert says in refused[0] + + +# --- a whole reply ---------------------------------------------------------------------- + + +def test_every_part_with_something_to_answer_is_asked_about_once(): + backend = FakeBackend(*["[]"] * 12) + + attached = response_areas.attach(REPLY, backend) + + # The ME2 reply has twelve parts, each with a statement of its own. + assert len(backend.calls) == 12 + assert attached.proposals == {} + assert attached.refused == {} + assert attached.tokens == sum(len(prompt) for _, prompt in backend.calls) + 12 * 2 + + +def test_a_part_with_nothing_in_it_is_not_asked_about(): + empty = [{"title": "", "main_text": "A ball is thrown up.", "parts": [{"content": "", "options": [], "answer": "", "worked_solution": ""}]}] + backend = FakeBackend() # No replies: a call would raise rather than answer. + + attached = response_areas.attach(empty, backend) + + assert backend.calls == [] + assert attached.proposals == {} + + +def test_a_refused_proposal_is_handed_back_under_the_part_it_was_made_for(): + reply = [ + { + "title": "", + "main_text": "", + "parts": [ + {"content": "Find the mass.", "options": [], "answer": "0.106 kg", "worked_solution": ""}, + {"content": "Find the force.", "options": [], "answer": "30 N", "worked_solution": ""}, + ], + } + ] + backend = FakeBackend( + "I am afraid I cannot help with that.", + json.dumps([{"kind": "NUMERIC_UNITS", "pre_text": "$F=$", "answer": "30 N"}]), + ) + + attached = response_areas.attach(reply, backend) + + assert list(attached.refused) == ["q1.p1"] + assert "not JSON" in attached.refused["q1.p1"][0] + # The part after it is still asked about and still written. + assert list(attached.proposals) == ["q1.p2"] + + +# --- what the export says ---------------------------------------------------------------- + + +def test_the_exports_areas_are_read_by_part_in_the_order_the_platform_shows_them(): + found = response_areas.areas_of(EXPORT) + + assert found["q1.p1"] == [("NUMERIC_UNITS", "0.106 kg")] + assert found["q3.p1"] == [("MATH_SINGLE_LINE", "(pi/6)*(rho)*(U**2)*(R**2)")] + # Two boxes in one part, taken in the order the platform numbers them. + assert found["q4.p2"] == [ + ("NUMERIC_UNITS", "3.58e-7 m"), + ("NUMERIC_UNITS", "0.541 mm"), + ] + assert found["q4.p3"] == [("MULTIPLE_CHOICE", [True, False])] + # A part with no box is not a part with an empty one. + assert "q1.p2" not in found + + +def test_the_exports_own_areas_written_back_through_in2lambda_score_every_one(tmp_path): + wanted = response_areas.areas_of(EXPORT) + built = routes.to_set( + REPLY, name="Introduction", areas=proposals_of(wanted) + ) + + zip_path = routes.build(built, tmp_path / "out") + made = response_areas.areas_of(zip_path) + scored = response_areas.score(made, wanted) + + assert scored.misses == [] + assert scored.total == sum(len(boxes) for boxes in wanted.values()) + assert scored.matches == scored.total + # The zip is what the platform reads, so the areas are read back out of it. + assert "question_000_Hydraulic_scale.json" in zipfile.ZipFile(zip_path).namelist() + + +# --- scoring ----------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "kind,made,wanted,same", + [ + ("NUMERIC_UNITS", "2.65e25 m^-3", "2.65e+25 m^(-3)", True), + ("NUMERIC_UNITS", "0.106 kg", "0.106kg", True), + ("NUMERIC_UNITS", "0.1063 kg", "0.106 kg", True), + ("NUMERIC_UNITS", "0.2 kg", "0.106 kg", False), + ("NUMERIC_UNITS", "0.106 g", "0.106 kg", False), + ("NUMERIC_UNITS", "kg", "0.106 kg", False), + ("MATH_SINGLE_LINE", "(pi/6)*rho*U**2*R**2", "(pi/6)*(rho)*(U**2)*(R**2)", True), + ("MATH_SINGLE_LINE", "(pi/6) * rho * U^2 * R^2", "(pi/6)*(rho)*(U**2)*(R**2)", True), + ("MATH_SINGLE_LINE", "(pi/6)*rho*U**3*R**2", "(pi/6)*(rho)*(U**2)*(R**2)", False), + ("MULTIPLE_CHOICE", [True, False], [True, False], True), + ("MULTIPLE_CHOICE", [False, True], [True, False], False), + ("MULTIPLE_CHOICE", [True], [True, False], False), + ], +) +def test_an_answer_matches_the_export_once_units_and_symbols_are_normalised( + kind, made, wanted, same +): + assert response_areas.same_answer(kind, made, wanted) is same + + +def test_every_miss_names_the_part_the_box_and_both_answers(): + wanted = { + "q1.p1": [("NUMERIC_UNITS", "0.106 kg")], + "q2.p1": [("MATH_SINGLE_LINE", "2*x"), ("NUMERIC_UNITS", "30 N")], + } + made = { + "q1.p1": [("NUMERIC_UNITS", "0.2 kg")], + "q2.p1": [("NUMERIC_UNITS", "2 x")], + "q3.p1": [("MULTIPLE_CHOICE", [True, False])], + } + + scored = response_areas.score(made, wanted) + + assert (scored.matches, scored.total) == (0, 3) + assert scored.misses == [ + "q1.p1[1]: wanted NUMERIC_UNITS '0.106 kg', made NUMERIC_UNITS '0.2 kg'", + "q2.p1[1]: wanted MATH_SINGLE_LINE '2*x', made NUMERIC_UNITS '2 x'", + "q2.p1[2]: wanted NUMERIC_UNITS '30 N', made nothing", + "q3.p1[1]: wanted nothing, made MULTIPLE_CHOICE [True, False]", + ] + + +def test_a_part_scored_right_is_counted_and_not_listed(): + wanted = {"q1.p1": [("NUMERIC_UNITS", "0.106 kg")], "q1.p2": [("MATH_SINGLE_LINE", "2*x")]} + made = {"q1.p1": [("NUMERIC_UNITS", "0.106kg")], "q1.p2": [("MATH_SINGLE_LINE", "2*y")]} + + scored = response_areas.score(made, wanted) + + assert (scored.matches, scored.total) == (1, 2) + assert [miss.split(":")[0] for miss in scored.misses] == ["q1.p2[1]"] + + +# --- the conversion ---------------------------------------------------------------------- + + +def test_a_conversion_proposes_the_areas_and_writes_them_into_the_set(tmp_path): + numeric = json.dumps( + [{"kind": "NUMERIC_UNITS", "pre_text": "$m=$", "answer": "0.106 kg"}] + ) + backend = FakeBackend(json.dumps(REPLY), *[numeric] * 12) + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + ) + + # One call reading the documents, then one for each of the twelve parts. + assert len(backend.calls) == 13 + assert result.set.questions[0].parts[0].response_areas[0].answer == "0.106 kg" + assert list(result.areas) == [f"q{i}.p{j}" for i in range(1, 6) for j in range(1, len(REPLY[i - 1]["parts"]) + 1)] + written = json.loads( + zipfile.ZipFile(result.zip_path).read("question_000_Hydraulic_scale.json") + ) + area = written["parts"][0]["responseAreas"][0] + assert area["response"]["responseInput"]["responseType"] == "NUMERIC_UNITS" + assert area["evaluationFunctionName"] == "comparePhysicalQuantities" + + +def test_areas_given_to_a_conversion_are_written_and_nothing_is_asked(tmp_path): + # What a targets run hands back from the proposals it saved, and what the + # gate replays: no part is asked about again. + backend = FakeBackend() # No replies: a call would raise rather than answer. + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + route_a=REPLY, + areas={"q1.p1": [{"kind": "NUMERIC_UNITS", "pre_text": "$m=$", "answer": "0.106 kg"}]}, + ) + + assert backend.calls == [] + assert result.tokens == 0 + assert [len(p.response_areas) for p in result.set.questions[0].parts] == [1, 0] + + +def test_a_conversion_told_there_are_no_areas_asks_nothing(tmp_path): + backend = FakeBackend() # No replies: a call would raise rather than answer. + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + route_a=REPLY, + areas={}, + ) + + assert backend.calls == [] + assert result.areas == {} + assert all(p.response_areas == [] for q in result.set.questions for p in q.parts) + + +def test_a_refused_proposal_is_flagged_and_the_set_is_still_written(tmp_path): + backend = FakeBackend("no thank you", *["[]"] * 11) + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=backend, + settings=Settings(), + route_a=REPLY, + ) + + assert "q1.p1.areas" in [one.field for one in result.flags] + assert result.zip_path.is_file() + + +def test_the_areas_are_a_stage_of_their_own_and_a_line_of_the_report(tmp_path): + seen = [] + numeric = json.dumps([{"kind": "NUMERIC_UNITS", "pre_text": "", "answer": "1 m"}]) + + result = routes.convert( + ME2 / "questions.md", + solutions=ME2 / "solutions.md", + out_dir=tmp_path / "out", + backend=FakeBackend(*[numeric] * 12), + settings=Settings(), + route_a=REPLY, + on_stage=lambda name, message: seen.append((name, message)), + ) + + assert [name for name, _ in seen] == ["ocr", "route A", "route B", "areas", "fields", "build"] + # The stage line says what the call cost as route A's does; the report, which + # is read after the run, says what the set got. + assert dict(seen)["areas"].startswith("12 for 12 parts, ") + assert "areas 12 for 12 parts" in result.report() + + +# --- a target ------------------------------------------------------------------------------ + + +def target_of(tmp_path): + """The ME2 documents and export as a target, as test_targets makes one.""" + from test_targets import make_target + + make_target(tmp_path / "corpus", "ME2") + (found,) = targets.find(tmp_path / "corpus") + return found + + +def test_a_target_saves_what_it_proposed_and_replays_it_with_no_call(tmp_path, monkeypatch): + from test_targets import fake_convert + + calls = fake_convert(monkeypatch, areas={"q1.p1": [{"kind": "NUMERIC_UNITS", "pre_text": "", "answer": "0.106 kg"}]}) + target = target_of(tmp_path) + filters = tmp_path / "filters" + (filters / "ME2").mkdir(parents=True) + (filters / "ME2" / targets.FILTER_NAME).write_text("-- filter") + + first = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend(), + ) + saved = json.loads((filters / "ME2" / targets.AREAS_NAME).read_text()) + second = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend(), replay=True, + ) + + assert first.error is None and second.error is None + assert saved == {"q1.p1": [{"kind": "NUMERIC_UNITS", "pre_text": "", "answer": "0.106 kg"}]} + # The second run converted what the first proposed rather than proposing again. + assert calls[1]["areas"] == saved + + +def test_a_replay_refuses_a_target_whose_areas_are_not_saved(tmp_path, monkeypatch): + from test_targets import fake_convert + + calls = fake_convert(monkeypatch) + target = target_of(tmp_path) + filters = tmp_path / "filters" + (filters / "ME2").mkdir(parents=True) + (filters / "ME2" / targets.FILTER_NAME).write_text("-- filter") + (filters / "ME2" / targets.REPLY_NAME).write_text(json.dumps(REPLY)) + + result = targets.run_one( + target, filters=filters, out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend(), replay=True, + ) + + assert targets.AREAS_NAME in result.error + assert f"--filters {filters}" in result.error + assert calls == [] + + +def test_a_targets_report_counts_the_areas_it_matched_and_lists_the_misses(tmp_path, monkeypatch): + from test_targets import fake_convert + + wanted = response_areas.areas_of(EXPORT) + # Every area of the export but one, which the report must name. + proposals = proposals_of(wanted) + proposals["q1.p1"] = [{"kind": "NUMERIC_UNITS", "pre_text": "", "answer": "9 kg"}] + fake_convert(monkeypatch, areas=proposals) + target = target_of(tmp_path) + + result = targets.run_one( + target, filters=tmp_path / "filters", out_dir=tmp_path / "out", + cache_dir=tmp_path / "cache", backend=FakeBackend("-- filter"), + ) + + total = sum(len(boxes) for boxes in wanted.values()) + assert (result.areas.matches, result.areas.total) == (total - 1, total) + assert f"areas ME2: {total - 1} of {total} match" in result.report() + assert [line for line in result.report() if line.startswith("miss")] == [ + "miss ME2: q1.p1[1]: wanted NUMERIC_UNITS '0.106 kg', made NUMERIC_UNITS '9 kg'" + ] + + +# --- live ----------------------------------------------------------------------------------- + + +@live +@pytest.mark.skipif(not ME2_TARGET.is_dir(), reason="private corpus") +def test_the_me2_target_scores_its_areas_against_the_export(tmp_path): + # The ticket's run: the areas the agent proposes for the ME2 pair, against + # the ones the platform exported. The report is written under out/ and read + # back from there, so what the pull request quotes is a file. + (found,) = targets.find(ME2_TARGET.parent, [Path(ME2_TARGET.name)]) + + result = targets.run_one( + found, + filters=tmp_path / "filters", + out_dir=tmp_path / "out", + # The gate's cache, so that the pages are not read by Mathpix again. + cache_dir=gate.DEFAULT_CACHE_DIR, + ) + + OUT.mkdir(parents=True, exist_ok=True) + report = OUT / "t44-me2-areas.txt" + report.write_text("\n".join(result.report()) + "\n") + print("\n" + report.read_text()) + assert result.error is None + assert result.areas.total == sum( + len(boxes) for boxes in response_areas.areas_of(found.export).values() + ) diff --git a/tests/test_routes.py b/tests/test_routes.py index c6010db..777e12b 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -133,7 +133,9 @@ def test_convert_reports_the_stray_minus_as_a_flag(tmp_path): ME2 / "questions.md", solutions=ME2 / "solutions.md", out_dir=tmp_path / "out", - backend=FakeBackend(json.dumps(REPLY)), + # The parts are asked for their answer boxes after the fields are + # settled, and this document's are tested in test_response_areas. + backend=FakeBackend(json.dumps(REPLY), default="[]"), settings=Settings(), ) assert [(f.field, f.reason) for f in result.flags] == [ @@ -156,6 +158,7 @@ def test_a_reply_given_to_convert_is_route_as_and_no_call_is_made(tmp_path): backend=backend, settings=Settings(), route_a=REPLY, + areas={}, ) assert backend.calls == [] @@ -505,11 +508,11 @@ def test_convert_reports_each_stage_as_it_happens(tmp_path): ME2 / "questions.md", solutions=ME2 / "solutions.md", out_dir=tmp_path / "out", - backend=FakeBackend(json.dumps(REPLY)), + backend=FakeBackend(json.dumps(REPLY), default="[]"), settings=Settings(), on_stage=lambda name, message: seen.append((name, message)), ) - assert [name for name, _ in seen] == ["ocr", "route A", "route B", "fields", "build"] + assert [name for name, _ in seen] == ["ocr", "route A", "route B", "areas", "fields", "build"] assert dict(seen)["ocr"] == "questions.md: read; solutions.md: read" assert dict(seen)["route B"] == "did not run: no filter" # The stage line is the line the report prints for the same run. diff --git a/tests/test_targets.py b/tests/test_targets.py index f8c2800..6f7baf6 100644 --- a/tests/test_targets.py +++ b/tests/test_targets.py @@ -40,27 +40,32 @@ def make_target(root, name, *, questions="sheet.md", solutions="sheet_solutions. return folder -def fake_convert(monkeypatch, flags=(), error=None, reply=None): +def fake_convert(monkeypatch, flags=(), error=None, reply=None, areas=None): """Stands in for a conversion: builds the fixture reply, records the call. A saved reply handed back is what route A answered, as `convert` uses it, so - a second run over a target reads the first run's reply where it has one. + a second run over a target reads the first run's reply where it has one. The + answer boxes work the same way: `areas` is what a run that was given none + proposes. """ calls = [] reply = REPLY if reply is None else reply + areas = {} if areas is None else areas def convert(document, solutions=None, **options): calls.append({"document": document, "solutions": solutions, **options}) if error is not None: raise error answered = options.get("route_a") or reply - built = routes.to_set(answered, name=options["name"]) + boxes = areas if options.get("areas") is None else options["areas"] + built = routes.to_set(answered, name=options["name"], areas=boxes) return routes.Converted( set=built, zip_path=routes.build(built, options["out_dir"]), flags=list(flags), reply=answered, route_a=answered, + areas=boxes, tokens=1200, ) @@ -345,10 +350,13 @@ def test_a_replay_refuses_a_target_whose_filter_or_reply_is_not_saved( no_reply = targets.run_one(target, **ran) (filters / "ME2").mkdir(parents=True) (filters / "ME2" / targets.REPLY_NAME).write_text(json.dumps(REPLY)) + no_areas = targets.run_one(target, **ran) + (filters / "ME2" / targets.AREAS_NAME).write_text("{}") no_filter = targets.run_one(target, **ran) assert targets.REPLY_NAME in no_reply.error assert f"--filters {filters}" in no_reply.error + assert targets.AREAS_NAME in no_areas.error assert targets.FILTER_NAME in no_filter.error assert calls == [] @@ -363,6 +371,7 @@ def test_a_replay_of_a_saved_target_reports_its_differences_and_calls_nothing( (filters / "ME2").mkdir(parents=True) (filters / "ME2" / targets.FILTER_NAME).write_text("-- filter") (filters / "ME2" / targets.REPLY_NAME).write_text(json.dumps(REPLY)) + (filters / "ME2" / targets.AREAS_NAME).write_text("{}") backend = FakeBackend() result = targets.run_one( @@ -376,9 +385,9 @@ def test_a_replay_of_a_saved_target_reports_its_differences_and_calls_nothing( assert backend.calls == [] -def test_a_replay_of_a_scanned_target_needs_only_the_reply(tmp_path, monkeypatch): - # There is no filter for a PDF target to save, so the reply is all a replay - # of one reads. +def test_a_replay_of_a_scanned_target_needs_no_filter(tmp_path, monkeypatch): + # There is no filter for a PDF target to save, so the reply and the answer + # boxes are all a replay of one reads. calls = fake_convert(monkeypatch) make_target( tmp_path / "corpus", "ME2", @@ -388,6 +397,7 @@ def test_a_replay_of_a_scanned_target_needs_only_the_reply(tmp_path, monkeypatch filters = tmp_path / "filters" (filters / "ME2").mkdir(parents=True) (filters / "ME2" / targets.REPLY_NAME).write_text(json.dumps(REPLY)) + (filters / "ME2" / targets.AREAS_NAME).write_text("{}") result = targets.run_one( target, filters=filters, out_dir=tmp_path / "out",