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 @@ -143,6 +143,31 @@ def get_tool_by_prompt_registry_id(
image_tag=settings.STRUCTURE_TOOL_IMAGE_TAG,
)

@staticmethod
def get_resolved_settings(prompt_registry_id: str) -> dict[str, Any]:
"""Return the settings export already resolved for this exported tool.

``frame_export_json`` resolves adapter-valued settings at export time -
notably ``challenge_llm``, which falls back to the default profile's LLM
when the project set none - and stores them under
``tool_metadata[tool_settings]``. ``Tool`` (built from ``tool_spec`` /
``tool_property``) does not carry them, so callers that only have a
``Tool`` cannot see the resolved values.

Returns an empty dict when the registry row is missing or carries no
settings, so callers can treat "no resolved settings" as a no-op.
"""
try:
prompt_registry_tool = PromptStudioRegistry.objects.get(pk=prompt_registry_id)
except Exception as e:
logger.warning(
f"Error while fetching resolved settings for prompt registry ID "
f"{prompt_registry_id}: {e}"
)
return {}
metadata = prompt_registry_tool.tool_metadata or {}
return metadata.get(JsonSchemaKey.TOOL_SETTINGS, {}) or {}

Comment on lines +146 to +170

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

New helper logs a WARNING on every non-Prompt-Studio tool-instance creation.

get_resolved_settings is called unconditionally from ToolInstanceSerializer.create() for every tool type via str(tool_uid). prompt_registry_id is a UUID primary key, so any non-Prompt-Studio tool_uid (the common case — built-in/registry tools) will fail PromptStudioRegistry.objects.get(pk=...) and hit the except Exception branch, logging a warning on every single such creation. The docstring frames "missing settings" as an expected no-op, but the implementation logs it as a warning regardless, which will flood production logs and mask genuine errors.

Short-circuit on an invalid UUID before touching the DB/logger:

🔇 Proposed fix
+import uuid
+
     `@staticmethod`
     def get_resolved_settings(prompt_registry_id: str) -> dict[str, Any]:
         ...
+        try:
+            uuid.UUID(str(prompt_registry_id))
+        except (ValueError, AttributeError, TypeError):
+            # Not a Prompt Studio registry ID (e.g. a built-in tool's uid);
+            # nothing to resolve, and not worth a warning.
+            return {}
         try:
             prompt_registry_tool = PromptStudioRegistry.objects.get(pk=prompt_registry_id)
         except Exception as e:
             logger.warning(
                 f"Error while fetching resolved settings for prompt registry ID "
                 f"{prompt_registry_id}: {e}"
             )
             return {}

