Skip to content

fix(remediate): keep out-of-scope pages' canonical markdown on a flagged run #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rl0007 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(remediate): keep out-of-scope pages' canonical markdown on a flagged run #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions wikify/engine/remediate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Comment on lines +102 to +103

@greptile-apps greptile-apps Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stitching rewrites preserved pages

When a passing page and an adjacent remediation target contain compatible cross-page tables, the all-page stitching pass mutates both pages and writes both results back, causing the out-of-scope page's accepted canonical table content to be merged or stripped.

Knowledge Base Used: Remediation and verification

Prompt To Fix With AI 
This is a comment left during a code review.
Path: wikify/engine/remediate.py
Line: 102-103

Comment:
**Stitching rewrites preserved pages**

When a passing page and an adjacent remediation target contain compatible cross-page tables, the all-page stitching pass mutates both pages and writes both results back, causing the out-of-scope page's accepted canonical table content to be merged or stripped.

**Knowledge Base Used:** [Remediation and verification](https://app.greptile.com/bwh-tech/-/custom-context/knowledge-base/bwhtech/wikify/-/docs/remediation-and-verification.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

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/
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 16 additions & 2 deletions wikify/engine/store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down
38 changes: 38 additions & 0 deletions wikify/tests/test_remediate_pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down
Loading