diff --git a/skills/deeppapernote/SKILL.md b/skills/deeppapernote/SKILL.md index 404283d..ad0f18f 100644 --- a/skills/deeppapernote/SKILL.md +++ b/skills/deeppapernote/SKILL.md @@ -150,6 +150,7 @@ Formal Save states: - The `创新点` section should not be empty praise. It should enumerate the paper's actual innovations and briefly explain why each one matters. - High-quality notes should usually contain multiple meaningful `###` subheadings in the technical sections when the paper is non-trivial. - Generate the complete figure/table decision table and satisfy the generated `writing_contract.figure_table_contract` before drafting or saving. +- After the synthesis bundle is built, complete the model-led Visual Review Gate and Figure/Table Decision Freeze before creating `note_plan`; no `review_pending` item may cross that boundary. - Pass the grounding and final-note figure gates before advancing; revise any failed decision coverage, insertion, structure, or status check. - An `insert` decision is complete only after Formal Save materializes the selected image into the paper-local `images/` directory and the write succeeds. - The note must pass a style gate: no mixed Chinese-English prose lines except stable proper nouns or citation metadata. diff --git a/skills/deeppapernote/references/figure-placement.md b/skills/deeppapernote/references/figure-placement.md index f38511b..ce3cf01 100644 --- a/skills/deeppapernote/references/figure-placement.md +++ b/skills/deeppapernote/references/figure-placement.md @@ -67,8 +67,36 @@ Figure/table insertion has two separate gates: - visual usability: the crop actually contains the visual body needed by the reader A label or caption match is not insertion approval. +Deterministic signals may reject an obvious defect, but only an actual image inspection may approve insertion. Fail closed when visual usability is weak: keep the placeholder instead of inserting the candidate. +Every selected candidate begins as `review_pending`. Review it after the synthesis bundle exists and before `note_plan`: + +1. Open the exact candidate crop and its complete source page preview. +2. Compare them with the matched Figure/Table identity and external caption. +3. Confirm that the crop is a Visual-Body Crop: the complete scientific visual is present, all internal labels remain readable, and the external caption and surrounding paper prose are absent. +4. Record the review in that candidate's existing decision entry using the generated `writing_contract.figure_table_contract.visual_review` fields and values. + +A passing review is valid only for the decision entry's current asset SHA-256. Any asset change returns the item to `review_pending`. Complete the Figure/Table Decision Freeze before planning or drafting; grounding must reject an unresolved or stale review. + +## Visual-Body Crop Boundary + +Keep axes, tick labels, legends, color bars, scale bars, panel letters, table headers, method labels, and other text that belongs inside the scientific visual. Keep a small safety margin so edge labels are not clipped. Exclude the source caption and unrelated running prose from the image pixels while retaining the caption as metadata and later Markdown explanation. + +If the external caption cannot be separated without damaging the scientific visual, fail closed and keep the placeholder. The first implementation reviews complete Figure/Table bodies only; do not split panels into separate assets. + +## Bounded Crop Repair + +Use a Bounded Crop Repair only for a geometry-only failure allowed by the generated contract. Put the page-relative normalized bbox and repair request into the same decision entry, then run: + +```bash +python3 scripts/plan_figure_table_decisions.py \ + --review-decisions \ + --output +``` + +The script validates the bbox, rerenders once from the source PDF at the contract's 300 dpi, refreshes the asset SHA-256, and returns the item to `review_pending` for a full fresh review. A terminal defect or a failed repaired crop remains a placeholder; never request a second recrop. + Reject candidates that are: - caption-only crops - tables with no visible table body diff --git a/skills/deeppapernote/scripts/build_synthesis_bundle.py b/skills/deeppapernote/scripts/build_synthesis_bundle.py index 3779ddd..539171b 100644 --- a/skills/deeppapernote/scripts/build_synthesis_bundle.py +++ b/skills/deeppapernote/scripts/build_synthesis_bundle.py @@ -325,6 +325,15 @@ def compact_writing_contract() -> dict: ) usable_insert_candidate = dict(WRITING_CONTRACT_RULES["usable_insert_candidate"]) usable_insert_candidate["kinds"] = list(usable_insert_candidate["kinds"]) + visual_review_contract = deepcopy(WRITING_CONTRACT_RULES["visual_review_contract"]) + for field in ( + "review_fields", + "review_status_values", + "review_evidence_fields", + "repairable_failure_reasons", + "terminal_failure_reasons", + ): + visual_review_contract[field] = list(visual_review_contract[field]) analysis_coverage = deepcopy(WRITING_CONTRACT_RULES["analysis_coverage_contract"]) analysis_coverage["central_claim_fields"] = list( analysis_coverage["central_claim_fields"] @@ -394,6 +403,7 @@ def compact_writing_contract() -> dict: WRITING_CONTRACT_RULES["automatic_fail_closed_visual_statuses"] ), "manual_review_claim_requires_image_inspection": True, + "visual_review": visual_review_contract, }, "analysis_coverage_contract": analysis_coverage, } diff --git a/skills/deeppapernote/scripts/common.py b/skills/deeppapernote/scripts/common.py index e3ecd10..bb36709 100644 --- a/skills/deeppapernote/scripts/common.py +++ b/skills/deeppapernote/scripts/common.py @@ -72,6 +72,17 @@ def ensure_parent(path: str | Path) -> None: Path(path).expanduser().resolve().parent.mkdir(parents=True, exist_ok=True) +def file_sha256(path: str | Path) -> str: + candidate = Path(path).expanduser() + if not candidate.is_file(): + return "" + digest = hashlib.sha256() + with candidate.open("rb") as handle: + for chunk in iter(lambda: handle.read(1024 * 1024), b""): + digest.update(chunk) + return digest.hexdigest() + + def emit(payload: dict[str, Any], output_path: str | None = None) -> None: text = json.dumps(payload, ensure_ascii=False, indent=2) if output_path: diff --git a/skills/deeppapernote/scripts/contracts.py b/skills/deeppapernote/scripts/contracts.py index 1aeb02a..e4a3dbe 100644 --- a/skills/deeppapernote/scripts/contracts.py +++ b/skills/deeppapernote/scripts/contracts.py @@ -316,7 +316,14 @@ def required_field_value_error( "candidate_chunks", "section_texts", ), - "figure_decision_values": ("insert", "placeholder", "low_priority", "visual_defect", "skip"), + "figure_decision_values": ( + "review_pending", + "insert", + "placeholder", + "low_priority", + "visual_defect", + "skip", + ), "usable_insert_candidate": { "kinds": ("figure", "table"), "visual_quality_status": "usable_candidate", @@ -335,6 +342,49 @@ def required_field_value_error( "reject_visual_quality", "asset_candidate_missing", ), + "visual_review_contract": { + "selected_render_dpi": 300, + "page_preview_dpi": 96, + "review_fields": ( + "status", + "reviewed_asset_sha256", + "preserved_scientific_elements", + "omitted_scientific_elements", + "notes", + "failure_reason", + "repair_attempts", + "revised_bbox", + ), + "review_status_values": ("pending", "pass", "fail", "repair_requested"), + "repair_limit": 1, + "asset_sha256_bound": True, + "caption_free_visual_body_required": True, + "decision_freeze_before": "note_plan", + "review_evidence_fields": ( + "candidate_path", + "page_preview_path", + "source_pdf_path", + "source_page", + "caption", + "bbox_pt", + "normalized_bbox", + "render_dpi", + ), + "repairable_failure_reasons": ( + "caption_contamination", + "surrounding_prose_contamination", + "scientific_content_clipped", + "insufficient_safety_margin", + ), + "terminal_failure_reasons": ( + "identity_mismatch", + "caption_inseparable", + "ambiguous_visual_body", + "unreadable_source", + "scientific_content_missing", + "repair_limit_exhausted", + ), + }, "note_plan_depth_requirements": { "required_section_focus_min_chars": 20, "required_section_focus_fields": ("focus", "reading_goal", "purpose"), diff --git a/skills/deeppapernote/scripts/extract_pdf_assets.py b/skills/deeppapernote/scripts/extract_pdf_assets.py index 25126e5..638b108 100644 --- a/skills/deeppapernote/scripts/extract_pdf_assets.py +++ b/skills/deeppapernote/scripts/extract_pdf_assets.py @@ -39,7 +39,8 @@ except ImportError: # pragma: no cover pytesseract = None -FIGURE_RENDER_DPI = 200 +FIGURE_RENDER_DPI = 96 +PAGE_PREVIEW_DPI = 96 MIN_FIGURE_HEIGHT_PT = 60 MIN_FIGURE_WIDTH_PT = 100 @@ -628,8 +629,6 @@ def _estimate_figure_bbox_above_caption( the nearest body-text block above and the caption. """ caption_y_top = caption_anchor["bbox"][1] - caption_y_bottom = caption_anchor["bbox"][3] - upper_bound = 0.0 if prev_anchor is not None: upper_bound = prev_anchor["bbox"][3] + 2.0 @@ -647,10 +646,32 @@ def _estimate_figure_bbox_above_caption( relevant.append((r[0], r[1], r[2], clipped_y1)) if relevant: - caption_x0, _, caption_x1, _ = caption_anchor["bbox"] - x0 = min([r[0] for r in relevant] + [caption_x0]) + visual_bbox = ( + min(r[0] for r in relevant), + min(r[1] for r in relevant), + max(r[2] for r in relevant), + max(r[3] for r in relevant), + ) + for line in _collect_text_lines(page): + text = normalize_whitespace(str(line.get("text", ""))) + bb = tuple(line.get("bbox", ())) + if ( + len(bb) != 4 + or len(text) > 80 + or CAPTION_RE.match(text) + or bb[1] >= caption_y_top - 1.0 + or bb[3] <= upper_bound + ): + continue + horizontal_gap = max(visual_bbox[0] - bb[2], bb[0] - visual_bbox[2], 0.0) + vertical_gap = max(visual_bbox[1] - bb[3], bb[1] - visual_bbox[3], 0.0) + if (vertical_gap == 0.0 and horizontal_gap <= 36.0) or ( + horizontal_gap == 0.0 and vertical_gap <= 12.0 + ): + relevant.append((bb[0], bb[1], bb[2], min(bb[3], caption_y_top - 2.0))) + x0 = min(r[0] for r in relevant) y0 = min(r[1] for r in relevant) - x1 = max([r[2] for r in relevant] + [caption_x1]) + x1 = max(r[2] for r in relevant) y1 = max(r[3] for r in relevant) else: body_blocks = _find_body_text_blocks(page) @@ -663,9 +684,8 @@ def _estimate_figure_bbox_above_caption( x1 = page_rect.x1 y1 = caption_y_top - 2.0 - y1 = max(y1, caption_y_bottom + 2.0) - bbox = _clip_to_page((x0, y0, x1, y1), page_rect) + bbox = (bbox[0], bbox[1], bbox[2], min(bbox[3], caption_y_top - 1.0)) width = bbox[2] - bbox[0] height = bbox[3] - bbox[1] if width < MIN_FIGURE_WIDTH_PT or height < MIN_FIGURE_HEIGHT_PT: @@ -927,13 +947,13 @@ def _finalize_table_bbox( caption_anchor: dict, extra_rects: list[tuple[float, float, float, float]], page_rect, + *, + direction: str, ) -> tuple[float, float, float, float] | None: if not extra_rects: return None - caption_x0, caption_y0, caption_x1, caption_y1 = caption_anchor["bbox"] - accepted: list[tuple[float, float, float, float]] = list(extra_rects) + [ - (caption_x0, caption_y0, caption_x1, caption_y1) - ] + _, caption_y0, _, caption_y1 = caption_anchor["bbox"] + accepted: list[tuple[float, float, float, float]] = list(extra_rects) y0 = min(b[1] for b in accepted) y1 = max(b[3] for b in accepted) @@ -954,6 +974,10 @@ def _finalize_table_bbox( y1 = max(b[3] for b in accepted) bbox = _clip_to_page((x0, y0, x1, y1), page_rect, padding=6.0) + if direction == "down": + bbox = (bbox[0], max(bbox[1], caption_y1 + 1.0), bbox[2], bbox[3]) + else: + bbox = (bbox[0], bbox[1], bbox[2], min(bbox[3], caption_y0 - 1.0)) width = bbox[2] - bbox[0] height = bbox[3] - bbox[1] if width < MIN_FIGURE_WIDTH_PT or height < MIN_FIGURE_HEIGHT_PT: @@ -998,7 +1022,6 @@ def _estimate_table_bbox_with_rows( the same y-range, in case the paper places company-logo plots inside a table cell. """ - caption_y0 = caption_anchor["bbox"][1] caption_y1 = caption_anchor["bbox"][3] upper_bound = page_rect.y0 @@ -1040,11 +1063,19 @@ def _estimate_table_bbox_with_rows( if up_data > down_data: chosen = up_lines chosen_data_rows = up_data + direction = "up" else: chosen = down_lines chosen_data_rows = down_data + direction = "down" - bbox = _finalize_table_bbox(page, caption_anchor, chosen, page_rect) + bbox = _finalize_table_bbox( + page, + caption_anchor, + chosen, + page_rect, + direction=direction, + ) if bbox is None: return None return bbox, chosen_data_rows @@ -1176,6 +1207,7 @@ def main() -> None: asset_root = Path(args.assets_dir).expanduser().resolve() if args.assets_dir else default_assets_dir(record) images_dir = asset_root / "images" + previews_dir = asset_root / "page_previews" images_dir.mkdir(parents=True, exist_ok=True) figure_dpi = args.figure_dpi @@ -1209,6 +1241,20 @@ def main() -> None: image_assets.extend(page_images) page_figures = extract_figure_regions(page, page_number, images_dir, dpi=figure_dpi) + page_preview_path = "" + if page_figures: + preview_path = previews_dir / f"page_{page_number:03d}.png" + save_image_bytes( + preview_path, + _render_crop( + page, + (page.rect.x0, page.rect.y0, page.rect.x1, page.rect.y1), + PAGE_PREVIEW_DPI, + ), + ) + page_preview_path = str(preview_path) + for figure in page_figures: + figure["page_preview_path"] = page_preview_path figure_assets.extend(page_figures) page_records.append( @@ -1219,6 +1265,7 @@ def main() -> None: "ocr_used": extraction_method == "ocr", "image_count": len(page_images), "figure_count": len(page_figures), + "page_preview_path": page_preview_path, "page_text": text or ocr_text, "text_preview": (text or ocr_text)[:240], } diff --git a/skills/deeppapernote/scripts/lint_grounding.py b/skills/deeppapernote/scripts/lint_grounding.py index 4a96b83..3a48333 100644 --- a/skills/deeppapernote/scripts/lint_grounding.py +++ b/skills/deeppapernote/scripts/lint_grounding.py @@ -8,7 +8,13 @@ from pathlib import Path from typing import Any -from common import caption_label_key, emit, maybe_load_json_record, normalize_whitespace +from common import ( + caption_label_key, + emit, + file_sha256, + maybe_load_json_record, + normalize_whitespace, +) from contracts import ( NOTE_PLAN_FIELD_TYPES, NOTE_PLAN_REQUIRED_FIELDS, @@ -19,6 +25,148 @@ from source_corpus import SourceCorpusLoadError, load_source_corpus +def visual_review_is_valid(review: Any) -> bool: + contract = WRITING_CONTRACT_RULES["visual_review_contract"] + if not isinstance(review, dict): + return False + if set(review) != set(contract["review_fields"]): + return False + status = normalize_whitespace(str(review.get("status", ""))) + if status not in set(contract["review_status_values"]): + return False + reviewed_sha256 = review.get("reviewed_asset_sha256") + if not isinstance(reviewed_sha256, str): + return False + if not all( + isinstance(review.get(field), list) + and all(isinstance(value, str) for value in review[field]) + for field in ("preserved_scientific_elements", "omitted_scientific_elements") + ): + return False + if not isinstance(review.get("notes"), str) or not isinstance( + review.get("failure_reason"), str + ): + return False + failure_reason = normalize_whitespace(review["failure_reason"]) + attempts = review.get("repair_attempts") + if not isinstance(attempts, int) or isinstance(attempts, bool): + return False + if attempts < 0 or attempts > int(contract["repair_limit"]): + return False + bbox = review.get("revised_bbox") + if not isinstance(bbox, list): + return False + if bbox: + if len(bbox) != 4 or not all( + isinstance(value, (int, float)) and not isinstance(value, bool) + for value in bbox + ): + return False + x0, y0, x1, y1 = (float(value) for value in bbox) + if not (0.0 <= x0 < x1 <= 1.0 and 0.0 <= y0 < y1 <= 1.0): + return False + repairable = set(contract["repairable_failure_reasons"]) + terminal = set(contract["terminal_failure_reasons"]) + if status == "pending": + return not reviewed_sha256 and not failure_reason + if status == "pass": + return bool( + reviewed_sha256 + and not failure_reason + and not review["omitted_scientific_elements"] + ) + if status == "repair_requested": + return bool( + reviewed_sha256 + and failure_reason in repairable + and bbox + and attempts < int(contract["repair_limit"]) + ) + if status == "fail": + return bool( + reviewed_sha256 + and ( + failure_reason in terminal + or ( + attempts >= int(contract["repair_limit"]) + and failure_reason in repairable + ) + ) + ) + return True + + +def visual_review_evidence_is_valid(item: dict[str, Any]) -> bool: + evidence = item.get("review_evidence", {}) + contract = WRITING_CONTRACT_RULES["visual_review_contract"] + if not isinstance(evidence, dict): + return False + if set(evidence) != set(contract["review_evidence_fields"]): + return False + source_path = Path(str(item.get("source_image_path", ""))).expanduser() + candidate_path = Path(str(evidence.get("candidate_path", ""))).expanduser() + preview_path = Path(str(evidence.get("page_preview_path", ""))).expanduser() + pdf_path = Path(str(evidence.get("source_pdf_path", ""))).expanduser() + if not ( + source_path.is_file() + and candidate_path.is_file() + and source_path.resolve() == candidate_path.resolve() + and preview_path.is_file() + and pdf_path.is_file() + ): + return False + if not isinstance(evidence.get("source_page"), int) or int( + evidence.get("source_page", 0) + ) <= 0: + return False + if not isinstance(evidence.get("caption"), str): + return False + bbox_pt = evidence.get("bbox_pt") + if not isinstance(bbox_pt, list) or len(bbox_pt) != 4 or not all( + isinstance(value, (int, float)) and not isinstance(value, bool) + for value in bbox_pt + ): + return False + x0, y0, x1, y1 = (float(value) for value in bbox_pt) + if not (x0 < x1 and y0 < y1): + return False + normalized = evidence.get("normalized_bbox") + if not isinstance(normalized, list) or len(normalized) != 4 or not all( + isinstance(value, (int, float)) and not isinstance(value, bool) + for value in normalized + ): + return False + nx0, ny0, nx1, ny1 = (float(value) for value in normalized) + if not (0.0 <= nx0 < nx1 <= 1.0 and 0.0 <= ny0 < ny1 <= 1.0): + return False + return evidence.get("render_dpi") == contract["selected_render_dpi"] + + +def final_visual_failure_is_valid(item: dict[str, Any]) -> bool: + if normalize_whitespace(str(item.get("decision", ""))) != "visual_defect": + return False + review = item.get("visual_review", {}) + if not visual_review_is_valid(review) or not visual_review_evidence_is_valid(item): + return False + if normalize_whitespace(str(review.get("status", ""))) != "fail": + return False + failure_reason = normalize_whitespace(str(review.get("failure_reason", ""))) + contract = WRITING_CONTRACT_RULES["visual_review_contract"] + allowed = set(contract["terminal_failure_reasons"]) + if int(review.get("repair_attempts", 0) or 0) >= int(contract["repair_limit"]): + allowed.update(contract["repairable_failure_reasons"]) + source_path = normalize_whitespace(str(item.get("source_image_path", ""))) + current_sha256 = file_sha256(source_path) + return bool( + failure_reason in allowed + and current_sha256 + and normalize_whitespace(str(item.get("source_image_sha256", ""))) + == current_sha256 + and normalize_whitespace(str(review.get("reviewed_asset_sha256", ""))) + == current_sha256 + ) + + def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "lint grounding") p.add_argument("--note-plan", required=True, help="note_plan JSON path or JSON string.") @@ -411,6 +559,13 @@ def validate_figure_decisions( source_id=item.get("source_id") or item.get("label") or "", ) ) + if decision == "review_pending": + issues.append( + issue( + "figure_visual_review_unresolved", + source_id=item.get("source_id") or item.get("label") or "", + ) + ) if decision == "insert" and not normalize_whitespace( str(item.get("source_image_path", "")) ): @@ -420,13 +575,57 @@ def validate_figure_decisions( source_id=item.get("source_id") or item.get("label") or "", ) ) + if decision == "insert": + source_path = normalize_whitespace(str(item.get("source_image_path", ""))) + current_sha256 = file_sha256(source_path) + review = item.get("visual_review", {}) + if not visual_review_evidence_is_valid(item): + issues.append( + issue( + "figure_visual_review_evidence_invalid", + source_id=item.get("source_id") or item.get("label") or "", + ) + ) + if not visual_review_is_valid(review): + issues.append( + issue( + "figure_visual_review_invalid", + source_id=item.get("source_id") or item.get("label") or "", + ) + ) + reviewed_sha256 = ( + normalize_whitespace(str(review.get("reviewed_asset_sha256", ""))) + if isinstance(review, dict) + else "" + ) + recorded_sha256 = normalize_whitespace( + str(item.get("source_image_sha256", "")) + ) + if ( + not isinstance(review, dict) + or not visual_review_is_valid(review) + or normalize_whitespace(str(review.get("status", ""))) != "pass" + or not current_sha256 + or reviewed_sha256 != current_sha256 + or recorded_sha256 != current_sha256 + ): + issues.append( + issue( + "figure_visual_review_stale", + source_id=item.get("source_id") or item.get("label") or "", + ) + ) is_required_insert_candidate = ( normalize_whitespace(str(item.get("kind", ""))) in insertable_kinds and normalize_whitespace(str(item.get("visual_quality_status", ""))) == usable_insert["visual_quality_status"] and normalize_whitespace(str(item.get("source_image_path", ""))) ) - if is_required_insert_candidate and decision != "insert": + if ( + is_required_insert_candidate + and decision not in {"insert", "review_pending"} + and not final_visual_failure_is_valid(item) + ): skip_reason = normalize_whitespace(str(item.get("skip_reason", ""))) issues.append( issue( diff --git a/skills/deeppapernote/scripts/plan_figure_table_decisions.py b/skills/deeppapernote/scripts/plan_figure_table_decisions.py index 15e28a5..ab824b0 100644 --- a/skills/deeppapernote/scripts/plan_figure_table_decisions.py +++ b/skills/deeppapernote/scripts/plan_figure_table_decisions.py @@ -12,21 +12,31 @@ caption_label_key, caption_preference_score, emit, + file_sha256, maybe_load_json_record, normalize_whitespace, ) from contracts import WRITING_CONTRACT_RULES +from extract_pdf_assets import _render_crop, save_image_bytes from source_corpus import SourceCorpusLoadError, load_source_corpus DECISION_VALUES = set(WRITING_CONTRACT_RULES["figure_decision_values"]) INSERTABLE_KINDS = set(WRITING_CONTRACT_RULES["usable_insert_candidate"]["kinds"]) +REVIEW_RENDER_DPI = int( + WRITING_CONTRACT_RULES["visual_review_contract"]["selected_render_dpi"] +) def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "plan figure/table decisions") - p.add_argument("--source-manifest", required=True, help="Source manifest JSON path or string.") + p.add_argument("--source-manifest", default="", help="Source manifest JSON path or string.") p.add_argument("--figures", default="", help="Figure plan JSON path or string.") p.add_argument("--assets", default="", help="PDF assets JSON path or string.") + p.add_argument( + "--review-decisions", + default="", + help="Existing decision JSON whose requested bounded repairs should be applied.", + ) p.add_argument("--output", default="", help="Output JSON path.") p.add_argument("--paper-id", default="", help="Canonical paper id.") return p @@ -167,15 +177,211 @@ def source_image_filename(plan_item: dict[str, Any]) -> str: return re.split(r"[\\/]", path)[-1] if path else "" -def should_insert(caption: dict[str, Any], plan_item: dict[str, Any], status: str) -> bool: +def should_review(caption: dict[str, Any], plan_item: dict[str, Any], status: str) -> bool: if status != "usable_candidate": return False if caption.get("kind") not in INSERTABLE_KINDS: return False - return bool(source_image_path(plan_item)) + path = source_image_path(plan_item) + return bool(path and Path(path).expanduser().is_file()) + + +def matched_figure_asset(assets_wrapper: dict[str, Any], label: str) -> dict[str, Any]: + target = normalize_label(label) + for asset in assets_wrapper.get("figure_assets", []) or []: + if isinstance(asset, dict) and normalize_label(str(asset.get("label", ""))) == target: + return asset + return {} + + +def normalized_bbox(page_rect, bbox: list[float]) -> list[float]: + width = max(float(page_rect.width), 1.0) + height = max(float(page_rect.height), 1.0) + return [ + round((bbox[0] - page_rect.x0) / width, 6), + round((bbox[1] - page_rect.y0) / height, 6), + round((bbox[2] - page_rect.x0) / width, 6), + round((bbox[3] - page_rect.y0) / height, 6), + ] + + +def prepare_review_candidate( + caption: dict[str, Any], + plan_item: dict[str, Any], + assets_wrapper: dict[str, Any], +) -> dict[str, Any]: + image_path = source_image_path(plan_item) + filename = source_image_filename(plan_item) + asset = matched_figure_asset(assets_wrapper, str(caption.get("label", ""))) + pdf_path = Path(str(assets_wrapper.get("pdf_path", ""))).expanduser() + bbox = asset.get("bbox_pt", []) if isinstance(asset.get("bbox_pt"), list) else [] + page_number = int(asset.get("page_number", 0) or 0) + evidence = { + "candidate_path": image_path, + "page_preview_path": str(asset.get("page_preview_path", "")), + "source_pdf_path": str(pdf_path) if str(pdf_path) != "." else "", + "source_page": page_number, + "caption": caption.get("caption", ""), + "bbox_pt": bbox, + "normalized_bbox": [], + "render_dpi": 0, + } + if pdf_path.is_file() and page_number > 0 and len(bbox) == 4: + doc = None + try: + from common import fitz + + if fitz is not None: + doc = fitz.open(pdf_path.resolve()) + page = doc[page_number - 1] + source = Path(image_path).expanduser() + output_path = source.with_name(f"{source.stem}_review.png") + save_image_bytes( + output_path, + _render_crop(page, tuple(float(value) for value in bbox), REVIEW_RENDER_DPI), + ) + image_path = str(output_path) + filename = output_path.name + evidence["candidate_path"] = image_path + evidence["normalized_bbox"] = normalized_bbox(page.rect, bbox) + evidence["render_dpi"] = REVIEW_RENDER_DPI + except (OSError, RuntimeError, ValueError, IndexError): + pass + finally: + if doc is not None: + doc.close() + return { + "path": image_path, + "filename": filename, + "sha256": file_sha256(image_path), + "review_evidence": evidence, + } + +def validate_normalized_bbox(value: Any) -> list[float]: + if not isinstance(value, list) or len(value) != 4: + raise SystemExit("revised_bbox must be [x0, y0, x1, y1].") + if not all( + isinstance(item, (int, float)) and not isinstance(item, bool) + for item in value + ): + raise SystemExit("revised_bbox coordinates must be numeric.") + bbox = [float(item) for item in value] + x0, y0, x1, y1 = bbox + if not (0.0 <= x0 < x1 <= 1.0 and 0.0 <= y0 < y1 <= 1.0): + raise SystemExit( + "revised_bbox coordinates must be ordered and within [0, 1]." + ) + return bbox + + +def apply_requested_repairs(wrapper: dict[str, Any]) -> dict[str, Any]: + decisions = wrapper.get("decisions", []) + if not isinstance(decisions, list): + raise SystemExit("--review-decisions requires a decisions list.") + contract = WRITING_CONTRACT_RULES["visual_review_contract"] + repairable = set(contract["repairable_failure_reasons"]) + repair_limit = int(contract["repair_limit"]) + + for item in decisions: + if not isinstance(item, dict): + continue + review = item.get("visual_review", {}) + if not isinstance(review, dict) or review.get("status") != "repair_requested": + continue + source_path = Path(str(item.get("source_image_path", ""))).expanduser() + current_sha256 = file_sha256(str(source_path)) + if not current_sha256 or review.get("reviewed_asset_sha256") != current_sha256: + raise SystemExit("Bounded crop repair requires a current reviewed asset SHA-256.") + attempts = int(review.get("repair_attempts", 0) or 0) + if attempts >= repair_limit: + review.update( + { + "status": "fail", + "failure_reason": "repair_limit_exhausted", + "reviewed_asset_sha256": current_sha256, + } + ) + item["decision"] = "visual_defect" + item["skip_reason"] = "repair_limit_exhausted" + continue + failure_reason = normalize_whitespace(str(review.get("failure_reason", ""))) + if failure_reason not in repairable: + raise SystemExit( + f"Failure reason is not repairable by recropping: {failure_reason}" + ) + revised_bbox = validate_normalized_bbox(review.get("revised_bbox")) + evidence = item.get("review_evidence", {}) + if not isinstance(evidence, dict): + raise SystemExit("Bounded crop repair requires review_evidence.") + pdf_path = Path(str(evidence.get("source_pdf_path", ""))).expanduser() + page_number = int(evidence.get("source_page", 0) or 0) + if not pdf_path.is_file() or page_number <= 0: + raise SystemExit("Bounded crop repair requires a source PDF and page.") + + from common import fitz + + if fitz is None: + raise SystemExit("Bounded crop repair requires PyMuPDF (`fitz`).") + doc = fitz.open(pdf_path.resolve()) + try: + page = doc[page_number - 1] + page_rect = page.rect + x0, y0, x1, y1 = revised_bbox + bbox_pt = [ + page_rect.x0 + x0 * page_rect.width, + page_rect.y0 + y0 * page_rect.height, + page_rect.x0 + x1 * page_rect.width, + page_rect.y0 + y1 * page_rect.height, + ] + repair_path = source_path.with_name( + f"{source_path.stem}_repair{attempts + 1}.png" + ) + save_image_bytes( + repair_path, + _render_crop(page, tuple(bbox_pt), REVIEW_RENDER_DPI), + ) + finally: + doc.close() + + repaired_sha256 = file_sha256(str(repair_path)) + item["decision"] = "review_pending" + item["source_image_path"] = str(repair_path) + item["source_image_filename"] = repair_path.name + item["source_image_sha256"] = repaired_sha256 + if item.get("relative_markdown_embed"): + item["relative_markdown_embed"] = ( + f"![{item.get('source_id') or repair_path.name}]" + f"(images/{repair_path.name})" + ) + evidence.update( + { + "candidate_path": str(repair_path), + "bbox_pt": [round(value, 6) for value in bbox_pt], + "normalized_bbox": revised_bbox, + "render_dpi": REVIEW_RENDER_DPI, + } + ) + item["review_evidence"] = evidence + item["visual_review"] = { + "status": "pending", + "reviewed_asset_sha256": "", + "preserved_scientific_elements": [], + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "", + "repair_attempts": attempts + 1, + "revised_bbox": revised_bbox, + } + wrapper["decisions"] = decisions + return wrapper -def decide(caption: dict[str, Any], plan_item: dict[str, Any] | None) -> dict[str, Any]: + +def decide( + caption: dict[str, Any], + plan_item: dict[str, Any] | None, + assets_wrapper: dict[str, Any] | None = None, +) -> dict[str, Any]: label = normalize_whitespace(str(caption.get("label", ""))) fallback_caption = normalize_whitespace(str(caption.get("caption", "")))[:40] base = { @@ -202,8 +408,14 @@ def decide(caption: dict[str, Any], plan_item: dict[str, Any] | None) -> dict[st base["plan_kind"] = plan_item.get("kind", "") base["visual_quality_status"] = status base["reason"] = plan_item.get("reason", "") or "selected_by_figure_plan" - filename = source_image_filename(plan_item) - image_path = source_image_path(plan_item) + assets_wrapper = assets_wrapper or {} + prepared = ( + prepare_review_candidate(caption, plan_item, assets_wrapper) + if status == "usable_candidate" + else {} + ) + filename = str(prepared.get("filename") or source_image_filename(plan_item)) + image_path = str(prepared.get("path") or source_image_path(plan_item)) if filename: base["source_image_filename"] = filename base["relative_markdown_embed"] = f"![{label or filename}](images/{filename})" @@ -212,10 +424,22 @@ def decide(caption: dict[str, Any], plan_item: dict[str, Any] | None) -> dict[st if status in {"reject", "reject_visual_quality"}: base["decision"] = "visual_defect" base["skip_reason"] = "visual_quality_gate_rejected_candidate" - elif should_insert(caption, plan_item, status): - base["decision"] = "insert" + elif should_review(caption, plan_item, status): + base["decision"] = "review_pending" base["materialization_status"] = "pending" base["skip_reason"] = "" + base["source_image_sha256"] = str(prepared.get("sha256") or file_sha256(image_path)) + base["review_evidence"] = prepared.get("review_evidence", {}) + base["visual_review"] = { + "status": "pending", + "reviewed_asset_sha256": "", + "preserved_scientific_elements": [], + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + } else: base["decision"] = "placeholder" if status == "usable_candidate": @@ -231,10 +455,15 @@ def build_decisions( source_manifest: dict[str, Any], figures_wrapper: dict[str, Any], source_manifest_input: str = "", + assets_wrapper: dict[str, Any] | None = None, ) -> list[dict[str, Any]]: planned = planned_items(figures_wrapper) decisions = [ - decide(caption, planned.get(normalize_label(str(caption.get("label", ""))))) + decide( + caption, + planned.get(normalize_label(str(caption.get("label", "")))), + assets_wrapper, + ) for caption in source_caption_items(source_manifest, source_manifest_input) ] for decision in decisions: @@ -246,10 +475,26 @@ def build_decisions( def main() -> None: args = parser().parse_args() + if args.review_decisions: + payload = apply_requested_repairs(load_record(args.review_decisions)) + payload["status"] = "ok" + payload["script"] = "plan_figure_table_decisions.py" + emit(payload, args.output) + return + if not args.source_manifest: + raise SystemExit( + "plan_figure_table_decisions.py requires --source-manifest " + "or --review-decisions." + ) source_manifest = load_record(args.source_manifest) figures = load_record(args.figures) if args.figures else {} - _assets = load_record(args.assets) if args.assets else {} - decisions = build_decisions(source_manifest, figures, args.source_manifest) + assets = load_record(args.assets) if args.assets else {} + decisions = build_decisions( + source_manifest, + figures, + args.source_manifest, + assets, + ) payload = { "status": "ok", "script": "plan_figure_table_decisions.py", diff --git a/skills/deeppapernote/scripts/write_obsidian_note.py b/skills/deeppapernote/scripts/write_obsidian_note.py index 2ebb02e..e220f0b 100644 --- a/skills/deeppapernote/scripts/write_obsidian_note.py +++ b/skills/deeppapernote/scripts/write_obsidian_note.py @@ -13,6 +13,7 @@ from common import ( emit, ensure_parent, + file_sha256, maybe_load_json_record, resolve_domain_subdir, resolve_note_asset_dir, @@ -114,6 +115,18 @@ def materialize_insert_decisions( if not source_value or not source_image.is_file(): label = item.get("source_id") or item.get("label") or item.get("item_id") or "unknown" raise SystemExit(f"Insert decision source image does not exist for {label}: {source_value}") + current_sha256 = file_sha256(source_image) + review = item.get("visual_review", {}) + if ( + not isinstance(review, dict) + or str(review.get("status", "")).strip() != "pass" + or str(review.get("reviewed_asset_sha256", "")).strip() != current_sha256 + or str(item.get("source_image_sha256", "")).strip() != current_sha256 + ): + label = item.get("source_id") or item.get("label") or item.get("item_id") or "unknown" + raise SystemExit( + f"Insert decision for {label} does not match its reviewed asset SHA-256." + ) filename = safe_image_filename( str(item.get("source_image_filename", "")), source_image, @@ -130,12 +143,17 @@ def materialize_insert_decisions( raise SystemExit(f"Unsafe figure image destination: {dest_image}") if source_image.resolve() != dest_image.resolve(): shutil.copy2(source_image, dest_image) + if file_sha256(dest_image) != current_sha256: + raise SystemExit( + f"Materialized image bytes do not match the reviewed asset SHA-256: {filename}" + ) materialized.append( { "source_id": item.get("source_id") or item.get("label") or item.get("item_id") or "", "source_image": str(source_image.resolve()), "dest_image_path": str(dest_image), "relative_markdown_path": expected_relative, + "reviewed_asset_sha256": current_sha256, } ) return materialized diff --git a/tests/test_build_synthesis_bundle_contract.py b/tests/test_build_synthesis_bundle_contract.py index 7fac8d2..5208637 100644 --- a/tests/test_build_synthesis_bundle_contract.py +++ b/tests/test_build_synthesis_bundle_contract.py @@ -95,6 +95,25 @@ def test_bundle_compact_writing_contract_keeps_depth_rules_without_old_bundle_fi WRITING_CONTRACT_RULES["automatic_fail_closed_visual_statuses"] ) assert contract["figure_table_contract"]["manual_review_claim_requires_image_inspection"] is True + review_contract = contract["figure_table_contract"]["visual_review"] + assert review_contract["selected_render_dpi"] == 300 + assert review_contract["page_preview_dpi"] == 96 + assert review_contract["repair_limit"] == 1 + assert review_contract["repairable_failure_reasons"] == [ + "caption_contamination", + "surrounding_prose_contamination", + "scientific_content_clipped", + "insufficient_safety_margin", + ] + assert review_contract["terminal_failure_reasons"] == [ + "identity_mismatch", + "caption_inseparable", + "ambiguous_visual_body", + "unreadable_source", + "scientific_content_missing", + "repair_limit_exhausted", + ] + assert "review_pending" in contract["figure_table_contract"]["decision_values"] assert contract["note_plan_contract"]["analysis_coverage_field"] == "central_claims[*]" assert contract["analysis_coverage_contract"]["required_plan_fields"] == list( WRITING_CONTRACT_RULES["analysis_coverage_contract"]["required_plan_fields"] diff --git a/tests/test_contracts_consistency.py b/tests/test_contracts_consistency.py index 7bf02ce..ac7518b 100644 --- a/tests/test_contracts_consistency.py +++ b/tests/test_contracts_consistency.py @@ -417,6 +417,15 @@ def test_bundle_exposes_complete_canonical_figure_contract() -> None: figure_contract = bundle( metadata={}, evidence_wrapper={}, figures_wrapper={}, assets_wrapper={} )["writing_contract"]["figure_table_contract"] + visual_review = dict(WRITING_CONTRACT_RULES["visual_review_contract"]) + for field in ( + "review_fields", + "review_status_values", + "review_evidence_fields", + "repairable_failure_reasons", + "terminal_failure_reasons", + ): + visual_review[field] = list(visual_review[field]) assert figure_contract == { "placeholder_first": True, @@ -444,6 +453,7 @@ def test_bundle_exposes_complete_canonical_figure_contract() -> None: WRITING_CONTRACT_RULES["automatic_fail_closed_visual_statuses"] ), "manual_review_claim_requires_image_inspection": True, + "visual_review": visual_review, } @@ -459,6 +469,7 @@ def test_figure_protocol_docs_keep_single_owners() -> None: assert "complete figure/table decision table" in skill assert "grounding and final-note figure gates" in skill assert "Formal Save materializes the selected image" in skill + assert "Figure/Table Decision Freeze" in skill for duplicate in ( "needs_visual_quality_check", "reject_visual_quality", @@ -472,6 +483,10 @@ def test_figure_protocol_docs_keep_single_owners() -> None: assert "identity match" in placement assert "visual usability" in placement assert "asset_candidate_missing" in placement + assert "Visual-Body Crop" in placement + assert "complete source page" in placement + assert "Bounded Crop Repair" in placement + assert "before `note_plan`" in placement for duplicate in ( "kept_placeholder_visual_defect", "kept_placeholder_materialization_blocked", diff --git a/tests/test_extract_pdf_assets_quality.py b/tests/test_extract_pdf_assets_quality.py index 0fb4531..cc1a932 100644 --- a/tests/test_extract_pdf_assets_quality.py +++ b/tests/test_extract_pdf_assets_quality.py @@ -67,6 +67,72 @@ def write_fetch_input(path: Path, pdf_path: Path, *, title: str) -> None: ) +def write_captioned_figure_pdf(path: Path) -> None: + if fitz is None: + pytest.skip("PyMuPDF is required for PDF asset integration tests.") + doc = fitz.open() + try: + page = doc.new_page(width=600.0, height=800.0) + page.draw_rect(fitz.Rect(80.0, 80.0, 520.0, 300.0), width=2.0) + page.draw_line(fitz.Point(90.0, 280.0), fitz.Point(500.0, 100.0), width=2.0) + page.insert_text((88.0, 98.0), "panel A", fontsize=10) + page.insert_text((45.0, 190.0), "axis title", fontsize=10) + page.insert_text((466.0, 292.0), "edge label", fontsize=10) + page.insert_text( + (80.0, 340.0), + "Figure 1. Architecture overview with a long external caption.", + fontsize=10, + ) + page.insert_text( + (80.0, 372.0), + "Surrounding paper prose must not become image pixels.", + fontsize=10, + ) + doc.save(path) + finally: + doc.close() + + +def write_tables_with_captions_above_and_below(path: Path) -> None: + if fitz is None: + pytest.skip("PyMuPDF is required for PDF asset integration tests.") + doc = fitz.open() + try: + for caption_above in (True, False): + page = doc.new_page(width=600.0, height=800.0) + caption_y = 80.0 if caption_above else 230.0 + table_top = 110.0 if caption_above else 70.0 + page.insert_text( + (80.0, caption_y), + ( + f"Table {1 if caption_above else 2}. Main benchmark results across " + "methods, scores, compute costs, and all evaluation settings." + ), + fontsize=10, + ) + rows = [ + ("Method", "Score", "Cost"), + ("Alpha", "91.2", "8.0"), + ("Beta", "89.4", "5.5"), + ("Gamma", "87.1", "4.2"), + ] + for row_index, row in enumerate(rows): + baseline = table_top + row_index * 24.0 + for column_index, value in enumerate(row): + page.insert_text( + (90.0 + column_index * 150.0, baseline), + value, + fontsize=10, + ) + page.draw_rect( + fitz.Rect(80.0, table_top - 14.0, 500.0, table_top + 80.0), + width=1.0, + ) + doc.save(path) + finally: + doc.close() + + def test_extract_pdf_assets_emits_asset_coverage(tmp_path: Path) -> None: pdf_path = tmp_path / "paper.pdf" write_test_pdf(pdf_path, ["Page 1", "Page 2", "Page 3"]) @@ -99,6 +165,83 @@ def test_extract_pdf_assets_emits_asset_coverage(tmp_path: Path) -> None: } +def test_extract_pdf_assets_emits_caption_free_visual_body_and_page_preview( + tmp_path: Path, +) -> None: + pdf_path = tmp_path / "paper.pdf" + write_captioned_figure_pdf(pdf_path) + input_path = tmp_path / "input.json" + output_path = tmp_path / "assets.json" + write_fetch_input(input_path, pdf_path, title="Caption Free Figure") + + subprocess.run( + [ + sys.executable, + str(EXTRACT_PDF_ASSETS_SCRIPT), + "--input", + str(input_path), + "--output", + str(output_path), + "--assets-dir", + str(tmp_path / "assets"), + "--figure-dpi", + "72", + ], + check=True, + ) + + payload = json.loads(output_path.read_text(encoding="utf-8")) + asset = payload["figure_assets"][0] + assert asset["label"] == "Figure 1" + assert asset["bbox_pt"][0] <= 45.0 + assert asset["bbox_pt"][2] >= 520.0 + assert asset["bbox_pt"][3] < 329.0 + assert asset["caption_text"].startswith("Figure 1.") + assert Path(asset["page_preview_path"]).is_file() + + +def test_extract_pdf_assets_excludes_table_captions_above_and_below( + tmp_path: Path, +) -> None: + pdf_path = tmp_path / "tables.pdf" + write_tables_with_captions_above_and_below(pdf_path) + input_path = tmp_path / "input.json" + output_path = tmp_path / "assets.json" + write_fetch_input(input_path, pdf_path, title="Caption Free Tables") + + subprocess.run( + [ + sys.executable, + str(EXTRACT_PDF_ASSETS_SCRIPT), + "--input", + str(input_path), + "--output", + str(output_path), + "--assets-dir", + str(tmp_path / "assets"), + "--figure-dpi", + "72", + ], + check=True, + ) + + payload = json.loads(output_path.read_text(encoding="utf-8")) + tables = {item["label"]: item for item in payload["figure_assets"]} + assert set(tables) == {"Table 1", "Table 2"} + + doc = fitz.open(pdf_path) + try: + first_caption = _find_caption_blocks(doc[0])[0]["bbox"] + second_caption = _find_caption_blocks(doc[1])[0]["bbox"] + finally: + doc.close() + + assert tables["Table 1"]["bbox_pt"][1] > first_caption[3] + assert tables["Table 1"]["bbox_pt"][1] <= 100.0 + assert tables["Table 2"]["bbox_pt"][3] < second_caption[1] + assert tables["Table 2"]["bbox_pt"][3] >= 150.0 + + def test_extract_pdf_assets_default_scans_short_pdf_without_truncation(tmp_path: Path) -> None: pdf_path = tmp_path / "paper.pdf" write_test_pdf(pdf_path, ["Page 1", "Page 2", "Page 3"]) @@ -314,7 +457,7 @@ def test_restrict_row_to_caption_column_skips_near_mid_opposite_row() -> None: assert restricted is None -def test_figure_bbox_uses_caption_width_for_narrow_vector_figure() -> None: +def test_figure_bbox_does_not_use_caption_width_to_rescue_narrow_visual() -> None: if fitz is None: pytest.skip("PyMuPDF is required for figure bbox tests.") doc = fitz.open() @@ -335,10 +478,7 @@ def test_figure_bbox_uses_caption_width_for_narrow_vector_figure() -> None: bbox = _estimate_figure_bbox_above_caption(page, anchor, None, page.rect) - assert bbox is not None - assert bbox[0] <= anchor["bbox"][0] - assert bbox[2] >= anchor["bbox"][2] - assert bbox[2] < 300.0 + assert bbox is None finally: doc.close() diff --git a/tests/test_figure_table_decisions.py b/tests/test_figure_table_decisions.py index a6238d4..af7cd41 100644 --- a/tests/test_figure_table_decisions.py +++ b/tests/test_figure_table_decisions.py @@ -1,10 +1,18 @@ from __future__ import annotations +import hashlib import json import subprocess import sys from pathlib import Path +import pytest + +try: + import fitz # type: ignore +except ImportError: # pragma: no cover + fitz = None + PROJECT_ROOT = Path(__file__).resolve().parents[1] DECISIONS_SCRIPT = PROJECT_ROOT / "skills" / "deeppapernote" / "scripts" / "plan_figure_table_decisions.py" @@ -26,12 +34,16 @@ def write_raw_sections(path: Path) -> Path: return path -def run_decisions(tmp_path: Path, source_manifest: dict, figures: dict) -> dict: +def run_decisions( + tmp_path: Path, + source_manifest: dict, + figures: dict, + assets: dict | None = None, +) -> dict: source_path = write_json(tmp_path / "source_manifest.json", source_manifest) figures_path = write_json(tmp_path / "figures.json", figures) output_path = tmp_path / "figure_table_decisions.json" - subprocess.run( - [ + command = [ sys.executable, str(DECISIONS_SCRIPT), "--source-manifest", @@ -40,10 +52,41 @@ def run_decisions(tmp_path: Path, source_manifest: dict, figures: dict) -> dict: str(figures_path), "--output", str(output_path), + ] + if assets is not None: + assets_path = write_json(tmp_path / "assets.json", assets) + command.extend(["--assets", str(assets_path)]) + subprocess.run(command, check=True) + return json.loads(output_path.read_text(encoding="utf-8")) + + +def run_review_decisions( + tmp_path: Path, + decisions: dict, + *, + check: bool = True, +) -> tuple[subprocess.CompletedProcess[str], dict | None]: + decisions_path = write_json(tmp_path / "review_decisions.json", decisions) + output_path = tmp_path / "reviewed_decisions.json" + result = subprocess.run( + [ + sys.executable, + str(DECISIONS_SCRIPT), + "--review-decisions", + str(decisions_path), + "--output", + str(output_path), ], - check=True, + check=check, + capture_output=True, + text=True, ) - return json.loads(output_path.read_text(encoding="utf-8")) + payload = ( + json.loads(output_path.read_text(encoding="utf-8")) + if output_path.is_file() + else None + ) + return result, payload def test_figure_table_decisions_cover_every_caption(tmp_path: Path) -> None: @@ -175,7 +218,11 @@ def test_figure_table_decisions_fail_closed_on_visual_defect(tmp_path: Path) -> assert decision["skip_reason"] == "visual_quality_gate_rejected_candidate" -def test_figure_table_decisions_insert_usable_candidate(tmp_path: Path) -> None: +def test_figure_table_decisions_require_visual_review_for_usable_candidate( + tmp_path: Path, +) -> None: + source_image = tmp_path / "page_002_fig_figure_1.png" + source_image.write_bytes(b"candidate-image") source_manifest = { "paper_id": "paper:figures", "captions": { @@ -201,7 +248,7 @@ def test_figure_table_decisions_insert_usable_candidate(tmp_path: Path) -> None: "priority": 1, "figure_asset_candidate": { "filename": "page_002_fig_figure_1.png", - "path": "/tmp/images/page_002_fig_figure_1.png", + "path": str(source_image), "candidate_status": "usable_candidate", "quality_signals": {"visual_quality_status": "usable"}, }, @@ -213,11 +260,242 @@ def test_figure_table_decisions_insert_usable_candidate(tmp_path: Path) -> None: payload = run_decisions(tmp_path, source_manifest, figures) decision = payload["decisions"][0] - assert decision["decision"] == "insert" - assert payload["summary"]["by_decision"]["insert"] == 1 + assert decision["decision"] == "review_pending" + assert decision["source_image_sha256"] == hashlib.sha256(b"candidate-image").hexdigest() + assert decision["visual_review"]["status"] == "pending" + assert decision["visual_review"]["reviewed_asset_sha256"] == "" + assert payload["summary"]["by_decision"]["review_pending"] == 1 + + +def test_figure_table_decisions_rerender_selected_candidate_at_300_dpi( + tmp_path: Path, +) -> None: + if fitz is None: + pytest.skip("PyMuPDF is required for selected-candidate rendering.") + pdf_path = tmp_path / "paper.pdf" + doc = fitz.open() + try: + page = doc.new_page(width=200.0, height=300.0) + page.draw_rect(fitz.Rect(20.0, 30.0, 180.0, 130.0), width=2.0) + doc.save(pdf_path) + finally: + doc.close() + + preview_path = tmp_path / "page_001.png" + preview_path.write_bytes(b"page-preview") + low_res_path = tmp_path / "page_001_fig_figure_1.png" + low_res_path.write_bytes(b"low-res-candidate") + source_manifest = { + "captions": { + "figures": [{"id": "Figure 1", "caption": "Architecture", "page": 1}], + "tables": [], + } + } + figures = { + "figure_plan": { + "figures": [ + { + "id": "Figure 1", + "kind": "method_overview", + "section": "方法主线", + "priority": 1, + "figure_asset_candidate": { + "filename": low_res_path.name, + "path": str(low_res_path), + "candidate_status": "usable_candidate", + }, + } + ] + } + } + assets = { + "pdf_path": str(pdf_path), + "figure_assets": [ + { + "label": "Figure 1", + "page_number": 1, + "bbox_pt": [20.0, 30.0, 180.0, 130.0], + "path": str(low_res_path), + "page_preview_path": str(preview_path), + } + ], + } + + payload = run_decisions(tmp_path, source_manifest, figures, assets) + decision = payload["decisions"][0] + reviewed_path = Path(decision["source_image_path"]) + + assert reviewed_path.name == "page_001_fig_figure_1_review.png" + assert reviewed_path.is_file() + pixmap = fitz.Pixmap(str(reviewed_path)) + assert pixmap.width == 667 + assert pixmap.height == 417 + assert decision["review_evidence"] == { + "candidate_path": str(reviewed_path), + "page_preview_path": str(preview_path), + "source_pdf_path": str(pdf_path), + "source_page": 1, + "caption": "Architecture", + "bbox_pt": [20.0, 30.0, 180.0, 130.0], + "normalized_bbox": [0.1, 0.1, 0.9, 0.433333], + "render_dpi": 300, + } + + +def test_figure_table_decisions_apply_one_normalized_bbox_repair( + tmp_path: Path, +) -> None: + if fitz is None: + pytest.skip("PyMuPDF is required for bounded crop repair.") + pdf_path = tmp_path / "paper.pdf" + doc = fitz.open() + try: + page = doc.new_page(width=200.0, height=400.0) + page.draw_rect(fitz.Rect(20.0, 80.0, 180.0, 240.0), width=2.0) + doc.save(pdf_path) + finally: + doc.close() + source_image = tmp_path / "candidate.png" + source_image.write_bytes(b"caption-contaminated") + digest = hashlib.sha256(b"caption-contaminated").hexdigest() + decisions = { + "status": "ok", + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "review_pending", + "source_image_path": str(source_image), + "source_image_filename": source_image.name, + "source_image_sha256": digest, + "review_evidence": { + "candidate_path": str(source_image), + "page_preview_path": str(tmp_path / "page.png"), + "source_pdf_path": str(pdf_path), + "source_page": 1, + "caption": "Architecture", + "bbox_pt": [0.0, 0.0, 200.0, 300.0], + "normalized_bbox": [0.0, 0.0, 1.0, 0.75], + "render_dpi": 300, + }, + "visual_review": { + "status": "repair_requested", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["diagram"], + "omitted_scientific_elements": [], + "notes": "Remove external caption below the diagram.", + "failure_reason": "caption_contamination", + "repair_attempts": 0, + "revised_bbox": [0.1, 0.2, 0.9, 0.6], + }, + } + ], + } + + _, payload = run_review_decisions(tmp_path, decisions) + assert payload is not None + decision = payload["decisions"][0] + repair_path = Path(decision["source_image_path"]) + + assert repair_path.name == "candidate_repair1.png" + assert repair_path.is_file() + pixmap = fitz.Pixmap(str(repair_path)) + assert pixmap.width == 667 + assert pixmap.height == 667 + assert decision["decision"] == "review_pending" + assert decision["visual_review"]["status"] == "pending" + assert decision["visual_review"]["repair_attempts"] == 1 + assert decision["visual_review"]["revised_bbox"] == [0.1, 0.2, 0.9, 0.6] + assert decision["review_evidence"]["bbox_pt"] == [20.0, 80.0, 180.0, 240.0] + assert decision["source_image_sha256"] == hashlib.sha256( + repair_path.read_bytes() + ).hexdigest() + + +@pytest.mark.parametrize( + "bbox", + ( + [], + [-0.1, 0.1, 0.8, 0.8], + [0.1, 0.1, 1.1, 0.8], + [0.8, 0.1, 0.2, 0.8], + [0.1, 0.8, 0.8, 0.2], + ), +) +def test_figure_table_decisions_reject_invalid_normalized_bbox( + tmp_path: Path, + bbox: list[float], +) -> None: + source_image = tmp_path / "candidate.png" + source_image.write_bytes(b"candidate") + digest = hashlib.sha256(b"candidate").hexdigest() + decisions = { + "decisions": [ + { + "decision": "review_pending", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "visual_review": { + "status": "repair_requested", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": [], + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "caption_contamination", + "repair_attempts": 0, + "revised_bbox": bbox, + }, + } + ] + } + + result, payload = run_review_decisions(tmp_path, decisions, check=False) + + assert result.returncode != 0 + assert payload is None + assert "revised_bbox" in result.stderr + + +def test_figure_table_decisions_fail_closed_after_repair_limit( + tmp_path: Path, +) -> None: + source_image = tmp_path / "candidate_repair1.png" + source_image.write_bytes(b"still-contaminated") + digest = hashlib.sha256(b"still-contaminated").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "decision": "review_pending", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "visual_review": { + "status": "repair_requested", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["diagram"], + "omitted_scientific_elements": [], + "notes": "Caption remains after the first repair.", + "failure_reason": "caption_contamination", + "repair_attempts": 1, + "revised_bbox": [0.1, 0.1, 0.9, 0.7], + }, + } + ] + } + + _, payload = run_review_decisions(tmp_path, decisions) + assert payload is not None + decision = payload["decisions"][0] + + assert decision["decision"] == "visual_defect" + assert decision["skip_reason"] == "repair_limit_exhausted" + assert decision["visual_review"]["status"] == "fail" + assert decision["visual_review"]["failure_reason"] == "repair_limit_exhausted" def test_figure_table_decisions_dedupe_figure_and_fig_variants(tmp_path: Path) -> None: + source_image = tmp_path / "page_011_fig_figure_14.png" + source_image.write_bytes(b"figure-14") source_manifest = { "paper_id": "paper:figures", "captions": { @@ -249,7 +527,7 @@ def test_figure_table_decisions_dedupe_figure_and_fig_variants(tmp_path: Path) - "priority": 2, "figure_asset_candidate": { "filename": "page_011_fig_figure_14.png", - "path": "/tmp/images/page_011_fig_figure_14.png", + "path": str(source_image), "candidate_status": "usable_candidate", }, } @@ -261,12 +539,14 @@ def test_figure_table_decisions_dedupe_figure_and_fig_variants(tmp_path: Path) - assert payload["summary"]["total_items"] == 1 assert payload["decisions"][0]["source_id"] == "Figure 14" - assert payload["decisions"][0]["decision"] == "insert" + assert payload["decisions"][0]["decision"] == "review_pending" def test_figure_table_decisions_insert_selected_usable_figure_regardless_priority( tmp_path: Path, ) -> None: + source_image = tmp_path / "page_002_fig_figure_1.png" + source_image.write_bytes(b"figure-1") source_manifest = { "captions": { "figures": [{"id": "Figure 1", "caption": "Auxiliary distribution", "page": 2}], @@ -283,7 +563,7 @@ def test_figure_table_decisions_insert_selected_usable_figure_regardless_priorit "priority": 3, "figure_asset_candidate": { "filename": "page_002_fig_figure_1.png", - "path": "/tmp/images/page_002_fig_figure_1.png", + "path": str(source_image), "candidate_status": "usable_candidate", }, } @@ -294,11 +574,13 @@ def test_figure_table_decisions_insert_selected_usable_figure_regardless_priorit payload = run_decisions(tmp_path, source_manifest, figures) decision = payload["decisions"][0] - assert decision["decision"] == "insert" + assert decision["decision"] == "review_pending" assert decision["plan_kind"] == "data_or_task" def test_figure_table_decisions_insert_selected_usable_tables(tmp_path: Path) -> None: + source_image = tmp_path / "page_005_fig_table_2.png" + source_image.write_bytes(b"table-2") source_manifest = { "captions": { "figures": [], @@ -314,7 +596,7 @@ def test_figure_table_decisions_insert_selected_usable_tables(tmp_path: Path) -> "priority": 1, "figure_asset_candidate": { "filename": "page_005_fig_table_2.png", - "path": "/tmp/images/page_005_fig_table_2.png", + "path": str(source_image), "candidate_status": "usable_candidate", }, } @@ -325,5 +607,5 @@ def test_figure_table_decisions_insert_selected_usable_tables(tmp_path: Path) -> payload = run_decisions(tmp_path, source_manifest, figures) decision = payload["decisions"][0] - assert decision["decision"] == "insert" + assert decision["decision"] == "review_pending" assert decision["skip_reason"] == "" diff --git a/tests/test_lint_grounding.py b/tests/test_lint_grounding.py index d32bc80..a96b25b 100644 --- a/tests/test_lint_grounding.py +++ b/tests/test_lint_grounding.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import json import subprocess import sys @@ -30,6 +31,23 @@ def write_raw_sections(path: Path) -> Path: return path +def visual_review_evidence(tmp_path: Path, source_image: Path) -> dict: + page_preview = tmp_path / "page_preview.png" + page_preview.write_bytes(b"page-preview") + source_pdf = tmp_path / "source.pdf" + source_pdf.write_bytes(b"source-pdf") + return { + "candidate_path": str(source_image), + "page_preview_path": str(page_preview), + "source_pdf_path": str(source_pdf), + "source_page": 4, + "caption": "Architecture", + "bbox_pt": [10.0, 20.0, 190.0, 220.0], + "normalized_bbox": [0.05, 0.05, 0.95, 0.55], + "render_dpi": 300, + } + + def run_lint_grounding( tmp_path: Path, note_plan: dict, @@ -430,6 +448,342 @@ def test_lint_grounding_rejects_invalid_figure_table_decision_value( assert "figure_table_decision_value_invalid" in codes +def test_lint_grounding_rejects_unresolved_visual_review(tmp_path: Path) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"review-pending") + manifest = source_manifest() + manifest["captions"] = { + "figures": [{"id": "Figure 1", "caption": "Architecture", "page": 4}], + "tables": [], + } + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "review_pending", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": hashlib.sha256(b"review-pending").hexdigest(), + "visual_review": { + "status": "pending", + "reviewed_asset_sha256": "", + "preserved_scientific_elements": [], + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + manifest, + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_unresolved" in codes + + +def test_lint_grounding_rejects_stale_visual_review(tmp_path: Path) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"changed-after-review") + manifest = source_manifest() + manifest["captions"] = { + "figures": [{"id": "Figure 1", "caption": "Architecture", "page": 4}], + "tables": [], + } + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": hashlib.sha256(b"original-candidate").hexdigest(), + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": hashlib.sha256(b"original-candidate").hexdigest(), + "preserved_scientific_elements": ["axes", "legend", "panel labels"], + "omitted_scientific_elements": [], + "notes": "Complete visual body; external caption excluded.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + manifest, + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_stale" in codes + + +def test_lint_grounding_accepts_current_passing_visual_review(tmp_path: Path) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"reviewed-candidate") + digest = hashlib.sha256(b"reviewed-candidate").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "review_evidence": visual_review_evidence(tmp_path, source_image), + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["axes", "legend", "panel labels"], + "omitted_scientific_elements": [], + "notes": "Complete visual body; external caption excluded.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + + assert result["passes_grounding"] is True + + +def test_lint_grounding_rejects_insert_without_visual_review_evidence( + tmp_path: Path, +) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"reviewed-candidate") + digest = hashlib.sha256(b"reviewed-candidate").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["complete figure"], + "omitted_scientific_elements": [], + "notes": "Caption-free visual body.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_evidence_invalid" in codes + + +def test_lint_grounding_rejects_malformed_visual_review(tmp_path: Path) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"reviewed-candidate") + digest = hashlib.sha256(b"reviewed-candidate").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "visual_review": { + "status": "approved", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": "axes and legend", + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + "self_certified": True, + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_invalid" in codes + + +def test_lint_grounding_rejects_inconsistent_passing_visual_review( + tmp_path: Path, +) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"reviewed-candidate") + digest = hashlib.sha256(b"reviewed-candidate").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "review_evidence": visual_review_evidence(tmp_path, source_image), + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["complete figure"], + "omitted_scientific_elements": [], + "notes": "", + "failure_reason": "identity_mismatch", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_invalid" in codes + + +def test_lint_grounding_rejects_passing_review_with_omitted_scientific_elements( + tmp_path: Path, +) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"reviewed-candidate") + digest = hashlib.sha256(b"reviewed-candidate").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "insert", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "review_evidence": visual_review_evidence(tmp_path, source_image), + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["plot body"], + "omitted_scientific_elements": ["axis title"], + "notes": "", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + codes = {issue["code"] for issue in result["issues"]} + + assert result["passes_grounding"] is False + assert "figure_visual_review_invalid" in codes + + +def test_lint_grounding_accepts_terminal_visual_defect(tmp_path: Path) -> None: + source_image = tmp_path / "figure.png" + source_image.write_bytes(b"wrong-figure") + digest = hashlib.sha256(b"wrong-figure").hexdigest() + decisions = { + "decisions": [ + { + "source_id": "Figure 1", + "kind": "figure", + "decision": "visual_defect", + "visual_quality_status": "usable_candidate", + "source_image_path": str(source_image), + "source_image_sha256": digest, + "skip_reason": "identity_mismatch", + "review_evidence": visual_review_evidence(tmp_path, source_image), + "visual_review": { + "status": "fail", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": [], + "omitted_scientific_elements": ["matched Figure 1 visual body"], + "notes": "The crop belongs to another figure.", + "failure_reason": "identity_mismatch", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + + result = run_lint_grounding( + tmp_path, + grounded_note_plan(), + source_manifest(), + slim_bundle(), + decisions, + ) + + assert result["passes_grounding"] is True + + def test_lint_grounding_rejects_placeholder_decision_for_usable_candidate(tmp_path: Path) -> None: manifest = source_manifest() manifest["captions"] = { diff --git a/tests/test_lint_note.py b/tests/test_lint_note.py index a6e266b..5ca4761 100644 --- a/tests/test_lint_note.py +++ b/tests/test_lint_note.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import json import subprocess import sys @@ -1322,11 +1323,29 @@ def passing_lint_payload() -> dict: } +def reviewed_insert_fields(source_image: Path) -> dict: + digest = hashlib.sha256(source_image.read_bytes()).hexdigest() + return { + "source_image_sha256": digest, + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["complete figure"], + "omitted_scientific_elements": [], + "notes": "Caption-free visual body.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + + def test_write_obsidian_note_materializes_insert_decision(tmp_path) -> None: vault = tmp_path / "vault" vault.mkdir() source_image = tmp_path / "page_001_fig_figure_1.png" source_image.write_bytes(b"fake-png") + digest = hashlib.sha256(b"fake-png").hexdigest() lint_path = tmp_path / "lint.json" lint_path.write_text(json.dumps(passing_lint_payload()), encoding="utf-8") decisions_path = tmp_path / "figure_decisions.json" @@ -1339,6 +1358,17 @@ def test_write_obsidian_note_materializes_insert_decision(tmp_path) -> None: "decision": "insert", "source_image_path": str(source_image), "source_image_filename": source_image.name, + "source_image_sha256": digest, + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": digest, + "preserved_scientific_elements": ["complete figure"], + "omitted_scientific_elements": [], + "notes": "Caption-free visual body.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, } ] } @@ -1381,6 +1411,72 @@ def test_write_obsidian_note_materializes_insert_decision(tmp_path) -> None: assert Path(materialized["dest_image_path"]).read_bytes() == b"fake-png" +def test_write_obsidian_note_rejects_stale_reviewed_insert_bytes(tmp_path) -> None: + vault = tmp_path / "vault" + vault.mkdir() + source_image = tmp_path / "page_001_fig_figure_1.png" + reviewed_digest = hashlib.sha256(b"reviewed").hexdigest() + source_image.write_bytes(b"changed-after-review") + lint_path = tmp_path / "lint.json" + lint_path.write_text(json.dumps(passing_lint_payload()), encoding="utf-8") + decisions_path = tmp_path / "figure_decisions.json" + decisions_path.write_text( + json.dumps( + { + "decisions": [ + { + "source_id": "Figure 1", + "decision": "insert", + "source_image_path": str(source_image), + "source_image_filename": source_image.name, + "source_image_sha256": reviewed_digest, + "visual_review": { + "status": "pass", + "reviewed_asset_sha256": reviewed_digest, + "preserved_scientific_elements": ["complete figure"], + "omitted_scientific_elements": [], + "notes": "Caption-free visual body.", + "failure_reason": "", + "repair_attempts": 0, + "revised_bbox": [], + }, + } + ] + } + ), + encoding="utf-8", + ) + script_path = ( + Path(__file__).resolve().parents[1] + / "skills" + / "deeppapernote" + / "scripts" + / "write_obsidian_note.py" + ) + + result = subprocess.run( + [ + sys.executable, + str(script_path), + "--title", + "Stale Figure Review", + "--content", + "# Stale Figure Review\n\n![Figure 1](images/page_001_fig_figure_1.png)\n*Fig. 1 caption.*\n", + "--lint-json", + str(lint_path), + "--figure-decisions", + str(decisions_path), + "--vault", + str(vault), + ], + capture_output=True, + text=True, + ) + + assert result.returncode != 0 + assert "reviewed asset SHA-256" in result.stderr + + def test_write_obsidian_note_rejects_unreferenced_insert_decision(tmp_path) -> None: vault = tmp_path / "vault" vault.mkdir() @@ -1398,6 +1494,7 @@ def test_write_obsidian_note_rejects_unreferenced_insert_decision(tmp_path) -> N "decision": "insert", "source_image_path": str(source_image), "source_image_filename": source_image.name, + **reviewed_insert_fields(source_image), } ] } @@ -1446,6 +1543,7 @@ def test_write_obsidian_note_rejects_plain_path_for_insert_decision(tmp_path) -> "decision": "insert", "source_image_path": str(source_image), "source_image_filename": source_image.name, + **reviewed_insert_fields(source_image), } ] } @@ -1494,6 +1592,7 @@ def test_write_obsidian_note_rejects_unsafe_insert_filename(tmp_path) -> None: "decision": "insert", "source_image_path": str(source_image), "source_image_filename": "../escaped.png", + **reviewed_insert_fields(source_image), } ] }