From be9d774719236bd3d27a61d54b74d91b83349d2a Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 16:13:46 +0530 Subject: [PATCH 01/11] UN-2953, UN-3038, UN-3333 [FIX] Adapter id validation, page billing, OCR threshold UN-2953: validate_adapter_permissions read adapter ids straight out of tool_meta and added them unconditionally, so a tool instance holding "" for an adapter id made the JSON schema validator compare "" against the UUID enum and raise. The error was logged but not handled, repeating every validation pass until the pod stopped answering health checks. Skips empty/missing ids and uses .get() so a missing key no longer raises KeyError. Also initialises adapter_id per iteration -- previously a disabled entry could re-add the previous loop's id. UN-3038: push_usage_details billed len(pdf.pages) for every PDF, ignoring the adapter's pages_to_extract range, so a 5-page extraction from a 100-page document was charged 100 pages. Narrows the count to the selected pages, handling ranges, open-ended ranges, overlaps and out-of-range values, and falling back to the full count when the setting is absent or unparseable so usage is never under-reported. UN-3333: adds word_confidence_threshold to the LLMWhisperer v2 adapter schema (number, default 0.3, 0.0-1.0) so it is configurable from the adapter UI. The parameter is implemented in the LLMWhisperer backend but was never exposed. Not added to the v1 schema, which predates the OCR tuning parameters. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- .../tool_instance_v2/tool_instance_helper.py | 52 +++++++++++++----- .../src/static/json_schema.json | 16 +++++- unstract/sdk1/src/unstract/sdk1/x2txt.py | 53 +++++++++++++++++++ 3 files changed, 107 insertions(+), 14 deletions(-) diff --git a/backend/tool_instance_v2/tool_instance_helper.py b/backend/tool_instance_v2/tool_instance_helper.py index 138e48b5b2..70631fb47f 100644 --- a/backend/tool_instance_v2/tool_instance_helper.py +++ b/backend/tool_instance_v2/tool_instance_helper.py @@ -486,33 +486,61 @@ def validate_adapter_permissions( adapter_ids: set[str] = set() for llm in tool.properties.adapter.language_models: + adapter_id = None if llm.is_enabled and llm.adapter_id: - adapter_id = tool_meta[llm.adapter_id] + adapter_id = tool_meta.get(llm.adapter_id) elif llm.is_enabled: - adapter_id = tool_meta[AdapterPropertyKey.DEFAULT_LLM_ADAPTER_ID] + adapter_id = tool_meta.get( + AdapterPropertyKey.DEFAULT_LLM_ADAPTER_ID + ) - adapter_ids.add(adapter_id) + # UN-2953: a tool instance may carry "" for an adapter id. + # Adding it made the schema validator compare "" against the + # UUID enum and raise, once per validation pass. + if adapter_id: + adapter_ids.add(adapter_id) for vdb in tool.properties.adapter.vector_stores: + adapter_id = None if vdb.is_enabled and vdb.adapter_id: - adapter_id = tool_meta[vdb.adapter_id] + adapter_id = tool_meta.get(vdb.adapter_id) elif vdb.is_enabled: - adapter_id = tool_meta[AdapterPropertyKey.DEFAULT_VECTOR_DB_ADAPTER_ID] + adapter_id = tool_meta.get( + AdapterPropertyKey.DEFAULT_VECTOR_DB_ADAPTER_ID + ) - adapter_ids.add(adapter_id) + # UN-2953: a tool instance may carry "" for an adapter id. + # Adding it made the schema validator compare "" against the + # UUID enum and raise, once per validation pass. + if adapter_id: + adapter_ids.add(adapter_id) for embedding in tool.properties.adapter.embedding_services: + adapter_id = None if embedding.is_enabled and embedding.adapter_id: - adapter_id = tool_meta[embedding.adapter_id] + adapter_id = tool_meta.get(embedding.adapter_id) elif embedding.is_enabled: - adapter_id = tool_meta[AdapterPropertyKey.DEFAULT_EMBEDDING_ADAPTER_ID] + adapter_id = tool_meta.get( + AdapterPropertyKey.DEFAULT_EMBEDDING_ADAPTER_ID + ) - adapter_ids.add(adapter_id) + # UN-2953: a tool instance may carry "" for an adapter id. + # Adding it made the schema validator compare "" against the + # UUID enum and raise, once per validation pass. + if adapter_id: + adapter_ids.add(adapter_id) for text_extractor in tool.properties.adapter.text_extractors: + adapter_id = None if text_extractor.is_enabled and text_extractor.adapter_id: - adapter_id = tool_meta[text_extractor.adapter_id] + adapter_id = tool_meta.get(text_extractor.adapter_id) elif text_extractor.is_enabled: - adapter_id = tool_meta[AdapterPropertyKey.DEFAULT_X2TEXT_ADAPTER_ID] + adapter_id = tool_meta.get( + AdapterPropertyKey.DEFAULT_X2TEXT_ADAPTER_ID + ) - adapter_ids.add(adapter_id) + # UN-2953: a tool instance may carry "" for an adapter id. + # Adding it made the schema validator compare "" against the + # UUID enum and raise, once per validation pass. + if adapter_id: + adapter_ids.add(adapter_id) ToolInstanceHelper.validate_adapter_access(user=user, adapter_ids=adapter_ids) diff --git a/unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/static/json_schema.json b/unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/static/json_schema.json index 04d16a3e7a..95644c7222 100644 --- a/unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/static/json_schema.json +++ b/unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/static/json_schema.json @@ -58,8 +58,12 @@ "line_splitter_strategy": { "type": "string", "title": "Line Splitter Strategy", - "enum": ["left-priority", "mid-priority", "right-priority"], - "default":"left-priority", + "enum": [ + "left-priority", + "mid-priority", + "right-priority" + ], + "default": "left-priority", "description": "An advanced option for customizing the line splitting process." }, "horizontal_stretch_factor": { @@ -93,6 +97,14 @@ "default": false, "description": "States whether to reproduce horizontal lines in the document. Note: This parameter is not applicable if `mode` chosen is `native_text` and will not work if `mark_vertical_lines` is set to `false`." }, + "word_confidence_threshold": { + "type": "number", + "title": "Word confidence threshold", + "default": 0.3, + "minimum": 0.0, + "maximum": 1.0, + "description": "Minimum OCR confidence a word must reach to be included in the extracted text. Lower this when words are dropped because the scan is faint or noisy. Note: This parameter is not applicable if `mode` chosen is `native_text`." + }, "tag": { "type": "string", "title": "Tag", diff --git a/unstract/sdk1/src/unstract/sdk1/x2txt.py b/unstract/sdk1/src/unstract/sdk1/x2txt.py index 2024f8cdbc..2c769cf635 100644 --- a/unstract/sdk1/src/unstract/sdk1/x2txt.py +++ b/unstract/sdk1/src/unstract/sdk1/x2txt.py @@ -118,6 +118,55 @@ def process( self.push_usage_details(input_file_path, mime_type, fs=fs) return text_extraction_result + @staticmethod + def _parse_pages_to_extract(pages_to_extract: str, total_pages: int) -> int: + """Count the pages selected by an LLMWhisperer ``pages_to_extract`` spec. + + The spec is a comma separated list of single pages and ranges, where a + range may be open ended (``50-`` means "page 50 to the end"). Pages are + 1-indexed and may overlap, so they are collected into a set and clamped + to the document length. An empty spec means "all pages". + """ + selected: set[int] = set() + for part in pages_to_extract.split(","): + part = part.strip() + if not part: + continue + if "-" in part: + start_str, _, end_str = part.partition("-") + try: + start = int(start_str) + except ValueError: + continue + end = total_pages + if end_str: + try: + end = int(end_str) + except ValueError: + continue + selected.update(range(max(start, 1), min(end, total_pages) + 1)) + else: + try: + page = int(part) + except ValueError: + continue + if 1 <= page <= total_pages: + selected.add(page) + return len(selected) + + def _get_billable_page_count(self, page_count: int) -> int: + """Narrow ``page_count`` to the pages the adapter will actually extract. + + Falls back to the full count whenever the setting is absent, empty or + unparseable, so usage is never under-reported by a malformed value. + """ + config = getattr(self._x2text_instance, "config", None) or {} + pages_to_extract = str(config.get("pages_to_extract", "") or "").strip() + if not pages_to_extract: + return page_count + selected = self._parse_pages_to_extract(pages_to_extract, page_count) + return selected or page_count + def push_usage_details( self, input_file_path: str, @@ -133,6 +182,10 @@ def push_usage_details( with pdfplumber.open(pdf_contents) as pdf: # calculate the number of pages page_count = len(pdf.pages) + # UN-3038: when the adapter restricts extraction to a page range, + # only those pages are actually processed, so bill for them rather + # than for every page in the document. + page_count = self._get_billable_page_count(page_count) Audit().push_page_usage_data( platform_api_key=self._tool.get_env_or_die(ToolEnv.PLATFORM_API_KEY), file_size=file_size, From da198d0e156f4d3fecac1033f452c3a1e5a90468 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 16:19:55 +0530 Subject: [PATCH 02/11] UN-2902 [FIX] Honour the chunk limit in Keyword Table retrieval KeywordTableIndex's retriever caps results with `num_chunks_per_query` (default 10), not `similarity_top_k`. The code passed similarity_top_k, which as_retriever accepts and ignores, so the configured limit never took effect -- a profile set to 3 chunks still retrieved 10. Passes num_chunks_per_query instead. Filed as a frontend ticket, but the setting was being displayed correctly; only the retriever ignored it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- workers/executor/executors/retrievers/keyword_table.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/workers/executor/executors/retrievers/keyword_table.py b/workers/executor/executors/retrievers/keyword_table.py index 5c7db9ab4e..e2909146e3 100644 --- a/workers/executor/executors/retrievers/keyword_table.py +++ b/workers/executor/executors/retrievers/keyword_table.py @@ -51,9 +51,13 @@ def retrieve(self) -> set[str]: llm=llm, # Use the provided LLM instead of defaulting to OpenAI ) - # Create retriever from keyword index + # Create retriever from keyword index. + # UN-2902: KeywordTableIndex's retriever caps results with + # `num_chunks_per_query` (default 10), not `similarity_top_k` -- + # that kwarg is accepted and ignored, so a profile asking for 3 + # chunks still got 10. keyword_retriever = keyword_index.as_retriever( - similarity_top_k=self.top_k, + num_chunks_per_query=self.top_k, ) # Retrieve nodes using keyword matching From aa0df6f89a3e596153e68d003c522f2dcfe646c2 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 16:20:54 +0530 Subject: [PATCH 03/11] UN-3176 [FIX] Apply the 15-significant-figure limit for values below 1 _sanitize_for_bigquery documents a 15-significant-figure cap for PARSE_JSON compatibility, but derived a DECIMAL-place count from the value's magnitude. That only equals 15 significant figures for values >= 1. For 0.0053325 the magnitude is -2, so it asked for 17 decimals -- more precision than the safe zone permits -- and the value was returned unchanged. Formats with `.15g` so the significant-figure limit applies at any magnitude. Verified round-tripping for small values, Unix timestamps, large mantissas and binary-artifact values such as 0.1 + 0.2. NOTE: this corrects a real precision defect but is NOT confirmed to be the whole of the reported failure. The ticket's rejected value (0.0053325) is already representable and survives sanitization unchanged, so reproducing the BigQuery-side error needs the customer's table and PARSE_JSON expression. Flagged for follow-up rather than closed on this commit alone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- .../connectors/databases/bigquery/bigquery.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py index d9e05a7055..723450b342 100644 --- a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py +++ b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py @@ -106,13 +106,16 @@ def _sanitize_for_bigquery(data: Any) -> Any: if data == 0: return 0.0 - # Limit total significant figures to 15 for IEEE 754 compatibility - # BigQuery PARSE_JSON requires values that round-trip cleanly - # For large numbers (like Unix timestamps), this reduces decimal precision - # For small numbers (like costs), full precision is preserved - magnitude = math.floor(math.log10(abs(data))) + 1 - safe_decimals = max(0, 15 - magnitude) - return float(f"{data:.{safe_decimals}f}") + # Limit total significant figures to 15 for IEEE 754 compatibility. + # BigQuery PARSE_JSON requires values that round-trip cleanly. + # + # UN-3176: the previous form derived a DECIMAL-place count from the + # magnitude, which only matches "15 significant figures" for values + # >= 1. For a value like 0.0053325 the magnitude is -2, giving 17 + # decimals -- more precision than the safe zone allows, so the + # value was passed through unchanged. Formatting with `g` applies + # the significant-figure limit directly, for any magnitude. + return float(f"{data:.15g}") elif isinstance(data, dict): return {k: BigQuery._sanitize_for_bigquery(v) for k, v in data.items()} From aa4f086ac1802e906f1a1e0b35f82c53e18d23b3 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 17:44:02 +0530 Subject: [PATCH 04/11] UN-3133 [FIX] Surface context-window overflow as its own error litellm's ContextWindowExceededError derives from BadRequestError, so it was caught by the generic openai.APIError branch in parse_litellm_err and wrapped as a plain SdkError reading "Error from ." plus the raw 400 text. Users hitting the token limit saw a generic failure and had to read worker logs to find the cause (reported on execution f308a67f-02a1-437e-8531-05e067a94e02). Adds a ContextWindowExceededError SdkError subclass, maps it ahead of the generic wrap, and returns early so the actionable guidance (reduce chunk size, limit pages extracted, or use a larger-context model) is not overwritten by the generic tail. The provider's own text is kept in a code block underneath for support. Other litellm errors are unchanged. Context from the source thread: the customer's PRIMARY complaint there was inconsistent JSON structure across questions, which Jagadeesh identified as a prompting issue, not this one. Only the mis-surfaced token-limit error is in scope for UN-3133. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- unstract/sdk1/src/unstract/sdk1/exceptions.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/unstract/sdk1/src/unstract/sdk1/exceptions.py b/unstract/sdk1/src/unstract/sdk1/exceptions.py index 40889af877..1531ca6dd9 100644 --- a/unstract/sdk1/src/unstract/sdk1/exceptions.py +++ b/unstract/sdk1/src/unstract/sdk1/exceptions.py @@ -2,6 +2,7 @@ import re import openai +from litellm import exceptions as litellm_exceptions logger = logging.getLogger(__name__) @@ -85,6 +86,23 @@ class RateLimitError(SdkError): DEFAULT_MESSAGE = "Running into rate limit errors, please try again later" +class ContextWindowExceededError(SdkError): + """The prompt (plus context) exceeded the model's context window. + + UN-3133: litellm raises ``ContextWindowExceededError``, which derives from + ``BadRequestError`` and so from ``openai.APIError``. Without a distinct + class it was wrapped as a plain ``SdkError`` carrying the provider's raw + 400 text, so the UI showed a generic "Error from " and the real + cause was only findable by reading worker logs. + """ + + DEFAULT_MESSAGE = ( + "The document and prompt together exceed the model's context window. " + "Reduce the chunk size, limit the pages extracted, or use a model with " + "a larger context window." + ) + + class FileStorageError(SdkError): DEFAULT_MESSAGE = ( "Error while connecting with the storage. " @@ -156,6 +174,26 @@ def parse_litellm_err(e: Exception, provider_name: str | None = None) -> SdkErro ) cleaned_message = strip_litellm_prefix(str(e)) + + # UN-3133: surface a context-window overflow as its own error type with an + # actionable message. It is a BadRequestError subclass, so without this it + # collapses into the generic wrap below and the user sees only the + # provider's raw 400. + if isinstance(e, litellm_exceptions.ContextWindowExceededError): + err = ContextWindowExceededError( + ContextWindowExceededError.DEFAULT_MESSAGE, + actual_err=e, + status_code=status_code, + ) + # Return early: the generic tail below would overwrite the actionable + # guidance with "Error from ." and leave only the raw 400. + # The provider text is kept underneath it for support/debugging. + err.message = ( + f"{ContextWindowExceededError.DEFAULT_MESSAGE}" + f"\n```\n{cleaned_message}\n```" + ) + return err + err = SdkError(cleaned_message, actual_err=e, status_code=status_code) if not provider_name: From f45d4e0f9e505d21c0822265119d0a4e114a9431 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 21:48:10 +0530 Subject: [PATCH 05/11] UN-2900 [FIX] Flag variables that cannot resolve under single-pass extraction Single pass builds ONE combined prompt -- every field declared up front in a single JSON schema, answered in one LLM call -- so no prompt's output exists to feed another prompt's variable. The runtime reflects that: the enterprise single_pass_extraction plugin calls the shared replacement service with structured_output={}, and both replace_static_variable and replace_dynamic_variable return the prompt UNCHANGED when their lookup misses. The literal {{...}} is then sent to the LLM, silently degrading the answer. This is not specific to custom_data, despite the ticket title. CUSTOM_DATA is in fact the ONLY variable type that survives single pass, because it resolves from the tool's own custom_data and never consults the variable map. STATIC and DYNAMIC variables both fail on their own, with no custom_data involved: static : "check {{invoice_number}}" -> "check {{invoice_number}}" dynamic : "via {{https://.../x[cust_id]}}" -> unchanged custom : "{{custom_data.client.name}}" -> "Acme GmbH" (works) Adds find_unresolvable_single_pass_variables() and surfaces the result per prompt as single_pass_unresolvable_variables when the tool has single-pass enabled. Warning only -- deliberately NOT a save-time block, because existing projects may already carry this combination and users toggle single pass on and off; a hard refusal would break them retroactively and be order-dependent. Classification pairs with VariableReplacementService in the worker, which keeps its own copy of the variable regexes; noted in the docstring since drift would make validation and runtime disagree. Known gap: this covers Prompt Studio authoring, not an already-exported tool running single pass via API deployment, which keeps failing silently until the tool is re-saved. That argues for pairing this with placeholder-stripping at runtime later, not for widening this change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- .../prompt_variable_service.py | 35 +++++++++++++++++++ .../prompt_studio_core_v2/serializers.py | 16 +++++++++ 2 files changed, 51 insertions(+) diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_variable_service.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_variable_service.py index 008ba078c0..2a4a7f7b3b 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_variable_service.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_variable_service.py @@ -64,6 +64,41 @@ def identify_variable_type(variable: str) -> VariableType: variable_type = VariableType.STATIC return variable_type + @staticmethod + def find_unresolvable_single_pass_variables(prompt: str) -> list[str]: + """Variables in ``prompt`` that cannot resolve under single-pass extraction. + + UN-2900. Single pass builds ONE combined prompt — every field is declared + up front in a single JSON schema and answered in one LLM call — so no + prompt's output exists to feed another prompt's variable. The runtime + reflects this: the enterprise ``single_pass_extraction`` plugin calls the + shared replacement service with ``structured_output={}``, and both + ``replace_static_variable`` and ``replace_dynamic_variable`` return the + prompt UNCHANGED when their lookup misses. The literal ``{{...}}`` is then + sent to the LLM, silently degrading the answer. + + CUSTOM_DATA is the one exception: it is resolved from the tool's own + ``custom_data`` and never consults the variable map, so it works + identically in both modes and is not reported here. + + Returns the offending variable strings, in prompt order, or an empty list. + + NOTE: classification pairs with ``VariableReplacementService`` in the + worker (``executor/executors/variable_replacement.py``), which keeps its + own copy of these regexes. If one side's patterns change, this validation + and the runtime behaviour will disagree. + """ + unresolvable: list[str] = [] + for variable in PromptStudioVariableService.extract_variables_from_prompt( + prompt=prompt + ): + variable_type = PromptStudioVariableService.identify_variable_type( + variable=variable + ) + if variable_type != VariableType.CUSTOM_DATA: + unresolvable.append(variable) + return unresolvable + @staticmethod def extract_variables_from_prompt(prompt: str) -> list[str]: variable: list[str] = [] diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index acb3a243d7..0acfd8a369 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -18,6 +18,9 @@ from prompt_studio.prompt_profile_manager_v2.models import ProfileManager from prompt_studio.prompt_studio_core_v2.constants import ToolStudioKeys as TSKeys from prompt_studio.prompt_studio_core_v2.exceptions import DefaultProfileError +from prompt_studio.prompt_studio_core_v2.prompt_variable_service import ( + PromptStudioVariableService, +) from prompt_studio.prompt_studio_output_manager_v2.output_manager_util import ( OutputManagerUtils, ) @@ -226,6 +229,19 @@ def to_representation(self, instance): # type: ignore # Add coverage to serialized data serialized_data["coverage"] = coverage + + # UN-2900: warn (do not block) when a prompt uses variables that + # cannot resolve under single pass. Surfaced per prompt so the user + # sees it while authoring instead of discovering it as a degraded + # answer after paying for the run. + serialized_data["single_pass_unresolvable_variables"] = ( + PromptStudioVariableService.find_unresolvable_single_pass_variables( + prompt=prompt.prompt + ) + if instance.single_pass_extraction_mode and prompt.prompt + else [] + ) + output.append(serialized_data) data[TSKeys.PROMPTS] = output From c830880f6b8fb9c7a9dd5be926554ba572c84c86 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 28 Aug 2026 23:16:59 +0530 Subject: [PATCH 06/11] UN-2900 [FIX] Return unresolvable-variable warning on prompt save too Follow-up to f45d4e0f9, which computed the warning only in CustomToolSerializer.to_representation -- so it appeared on tool fetch but not on the prompt save response, and the UI had no fresh value after an edit. Moves the computation to ToolStudioPromptSerializer as a SerializerMethodField. That serializer is what the prompt CRUD view returns AND what CustomToolSerializer nests per prompt, so one implementation now covers both page load and save. Removes the duplicated assignment from CustomToolSerializer. Adds select_related("tool_id") to the prompt query in CustomToolSerializer.to_representation: the new field reads the parent tool's single_pass_extraction_mode, which would otherwise be one query per prompt -- the exact N+1 CustomToolListSerializer's docstring calls out. Verified: single-pass off -> [] regardless of content; on -> static and dynamic variables reported, custom_data excluded, empty prompt safe (5/5). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- .../prompt_studio_core_v2/serializers.py | 27 +++++++------------ .../prompt_studio_v2/serializers.py | 26 ++++++++++++++++++ 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index 0acfd8a369..2eadce85ed 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -18,9 +18,6 @@ from prompt_studio.prompt_profile_manager_v2.models import ProfileManager from prompt_studio.prompt_studio_core_v2.constants import ToolStudioKeys as TSKeys from prompt_studio.prompt_studio_core_v2.exceptions import DefaultProfileError -from prompt_studio.prompt_studio_core_v2.prompt_variable_service import ( - PromptStudioVariableService, -) from prompt_studio.prompt_studio_output_manager_v2.output_manager_util import ( OutputManagerUtils, ) @@ -189,9 +186,15 @@ def to_representation(self, instance): # type: ignore ) # Fetch prompt instances - prompt_instances: ToolStudioPrompt = ToolStudioPrompt.objects.filter( - tool_id=data.get(TSKeys.TOOL_ID) - ).order_by("sequence_number") + # select_related("tool_id"): ToolStudioPromptSerializer's + # single_pass_unresolvable_variables (UN-2900) reads the parent tool's + # single_pass_extraction_mode, which would otherwise be one query per + # prompt here. + prompt_instances: ToolStudioPrompt = ( + ToolStudioPrompt.objects.filter(tool_id=data.get(TSKeys.TOOL_ID)) + .select_related("tool_id") + .order_by("sequence_number") + ) data["created_by_email"] = ( instance.created_by.email if instance.created_by else "" @@ -230,18 +233,6 @@ def to_representation(self, instance): # type: ignore # Add coverage to serialized data serialized_data["coverage"] = coverage - # UN-2900: warn (do not block) when a prompt uses variables that - # cannot resolve under single pass. Surfaced per prompt so the user - # sees it while authoring instead of discovering it as a degraded - # answer after paying for the run. - serialized_data["single_pass_unresolvable_variables"] = ( - PromptStudioVariableService.find_unresolvable_single_pass_variables( - prompt=prompt.prompt - ) - if instance.single_pass_extraction_mode and prompt.prompt - else [] - ) - output.append(serialized_data) data[TSKeys.PROMPTS] = output diff --git a/backend/prompt_studio/prompt_studio_v2/serializers.py b/backend/prompt_studio/prompt_studio_v2/serializers.py index 4026f32b94..5463113539 100644 --- a/backend/prompt_studio/prompt_studio_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_v2/serializers.py @@ -23,6 +23,8 @@ class Meta: class ToolStudioPromptSerializer(AuditSerializer): + single_pass_unresolvable_variables = serializers.SerializerMethodField() + class Meta: model = ToolStudioPrompt fields = "__all__" @@ -30,6 +32,30 @@ class Meta: # the DRF auto-validator that 400s on re-save / PUT before the view runs. validators = [] + def get_single_pass_unresolvable_variables(self, obj) -> list[str]: + """UN-2900: variables in this prompt that single pass cannot resolve. + + Empty unless the parent tool has single-pass extraction enabled. Lives + on this serializer rather than the tool serializer so the same value is + returned by BOTH the tool detail fetch (which nests this serializer per + prompt) and the prompt save response, without duplicating the logic. + + ``tool_id`` is the FK to CustomTool; callers that serialize many prompts + should ``select_related("tool_id")`` to avoid a query per prompt. + """ + from prompt_studio.prompt_studio_core_v2.prompt_variable_service import ( + PromptStudioVariableService, + ) + + tool = getattr(obj, "tool_id", None) + if not tool or not getattr(tool, "single_pass_extraction_mode", False): + return [] + if not obj.prompt: + return [] + return PromptStudioVariableService.find_unresolvable_single_pass_variables( + prompt=obj.prompt + ) + class ToolStudioIndexSerializer(serializers.Serializer): file_name = serializers.CharField() From 1f8a4a7cb4b18b5bdd1e40949a3839339c74d205 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Sat, 29 Aug 2026 01:11:48 +0530 Subject: [PATCH 07/11] UN-3176 [FIX] Stop reporting BigQuery value errors as missing columns Every google.api_core BadRequest was mapped to ColumnMissingException, whose message tells the user to "make sure all the columns exist in your table as per the destination DB configuration". BigQuery also returns BadRequest for VALUE-level failures -- including this ticket's "cannot round-trip through string representation; error in PARSE_JSON expression" -- so users were sent to check a schema that was never wrong. That misdirection is likely why this was filed as a datatype-conversion bug. Adds BigQueryValueException and discriminates before wrapping: prefers the structured errors[] payload, falls back to message signatures for the round-trip / PARSE_JSON / invalid-JSON cases BigQuery does not tag. Anything unrecognised falls through to the existing ColumnMissingException, so this only narrows messages that were already wrong. Verified 7/7 including the ticket's verbatim error text and two genuine missing-column messages that must NOT be reclassified. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF --- .../connectors/databases/bigquery/bigquery.py | 42 +++++++++++++++++++ .../connectors/databases/exceptions.py | 23 ++++++++++ 2 files changed, 65 insertions(+) diff --git a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py index 723450b342..99985496eb 100644 --- a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py +++ b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py @@ -13,6 +13,7 @@ from unstract.connectors.databases.exceptions import ( BigQueryForbiddenException, BigQueryNotFoundException, + BigQueryValueException, ColumnMissingException, ) from unstract.connectors.databases.sql_safety import ( @@ -321,6 +322,17 @@ def execute_query( detail=e.message, table_name=table_name ) from e except google.api_core.exceptions.BadRequest as e: + # UN-3176: BigQuery returns BadRequest for VALUE-level failures as + # well as for schema mismatches. Mapping them all to + # ColumnMissingException told users to check their columns when the + # columns were fine (e.g. a float that will not round-trip through + # PARSE_JSON), which misdirects the investigation. Discriminate + # before wrapping. + if BigQuery._is_value_error(e): + logger.error(f"Value rejected by BigQuery on insert: {str(e)}") + raise BigQueryValueException( + detail=e.message, table_name=table_name + ) from e logger.error(f"Column missing in inserting data: {str(e)}") db, schema, table = table_name.split(".") raise ColumnMissingException( @@ -330,6 +342,36 @@ def execute_query( table_name=table, ) from e + @staticmethod + def _is_value_error(e: Any) -> bool: + """True if a BigQuery BadRequest is about the DATA, not the schema. + + UN-3176. Prefers the structured ``errors`` payload (a list of + ``{reason, message}``), because ``invalidQuery`` covers the value-level + rejections we care about here, and falls back to the message text for + the signatures BigQuery does not tag -- notably the PARSE_JSON + round-trip failure in this ticket. Unknown shapes fall through to the + existing column-missing behaviour, so this only ever narrows a message + that was already wrong for these cases. + """ + value_error_markers = ( + "parse_json", + "round-trip through string representation", + "invalid json", + "cannot round-trip", + "failed to parse json", + ) + text = f"{getattr(e, 'message', '') or ''} {str(e)}".lower() + if any(marker in text for marker in value_error_markers): + return True + for error in getattr(e, "errors", None) or []: + if not isinstance(error, dict): + continue + message = str(error.get("message", "")).lower() + if any(marker in message for marker in value_error_markers): + return True + return False + def get_information_schema(self, table_name: str) -> dict[str, str]: """Function to generate information schema of the big query table. diff --git a/unstract/connectors/src/unstract/connectors/databases/exceptions.py b/unstract/connectors/src/unstract/connectors/databases/exceptions.py index b1a9b30853..c699ff4542 100644 --- a/unstract/connectors/src/unstract/connectors/databases/exceptions.py +++ b/unstract/connectors/src/unstract/connectors/databases/exceptions.py @@ -90,6 +90,29 @@ def __init__(self, detail: str, table_name: str) -> None: super().__init__(detail=final_detail) +class BigQueryValueException(UnstractDBConnectorException): + """A BigQuery BadRequest caused by the DATA, not the table schema. + + UN-3176: BigQuery returns BadRequest for value-level failures (a float that + will not round-trip through PARSE_JSON, a malformed JSON literal) as well as + for genuine schema mismatches. Mapping every BadRequest to + ColumnMissingException told users to "make sure all the columns exist" when + the columns were fine, which sent at least one investigation down the wrong + path. + """ + + def __init__(self, detail: Any, table_name: str) -> None: + default_detail = ( + f"Error writing to '{table_name}'. \n" + f"BigQuery rejected a value in the row being inserted -- the table " + f"schema is not the problem. This usually means a number could not " + f"be represented exactly, or a JSON column received text that is " + f"not valid JSON.\n" + ) + final_detail = _format_exception_detail(default_detail, detail) + super().__init__(detail=final_detail) + + class ColumnMissingException(UnstractDBConnectorException): def __init__( self, From 68eaec46f2b4954d70311f42b0af93e39858469a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:41:13 +0000 Subject: [PATCH 08/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- backend/tool_instance_v2/tool_instance_helper.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/backend/tool_instance_v2/tool_instance_helper.py b/backend/tool_instance_v2/tool_instance_helper.py index 70631fb47f..d44a77c2a0 100644 --- a/backend/tool_instance_v2/tool_instance_helper.py +++ b/backend/tool_instance_v2/tool_instance_helper.py @@ -490,9 +490,7 @@ def validate_adapter_permissions( if llm.is_enabled and llm.adapter_id: adapter_id = tool_meta.get(llm.adapter_id) elif llm.is_enabled: - adapter_id = tool_meta.get( - AdapterPropertyKey.DEFAULT_LLM_ADAPTER_ID - ) + adapter_id = tool_meta.get(AdapterPropertyKey.DEFAULT_LLM_ADAPTER_ID) # UN-2953: a tool instance may carry "" for an adapter id. # Adding it made the schema validator compare "" against the @@ -532,9 +530,7 @@ def validate_adapter_permissions( if text_extractor.is_enabled and text_extractor.adapter_id: adapter_id = tool_meta.get(text_extractor.adapter_id) elif text_extractor.is_enabled: - adapter_id = tool_meta.get( - AdapterPropertyKey.DEFAULT_X2TEXT_ADAPTER_ID - ) + adapter_id = tool_meta.get(AdapterPropertyKey.DEFAULT_X2TEXT_ADAPTER_ID) # UN-2953: a tool instance may carry "" for an adapter id. # Adding it made the schema validator compare "" against the From fc2962cb03a6699cde41de50c9b749ebde8f0d6e Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 00:28:15 +0530 Subject: [PATCH 09/11] [FIX] Correct two false comment claims in the BigQuery sanitizer (#2, #5) #2 _sanitize_for_bigquery: the UN-3176 comment said the old magnitude-derived decimal count over-preserved precision for values BELOW 1, citing 0.0053325 as a value "passed through unchanged". That is inverted. For magnitude m < 0 the value carries |m| leading zeros after the point, so `15 - m` decimals preserves exactly 15 significant figures; the old form was already correct for every m <= 15. It over-preserves only ABOVE 10^15, where the count floors at 0 and the full integer part is emitted. A 200k-sample sweep over magnitudes 10^-12 to 10^+20 shows old and new differ only at 10^15 and above -- and old(0.0053325) == new(0.0053325) exactly. The code change is right; the stated cause was not. #5 _is_value_error: the docstring claimed it "prefers the structured errors payload" because "invalidQuery covers the value-level rejections". Neither is true -- the message text is checked first and returns before the payload is reached, and `reason` is never inspected at all. Restated to describe what the function does. The e.errors loop is kept and is provably live: str() of a google.api_core BadRequest is just "400 " and omits the payload, so a marker present only there is still matched. #6 serializers.py: drop a stray blank line added by the diff. No behaviour change. --- .../prompt_studio_core_v2/serializers.py | 1 - .../connectors/databases/bigquery/bigquery.py | 25 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index 2eadce85ed..0dead92f4f 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -232,7 +232,6 @@ def to_representation(self, instance): # type: ignore # Add coverage to serialized data serialized_data["coverage"] = coverage - output.append(serialized_data) data[TSKeys.PROMPTS] = output diff --git a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py index 99985496eb..f3b46d21c7 100644 --- a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py +++ b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py @@ -110,12 +110,12 @@ def _sanitize_for_bigquery(data: Any) -> Any: # Limit total significant figures to 15 for IEEE 754 compatibility. # BigQuery PARSE_JSON requires values that round-trip cleanly. # - # UN-3176: the previous form derived a DECIMAL-place count from the - # magnitude, which only matches "15 significant figures" for values - # >= 1. For a value like 0.0053325 the magnitude is -2, giving 17 - # decimals -- more precision than the safe zone allows, so the - # value was passed through unchanged. Formatting with `g` applies - # the significant-figure limit directly, for any magnitude. + # UN-3176: the previous form asked for `15 - magnitude` DECIMAL + # places. Below 10^15 that happens to equal 15 significant figures, + # but above it the count floors at 0 and the full integer part was + # emitted -- more precision than the safe zone allows. Formatting + # with `g` applies the significant-figure limit directly, at any + # magnitude. return float(f"{data:.15g}") elif isinstance(data, dict): @@ -346,13 +346,12 @@ def execute_query( def _is_value_error(e: Any) -> bool: """True if a BigQuery BadRequest is about the DATA, not the schema. - UN-3176. Prefers the structured ``errors`` payload (a list of - ``{reason, message}``), because ``invalidQuery`` covers the value-level - rejections we care about here, and falls back to the message text for - the signatures BigQuery does not tag -- notably the PARSE_JSON - round-trip failure in this ticket. Unknown shapes fall through to the - existing column-missing behaviour, so this only ever narrows a message - that was already wrong for these cases. + UN-3176. Matches known value-level signatures -- notably the PARSE_JSON + round-trip failure in this ticket -- against the exception's own text + and then against each entry of its structured ``errors`` payload, which + ``str(e)`` does not include. Unknown shapes fall through to the existing + column-missing behaviour, so this only ever narrows a message that was + already wrong for these cases. """ value_error_markers = ( "parse_json", From df75e378c1950e9c3c5e1790ae58f60fbe79cd80 Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 00:37:19 +0530 Subject: [PATCH 10/11] [FIX] Stop restating the UN-3176 cause; state only what `g` does (#2, #5) Adversarial verification refuted the causal story in the previous commit. It claimed the old and new forms agree below 10^15. They do not: when |x| sits just below a power of ten, log10 returns an exact integer (the true value is within half an ULP), so the derived magnitude is one too large and the old form emitted 14 significant figures, not 15. Reproduced against the shipped function -- old(9.99999999999999e-05) == 0.0001 while new() preserves all 15 digits, and the same happens in every decade below 1e-4. That is the second false account of this line in as many commits, so this one stops narrating the old form's failure mode and states only the invariant that matters: `g` asks for significant figures, the old form asked for decimal places, and the two are not the same quantity. _is_value_error: the "str(e) does not include the payload" claim was true for the path this code sees but stated absolutely. GoogleAPICallError.__str__ (api-core 2.24.2) 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 hasattr filter. Says that instead. Verified the loop is still load-bearing: a marker present only in the dict payload is absent from str(e) ('400 Schema mismatch on insert') and the function still returns True. No behaviour change. unstract/connectors tests/databases: 36 passed. --- .../connectors/databases/bigquery/bigquery.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py index f3b46d21c7..75437216a9 100644 --- a/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py +++ b/unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py @@ -110,12 +110,10 @@ def _sanitize_for_bigquery(data: Any) -> Any: # Limit total significant figures to 15 for IEEE 754 compatibility. # BigQuery PARSE_JSON requires values that round-trip cleanly. # - # UN-3176: the previous form asked for `15 - magnitude` DECIMAL - # places. Below 10^15 that happens to equal 15 significant figures, - # but above it the count floors at 0 and the full integer part was - # emitted -- more precision than the safe zone allows. Formatting - # with `g` applies the significant-figure limit directly, at any - # magnitude. + # UN-3176: `g` asks for significant figures directly. The previous + # form asked for a DECIMAL-place count derived from the magnitude, + # which is not the same quantity and drifted from 15 sig figs at + # both ends of the range. return float(f"{data:.15g}") elif isinstance(data, dict): @@ -348,10 +346,13 @@ def _is_value_error(e: Any) -> bool: UN-3176. Matches known value-level signatures -- notably the PARSE_JSON round-trip failure in this ticket -- against the exception's own text - and then against each entry of its structured ``errors`` payload, which - ``str(e)`` does not include. Unknown shapes fall through to the existing - column-missing behaviour, so this only ever narrows a message that was - already wrong for these cases. + and then against each entry of its structured ``errors`` payload. The + payload is checked separately because BigQuery's REST path fills it + with plain dicts, and ``GoogleAPICallError.__str__`` folds an entry + into the string only when it exposes ``.code``/``.message`` attributes. + Unknown shapes fall through to the existing column-missing behaviour, + so this only ever narrows a message that was already wrong for these + cases. """ value_error_markers = ( "parse_json", From 07ed225eadf84899db985c344b017b6351916c5d Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Mon, 31 Aug 2026 01:22:40 +0530 Subject: [PATCH 11/11] [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). --- .../tests/databases/test_bigquery_db.py | 108 ++++++++++++++++ .../tests/test_llm_whisperer_v2_params.py | 122 +++++++++++++++++- 2 files changed, 229 insertions(+), 1 deletion(-) diff --git a/unstract/connectors/tests/databases/test_bigquery_db.py b/unstract/connectors/tests/databases/test_bigquery_db.py index 695d28a337..de6d157926 100644 --- 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 index d8ca030e31..0016ce11a0 100644 --- 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