Note: the existing test double for this function (test_get_resolved_settings_reads_the_tool_settings_block) would still pass unchanged since it only exercises the DB-lookup branch.

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 162-162: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py`
around lines 146 - 170, Update get_resolved_settings to validate
prompt_registry_id as a UUID before calling PromptStudioRegistry.objects.get or
logging; return an empty dict immediately for invalid identifiers so
non-Prompt-Studio tool instances remain a silent no-op, while preserving the
existing database lookup behavior for valid UUIDs.

Source: Linters/SAST tools

@staticmethod
def update_or_create_psr_tool(
custom_tool: CustomTool,
Expand Down
20 changes: 19 additions & 1 deletion backend/tool_instance_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from adapter_processor_v2.adapter_processor import AdapterProcessor
from adapter_processor_v2.models import AdapterInstance
from prompt_studio.prompt_studio_registry_v2.constants import PromptStudioRegistryKeys
from prompt_studio.prompt_studio_registry_v2.prompt_studio_registry_helper import (
PromptStudioRegistryHelper,
)
from rest_framework.serializers import ListField, Serializer, UUIDField, ValidationError
from workflow_manager.workflow_v2.constants import WorkflowKey
from workflow_manager.workflow_v2.models.workflow import Workflow
Expand Down Expand Up @@ -170,11 +173,26 @@ def create(self, validated_data: dict[str, Any]) -> Any:
validated_data[TIKey.PK] = uuid.uuid4()
# TODO: Use version from tool props
validated_data[TIKey.VERSION] = ""
tool_settings = ToolProcessor.get_default_settings(tool)
# `get_default_settings` seeds an adapter-valued property with "" when
# the spec carries no default (e.g. challenge_llm). The exported tool
# already resolved a real value for it, so prefer that - otherwise the
# instance stores "", which fails deployment validation against the
# adapter enum. Only spec-declared keys are overlaid; the export's
# settings are a superset (llm, vector-db, ...) and the rest are not
# part of the instance schema. Non-Prompt-Studio tools resolve to {}
# here, making this a no-op for them.
resolved_settings = PromptStudioRegistryHelper.get_resolved_settings(
prompt_registry_id=str(tool_uid)
)
for key in tool_settings:
if key in resolved_settings:
tool_settings[key] = resolved_settings[key]
validated_data[TIKey.METADATA] = {
# TODO: Review and remove tool instance ID
WorkflowKey.WF_TOOL_INSTANCE_ID: str(validated_data[TIKey.PK]),
PromptStudioRegistryKeys.PROMPT_REGISTRY_ID: str(tool_uid),
**ToolProcessor.get_default_settings(tool),
**tool_settings,
Comment on lines +176 to +195

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'single_pass_extraction_mode|ENABLE_SINGLE_PASS_EXTRACTION' backend/prompt_studio/prompt_studio_registry_v2/

Repository: Zipstack/unstract

Length of output: 702


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ToolInstanceV2 serializer relevant section =="
rg -n "get_resolved_settings|get_default_settings|tool_settings|single_pass_extraction_mode|ENABLE_SINGLE_PASS_EXTRACTION|METADATA|TIKey" backend/tool_instance_v2/serializers.py

echo
echo "== prompt_studio_registry_helper relevant exports =="
sed -n '280,325p' backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py
sed -n '50,90p' backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py

echo
echo "== constants =="
rg -n "class JsonSchemaKey|ENABLE_SINGLE_PASS_EXTRACTION|SINGLE_PASS|single_pass_extraction_mode|ENABLE_HIGHLIGHT|ENABLE_WORD_CONFIDENCE|CHALLENGE_LLM|ENABLE_CHALLENGE|SUMMARIZE_AS_SOURCE" backend/prompt_studio/prompt_studio_registry_v2/constants.py

Repository: Zipstack/unstract

Length of output: 5802


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tool_instance serializer diff/stat =="
git diff -- backend/tool_instance_v2/serializers.py | sed -n '1,120p' || true
git diff -- backend/tool_instance_v2/ --stat || true

echo
echo "== relevant tests mentioning single_pass_extraction_mode / ENABLE_SINGLE_PASS_EXTRACTION =="
rg -n "single_pass_extraction_mode|ENABLE_SINGLE_PASS_EXTRACTION|enable_single_pass_extraction" backend tests | sed -n '1,160p'

echo
echo "== read-only structural/behavioral verifier for overlay keys =="
python3 - <<'PY'
from pathlib import Path
import ast, re

spec_file = Path("backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py")
serializer_file = Path("backend/tool_instance_v2/serializers.py")
const_file = Path("backend/prompt_studio/prompt_studio_registry_v2/constants.py")

tree = ast.parse(spec_file.read_text())
export_props_found = []
default_props_found = []
for node in ast.walk(tree):
    if isinstance(node, ast.Dict):
        # find dicts whose keys include ENABLE_SINGLE_PASS_EXTRACTION or single_pass_extraction_mode
        keys = [v.value if isinstance(v, ast.Constant) else None for v in [k for k in node.keys]]
        if "enable_single_pass_extraction" in keys or "single_pass_extraction_mode" in keys:
            if node.lineno < 150 and any(isinstance(k, ast.Constant) and k.value in ("single_pass_extraction_mode", "enable_challenge") for k in node.keys):
                export_props_found = keys[:10]
            elif node.lineno > 250 and node.lineno < 350:
                export_props_found = keys[:25]
        if node.col_offset == 0 and node.lineno < 120:
            default_props_found = [v.value if isinstance(v, ast.Constant) else None for v in [k for k in node.keys]]

serializer_text = serializer_file.read_text()
const_tree = ast.parse(const_file.read_text())
const_dict = {}
for node in ast.walk(const_tree):
    if isinstance(node, ast.Assign):
        for t in node.targets:
            if isinstance(t, ast.Name) and t.id in (
                "CHALLENGE_LLM",
                "ENABLE_CHALLENGE",
                "SUMMARIZE_AS_SOURCE",
                "ENABLE_SINGLE_PASS_EXTRACTION",
                "ENABLE_HIGHLIGHT",
                "ENABLE_WORD_CONFIDENCE",
            ):
                const_dict[t.id] = node.value.value if isinstance(node.value, ast.Constant) else None

export_alias = const_dict["ENABLE_SINGLE_PASS_EXTRACTION"]
spec_key = "single_pass_extraction_mode"
overlay_match = bool(re.search(r"for key in tool_settings:(\s*\n\s*if key in resolved_settings:|.\n\s*if export_key in resolved_settings:)", serializer_text))
print("export_setting_key=", export_alias)
print("spec_property_key=", spec_key)
print("export_setting_matches_spec_property=", export_alias == spec_key)
print("overlay_checks_resolved_settings_key=", overlay_match)
print("overlapping_props=", [k for k in sorted(export_props_found or []) if k in sorted(default_props_found or [])])
PY

Repository: Zipstack/unstract

Length of output: 2922


Overlay silently drops single_pass_extraction_mode.

tool_settings uses the spec property name (single_pass_extraction_mode), but get_resolved_settings() exports the model field under JsonSchemaKey.ENABLE_SINGLE_PASS_EXTRACTION (enable_single_pass_extraction). Since the overlay only matches key in resolved_settings, a Prompt-Studio tool exported with single-pass extraction enabled will seed the new instance with single_pass_extraction_mode: False, preserving the same silent-setting-drop shape as the challenge_llm bug this PR fixes.

🔧 One way to close the gap
+        # frame_export_json exports a couple of settings under a different
+        # key than the spec property name.
+        _EXPORT_KEY_ALIASES = {"single_pass_extraction_mode": "enable_single_pass_extraction"}
         for key in tool_settings:
-            if key in resolved_settings:
-                tool_settings[key] = resolved_settings[key]
+            export_key = _EXPORT_KEY_ALIASES.get(key, key)
+            if export_key in resolved_settings:
+                tool_settings[key] = resolved_settings[export_key]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tool_settings = ToolProcessor.get_default_settings(tool)
# `get_default_settings` seeds an adapter-valued property with "" when
# the spec carries no default (e.g. challenge_llm). The exported tool
# already resolved a real value for it, so prefer that - otherwise the
# instance stores "", which fails deployment validation against the
# adapter enum. Only spec-declared keys are overlaid; the export's
# settings are a superset (llm, vector-db, ...) and the rest are not
# part of the instance schema. Non-Prompt-Studio tools resolve to {}
# here, making this a no-op for them.
resolved_settings = PromptStudioRegistryHelper.get_resolved_settings(
prompt_registry_id=str(tool_uid)
)
for key in tool_settings:
if key in resolved_settings:
tool_settings[key] = resolved_settings[key]
validated_data[TIKey.METADATA] = {
# TODO: Review and remove tool instance ID
WorkflowKey.WF_TOOL_INSTANCE_ID: str(validated_data[TIKey.PK]),
PromptStudioRegistryKeys.PROMPT_REGISTRY_ID: str(tool_uid),
**ToolProcessor.get_default_settings(tool),
**tool_settings,
tool_settings = ToolProcessor.get_default_settings(tool)
# `get_default_settings` seeds an adapter-valued property with "" when
# the spec carries no default (e.g. challenge_llm). The exported tool
# already resolved a real value for it, so prefer that - otherwise the
# instance stores "", which fails deployment validation against the
# adapter enum. Only spec-declared keys are overlaid; the export's
# settings are a superset (llm, vector-db, ...) and the rest are not
# part of the instance schema. Non-Prompt-Studio tools resolve to {}
# here, making this a no-op for them.
resolved_settings = PromptStudioRegistryHelper.get_resolved_settings(
prompt_registry_id=str(tool_uid)
)
# frame_export_json exports a couple of settings under a different
# key than the spec property name.
_EXPORT_KEY_ALIASES = {"single_pass_extraction_mode": "enable_single_pass_extraction"}
for key in tool_settings:
export_key = _EXPORT_KEY_ALIASES.get(key, key)
if export_key in resolved_settings:
tool_settings[key] = resolved_settings[export_key]
validated_data[TIKey.METADATA] = {
# TODO: Review and remove tool instance ID
WorkflowKey.WF_TOOL_INSTANCE_ID: str(validated_data[TIKey.PK]),
PromptStudioRegistryKeys.PROMPT_REGISTRY_ID: str(tool_uid),
**tool_settings,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tool_instance_v2/serializers.py` around lines 176 - 195, Update the
settings overlay in the tool-instance serializer to map the resolved export
field JsonSchemaKey.ENABLE_SINGLE_PASS_EXTRACTION to the spec property
single_pass_extraction_mode before writing tool_settings. Preserve the existing
direct-key overlay for other settings and ensure enabled single-pass extraction
is not replaced by the default False.

}
if TIKey.STEP not in validated_data:
validated_data[TIKey.STEP] = workflow.tool_instances.count() + 1
Expand Down
Empty file.
Loading
Loading