Fix code review findings: image refs, cache ordering, and cleanup · alice.mosphere.at/claude-skill-pdf-to-markdown@06e9c3f

+138 -65

2 changed files

expand all

scripts

extractor.py

pdf_to_md.py

+24 -3

scripts/extractor.py

··· 11 11 # Format: major.minor.patch 12 12 # 3.1.0: Page separators now use  instead of ----- 13 13 # Image extraction includes nested XObjects (full=True) 14 - EXTRACTOR_VERSION = "3.1.0" 14 + # 3.2.0: Fast mode now includes image references in markdown (write_images=True) 15 + # Cache keys now include no_images flag to avoid contamination 16 + EXTRACTOR_VERSION = "3.2.0" 15 17 16 18 17 19 def check_docling_models(): ··· 27 29 return False 28 30 29 31 30 - def extract_pdf_fast(pdf_path: str, show_progress: bool = False) -> str: 32 + def extract_pdf_fast( 33 + pdf_path: str, image_dir: str = None, show_progress: bool = False 34 + ) -> str: 31 35 """ 32 36 Fast PDF extraction using PyMuPDF with text-based table detection. 33 37 ··· 36 40 37 41 Args: 38 42 pdf_path: Path to the PDF file 43 + image_dir: Directory to save extracted images (None = skip images) 39 44 show_progress: Whether to show progress output 40 45 41 46 Returns: 42 - Markdown string of the PDF content 47 + Markdown string of the PDF content with image references if image_dir provided 43 48 """ 44 49 import pymupdf4llm 45 50 ··· 52 57 pdf_path, 53 58 show_progress=show_progress, 54 59 table_strategy="text", # Better for mixed table types 60 + write_images=image_dir is not None, 61 + image_path=image_dir, 55 62 ) 56 63 57 64 # Replace pymupdf4llm's default page separator with explicit sentinel. ··· 143 150 144 151 # Convert the document 145 152 result = converter.convert(pdf_path) 153 + 154 + # Check for conversion errors 155 + if hasattr(result, "errors") and result.errors: 156 + for error in result.errors: 157 + print(f"WARNING: Docling conversion error: {error}", file=sys.stderr) 158 + 159 + # Check conversion status 160 + from docling.datamodel.base_models import ConversionStatus 161 + 162 + if hasattr(result, "status") and result.status != ConversionStatus.SUCCESS: 163 + print( 164 + f"WARNING: Docling conversion status: {result.status.name}", 165 + file=sys.stderr, 166 + ) 146 167 147 168 # Save images to output directory (order matters for placeholder replacement) 148 169 image_paths = []

+114 -62

scripts/pdf_to_md.py

··· 42 42 43 43 44 44 def get_cache_key( 45 - pdf_path: str, docling: bool = False, images_scale: float = 4.0 45 + pdf_path: str, 46 + docling: bool = False, 47 + images_scale: float = 4.0, 48 + no_images: bool = False, 46 49 ) -> str: 47 50 """Generate cache key from file content + size + mode (path-independent). 48 51 ··· 50 53 pdf_path: Path to the PDF file 51 54 docling: Whether Docling mode is used 52 55 images_scale: Image resolution multiplier (only affects cache key in Docling mode) 56 + no_images: Whether image extraction is disabled 53 57 """ 54 58 p = Path(pdf_path).resolve() 55 59 stat = p.stat() ··· 73 77 mode = f"docling_{images_scale}" 74 78 else: 75 79 mode = "fast" 80 + 81 + # Include no_images flag to avoid cache contamination 82 + if no_images: 83 + mode += "_noimages" 84 + 76 85 raw = f"{file_size}|{hasher.hexdigest()}|{mode}" 77 86 return hashlib.sha256(raw.encode()).hexdigest()[:16] 78 87 ··· 82 91 return CACHE_DIR / cache_key 83 92 84 93 94 + def get_cached_total_pages(cache_key: str) -> int: 95 + """Get total_pages from cache metadata without loading the full content. 96 + 97 + Used to enable page range parsing before loading the full cache. 98 + Returns 0 if cache metadata is unavailable. 99 + """ 100 + cache_dir = get_cache_dir(cache_key) 101 + metadata_file = cache_dir / "metadata.json" 102 + try: 103 + with open(metadata_file) as f: 104 + metadata = json.load(f) 105 + return metadata.get("total_pages", 0) 106 + except (FileNotFoundError, json.JSONDecodeError, OSError): 107 + return 0 108 + 109 + 85 110 def is_cache_valid( 86 - pdf_path: str, docling: bool = False, images_scale: float = 4.0 111 + pdf_path: str, 112 + docling: bool = False, 113 + images_scale: float = 4.0, 114 + no_images: bool = False, 87 115 ) -> tuple: 88 116 """ 89 117 Check if valid cache exists for this PDF. ··· 94 122 from extractor import EXTRACTOR_VERSION 95 123 96 124 try: 97 - cache_key = get_cache_key(pdf_path, docling=docling, images_scale=images_scale) 125 + cache_key = get_cache_key( 126 + pdf_path, docling=docling, images_scale=images_scale, no_images=no_images 127 + ) 98 128 except (FileNotFoundError, OSError): 99 129 return False, "" 100 130 ··· 188 218 total_pages: int, 189 219 docling: bool = False, 190 220 images_scale: float = 4.0, 221 + no_images: bool = False, 191 222 ): 192 223 """Save full extraction to cache using atomic writes. 193 224 ··· 203 234 p = Path(pdf_path).resolve() 204 235 stat = p.stat() 205 236 mode = f"docling_{images_scale}" if docling else "fast" 237 + if no_images: 238 + mode += "_noimages" 206 239 207 240 metadata = { 208 241 "source_path": str(p), ··· 214 247 "extractor_version": EXTRACTOR_VERSION, 215 248 "mode": mode, 216 249 "images_scale": images_scale if docling else None, 250 + "no_images": no_images, 217 251 } 218 252 219 253 # Write to temp files first, then atomic rename ··· 714 748 ) 715 749 return markdown 716 750 else: 717 - from extractor import extract_pdf_to_markdown, extract_images 751 + from extractor import extract_pdf_fast 718 752 719 - # Fast mode: separate text and image extraction 720 - markdown = extract_pdf_to_markdown( 721 - pdf_path, accurate=False, show_progress=show_progress 753 + # Fast mode: pymupdf4llm handles both text and image extraction 754 + markdown = extract_pdf_fast( 755 + pdf_path, 756 + image_dir=image_dir if not no_images else None, 757 + show_progress=show_progress, 722 758 ) 723 - 724 - if not no_images and image_dir: 725 - extract_images(pdf_path, image_dir, show_progress=show_progress) 726 759 727 760 return markdown 728 761 ··· 947 980 if pdf_exists and not args.input.lower().endswith(".pdf"): 948 981 print(f"WARNING: File may not be a PDF: {args.input}", file=sys.stderr) 949 982 950 - # Check dependencies for the requested mode (skip if using cache fallback) 951 - if not cache_fallback and not check_dependencies(docling_mode=args.docling): 983 + # Error if --pages used with --docling (page slicing not supported) 984 + # Check early before any expensive operations 985 + if args.pages and args.docling: 986 + print( 987 + "ERROR: --pages is not supported in Docling mode (no page delimiters in output). " 988 + "Use fast mode (default) for page slicing, or omit --pages.", 989 + file=sys.stderr, 990 + ) 952 991 sys.exit(1) 953 992 954 - # Get total pages (from PDF or from cached metadata) 993 + # Determine if progress should be shown 994 + show_progress = sys.stderr.isatty() and not args.no_progress and not args.stdout 995 + 996 + # Check cache FIRST (before dependency check) 997 + # This allows using cached results even without extraction dependencies installed 998 + cache_hit = False 999 + cache_key = "" 1000 + result = None 1001 + image_dir = None 1002 + total_pages = 0 1003 + 955 1004 if cache_fallback: 956 - # Use metadata we already loaded during cache lookup 1005 + # PDF missing, but we found a cached extraction 1006 + cache_key = fallback_cache_dir.name 957 1007 total_pages = fallback_metadata.get("total_pages", 0) 958 - else: 1008 + cache_hit = True # Will load below after page range parsing 1009 + elif not args.no_cache: 1010 + # Check if we have a valid cache for this PDF 1011 + valid, cache_key = is_cache_valid( 1012 + args.input, 1013 + docling=args.docling, 1014 + images_scale=args.images_scale, 1015 + no_images=args.no_images, 1016 + ) 1017 + if valid: 1018 + # Get total_pages from cache metadata (doesn't load full content) 1019 + total_pages = get_cached_total_pages(cache_key) 1020 + if total_pages > 0: 1021 + cache_hit = True # Will load below after page range parsing 1022 + 1023 + # If no cache hit, we need dependencies and page count from PDF 1024 + if not cache_hit: 1025 + # Check dependencies only when extraction is needed 1026 + if not check_dependencies(docling_mode=args.docling): 1027 + sys.exit(1) 1028 + 959 1029 from extractor import get_page_count 960 1030 961 1031 total_pages = get_page_count(args.input) ··· 972 1042 file=sys.stderr, 973 1043 ) 974 1044 sys.exit(1) 975 - # Warn if --pages used with --docling (page slicing not supported) 976 - # Clear requested_pages and use total_pages so metadata doesn't lie 977 - if args.pages and args.docling: 978 - print( 979 - "WARNING: --pages is not supported in Docling mode (no page delimiters in output). " 980 - "Full document will be returned.", 981 - file=sys.stderr, 982 - ) 983 - requested_pages = None # Don't attempt slicing 984 1045 985 1046 pages_to_output = len(requested_pages) if requested_pages else total_pages 986 1047 987 - # Determine if progress should be shown 988 - show_progress = sys.stderr.isatty() and not args.no_progress and not args.stdout 989 - 990 - # Check cache 991 - cache_hit = False 992 - cache_key = "" 993 - result = None 994 - image_dir = None 995 - 996 - # If using cache fallback (PDF missing), load directly from the found cache 997 - if cache_fallback: 998 - cache_key = fallback_cache_dir.name 1048 + # Now load from cache if we had a cache hit 1049 + if cache_hit: 1050 + if show_progress: 1051 + mode = "docling" if args.docling else "fast" 1052 + print(f"Loading from cache ({mode} mode)...", file=sys.stderr) 999 1053 result, image_dir, cached_total = load_from_cache( 1000 1054 cache_key, requested_pages, no_images=args.no_images 1001 1055 ) 1002 1056 if result is None: 1003 - # Cache was corrupted and PDF doesn't exist - can't recover 1004 - print( 1005 - "ERROR: Cache was corrupted and source PDF is not available.", 1006 - file=sys.stderr, 1007 - ) 1008 - sys.exit(1) 1009 - cache_hit = True 1010 - elif not args.no_cache: 1011 - valid, cache_key = is_cache_valid( 1012 - args.input, docling=args.docling, images_scale=args.images_scale 1013 - ) 1014 - if valid: 1015 - if show_progress: 1016 - mode = "docling" if args.docling else "fast" 1017 - print(f"Loading from cache ({mode} mode)...", file=sys.stderr) 1018 - result, image_dir, cached_total = load_from_cache( 1019 - cache_key, requested_pages, no_images=args.no_images 1020 - ) 1021 - # If cache was corrupted, treat as cache miss (will re-extract below) 1022 - if result is not None: 1023 - cache_hit = True 1057 + if cache_fallback: 1058 + # Cache was corrupted and PDF doesn't exist - can't recover 1059 + print( 1060 + "ERROR: Cache was corrupted and source PDF is not available.", 1061 + file=sys.stderr, 1062 + ) 1063 + sys.exit(1) 1064 + else: 1065 + # Cache was corrupted, treat as cache miss 1066 + cache_hit = False 1067 + # Need to check dependencies now since we're going to extract 1068 + if not check_dependencies(docling_mode=args.docling): 1069 + sys.exit(1) 1024 1070 1025 1071 # If no cache hit, extract full PDF 1026 1072 if not cache_hit: 1027 1073 # Get cache key if we don't have it 1028 1074 if not cache_key: 1029 1075 cache_key = get_cache_key( 1030 - args.input, docling=args.docling, images_scale=args.images_scale 1076 + args.input, 1077 + docling=args.docling, 1078 + images_scale=args.images_scale, 1079 + no_images=args.no_images, 1031 1080 ) 1032 1081 1033 1082 # Setup image directory for extraction (temporary) ··· 1071 1120 total_pages, 1072 1121 docling=args.docling, 1073 1122 images_scale=args.images_scale, 1123 + no_images=args.no_images, 1074 1124 ) 1075 1125 if show_progress: 1076 1126 print(f"Cached: {get_cache_dir(cache_key)}", file=sys.stderr) ··· 1085 1135 cached_image_dir = get_cache_dir(cache_key) / "images" 1086 1136 if cached_image_dir.exists() and any(cached_image_dir.iterdir()): 1087 1137 image_dir = cached_image_dir 1088 - # Clean up temp directory since images are now in cache 1089 - if temp_image_dir and os.path.exists(temp_image_dir): 1090 - shutil.rmtree(temp_image_dir) 1091 1138 else: 1092 - image_dir = temp_image_dir 1139 + # No images found (text-only PDF), don't set image_dir 1140 + image_dir = None 1141 + 1142 + # Always clean up temp directory when caching is enabled 1143 + if temp_image_dir and os.path.exists(temp_image_dir): 1144 + shutil.rmtree(temp_image_dir) 1093 1145 1094 1146 # Slice pages if requested (after caching full result) 1095 1147 if requested_pages:

Configure Feed

Configure Feed