[TEST] Cover the UN-3176 and UN-3038 logic this PR added

The PR shipped no tests. These pin the three behaviours whose regressions would be silent, and each was mutation-checked -- reverted or neutered the fix, confirmed the test fails, restored. BigQuery BadRequest discrimination (UN-3176), in the existing test_bigquery_db.py alongside the Forbidden/NotFound cases: - a value-level BadRequest routes to BigQueryValueException, and the message no longer tells the user to check columns that were never wrong - the same, with the marker present ONLY in the structured errors payload. GoogleAPICallError.__str__ folds an entry into the string only when it has .code/.message ATTRIBUTES, and BigQuery's REST path supplies plain dicts, so this is reachable solely through the payload loop. Deleting that loop fails this test and nothing else -- verified. - a genuine schema BadRequest still routes to ColumnMissingException, so the discrimination cannot drift into matching everything. _sanitize_for_bigquery (UN-3176): the two values where the old magnitude- derived decimal count and `:.15g` actually disagree -- 1234567890123456.0 at the high end, and 9.99999999999999e-05 at the low end, where log10 returns an exact integer, inflating the magnitude and costing a significant figure. A round number like 3.14159 passes under both forms and would prove nothing. Plus the NaN/Inf/zero guards and nested-structure recursion. Page-range billing (UN-3038): _parse_pages_to_extract over single pages, ranges, open-ended ranges, overlaps, inverted ranges and out-of-range values; the four degenerate configs that must fall back to the full count rather than bill zero; and one test asserting the count Audit actually receives, because asserting on _get_billable_page_count alone still passes when the call is dropped from push_usage_details -- which is the only place the number becomes a bill. Confirmed: removing that call site fails only the new test. No new test files -- both suites extend files already in the repo. unstract/connectors tests/databases: 44 passed (was 36). unstract/sdk1: 570 passed (was 554); the 11 failures are pre-existing and byte-identical to the base commit (missing pytest-asyncio, network-bound bedrock tests).
hari-kuriakose · Aug 28, 2026 · Aug 28, 2026 · Aug 28, 2026 · Aug 28, 2026 · Aug 28, 2026
commit 07ed225eadf84899db985c344b017b6351916c5d
diff --git a/unstract/connectors/tests/databases/test_bigquery_db.py b/unstract/connectors/tests/databases/test_bigquery_db.py
@@ -2,10 +2,13 @@
 from unittest.mock import MagicMock, patch
 
 import google.api_core.exceptions
+
 from unstract.connectors.databases.bigquery.bigquery import BigQuery
 from unstract.connectors.databases.exceptions import (
     BigQueryForbiddenException,
     BigQueryNotFoundException,
+    BigQueryValueException,
+    ColumnMissingException,
 )
 
 
@@ -130,6 +133,111 @@ def test_exception_empty_detail(self):
         # When detail is empty, should not have "Details:" section
         self.assertNotIn("Details:", error_msg)
 
