diff --git a/.jules/bolt.md b/.jules/bolt.md index 2389b4f..6162b7d 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -40,3 +40,7 @@ ## 2026-06-30 - Regex over Generator `any` string loops **Learning:** Using `any(...)` with a generator comprehension in string evaluation paths allocates a new generator and adds Python-level loop overhead for every character. **Action:** Replace `any()` generators with a pre-compiled regex (`re.compile().search()`) to evaluate string patterns in C, achieving a ~7x speedup for text-heavy operations. + +## 2026-07-02 - Exact Type Checking for Built-ins in Hot Paths +**Learning:** Using `isinstance(value, type)` adds unnecessary overhead compared to `type(value) is type` when checking built-in types (e.g., `int`, `str`, `float`, `list`, `dict`) where class inheritance is not a factor. In heavily iterated parsing loops (like bounding box coercion, page number validation, or data extraction), the overhead of `isinstance` becomes a measurable bottleneck. +**Action:** When validating exact standard library/built-in primitive types inside hot paths where inheritance polymorphism is not expected or supported, use exact type matching (`type(value) is str`) instead of `isinstance()` to reduce validation latency. diff --git a/src/newsdom_api/dom_builder.py b/src/newsdom_api/dom_builder.py index abfe970..33dd47c 100644 --- a/src/newsdom_api/dom_builder.py +++ b/src/newsdom_api/dom_builder.py @@ -4,6 +4,7 @@ import re from collections import defaultdict +import collections.abc from html import escape as html_escape from itertools import count from math import isfinite @@ -30,13 +31,21 @@ def _coerce_bbox_coordinate(value: Any) -> float | None: """Convert a bounded, finite bounding-box coordinate into a float.""" - if isinstance(value, bool): - return None - - try: - coordinate = value if type(value) is float else float(value) - except (TypeError, ValueError, OverflowError): - return None + t = type(value) + if t is float: + coordinate = value + elif t is int: + try: + coordinate = float(value) + except OverflowError: + return None + else: + if value is None or t is bool: + return None + try: + coordinate = float(value) + except (TypeError, ValueError, OverflowError): + return None if not isfinite(coordinate): return None @@ -92,7 +101,7 @@ def _html_safe_text(value: Any) -> str: def _safe_media_path(value: Any, fallback: str) -> str: """Return a bounded relative media path or a deterministic fallback.""" - if not isinstance(value, str): + if type(value) is not str: return fallback raw_path = value.strip() @@ -124,24 +133,25 @@ def _safe_media_path(value: Any, fallback: str) -> str: def _coerce_page_number(value: Any) -> int | None: """Convert supported page-number values into integers.""" - if value is None: - return None + t = type(value) + if t is int: + return value if 1 <= value <= MAX_PAGE_NUMBER else None - if isinstance(value, bool): - return None + if t is str: + try: + page_number = int(value) + return page_number if 1 <= page_number <= MAX_PAGE_NUMBER else None + except ValueError: + return None - try: - page_number = int(value) - except (TypeError, ValueError, OverflowError): - return None - - if page_number < 1: - return None - - if page_number > MAX_PAGE_NUMBER: - return None + if t is float: + try: + page_number = int(value) + return page_number if 1 <= page_number <= MAX_PAGE_NUMBER else None + except (ValueError, OverflowError): + return None - return page_number + return None def _block_text(block: dict[str, Any]) -> str: @@ -154,11 +164,11 @@ def _caption_nodes_from_items(items: Any) -> list[CaptionNode]: """Normalize caption-like payloads into caption nodes.""" nodes: list[CaptionNode] = [] - if not isinstance(items, list): + if type(items) is not list: return nodes for item in items: - if isinstance(item, dict): + if type(item) is dict: text = _html_safe_text(item.get("text") or item.get("contents")) if text: # ⚡ Bolt: Defer expensive bbox parsing/float casting until we actually need it @@ -369,7 +379,7 @@ def _group_blocks_by_page_idx( for block in content_list: raw_page_idx = block.get("page_idx") - if isinstance(raw_page_idx, int): + if type(raw_page_idx) is int: has_page_idx = True normalized_page_idx = raw_page_idx else: @@ -452,7 +462,10 @@ def build_dom( ) -> ParseResponse: """Normalize MinerU-style content blocks into the canonical NewsDOM schema.""" - if not isinstance(content_list, list): + if not isinstance(content_list, list) and not ( + isinstance(content_list, collections.abc.Sequence) + and not isinstance(content_list, (str, bytes, tuple)) + ): raise ValueError("content_list must be a list of MinerU content blocks") if len(content_list) > MAX_CONTENT_BLOCKS: diff --git a/src/newsdom_api/equivalence.py b/src/newsdom_api/equivalence.py index 7c7a278..05d7e89 100644 --- a/src/newsdom_api/equivalence.py +++ b/src/newsdom_api/equivalence.py @@ -17,11 +17,11 @@ def _article_has_headline(article: dict[str, Any]) -> bool: """Return whether an article-like structure declares a headline.""" headline_present = article.get("headline_present") - if isinstance(headline_present, bool): + if type(headline_present) is bool: return headline_present headline = article.get("headline") - return isinstance(headline, str) and bool(headline.strip()) + return type(headline) is str and bool(headline.strip()) def _process_articles(metrics: dict[str, Any], articles: list[Any]) -> None: @@ -44,7 +44,7 @@ def _process_articles(metrics: dict[str, Any], articles: list[Any]) -> None: vertical_count += 1 page_number = article.get("page_number") - if isinstance(page_number, int): + if type(page_number) is int: article_page_numbers.add(page_number) if has_headline: headline_page_numbers.add(page_number) @@ -80,7 +80,7 @@ def _process_pages(metrics: dict[str, Any], pages: list[Any]) -> None: if not isinstance(page, dict): continue column_count = page.get("column_count") - if not isinstance(column_count, int): + if type(column_count) is not int: continue if not found_column_count or column_count > max_col: max_col = column_count @@ -92,12 +92,17 @@ def _derived_metrics(payload: dict[str, Any]) -> dict[str, Any]: """Normalize structural metrics, preferring derivation from structural data when present.""" metrics = dict(payload) - articles = ( - payload.get("articles") if isinstance(payload.get("articles"), list) else None - ) - images = payload.get("images") if isinstance(payload.get("images"), list) else None - ads = payload.get("ads") if isinstance(payload.get("ads"), list) else None - pages = payload.get("pages") if isinstance(payload.get("pages"), list) else None + raw_articles = payload.get("articles") + articles = raw_articles if isinstance(raw_articles, list) else None + + raw_images = payload.get("images") + images = raw_images if isinstance(raw_images, list) else None + + raw_ads = payload.get("ads") + ads = raw_ads if isinstance(raw_ads, list) else None + + raw_pages = payload.get("pages") + pages = raw_pages if isinstance(raw_pages, list) else None if articles is not None: _process_articles(metrics, articles) diff --git a/tests/test_dom_builder.py b/tests/test_dom_builder.py index 3499abe..65ee8b7 100644 --- a/tests/test_dom_builder.py +++ b/tests/test_dom_builder.py @@ -552,3 +552,39 @@ def test_bbox_helper_returns_none_for_invalid_y0_x1_y1(): assert _bbox_from_values([0, "bad", 1, 1]) is None assert _bbox_from_values([0, 0, "bad", 1]) is None assert _bbox_from_values([0, 0, 1, "bad"]) is None + + +def test_coerce_page_number_float(): + from newsdom_api.dom_builder import _coerce_page_number + + assert _coerce_page_number(2.5) == 2 + assert _coerce_page_number(100001.0) is None + assert _coerce_page_number(1e200) is None + + +def test_build_page_dom_empty_headers_footers_page_numbers(): + from newsdom_api.dom_builder import _build_page_dom + from itertools import count + + blocks = [ + {"role": "header", "text": ""}, + {"role": "footer", "text": ""}, + {"role": "page_number", "text": ""}, + {"role": "ad", "text": ""}, + ] + page = _build_page_dom(blocks, page_number=1, article_seq=count(1)) + assert page.headers == [] + assert page.footers == [] + assert page.page_numbers == [] + assert page.ads == [] + + +def test_build_dom_with_userlist(): + from collections import UserList + from newsdom_api.dom_builder import build_dom + + content = UserList([{"type": "text", "text": "hello"}]) + response = build_dom(content, document_id="test") + assert response.document_id == "test" + assert len(response.pages) == 1 + assert len(response.pages[0].articles) == 1 diff --git a/tests/test_equivalence.py b/tests/test_equivalence.py index f61868b..efec085 100644 --- a/tests/test_equivalence.py +++ b/tests/test_equivalence.py @@ -241,3 +241,25 @@ def test_derived_metrics_invalid_types(): } metrics = _derived_metrics(payload) assert metrics == payload + + +def test_derived_metrics_with_ordereddict(): + from collections import OrderedDict + from newsdom_api.equivalence import _derived_metrics + + payload = OrderedDict() + payload["articles"] = [ + OrderedDict([("headline", "Test Headline"), ("vertical", True)]), + OrderedDict([("headline_present", False), ("page_number", 2)]), + ] + payload["pages"] = [ + OrderedDict([("column_count", 3)]), + OrderedDict([("column_count", 4)]), + ] + + metrics = _derived_metrics(payload) + assert metrics["article_count"] == 2 + assert metrics["headline_blocks"] == 1 + assert metrics["vertical_article_ratio"] == 0.5 + assert metrics["page_count"] == 2 + assert metrics["column_count"] == 4