Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
Expand Down
12 changes: 9 additions & 3 deletions backend/prompt_studio/prompt_studio_core_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,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 ""
Expand Down
26 changes: 26 additions & 0 deletions backend/prompt_studio/prompt_studio_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,39 @@ class Meta:


class ToolStudioPromptSerializer(AuditSerializer):
single_pass_unresolvable_variables = serializers.SerializerMethodField()

class Meta:
model = ToolStudioPrompt
fields = "__all__"
# View owns uniqueness (IntegrityError->DuplicateData on create); drop
# 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()
Expand Down
48 changes: 36 additions & 12 deletions backend/tool_instance_v2/tool_instance_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,33 +486,57 @@ 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.
Comment on lines +495 to +497

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [Lens 16, 15] — This comment (repeated ×4, verbatim) names a mechanism that does not exist on this path, and hides the bugs actually fixed

Also at :509-511, :523-525, :535-537.

The comment says adding "" "made the schema validator compare "" against the UUID enum and raise." adapter_ids is a function-local set whose only consumer is validate_adapter_access (:553) → AdapterInstance.objects.filter(id__in=adapter_ids). No schema validator, no enum on that path. The only validator in this file (:451) validates tool_meta — a different input — and runs after this function returns. A maintainer will go read the JSON schema and find nothing.

The real failure was a Django UUID-coercion error (AdapterInstance.id is a UUIDField, backend/adapter_processor_v2/models.py:63), or the old KeyError.

And the comment never mentions the two other bugs this diff silently fixes: the old code left adapter_id unassigned when is_enabled was False, so adapter_ids.add(adapter_id) re-added the previous iteration's id (leaking across all four loops) or raised UnboundLocalError on the first. The new adapter_id = None initialiser is the load-bearing change and is undocumented.

Fix: one accurate comment above the loop group — # Skip unset adapter ids: "" is not a valid UUID and breaks the id__in filter. Hoist the four near-identical loops into a helper.

Confidence: High. Independently reached by three agents and a manual trace.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [Lens 13 — Testing] — This permission-check rewrite ships with zero tests

Every tool_meta[...] became tool_meta.get(...) plus if adapter_id:, changing raise-semantics to skip-semantics on the sole input to an authorization check (validate_adapter_access, :553). Nothing exercises it.

grep -rn "validate_adapter_permissions" --include=*.py . returns only the definition (:482) and its one caller (:439) — no test file. The app's only suite, backend/tool_instance_v2/tests/test_challenge_llm_seed_overlay.py, touches neither adapters nor permissions.

Fix — three cases against a stubbed ToolProcessor.get_tool_by_uid:

  1. "" for an enabled adapter → no exception, "" excluded from the set;
  2. a real inaccessible adapter id → PermissionDenied still raised;
  3. key missing entirely → pin the intended contract explicitly (this is the case the .get() widening newly swallows).

Confidence: High.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from unstract.connectors.databases.exceptions import (
BigQueryForbiddenException,
BigQueryNotFoundException,
BigQueryValueException,
ColumnMissingException,
)
from unstract.connectors.databases.sql_safety import (
Expand Down Expand Up @@ -106,13 +107,14 @@
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: `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}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [Lens 3, 5] — %.15g destroys precision on ≥1e15 floats that the old code preserved exactly

A 16–17-significant-digit float that is exactly representable as an IEEE-754 double is now silently rewritten before landing in the customer's BigQuery table. No error, no log — the warehouse just holds a wrong number. The stated contract is "values that round-trip cleanly"; these values already did.

Both implementations run side by side:

1760509016282637.0    old=1760509016282637.0    new=1760509016282640.0   old_exact=True  new_exact=False
1234567890123456.0    old=1234567890123456.0    new=1234567890123460.0   old_exact=True  new_exact=False
9.99999999999999e-05  old=0.0001                new=9.99999999999999e-05 old_exact=False new_exact=True

Only the third line is the bug UN-3176 describes ("values below 1"). The first two are a new regression — and test_large_magnitude_is_limited_to_15_significant_figures (test_bigquery_db.py:206-211) locks it in as desired behaviour.

Fix: skip the cap when the value already round-trips — e.g. return data unchanged when data.is_integer() and abs(data) < 2**53, or when float(f"{data:.17g}") == data and the 15g form differs. Update the test accordingly.

Confidence: High on the numeric behaviour (independently reproduced). Medium on frequency — the sanitizer is only reached for floats nested in dict/list payloads (:270,287,410). If tool metadata carries epoch-microsecond floats or large numeric IDs, this is Critical.


elif isinstance(data, dict):
return {k: BigQuery._sanitize_for_bigquery(v) for k, v in data.items()}
Expand Down Expand Up @@ -318,6 +320,17 @@
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)}")

Check failure on line 330 in unstract/connectors/src/unstract/connectors/databases/bigquery/bigquery.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use "logging.exception()" instead.

See more on https://sonarcloud.io/project/issues?id=Zipstack_unstract&issues=AaBPC2wkpsdzSMnawhCp&open=AaBPC2wkpsdzSMnawhCp&pullRequest=2257
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(
Expand All @@ -327,6 +340,38 @@
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. 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. 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",
"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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading