From fc7125d497a3851fb6a55e4340fe5707f0032ed7 Mon Sep 17 00:00:00 2001 From: Rahul Agrawal <12agrawalrahul@gmail.com> Date: Thu, 3 Sep 2026 03:13:57 +0530 Subject: [PATCH] fix(remediate): keep out-of-scope pages' canonical markdown on a flagged run `remediate_pdf` seeded canonical markdown from the raw baseline for *every* page and then wrote canonical back for every page, so a `scope="flagged"` run (the default of `trigger_remediation`) reverted every already-remediated `pass` page to its baseline, discarding the adopted cleanup/VLM output. Pages outside the scope now keep the canonical markdown, composite and provenance they already carry; `scope="all"` is unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0126A9jCM6LTxnmCPH3hWbTR --- wikify/engine/remediate.py | 33 +++++++++++++++++---- wikify/engine/store.py | 18 ++++++++++-- wikify/tests/test_remediate_pipeline.py | 38 +++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/wikify/engine/remediate.py b/wikify/engine/remediate.py index bf29a54..21333ec 100644 --- a/wikify/engine/remediate.py +++ b/wikify/engine/remediate.py @@ -47,6 +47,24 @@ def _pick_winner(candidates: list[tuple]) -> tuple | None: return cleanup_c +def _canonical_seed(page: dict, in_scope: bool) -> tuple[str, float | None, str]: + """Where a page's canonical starts this run. + + An in-scope page starts from its raw baseline, so a remediation is only canonical + when this run adopts it. A page outside the scope keeps the canonical it already + carries — a `flagged` run must not revert already-remediated `pass` pages to + baseline and throw away their adopted output. + """ + if in_scope: + return page["baseline_markdown"] or "", page["composite"], "baseline" + composite = page["canonical_composite"] + return ( + page["canonical_markdown"] or page["baseline_markdown"] or "", + page["composite"] if composite is None else composite, + page["canonical_source"] or "baseline", + ) + + def remediate_pdf( source_document: str, pdf_path: str, @@ -78,10 +96,14 @@ def remediate_pdf( targets = pages if scope == "all" else [p for p in pages if p["verdict"] != "pass"] total = len(targets) - # Canonical defaults to each page's baseline; adopted remediations override below. - canon_md = {p["page_no"]: p["baseline_markdown"] or "" for p in pages} - canon_comp = {p["page_no"]: p["composite"] for p in pages} - canon_src = {p["page_no"]: "baseline" for p in pages} + target_page_nos = {p["page_no"] for p in targets} + canon_md: dict[int, str] = {} + canon_comp: dict[int, float | None] = {} + canon_src: dict[int, str] = {} + for p in pages: + canon_md[p["page_no"]], canon_comp[p["page_no"]], canon_src[p["page_no"]] = _canonical_seed( + p, p["page_no"] in target_page_nos + ) with fitz.open(pdf_path) as doc: # Running furniture (banners, doc-code/date stamps, 'Page X of Y', prepared/issued/ @@ -92,6 +114,7 @@ def remediate_pdf( furniture = det.find_furniture_lines([doc[p["page_no"] - 1].get_text("text") for p in pages]) doc_cost = 0.0 + adopted_count = 0 for i, p in enumerate(targets): page = doc[p["page_no"] - 1] gt = page.get_text("text") @@ -157,6 +180,7 @@ def remediate_pdf( store.set_remediation(p["name"], "vlm", "", base_ps, False, "; ".join(errors) or None) if adopted: + adopted_count += 1 canon_md[p["page_no"]] = record[1] canon_comp[p["page_no"]] = new_composite canon_src[p["page_no"]] = method @@ -193,7 +217,6 @@ def remediate_pdf( doc_cost += store.cost_of(llm.get_metrics()) store.add_document_cost(source_document, doc_cost) - adopted_count = sum(1 for src in canon_src.values() if src != "baseline") return { "targets": total, "adopted": adopted_count, diff --git a/wikify/engine/store.py b/wikify/engine/store.py index 72d154c..982625e 100644 --- a/wikify/engine/store.py +++ b/wikify/engine/store.py @@ -126,11 +126,25 @@ def add_document_cost(source_document: str, cost: float) -> None: def get_pages(source_document: str) -> list[dict]: - """Pages of a doc (ordered) with the fields remediation needs to route + re-score.""" + """Pages of a doc (ordered) with the fields remediation needs to route + re-score. + + The canonical fields ride along so a scoped run can leave the pages it doesn't + touch on the canonical they already carry. + """ return frappe.get_all( "Source Page", filters={"source_document": source_document}, - fields=["name", "page_no", "kind", "baseline_markdown", "verdict", "composite"], + fields=[ + "name", + "page_no", + "kind", + "baseline_markdown", + "verdict", + "composite", + "canonical_markdown", + "canonical_composite", + "canonical_source", + ], order_by="page_no asc", ) diff --git a/wikify/tests/test_remediate_pipeline.py b/wikify/tests/test_remediate_pipeline.py index 071e53b..98d7369 100644 --- a/wikify/tests/test_remediate_pipeline.py +++ b/wikify/tests/test_remediate_pipeline.py @@ -37,6 +37,11 @@ def _fake_chat(model, messages, label="", **kw): return {"choices": [{"message": {"content": content}}], "usage": {}} +def _cleanup_marks_the_page(md, model=None, project_context="", instruction=""): + """A content-preserving cleanup whose output is distinguishable from the baseline.""" + return md + "\n\nCLEANED" + + class TestRemediatePipeline(FrappeTestCase): def setUp(self): # These tests assume only VISUAL pages are judged ("text pages stay @@ -183,6 +188,39 @@ def test_cost_accumulates_on_page_and_document(self): store.add_document_cost(sd, added) self.assertAlmostEqual(frappe.db.get_value("Source Document", sd, "llm_cost"), 0.006) + def test_flagged_run_keeps_an_out_of_scope_page_canonical(self): + sd, path = self._parse() + with ( + patch("wikify.engine.llm.has_openrouter", return_value=True), + patch("wikify.engine.llm.chat_completion", side_effect=_fake_chat), + patch("wikify.engine.remediate.clean_markdown", side_effect=_cleanup_marks_the_page), + patch("wikify.engine.remediate.vlm.parse_page_image", return_value=_MERMAID), + ): + remediate_pdf(sd, path, scope="all") + + fields = ["canonical_markdown", "canonical_source", "canonical_composite", "verdict"] + before = frappe.db.get_value( + "Source Page", {"source_document": sd, "page_no": 1}, fields, as_dict=True + ) + self.assertEqual(before.verdict, "pass") + self.assertEqual(before.canonical_source, "cleanup") + self.assertIn("CLEANED", before.canonical_markdown) + + with ( + patch("wikify.engine.llm.has_openrouter", return_value=True), + patch("wikify.engine.llm.chat_completion", side_effect=_fake_chat), + patch("wikify.engine.remediate.clean_markdown", side_effect=_cleanup_marks_the_page), + patch("wikify.engine.remediate.vlm.parse_page_image", return_value=_MERMAID), + ): + remediate_pdf(sd, path, scope="flagged") + + after = frappe.db.get_value( + "Source Page", {"source_document": sd, "page_no": 1}, fields, as_dict=True + ) + self.assertEqual(after.canonical_markdown, before.canonical_markdown) + self.assertEqual(after.canonical_source, before.canonical_source) + self.assertEqual(after.canonical_composite, before.canonical_composite) + def test_remediate_flagged_scope_skips_passing_pages(self): sd, path = self._parse() with (