+    def test_bad_request_value_error_routes_to_value_exception(self):
+        """UN-3176: a BadRequest about the DATA is not a missing column."""
+        value_error_msg = (
+            "400 Invalid value: cannot round-trip through string representation; "
+            "error in PARSE_JSON expression"
+        )
+        mock_error = google.api_core.exceptions.BadRequest(value_error_msg)
+        mock_error.message = value_error_msg
+
+        context = self._execute_query_with_mock_error(
+            mock_error, BigQueryValueException
+        )
+
+        error_msg = str(context.exception.detail)
+        self.assertIn("BigQuery rejected a value", error_msg)
+        # The old message sent users to check a schema that was never wrong.
+        self.assertNotIn("make sure all the columns exist", error_msg)
+        self.assertIn("test.dataset.table", error_msg)
+
+    def test_bad_request_value_error_detected_from_errors_payload_only(self):
+        """The marker lives ONLY in the structured payload, never in str(e).
+
+        ``GoogleAPICallError.__str__`` folds an ``errors`` entry into the string
+        only when it exposes ``.code``/``.message`` ATTRIBUTES; BigQuery's REST
+        path fills ``errors`` with plain dicts, which fail that check. So this
+        case is reachable only by the payload loop in ``_is_value_error`` --
+        delete that loop and this test fails while every message-based test
+        above still passes.
+        """
+        mock_error = google.api_core.exceptions.BadRequest(
+            "400 Request failed",
+            errors=[
+                {
+                    "reason": "invalidQuery",
+                    "message": "Cannot round-trip through string representation",
+                }
+            ],
+        )
+        mock_error.message = "400 Request failed"
+
+        # Guard the premise: if the marker ever leaks into str(e), this test
+        # would pass through the text branch and prove nothing.
+        self.assertNotIn("round-trip", str(mock_error).lower())
+
+        context = self._execute_query_with_mock_error(
+            mock_error, BigQueryValueException
+        )
+        self.assertIn("BigQuery rejected a value", str(context.exception.detail))
+
+    def test_bad_request_schema_error_still_routes_to_column_missing(self):
+        """A genuine schema BadRequest must keep its existing behaviour."""
+        schema_error_msg = "400 no such field: unknown_column"
+        mock_error = google.api_core.exceptions.BadRequest(schema_error_msg)
+        mock_error.message = schema_error_msg
+
+        context = self._execute_query_with_mock_error(
+            mock_error, ColumnMissingException
+        )
+        self.assertIn("column", str(context.exception.detail).lower())
+
+
+class TestBigQuerySanitizeForBigQuery(unittest.TestCase):
+    """UN-3176: the 15-significant-figure limit for PARSE_JSON compatibility.
+
+    The previous implementation derived a DECIMAL-place count from the value's
+    magnitude, which is a different quantity from significant figures. The
+    values below are the two classes where the two forms actually disagree --
+    an ordinary round number such as 3.14159 is preserved identically by both
+    and would pass against either implementation.
+    """
+
+    def test_large_magnitude_is_limited_to_15_significant_figures(self):
+        """Above 10^15 the old decimal count floored at 0 and kept every digit."""
+        self.assertEqual(
+            BigQuery._sanitize_for_bigquery(1234567890123456.0),
+            1234567890123460.0,
+        )
+
+    def test_value_just_below_a_power_of_ten_keeps_15_significant_figures(self):
+        """``log10`` returns an exact integer here, inflating the magnitude by one.
+
+        The old form therefore asked for one decimal place too few and emitted
+        14 significant figures, collapsing this value to 0.0001.
+        """
+        value = 9.99999999999999e-05
+        self.assertEqual(BigQuery._sanitize_for_bigquery(value), value)
+
+    def test_special_values_are_dropped(self):
+        """NaN/Inf cannot be represented in JSON and must not reach BigQuery."""
+        self.assertIsNone(BigQuery._sanitize_for_bigquery(float("nan")))
+        self.assertIsNone(BigQuery._sanitize_for_bigquery(float("inf")))
+        self.assertIsNone(BigQuery._sanitize_for_bigquery(float("-inf")))
+
+    def test_zero_and_modest_precision_are_preserved(self):
+        self.assertEqual(BigQuery._sanitize_for_bigquery(0.0), 0.0)
+        self.assertEqual(BigQuery._sanitize_for_bigquery(0.001228), 0.001228)
+
+    def test_nested_structures_are_sanitized(self):
+        self.assertEqual(
+            BigQuery._sanitize_for_bigquery(
+                {"rows": [{"time": 1760509016.282637}], "name": "x"}
+            ),
+            {"rows": [{"time": 1760509016.28264}], "name": "x"},
+        )
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/unstract/sdk1/tests/test_llm_whisperer_v2_params.py b/unstract/sdk1/tests/test_llm_whisperer_v2_params.py
@@ -1,10 +1,16 @@
-"""Tests for the query params the LLMWhisperer V2 adapter sends."""
+"""Tests for the LLMWhisperer V2 adapter's query params and page-range billing.
+
+Both read the same ``pages_to_extract`` setting: the adapter sends it to the
+service, and ``X2Text`` bills for the pages it selects.
+"""
 
 import pytest
 from unstract.sdk1.adapters.x2text.llm_whisperer_v2.src.dto import (
     WhispererRequestParams,
 )
 from unstract.sdk1.adapters.x2text.llm_whisperer_v2.src.helper import LLMWhispererHelper
+from unstract.sdk1.constants import MimeType
+from unstract.sdk1.x2txt import X2Text
 
 
 def _params(config: dict) -> dict:
@@ -47,3 +53,117 @@ def test_page_separator_read_under_legacy_config_key() -> None:
 
     assert params["page_separator"] == "<<< {{page_no}} >>>"
     assert "page_seperator" not in params
