From ab47236b1688702d08f24389c863a5d0de31d53d Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Tue, 22 Sep 2026 23:06:11 +0100 Subject: [PATCH 1/4] implement: Add route B, a pandoc filter per folder, and compare it with route A (t37) --- docs/plan.md | 4 +- in2lambda_agent/pair.py | 33 +++++- in2lambda_agent/routes.py | 193 ++++++++++++++++++++++++++++++--- tests/fixtures/pair-filter.lua | 65 +++++++++++ tests/fixtures/role-filter.lua | 7 ++ tests/test_pair.py | 23 ++++ tests/test_routes.py | 149 +++++++++++++++++++++++++ 7 files changed, 451 insertions(+), 23 deletions(-) create mode 100644 tests/fixtures/pair-filter.lua create mode 100644 tests/fixtures/role-filter.lua diff --git a/docs/plan.md b/docs/plan.md index 8d343e1..13b9847 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -107,7 +107,9 @@ under a cent. One hundred sheets in ten folders through both routes cost $7 to $ The ME2 pair, live: 5 questions, 60 fields, 0 flags, titles equal to the export's, one model call, 66 seconds with the OCR cached. The PHYS40002 folder, route B: one filter written from the first sheet in 41 seconds; pandoc converted all 9 sheets, 65 questions, -467 fields, 1 field flagged. +467 fields, 1 field flagged. The PHYS40002 folder, both routes with the solutions files, +live: 9 sheets, 540 fields, 399 agreed at tier 1, 35 filled by the one route that returned +them, 106 adjudicated, 56 flagged, in 32 minutes. ## Order of work diff --git a/in2lambda_agent/pair.py b/in2lambda_agent/pair.py index a52be82..d9fba8d 100644 --- a/in2lambda_agent/pair.py +++ b/in2lambda_agent/pair.py @@ -7,22 +7,28 @@ questions document beside it is converted on its own. The pairing is by name. A solutions document is one whose stem ends in -`solutions` after a space, an underscore or a hyphen, in any case. Its questions -document is the file beside it whose stem is the stem before that ending and -whose suffix is the same. +`solutions` or `sol` after a space, an underscore or a hyphen, in any case. Its +questions document is the file beside it whose stem is the stem before that +ending and whose suffix is the same. + +`pairs_in` reads a whole folder that way: every sheet in it with its solutions +document, which is what a run over a folder converts. """ import re from pathlib import Path from typing import Optional -SOLUTIONS = re.compile(r"^(?P.+?)[ _-]solutions$", re.IGNORECASE) +SOLUTIONS = re.compile(r"^(?P.+?)[ _-](solutions|sol)$", re.IGNORECASE) """A solutions document's stem, and the questions document's stem within it. The separator is required, so `resolutions.pdf` is not a solutions document and `Solutions.pdf` names no questions document. """ +DOCUMENTS = (".tex", ".pdf", ".docx", ".md") +"""The suffixes a folder's sheets are looked for under.""" + def questions_stem(document: Path) -> Optional[str]: """The stem of the questions document a solutions document answers. @@ -98,6 +104,25 @@ def _files_in(folder: Path) -> list[Path]: return sorted(path for path in folder.iterdir() if path.is_file()) +def pairs_in(folder: Path) -> list[tuple[Path, Optional[Path]]]: + """The documents a folder run converts, in name order. + + Args: + folder: A folder of sheets and their solutions. + + Returns: + One pair per sheet: the questions document and the solutions document + beside it, or None where there is none. A solutions document whose + questions document is missing is left out, since there is no sheet for + it to answer. Subfolders, `figures/` among them, are not looked into. + """ + return [ + (path, solutions_beside(path)) + for path in _files_in(Path(folder)) + if path.suffix.lower() in DOCUMENTS and questions_stem(path) is None + ] + + def of(source: Path) -> tuple[Path, Optional[Path]]: """The two documents a run freezes, whichever of them the user named. diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index a2f657e..78867c9 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -6,7 +6,13 @@ quote of the markdown (`not_verbatim`). The two replies are compared field by field (`disputed`); a disputed field goes to a small second call that may pick one side or a passage of the source, never its own words (`adjudicate`); what neither settles is a flag -for a person (`reconcile`). `to_set` and `build` write the result with in2lambda. +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. `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 +each sheet the number of fields agreed, defaulted, adjudicated and flagged. A reply is a list of questions: {"title", "main_text", "parts": [{"content", "options", "answer", "worked_solution"}]}. Field keys are 1-based: `q2.p1.content`. @@ -25,6 +31,7 @@ from in2lambda.api.question import Question from in2lambda.api.set import Set +from in2lambda_agent import pair from in2lambda_agent.model import Backend, Reply, choose_backend from in2lambda_agent.settings import Settings, load_settings @@ -70,6 +77,37 @@ def fields(reply: Reply_) -> dict[str, str]: return found +def normalise(reply: Reply_) -> Reply_: + """A copy in which every question has a part. + + Route A's prompt gives a question with no sub-questions one part whose content is + empty. A filter leaves that question's parts out. The two shapes mean the same, so + both are written as the one empty part; otherwise each such question is a structural + dispute. + """ + copied = json.loads(json.dumps(reply)) + for q in copied: + if not q.get("parts"): + q["parts"] = [{"content": "", "options": [], "answer": "", "worked_solution": ""}] + return copied + + +def merge(questions: Reply_, solutions: Reply_) -> Reply_: + """Route B's two runs as one reply: the answers of the solutions document by position. + + The filter reads the two documents apart, so the solutions run holds answers and + worked solutions and nothing else. A question or part the solutions run does not + return keeps the empty answer and worked solution of the questions run. + """ + merged, answers = normalise(questions), normalise(solutions) + for q, s in zip(merged, answers): + for p, sp in zip(q["parts"], s["parts"]): + for name in ("answer", "worked_solution"): + if sp.get(name): + p[name] = sp[name] + return merged + + def not_verbatim(reply: Reply_, source: str) -> list[str]: """The fields that are not quotes of the source; titles are not quotes. @@ -170,10 +208,15 @@ def direct(markdown: str, solutions: Optional[str], backend: Backend) -> tuple[R # --- route B ------------------------------------------------------------------------- -def run_filter(lua: Path, document: Path) -> Reply_: - """Route B at run time: pandoc, the filter, and the JSON it wrote. No model.""" +def run_filter(lua: Path, document: Path, role: str = "questions") -> Reply_: + """Route B at run time: pandoc, the filter, and the JSON it wrote. No model. + + The role, `questions` or `solutions`, is passed to the filter as pandoc metadata, + which is how the filter tells a set's two documents apart. + """ out = subprocess.run( - ["pandoc", str(document), "--lua-filter", str(lua), "-t", "plain", "--wrap=none"], + ["pandoc", str(document), "--lua-filter", str(lua), "-M", f"in2lambda_role={role}", + "-t", "plain", "--wrap=none"], capture_output=True, check=True, ) return json.loads(out.stdout.decode("utf-8").strip()) @@ -237,6 +280,7 @@ class Flag: class Reconciled: fields: Reply_ agreed: int + defaulted: int adjudicated: int flags: list[Flag] = field(default_factory=list) @@ -252,13 +296,27 @@ def _set_field(reply: Reply_, key: str, text: str) -> None: def reconcile(a: Reply_, b: Reply_, source: str, backend: Optional[Backend] = None) -> Reconciled: - """Tiers 1 to 3: agreed fields kept, disputes adjudicated, the rest flagged. Starts from A.""" + """Tiers 1 to 3: agreed fields kept, disputes adjudicated, the rest flagged. Starts from A. + + A field only one route filled is not a disagreement about wording: the text of the + route that filled it is taken, counted as defaulted, and the adjudicator is not asked. + """ + a, b = normalise(a), normalise(b) merged = json.loads(json.dumps(a)) keys = disputed(a, b) structural = [k for k in keys if re.fullmatch(r"q\d+(\.p\d+)?", k)] - wording = [k for k in keys if k not in structural] fa, fb = fields(a), fields(b) - result = Reconciled(fields=merged, agreed=len(fa) - len(wording), adjudicated=len(wording)) + defaulted = [k for k in keys if k not in structural and not (fold(fa[k]) and fold(fb[k]))] + wording = [k for k in keys if k not in structural and k not in defaulted] + result = Reconciled( + fields=merged, + agreed=len(fa) - len(defaulted) - len(wording), + defaulted=len(defaulted), + adjudicated=len(wording), + ) + for k in defaulted: + if not fold(fa[k]): + _set_field(merged, k, fb[k]) for k in structural: result.flags.append(Flag(k, "present" if k in _structure(a) else "absent", "present" if k in _structure(b) else "absent", "one route did not find it")) verdicts = adjudicate(a, b, wording, source, backend) if wording and backend is not None else {} @@ -292,6 +350,12 @@ class Converted: flags: list[Flag] reply: Reply_ tokens: int = 0 + # The counts of the reconciliation, zero where route B did not run. + fields: int = 0 + agreed: int = 0 + defaulted: int = 0 + adjudicated: int = 0 + route_b_error: Optional[str] = None def markdown_of(document: Path, cache_dir: Path, settings: Settings) -> tuple[str, Path]: @@ -320,22 +384,43 @@ def convert( lua: Optional[Path] = None, name: str = "set", ) -> Converted: - """Route A, route B where a filter is given, reconcile, verify, write.""" + """Route A, route B where a filter is given, reconcile, verify, write. + + Route B reads the solutions document too, under its own role, and the two runs are + merged before the comparison. Where a filter run fails, the route A reply is the + result and `route_b_error` holds pandoc's message, so that one sheet of a folder does + not stop the other eight. + """ settings = settings or load_settings() backend = backend or choose_backend(settings) markdown, images = markdown_of(document, cache_dir, settings) solutions_md = markdown_of(solutions, cache_dir, settings)[0] if solutions else None source = markdown + ("\n" + solutions_md if solutions_md else "") reply, usage = direct(markdown, solutions_md, backend) - tokens = usage.usage.input_tokens + usage.usage.output_tokens + counts, error = (0, 0, 0, 0), None + flags = [Flag(k, fields(reply)[k], "", "not a quote of the source") for k in not_verbatim(reply, source)] if lua is not None: - other = run_filter(lua, document) - reconciled = reconcile(reply, other, source, backend) - reply, flags = reconciled.fields, reconciled.flags - else: - flags = [Flag(k, fields(reply)[k], "", "not a quote of the source") for k in not_verbatim(reply, source)] + try: + other = run_filter(lua, document) + if solutions is not None: + other = merge(other, run_filter(lua, solutions, role="solutions")) + except (subprocess.CalledProcessError, json.JSONDecodeError) as problem: + stderr = getattr(problem, "stderr", None) + error = (stderr.decode("utf-8", "replace") if stderr else str(problem)).strip() + else: + reconciled = reconcile(reply, other, source, backend) + reply, flags = reconciled.fields, reconciled.flags + counts = ( + reconciled.agreed + reconciled.defaulted + reconciled.adjudicated, + reconciled.agreed, reconciled.defaulted, reconciled.adjudicated, + ) built = to_set(reply, name=name, directory=images) - return Converted(set=built, zip_path=build(built, out_dir), flags=flags, reply=reply, tokens=tokens) + return Converted( + set=built, zip_path=build(built, out_dir), flags=flags, reply=reply, + tokens=usage.usage.input_tokens + usage.usage.output_tokens, + fields=counts[0], agreed=counts[1], defaulted=counts[2], adjudicated=counts[3], + route_b_error=error, + ) # --- route B: writing the filter ------------------------------------------------------- @@ -371,15 +456,87 @@ def brief(b: dict, depth: int = 0) -> list[str]: return "\n".join(l for b in ast["blocks"] for l in brief(b)) -def write_filter(document: Path, backend: Backend) -> tuple[str, Reply]: - """Route B's one call: a Lua filter for the structure of this document's set.""" +def write_filter(document: Path, solutions: Optional[Path], backend: Backend) -> tuple[str, Reply]: + """Route B's one call: a Lua filter for the structure of this document's set. + + Where a set writes its solutions in a second document, one filter reads both: the + call is shown the structure of each, and the filter it writes tells them apart by the + role `run_filter` passes. + """ version = subprocess.check_output(["pandoc", "--version"]).decode().split()[1] + both = "" if solutions is None else f""" + +The set's solutions are in a second document, which the same filter reads. Its block structure is: + +{structure(solutions)} + +The filter tells the two documents apart by pandoc's metadata: pandoc.utils.stringify(doc.meta.in2lambda_role) is "questions" or "solutions". Under "questions" emit the objects described above, leaving answer and worked_solution empty. Under "solutions" emit one object per question of the sheet, in the same order, each with an empty title and main_text and one object per part of that question in order, whose answer holds that part's final answer and whose worked_solution holds its working, content and options staying empty. The two runs are merged part by part by position, so a question the solutions document does not answer must still have its object in place. +""" prompt = f"""A problem sheet is read by pandoc {version}. Its block structure (pandoc's AST, abbreviated) is: {structure(document)} Write a Lua filter that replaces the whole document with one CodeBlock holding a JSON array: one object per question, in order, {{"title": "", "main_text": "...", "parts": [{{"content": "...", "options": [], "answer": "", "worked_solution": ""}}]}} -Rules: a question is a top-level item of the numbered list of questions, or a section where the sheet uses headings; its main_text is the question's own paragraphs; its parts are the items of a numbered list nested inside it, each part's content being that nested item's paragraphs; a question with no nested list has one part with empty content. Render each text with pandoc.write(pandoc.Pandoc(blocks), "commonmark_x", {{wrap_text = "wrap-none"}}), keeping maths and images. Leave title empty unless the sheet names its questions. Ignore headings and figures that belong to no question. Build the JSON string by hand: escape only the double quote, the backslash and ASCII control characters (bytes below 32) - never any other byte, so that UTF-8 text passes through unchanged. Return the filter as: function Pandoc(doc) ... return pandoc.Pandoc({{pandoc.CodeBlock(json)}}) end.""" +Rules: a question is a top-level item of the numbered list of questions, or a section where the sheet uses headings; its main_text is the question's own paragraphs; its parts are the items of a numbered list nested inside it, each part's content being that nested item's paragraphs; a question with no nested list has one part with empty content. Render each text with pandoc.write(pandoc.Pandoc(blocks), "commonmark_x", {{wrap_text = "wrap-none"}}), keeping maths and images. Leave title empty unless the sheet names its questions. Ignore headings and figures that belong to no question. Build the JSON string by hand: escape only the double quote, the backslash and ASCII control characters (bytes below 32) - never any other byte, so that UTF-8 text passes through unchanged. +{both} +Return the filter as: function Pandoc(doc) ... return pandoc.Pandoc({{pandoc.CodeBlock(json)}}) end.""" reply = backend.call(FILTER_SYSTEM, prompt) return re.sub(r"^```(lua)?\s*|\s*```$", "", reply.text.strip()), reply + + +# --- a folder of sheets ------------------------------------------------------------------ + + +@dataclass +class Folder: + filter: Path + sheets: list[tuple[str, Converted]] + tokens: int = 0 + + def report(self) -> list[str]: + """One line per sheet, and a line of the totals.""" + lines, totals = [], [0, 0, 0, 0, 0] + for name, sheet in self.sheets: + counts = [sheet.fields, sheet.agreed, sheet.defaulted, sheet.adjudicated, len(sheet.flags)] + totals = [total + count for total, count in zip(totals, counts)] + lines.append(f"{name}: {_counted(counts)}" + (f" (route B failed: {sheet.route_b_error})" if sheet.route_b_error else "")) + return lines + [f"{len(self.sheets)} sheets: {_counted(totals)}"] + + +def _counted(counts: list[int]) -> str: + return "{} fields, agreed {}, defaulted {}, adjudicated {}, flagged {}".format(*counts) + + +def convert_folder( + folder: Path, + *, + out_dir: Path = Path("out"), + cache_dir: Path = Path(".in2lambda-agent"), + backend: Optional[Backend] = None, + settings: Optional[Settings] = None, +) -> Folder: + """Every sheet of a folder, through both routes, under one filter. + + One model call writes the filter from the first sheet and its solutions document, + because the sheets of a folder share one structure, and pandoc then runs that filter + over every sheet with no further call. Each sheet's set is written under a folder + named after the sheet. + """ + settings = settings or load_settings() + backend = backend or choose_backend(settings) + pairs = pair.pairs_in(folder) + lua_source, usage = write_filter(pairs[0][0], pairs[0][1], backend) + out_dir = Path(out_dir) + out_dir.mkdir(parents=True, exist_ok=True) + lua = out_dir / "filter.lua" + lua.write_text(lua_source, encoding="utf-8") + result = Folder(filter=lua, sheets=[], tokens=usage.usage.input_tokens + usage.usage.output_tokens) + for sheet, solutions in pairs: + converted = convert( + sheet, solutions, out_dir=out_dir / sheet.stem, cache_dir=cache_dir, + backend=backend, settings=settings, lua=lua, name=sheet.stem, + ) + result.sheets.append((sheet.stem, converted)) + result.tokens += converted.tokens + return result diff --git a/tests/fixtures/pair-filter.lua b/tests/fixtures/pair-filter.lua new file mode 100644 index 0000000..a67bf7e --- /dev/null +++ b/tests/fixtures/pair-filter.lua @@ -0,0 +1,65 @@ +-- A filter of the shape the model writes for a folder: it reads the sheets by +-- their Question headings, and the solutions document, under the role pandoc +-- passes it, by the question number each answer paragraph opens with. +local function esc(s) + return (s:gsub('[%z\1-\31"\\]', function(c) + if c == '"' then return '\\"' + elseif c == '\\' then return '\\\\' + elseif c == '\n' then return '\\n' + else return string.format('\\u%04x', string.byte(c)) + end + end)) +end + +local function render(blocks) + local md = pandoc.write(pandoc.Pandoc(blocks), 'commonmark_x', { wrap_text = 'wrap-none' }) + return (md:gsub('^%s+', ''):gsub('%s+$', '')) +end + +local function questions(doc) + local out, current = {}, nil + for _, b in ipairs(doc.blocks) do + if b.t == 'Header' then + current = nil + if pandoc.utils.stringify(b.content):match('^Question') then + current = {} + out[#out + 1] = current + end + elseif current then + current[#current + 1] = b + end + end + local pieces = {} + for _, blocks in ipairs(out) do + pieces[#pieces + 1] = '{"title": "", "main_text": "' .. esc(render(blocks)) .. '", "parts": []}' + end + return pieces +end + +local function solutions(doc) + local answers, inside = {}, false + for _, b in ipairs(doc.blocks) do + if b.t == 'Header' then + inside = pandoc.utils.stringify(b.content):match('^Solutions') ~= nil + elseif inside and (b.t == 'Para' or b.t == 'Plain') then + local text = render({ b }) + local n = tonumber(text:match('^(%d+)')) + if n then + answers[n] = answers[n] or {} + table.insert(answers[n], text) + end + end + end + local pieces = {} + for i = 1, #answers do + pieces[#pieces + 1] = '{"title": "", "main_text": "", "parts": [{"content": "", "options": [], "answer": "", "worked_solution": "' + .. esc(table.concat(answers[i], '\n\n')) .. '"}]}' + end + return pieces +end + +function Pandoc(doc) + local role = pandoc.utils.stringify(doc.meta.in2lambda_role or 'questions') + local pieces = role == 'solutions' and solutions(doc) or questions(doc) + return pandoc.Pandoc({ pandoc.CodeBlock('[' .. table.concat(pieces, ',\n') .. ']') }) +end diff --git a/tests/fixtures/role-filter.lua b/tests/fixtures/role-filter.lua new file mode 100644 index 0000000..9e1f98b --- /dev/null +++ b/tests/fixtures/role-filter.lua @@ -0,0 +1,7 @@ +-- Echoes back the role pandoc was given, as a question title, which is how the +-- test tells the run over the questions from the run over the solutions. +function Pandoc(doc) + local role = pandoc.utils.stringify(doc.meta.in2lambda_role or '') + local json = '[{"title": "' .. role .. '", "main_text": "", "parts": []}]' + return pandoc.Pandoc({ pandoc.CodeBlock(json) }) +end diff --git a/tests/test_pair.py b/tests/test_pair.py index 0a88053..adeef5a 100644 --- a/tests/test_pair.py +++ b/tests/test_pair.py @@ -14,8 +14,11 @@ ("Tutorial_2_Solutions.pdf", "Tutorial_2"), ("sheet-solutions.md", "sheet"), ("Sheet 1 Solutions.docx", "Sheet 1"), + ("Sheet1_Sol.pdf", "Sheet1"), + ("worksheet-sol.md", "worksheet"), # No separator before `solutions`, so the word is part of a longer one. ("resolutions.pdf", None), + ("aerosol.pdf", None), # Nothing before the separator, so there is no stem to pair with. ("Solutions.pdf", None), ("Worksheet_1.pdf", None), @@ -77,6 +80,26 @@ def test_a_folder_that_is_not_there_holds_no_companion(tmp_path): ) +def test_the_sheets_of_a_folder_come_paired_and_in_name_order(tmp_path): + for name in ("Sheet_2.tex", "Sheet_1.tex", "Sheet_1_solutions.tex", "notes.png"): + (tmp_path / name).write_text("x") + (tmp_path / "figures").mkdir() + (tmp_path / "figures" / "ball.tex").write_text("x") + + assert pair.pairs_in(tmp_path) == [ + (tmp_path / "Sheet_1.tex", tmp_path / "Sheet_1_solutions.tex"), + (tmp_path / "Sheet_2.tex", None), + ] + + +def test_a_folders_solutions_file_is_not_a_sheet_of_its_own(tmp_path): + # Its questions document is not there, so there is nothing to compare two + # routes over. The folder run leaves it out; `of` still converts it alone. + (tmp_path / "Sheet_3_Sol.pdf").write_bytes(b"%PDF") + + assert pair.pairs_in(tmp_path) == [] + + def test_solutions_with_no_questions_run_alone(tmp_path): # The markers above the solutions are this document's questions, so the # document converts with no second file. diff --git a/tests/test_routes.py b/tests/test_routes.py index 195ea8e..7191082 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -165,9 +165,75 @@ def test_the_report_lists_only_what_a_person_must_read(): assert result.fields[1]["parts"][1]["content"] == REPLY[1]["parts"][1]["content"] +# --- route B over two documents ----------------------------------------------------------- + +# Short enough to read whole, so that what a test changes is the only difference. +SHEET = "A ball is thrown straight up.\n\nFind the greatest height.\n\n$h = v^2/2g = 20.4$" + + +def one(content="Find the greatest height.", **over): + part = {"content": content, "options": [], "answer": "", "worked_solution": ""} + return [{"title": "", "main_text": "A ball is thrown straight up.", "parts": [part | over]}] + + +def test_route_b_takes_its_answers_from_the_run_over_the_solutions(): + solutions = [ + {"parts": [{"answer": "$h = 20.4$", "worked_solution": "$h = v^2/2g$"}]}, + {"parts": [{"answer": "of a question this sheet does not have"}]}, + ] + merged = routes.merge(one(), solutions) + assert len(merged) == 1 + assert merged[0]["parts"][0]["content"] == "Find the greatest height." + assert merged[0]["parts"][0]["answer"] == "$h = 20.4$" + assert merged[0]["parts"][0]["worked_solution"] == "$h = v^2/2g$" + + +def test_a_question_with_no_sub_questions_is_one_empty_part_either_way(): + # Route A's prompt says so; a filter that leaves the parts out means the same. + partless = [{"title": "", "main_text": "A ball is thrown straight up.", "parts": []}] + result = routes.reconcile(one(content=""), partless, SHEET) + assert result.flags == [] + assert result.adjudicated == 0 + + +def test_a_field_only_one_route_found_is_taken_from_it_with_no_call(): + backend = FakeBackend() # No replies: a call would raise rather than answer. + result = routes.reconcile(one(), one(answer="$h = v^2/2g = 20.4$"), SHEET, backend) + assert backend.calls == [] + assert result.defaulted == 1 + assert result.adjudicated == 0 + assert result.agreed + result.defaulted == len(routes.fields(one())) + assert result.fields[0]["parts"][0]["answer"] == "$h = v^2/2g = 20.4$" + assert result.flags == [] + + # --- route B: the filter ---------------------------------------------------------------- +@pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +def test_the_filter_is_told_which_of_the_two_documents_it_is_reading(): + role = Path(__file__).parent / "fixtures" / "role-filter.lua" + sheet = Path(__file__).parent / "fixtures" / "sheet.md" + assert routes.run_filter(role, sheet)[0]["title"] == "questions" + assert routes.run_filter(role, sheet, role="solutions")[0]["title"] == "solutions" + + +@pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +def test_the_filter_call_sees_both_documents_and_how_to_tell_them_apart(): + fixtures = Path(__file__).parent / "fixtures" + backend = FakeBackend("```lua\nfunction Pandoc(doc) end\n```") + lua, _ = routes.write_filter(fixtures / "tex-sheet.tex", fixtures / "tex-sheet-2.tex", backend) + ((_, prompt),) = backend.calls + assert lua == "function Pandoc(doc) end" + assert "Kinematics" in prompt and "Energy" in prompt + assert "in2lambda_role" in prompt + + alone = FakeBackend("function Pandoc(doc) end") + routes.write_filter(fixtures / "tex-sheet.tex", None, alone) + ((_, prompt),) = alone.calls + assert "Energy" not in prompt and "in2lambda_role" not in prompt + + @pytest.mark.skipif(not PHYS.is_dir() or shutil.which("pandoc") is None, reason="private corpus and pandoc") def test_a_filter_written_for_the_set_reads_a_sheet_with_pandoc_alone(): reply = routes.run_filter(FILTER, PHYS / "mechanics_23-24_PS1.tex") @@ -177,9 +243,92 @@ def test_a_filter_written_for_the_set_reads_a_sheet_with_pandoc_alone(): assert routes.not_verbatim(reply, markdown) == [] +# --- a folder of sheets ------------------------------------------------------------------- + +FIXTURES = Path(__file__).parent / "fixtures" + +# What the model would answer for the two fixture sheets, written to match what +# tests/fixtures/pair-filter.lua reads out of them: the same questions, one part each. The +# worked solution of paired's first question is left out, so that route B's is defaulted +# into it, and the second question's main_text is shortened, so that one field is +# adjudicated. +PAIRED_DIRECT = [ + { + "title": "", "parts": [{"content": "", "options": [], "answer": "", "worked_solution": ""}], + "main_text": "A cylinder of radius $r$ rolls along the ground without slipping.\n\n(a) Find its angular velocity at speed $v$.\n\n(b) Find its kinetic energy.", + }, + { + "title": "", "main_text": "A spring of stiffness $k$ carries a mass $m$.", + "parts": [{"content": "", "options": [], "answer": "", + "worked_solution": "2(a) $T = 2\\pi\\sqrt{m/k}$\n\n2(b) $v = A\\sqrt{k/m}$"}], + }, +] +SHEET_DIRECT = [ + { + "title": "", "parts": [{"content": "", "options": [], "answer": "", "worked_solution": ""}], + "main_text": "A ball is thrown straight up at $20\\,\\mathrm{m/s}$.\n\n(a) Find the greatest height it reaches.\n\n(b) Find its time of flight.", + }, + { + "title": "", "parts": [{"content": "", "options": [], "answer": "", "worked_solution": ""}], + "main_text": "A block of mass $m$ rests on a slope of angle $\\theta$.\n\n(a) Name the three forces acting on the block.\n\n(b) Find the least coefficient of friction that holds it still.", + }, +] + + +@pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +def test_a_folder_runs_one_filter_over_every_sheet_and_reports_each(tmp_path): + folder = tmp_path / "sheets" + folder.mkdir() + for name in ("paired.md", "paired_solutions.md", "sheet.md"): + shutil.copy(FIXTURES / name, folder / name) + backend = FakeBackend( + (FIXTURES / "pair-filter.lua").read_text(), + json.dumps(PAIRED_DIRECT), + json.dumps([{"field": "q2.main_text", "choice": "A", "reason": "B carries the parts too"}]), + json.dumps(SHEET_DIRECT), + ) + result = routes.convert_folder(folder, out_dir=tmp_path / "out", backend=backend) + + assert (tmp_path / "out" / "filter.lua").is_file() + assert "in2lambda_role" in backend.calls[0][1] # the filter call saw both documents + assert [name for name, _ in result.sheets] == ["paired", "sheet"] + assert result.report() == [ + "paired: 10 fields, agreed 8, defaulted 1, adjudicated 1, flagged 0", + "sheet: 10 fields, agreed 10, defaulted 0, adjudicated 0, flagged 0", + "2 sheets: 20 fields, agreed 18, defaulted 1, adjudicated 1, flagged 0", + ] + assert all(converted.zip_path.is_file() for _, converted in result.sheets) + # The worked solution route A left empty is route B's, read from the solutions file. + paired = dict(result.sheets)["paired"] + assert paired.reply[0]["parts"][0]["worked_solution"].startswith("1(a) $\\omega") + + +@pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") +def test_a_sheet_whose_filter_run_fails_keeps_its_route_a_reply(tmp_path): + # One sheet of a folder must not stop the other eight. + lua = tmp_path / "broken.lua" + lua.write_text("this is not a filter\n") + backend = FakeBackend(json.dumps(SHEET_DIRECT)) + result = routes.convert(FIXTURES / "sheet.md", out_dir=tmp_path / "out", backend=backend, lua=lua) + + assert result.route_b_error + assert result.reply == SHEET_DIRECT + assert (result.fields, result.agreed, result.flags) == (0, 0, []) + + # --- live ------------------------------------------------------------------------------- +@live +@pytest.mark.skipif(not PHYS.is_dir(), reason="private corpus") +def test_the_phys_folder_converts_through_both_routes(tmp_path): + # The ticket's run: nine sheets and their solutions, one filter, one report. + result = routes.convert_folder(PHYS, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache") + print("\n" + "\n".join(result.report())) + assert len(result.sheets) == 9 + assert all(converted.zip_path.is_file() for _, converted in result.sheets) + + @live def test_the_me2_pair_converts_with_no_flag(tmp_path): target = Path("ExampleContents/targets/ME2_Fluids_introduction") From 36880eb6a0e21d472bb2e932577f7abb5cbb67d7 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Tue, 22 Sep 2026 23:30:50 +0100 Subject: [PATCH 2/4] implement: Add route B, a pandoc filter per folder, and compare it with route A (t37) --- in2lambda_agent/pair.py | 31 +++++++++++++++++++++---------- in2lambda_agent/routes.py | 11 +++++++++++ tests/test_pair.py | 11 +++++++++++ tests/test_routes.py | 15 +++++++++++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/in2lambda_agent/pair.py b/in2lambda_agent/pair.py index d9fba8d..a8bf72e 100644 --- a/in2lambda_agent/pair.py +++ b/in2lambda_agent/pair.py @@ -26,8 +26,14 @@ `Solutions.pdf` names no questions document. """ -DOCUMENTS = (".tex", ".pdf", ".docx", ".md") -"""The suffixes a folder's sheets are looked for under.""" +DOCUMENTS = (".tex", ".docx", ".md", ".pdf") +"""The suffixes a folder's sheets are looked for under, first preferred. + +A folder often holds one sheet twice, as the source and as the file compiled +from it: `Sheet_1.tex` beside `Sheet_1.pdf`. The earlier suffix is the sheet, +because pandoc reads it, and a PDF costs an OCR call and gives the pandoc +filter nothing to read. +""" def questions_stem(document: Path) -> Optional[str]: @@ -112,15 +118,20 @@ def pairs_in(folder: Path) -> list[tuple[Path, Optional[Path]]]: Returns: One pair per sheet: the questions document and the solutions document - beside it, or None where there is none. A solutions document whose - questions document is missing is left out, since there is no sheet for - it to answer. Subfolders, `figures/` among them, are not looked into. + beside it, or None where there is none. One sheet per stem, under the + suffix `DOCUMENTS` prefers, so that a sheet held twice converts once. A + solutions document whose questions document is missing is left out, + since there is no sheet for it to answer. Subfolders, `figures/` among + them, are not looked into. An empty list where the folder is not there. """ - return [ - (path, solutions_beside(path)) - for path in _files_in(Path(folder)) - if path.suffix.lower() in DOCUMENTS and questions_stem(path) is None - ] + sheets: dict[str, Path] = {} + for path in _files_in(Path(folder)): + if path.suffix.lower() not in DOCUMENTS or questions_stem(path) is not None: + continue + held = sheets.get(path.stem) + if held is None or DOCUMENTS.index(path.suffix.lower()) < DOCUMENTS.index(held.suffix.lower()): + sheets[path.stem] = path + return [(path, solutions_beside(path)) for path in sheets.values()] def of(source: Path) -> tuple[Path, Optional[Path]]: diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 78867c9..8931ba2 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -522,10 +522,21 @@ def convert_folder( because the sheets of a folder share one structure, and pandoc then runs that filter over every sheet with no further call. Each sheet's set is written under a folder named after the sheet. + + Raises: + ValueError: The folder holds no sheet, because the path names no folder or + because every document in it is a solutions document. """ settings = settings or load_settings() backend = backend or choose_backend(settings) pairs = pair.pairs_in(folder) + if not pairs: + raise ValueError( + f"{folder} holds no sheet to convert. A folder run converts the files in the " + f"folder whose suffix is one of {' '.join(pair.DOCUMENTS)} and whose name does " + "not end in `solutions` or `sol`. Name a single document to convert that " + "document on its own." + ) lua_source, usage = write_filter(pairs[0][0], pairs[0][1], backend) out_dir = Path(out_dir) out_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_pair.py b/tests/test_pair.py index adeef5a..7740867 100644 --- a/tests/test_pair.py +++ b/tests/test_pair.py @@ -92,6 +92,17 @@ def test_the_sheets_of_a_folder_come_paired_and_in_name_order(tmp_path): ] +def test_a_sheet_and_the_file_compiled_from_it_are_one_sheet(tmp_path): + # The folder holds the LaTeX source beside the PDF built from it. The two + # hold the same questions, so converting both would convert the sheet + # twice, under one name. Pandoc reads the source and cannot read the PDF, + # so the source is the sheet. + for name in ("Sheet_1.tex", "Sheet_1.pdf"): + (tmp_path / name).write_text("x") + + assert pair.pairs_in(tmp_path) == [(tmp_path / "Sheet_1.tex", None)] + + def test_a_folders_solutions_file_is_not_a_sheet_of_its_own(tmp_path): # Its questions document is not there, so there is nothing to compare two # routes over. The folder run leaves it out; `of` still converts it alone. diff --git a/tests/test_routes.py b/tests/test_routes.py index 7191082..e065c60 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -303,6 +303,21 @@ def test_a_folder_runs_one_filter_over_every_sheet_and_reports_each(tmp_path): assert paired.reply[0]["parts"][0]["worked_solution"].startswith("1(a) $\\omega") +def test_a_folder_with_no_sheet_in_it_names_what_a_folder_run_converts(tmp_path): + # A mistyped path and a folder holding solutions alone both pair to + # nothing. Neither reaches in2lambda, so this is the only place that can + # say what is wrong, and no model call is made for either. + lone = tmp_path / "sheets" + lone.mkdir() + (lone / "Sheet_1_solutions.tex").write_text("x") + backend = FakeBackend() # No replies: a call would raise rather than answer. + + for folder in (lone, tmp_path / "nope"): + with pytest.raises(ValueError, match="holds no sheet to convert"): + routes.convert_folder(folder, out_dir=tmp_path / "out", backend=backend) + assert backend.calls == [] + + @pytest.mark.skipif(shutil.which("pandoc") is None, reason="pandoc") def test_a_sheet_whose_filter_run_fails_keeps_its_route_a_reply(tmp_path): # One sheet of a folder must not stop the other eight. From c0cae7856be14df4dc9d1a079cb6da5984e4617b Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 00:44:12 +0100 Subject: [PATCH 3/4] implement: Add route B, a pandoc filter per folder, and compare it with route A (t37) --- docs/plan.md | 4 ++-- in2lambda_agent/routes.py | 11 ++++++++--- tests/test_routes.py | 10 ++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/plan.md b/docs/plan.md index 61834a1..0ee4cfe 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -110,8 +110,8 @@ The ME2 pair, live: 5 questions, 60 fields, 0 flags, titles equal to the export' model call, 66 seconds with the OCR cached. The PHYS40002 folder, route B: one filter written from the first sheet in 41 seconds; pandoc converted all 9 sheets, 65 questions, 467 fields, 1 field flagged. The PHYS40002 folder, both routes with the solutions files, -live: 9 sheets, 540 fields, 399 agreed at tier 1, 35 filled by the one route that returned -them, 106 adjudicated, 56 flagged, in 32 minutes. +live: 9 sheets, 543 fields, 388 agreed at tier 1, 54 taken from the one route that read +them, 101 adjudicated, 49 flagged, in 28 minutes. ## Order of work diff --git a/in2lambda_agent/routes.py b/in2lambda_agent/routes.py index 7a89a7b..91626ae 100644 --- a/in2lambda_agent/routes.py +++ b/in2lambda_agent/routes.py @@ -326,14 +326,18 @@ def reconcile(a: Reply_, b: Reply_, source: str, backend: Optional[Backend] = No A field only one route filled is not a disagreement about wording: the text of the route that filled it is taken, counted as defaulted, and the adjudicator is not asked. + A field of a question or part the other route did not find at all is defaulted too - + there is nothing to compare it with - though the structure itself is still flagged. """ a, b = normalise(a), normalise(b) merged = json.loads(json.dumps(a)) keys = disputed(a, b) structural = [k for k in keys if re.fullmatch(r"q\d+(\.p\d+)?", k)] fa, fb = fields(a), fields(b) - defaulted = [k for k in keys if k not in structural and not (fold(fa[k]) and fold(fb[k]))] - wording = [k for k in keys if k not in structural and k not in defaulted] + disagreed = [k for k in keys if k not in structural] + defaulted = [k for k in fa if any(k.startswith(s + ".") for s in structural)] + defaulted += [k for k in disagreed if not (fold(fa[k]) and fold(fb[k]))] + wording = [k for k in disagreed if fold(fa[k]) and fold(fb[k])] result = Reconciled( fields=merged, agreed=len(fa) - len(defaulted) - len(wording), @@ -341,7 +345,8 @@ def reconcile(a: Reply_, b: Reply_, source: str, backend: Optional[Backend] = No adjudicated=len(wording), ) for k in defaulted: - if not fold(fa[k]): + # Where the other route has no such field at all, A's is already in the merge. + if not fold(fa[k]) and k in fb: _set_field(merged, k, fb[k]) for k in structural: result.flags.append(Flag(k, "present" if k in _structure(a) else "absent", "present" if k in _structure(b) else "absent", "one route did not find it")) diff --git a/tests/test_routes.py b/tests/test_routes.py index 2a112ce..9638b78 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -280,6 +280,16 @@ def test_a_field_only_one_route_found_is_taken_from_it_with_no_call(): assert result.flags == [] +def test_the_fields_of_a_question_one_route_missed_are_defaulted_not_agreed(): + # Route B read nothing here. Counting the question's fields as agreed would + # report the two routes as having checked each other over a sheet only one of + # them read; there was nothing to compare, so they come from route A. + result = routes.reconcile(one(), [], SHEET) + assert (result.agreed, result.defaulted, result.adjudicated) == (0, 5, 0) + assert result.defaulted == len(routes.fields(one())) + assert [f.field for f in result.flags] == ["q1"] + + # --- route B: the filter ---------------------------------------------------------------- From b3e6486d425da7306367825f457008bd1c5656af Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 23 Sep 2026 01:30:57 +0100 Subject: [PATCH 4/4] Assert the live PHYS run pairs every sheet with its solutions and runs route B on each Co-Authored-By: Claude Fable 5.1 --- tests/test_routes.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_routes.py b/tests/test_routes.py index 9638b78..1e8ff60 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -21,6 +21,7 @@ from conftest import FakeBackend import in2lambda_agent.routes as routes +from in2lambda_agent import pair from in2lambda_agent.settings import Settings ME2 = Path(__file__).parent / "fixtures" / "me2" @@ -421,10 +422,20 @@ def test_a_sheet_whose_filter_run_fails_keeps_its_route_a_reply(tmp_path): @pytest.mark.skipif(not PHYS.is_dir(), reason="private corpus") def test_the_phys_folder_converts_through_both_routes(tmp_path): # The ticket's run: nine sheets and their solutions, one filter, one report. + # Every sheet has a solutions file, so a run that read none is not this run. + pairs = pair.pairs_in(PHYS) + assert len(pairs) == 9 and all(solutions is not None for _, solutions in pairs) result = routes.convert_folder(PHYS, out_dir=tmp_path / "out", cache_dir=tmp_path / "cache") print("\n" + "\n".join(result.report())) assert len(result.sheets) == 9 assert all(converted.zip_path.is_file() for _, converted in result.sheets) + # Route B ran on every sheet: a sheet whose filter failed falls back to route A. + assert [name for name, converted in result.sheets if converted.route_b_error] == [] + # And the solutions were read: every sheet has at least one answer and one worked solution. + for name, converted in result.sheets: + filled = routes.fields(converted.reply) + assert any(v.strip() for k, v in filled.items() if k.endswith(".answer")), name + assert any(v.strip() for k, v in filled.items() if k.endswith(".worked_solution")), name @live