+
+
+class _FakeAdapter:
+    """Stands in for the adapter instance, which only needs ``.config`` here."""
+
+    def __init__(self, config: dict | None) -> None:
+        self.config = config
+
+
+def _billable(config: dict | None, page_count: int) -> int:
+    """Run ``_get_billable_page_count`` against a stub adapter.
+
+    ``X2Text.__init__`` requires a live ``BaseTool``, and the method under test
+    reads nothing but ``self._x2text_instance.config``.
+    """
+    x2text = X2Text.__new__(X2Text)
+    x2text._x2text_instance = _FakeAdapter(config) if config is not None else None
+    return x2text._get_billable_page_count(page_count)
+
+
+@pytest.mark.parametrize(
+    ("spec", "total_pages", "expected"),
+    [
+        ("1,3,5", 10, 3),
+        ("2-4", 10, 3),
+        ("50-", 60, 11),  # open-ended range runs to the last page
+        ("1-3,2-4", 10, 4),  # overlapping ranges are counted once
+        ("0-3", 10, 3),  # pages are 1-indexed; page 0 does not exist
+        ("1-999", 10, 10),  # clamped to the document length
+        ("99", 10, 0),  # a single out-of-range page selects nothing
+        ("5-2", 10, 0),  # an inverted range selects nothing
+    ],
+)
+def test_parse_pages_to_extract_counts_selected_pages(
+    spec: str, total_pages: int, expected: int
+) -> None:
+    """UN-3038: billing follows the pages the adapter actually extracts."""
+    assert X2Text._parse_pages_to_extract(spec, total_pages) == expected
+
+
+def test_billable_page_count_narrows_to_the_selected_range() -> None:
+    """The whole point of UN-3038: 3 pages of a 100-page document bill as 3."""
+    assert _billable({"pages_to_extract": "2-4"}, 100) == 3
+
+
+@pytest.mark.parametrize(
+    "config",
+    [
+        {"pages_to_extract": ""},  # setting present but empty
+        {"pages_to_extract": "   "},  # whitespace only
+        {"pages_to_extract": "not-a-range"},  # unparseable
+        {"pages_to_extract": "99"},  # parses, but selects nothing
+        {},  # setting absent
+        None,  # no adapter instance at all
+    ],
+)
+def test_billable_page_count_falls_back_to_the_full_count(config: dict | None) -> None:
+    """Usage must never be UNDER-reported because a setting was malformed.
+
+    Each of these makes the page selection unusable; billing then falls back to
+    every page in the document rather than to zero.
+    """
+    assert _billable(config, 10) == 10
+
+
+def test_push_usage_details_reports_the_narrowed_page_count(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """The narrowing must be wired into the value Audit actually receives.
+
+    Asserting on ``_get_billable_page_count`` alone would still pass if the
+    call were dropped from ``push_usage_details``, which is the only place the
+    number becomes a bill.
+    """
+    import unstract.sdk1.x2txt as x2txt_module
+
+    recorded: dict[str, int] = {}
+
+    class _FakeAudit:
+        def push_page_usage_data(self, **kwargs: object) -> None:
+            recorded["page_count"] = kwargs["page_count"]
+
+    class _FakePdf:
+        pages = [object()] * 100
+
+        def __enter__(self) -> "_FakePdf":
+            return self
+
+        def __exit__(self, *exc: object) -> None:
+            return None
+
+    monkeypatch.setattr(x2txt_module, "Audit", _FakeAudit)
+    monkeypatch.setattr(x2txt_module.pdfplumber, "open", lambda _: _FakePdf())
+    monkeypatch.setattr(
+        x2txt_module.ToolUtils, "get_file_size", staticmethod(lambda *a, **k: 1024)
+    )
+
+    class _FakeFs:
+        def read(self, **kwargs: object) -> bytes:
+            return b"%PDF-1.4"
+
+    class _FakeTool:
+        def get_env_or_die(self, key: str) -> str:
+            return "test-key"
+
+    x2text = X2Text.__new__(X2Text)
+    x2text._x2text_instance = _FakeAdapter({"pages_to_extract": "2-4"})
+    x2text._tool = _FakeTool()
+    x2text._usage_kwargs = {}
+
+    x2text.push_usage_details("doc.pdf", MimeType.PDF, fs=_FakeFs())
+
+    # 100-page document, 3 pages extracted -> 3 pages billed.
+    assert recorded["page_count"] == 3