From a52feae5291d2e7f816e3708fb327b4791084a6c Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 24 Jul 2026 15:30:59 +0530 Subject: [PATCH 1/2] [FIX] Resolve Prompt Studio default LLM profile and unblock deploy when LLMChallenge is off Two failures hit during a full Prompt Studio run (extract -> export tool -> deploy API -> run). 1. fetch_response ignored the project default LLM profile `build_fetch_response_payload` and `_fetch_response` resolved the profile from the prompt's own FK only, and raised `DefaultProfileError` when it was null. Both sibling paths already fall back to the project default: index_document -> ProfileManager.get_default_llm_profile(tool) single_pass -> ProfileManager.get_default_llm_profile(tool) fetch_response -> prompt.profile_manager only The error text ("Default LLM profile is not configured. Please set an LLM profile as default to continue.") named the project default, so setting it looked correct and the call still failed - the message pointed at a setting the code never read. Both sites now fall back, and the message names both places a profile can come from. 2. An exported tool failed deployment validation with LLMChallenge off `deployment run` ended in pipeline status ERROR with "422 Unprocessable Entity: Tool validation failed". `get_default_settings` seeds a string property with no spec default as "", so a tool instance stores `challenge_llm: ""`. Because the property declared `adapterType: "LLM"`, `_update_schema_for_adapter_type` injected an enum of real adapter IDs, which "" can never satisfy - and this fired regardless of `enable_challenge`. `validate_adapter_access` did not catch it first: it filters `id__in=adapter_ids`, so "" matches no row and passes silently. The failure is an enum violation, not a required violation - a present-but-empty key satisfies `required`. So dropping `required` alone is not sufficient. `frame_spec` now declares `challenge_llm` as a plain string with a "" default when `enable_challenge` is off, and only marks it required/adapter-typed when the feature is on. With challenge enabled the enum is still injected and "" is still rejected, so an enabled challenge cannot silently run without an LLM. Scoped to the Prompt Studio spec, so no other tool or adapter type changes. Verified by composing the real `frame_spec` with the real `_update_schema_for_adapter_type`: enable_challenge=False {'challenge_llm': ''} -> VALID (was: RAISED enum) enable_challenge=True {'challenge_llm': ''} -> RAISED enum enable_challenge=True {'challenge_llm': } -> VALID Co-Authored-By: Claude Opus 4.8 (1M context) --- .../prompt_studio_core_v2/exceptions.py | 5 +-- .../prompt_studio_helper.py | 10 ++++++ .../prompt_studio_registry_helper.py | 32 ++++++++++++++----- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/backend/prompt_studio/prompt_studio_core_v2/exceptions.py b/backend/prompt_studio/prompt_studio_core_v2/exceptions.py index c4a7ffd204..036e87b7e1 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/exceptions.py +++ b/backend/prompt_studio/prompt_studio_core_v2/exceptions.py @@ -44,8 +44,9 @@ def __init__(self, detail: str | None = None, status_code: int = 500): class DefaultProfileError(APIException): status_code = 500 default_detail = ( - "Default LLM profile is not configured." - "Please set an LLM profile as default to continue." + "No LLM profile could be resolved for this prompt. " + "Either attach an LLM profile to the prompt, or set one as the " + "project default to continue." ) diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py index 91515db118..630421c705 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py @@ -721,6 +721,11 @@ def build_fetch_response_payload( profile_manager_id=profile_manager_id ) + # A prompt need not carry its own profile FK - fall back to the project + # default, matching index_document and single-pass extraction. + if not profile_manager: + profile_manager = ProfileManager.get_default_llm_profile(tool) + if not profile_manager: raise DefaultProfileError() @@ -1812,6 +1817,11 @@ def _fetch_response( profile_manager_id=profile_manager_id ) + # A prompt need not carry its own profile FK - fall back to the project + # default, matching index_document and single-pass extraction. + if not profile_manager: + profile_manager = ProfileManager.get_default_llm_profile(tool) + if not profile_manager: raise DefaultProfileError() diff --git a/backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py b/backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py index 4fee8c10bc..9edd47bdff 100644 --- a/backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py +++ b/backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py @@ -47,14 +47,25 @@ def frame_spec(tool: CustomTool) -> Spec: Returns: dict: spec dict """ + challenge_llm_property: dict[str, Any] = { + "type": "string", + "title": "Challenger LLM", + "adapterType": "LLM", + "description": "LLM to use for LLMChallenge", + "adapterIdKey": "challenge_llm_adapter_id", + } + if not tool.enable_challenge: + # With LLMChallenge off the tool instance stores challenge_llm as "" + # (no spec default to seed it). Declaring adapterType here would make + # the tool instance schema carry an enum of real adapter IDs only, + # which "" can never satisfy - so deployment validation rejects a + # tool that never used LLMChallenge in the first place. Leave the + # property a free-form string until the feature is switched on. + challenge_llm_property.pop("adapterType") + challenge_llm_property["default"] = "" + properties = { - "challenge_llm": { - "type": "string", - "title": "Challenger LLM", - "adapterType": "LLM", - "description": "LLM to use for LLMChallenge", - "adapterIdKey": "challenge_llm_adapter_id", - }, + "challenge_llm": challenge_llm_property, "enable_challenge": { "type": "boolean", "title": "Enable LLMChallenge", @@ -87,10 +98,15 @@ def frame_spec(tool: CustomTool) -> Spec: }, } + # challenge_llm is only meaningful when LLMChallenge is enabled. Marking + # it required unconditionally makes a tool instance that never set one + # fail validation at deployment time, long after export succeeded. + required = [JsonSchemaKey.CHALLENGE_LLM] if tool.enable_challenge else [] + spec = Spec( title=str(tool.tool_id), description=tool.description, - required=[JsonSchemaKey.CHALLENGE_LLM], + required=required, properties=properties, ) return spec From 5a4da0e71159a2d67315a431abdf6047adfdf0de Mon Sep 17 00:00:00 2001 From: Hari John Kuriakose Date: Fri, 24 Jul 2026 21:10:14 +0530 Subject: [PATCH 2/2] [TEST] Pin challenge_llm schema behaviour across the enable_challenge boundary Regression tests for the deploy-time 422 fixed in this PR. Exercises the real `frame_spec` composed with the real `_update_schema_for_adapter_type`, rather than reimplementing either - a copy of the logic would stay green even if the shipped code broke. The two function bodies are extracted from source and run against stubs, because Django is not importable in a plain checkout and both helpers live in Django-coupled modules. Mirrors the approach already used in `prompt_studio_core_v2/tests/test_build_index_payload.py`. Coverage, split across the enable_challenge boundary: challenge OFF - the reported bug - a value seeded by get_default_settings validates (was: RAISED enum) - no adapter enum is injected - challenge_llm is not required - an explicitly set adapter ID still validates challenge ON - guards against over-correcting - "" is rejected, and specifically with an `enum` violation - a missing key is rejected with `required` - a real adapter ID validates - the enum lists real adapter IDs scope - adapter enums outside challenge_llm are untouched, so the empty-value allowance cannot leak to every optional adapter on every tool Verified by mutation: reverting the fix in frame_spec fails exactly the three challenge-OFF assertions while the challenge-ON guards stay green. The tests assert the enum-vs-required distinction directly, since dropping `required` alone does not fix the bug - a present-but-empty key satisfies `required`. A future simplification back to an unconditional `adapterType` therefore fails loudly instead of silently restoring the 422. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/test_frame_spec_challenge_llm.py | 323 ++++++++++++++++++ 1 file changed, 323 insertions(+) create mode 100644 backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py diff --git a/backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py b/backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py new file mode 100644 index 0000000000..9a79c10374 --- /dev/null +++ b/backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py @@ -0,0 +1,323 @@ +"""Regression tests for ``PromptStudioRegistryHelper.frame_spec``'s handling of +``challenge_llm``. + +Pins the fix for the deploy-time failure where an exported tool that never used +LLMChallenge ended in pipeline status ERROR with +``422 Unprocessable Entity: Tool validation failed``. + +The mechanism these tests lock down: + + 1. ``get_default_settings`` seeds a ``"type": "string"`` property that has no + spec ``default`` with ``""``, so the tool instance stores + ``challenge_llm: ""``. + 2. A property declaring ``adapterType: "LLM"`` gets an ``enum`` of real + adapter IDs injected by ``_update_schema_for_adapter_type``. + 3. ``""`` is not in that enum, so validation rejects it -- and this fired + regardless of ``enable_challenge``. + +Note the failure is an **enum** violation, not a ``required`` one: a +present-but-empty key satisfies ``required``. Dropping ``challenge_llm`` from +``required`` alone therefore does *not* fix it, which is why ``frame_spec`` +makes the ``adapterType`` declaration itself conditional. These tests assert +that distinction directly so a future "simplification" back to an unconditional +``adapterType`` fails loudly. + +The tests exercise the **real** ``frame_spec`` body and the **real** +``_update_schema_for_adapter_type`` body composed together, rather than +reimplementing either -- a copy of the logic would stay green even if the +shipped code broke. + +Django is not importable in a plain checkout (no ``pytest-django``; the app +registry is not loaded), and both helpers live in Django-coupled modules. So +the two function bodies are extracted from source and executed against stubs, +mirroring the approach in +``prompt_studio_core_v2/tests/test_build_index_payload.py``. If either function +cannot be extracted -- because it was renamed or restructured -- the tests fail +rather than silently skip. +""" + +from __future__ import annotations + +import enum +import importlib.util +import re +import sys +import textwrap +import types +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import pytest + +jsonschema = pytest.importorskip( + "jsonschema", reason="jsonschema is required to exercise the real validator" +) + +BACKEND_DIR = Path(__file__).resolve().parents[3] +REPO_ROOT = BACKEND_DIR.parent + +REGISTRY_HELPER = ( + BACKEND_DIR + / "prompt_studio" + / "prompt_studio_registry_v2" + / "prompt_studio_registry_helper.py" +) +TOOL_PROCESSOR = BACKEND_DIR / "tool_instance_v2" / "tool_processor.py" +DTO_PATH = REPO_ROOT / "unstract" / "tool-registry" / "src" / "unstract" / "tool_registry" + + +class _AdapterTypes(str, enum.Enum): + LLM = "LLM" + EMBEDDING = "EMBEDDING" + VECTOR_DB = "VECTOR_DB" + X2TEXT = "X2TEXT" + OCR = "OCR" + + +def _stub_package(name: str, **attrs: Any) -> types.ModuleType: + module = types.ModuleType(name) + module.__path__ = [] # type: ignore[attr-defined] + for key, value in attrs.items(): + setattr(module, key, value) + sys.modules[name] = module + return module + + +def _load_spec_class() -> Any: + """Load the real ``Spec`` dataclass without importing the SDK-heavy package.""" + _stub_package("unstract") + _stub_package("unstract.sdk1") + _stub_package("unstract.sdk1.constants", AdapterTypes=_AdapterTypes) + _stub_package("unstract.tool_registry") + + constants_spec = importlib.util.spec_from_file_location( + "unstract.tool_registry.constants", DTO_PATH / "constants.py" + ) + constants = importlib.util.module_from_spec(constants_spec) + constants_spec.loader.exec_module(constants) + sys.modules["unstract.tool_registry.constants"] = constants + + dto_spec = importlib.util.spec_from_file_location( + "unstract.tool_registry.dto", DTO_PATH / "dto.py" + ) + dto = importlib.util.module_from_spec(dto_spec) + dto_spec.loader.exec_module(dto) + return dto.Spec + + +def _extract_function(path: Path, start_marker: str) -> str: + """Return the dedented source of a method beginning with ``start_marker``. + + Fails the test if the marker is absent, so a rename surfaces here instead of + quietly reducing coverage. + """ + source = path.read_text() + if start_marker not in source: + pytest.fail( + f"Could not find {start_marker!r} in {path}. If it was renamed or " + "restructured, update this test rather than deleting it." + ) + body = source[source.index(start_marker) :] + end = body.find("\n @staticmethod") + if end != -1: + body = body[:end] + return textwrap.dedent(body) + + +SPEC_CLS = _load_spec_class() + + +def _load_json_schema_key() -> Any: + spec = importlib.util.spec_from_file_location( + "_psr_constants", + BACKEND_DIR / "prompt_studio" / "prompt_studio_registry_v2" / "constants.py", + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module.JsonSchemaKey + + +def _real_frame_spec(): + body = _extract_function( + REGISTRY_HELPER, " def frame_spec(tool: CustomTool) -> Spec:" + ) + body = body.replace( + "def frame_spec(tool: CustomTool) -> Spec:", "def frame_spec(tool):" + ) + namespace: dict[str, Any] = { + "Spec": SPEC_CLS, + "Any": Any, + "JsonSchemaKey": _load_json_schema_key(), + } + exec(compile(body, str(REGISTRY_HELPER), "exec"), namespace) + return namespace["frame_spec"] + + +def _real_update_schema(): + """Extract ``_update_schema_for_adapter_type`` with a stubbed adapter lookup.""" + body = _extract_function(TOOL_PROCESSOR, " def _update_schema_for_adapter_type(") + body = re.sub( + r"def _update_schema_for_adapter_type\([^)]*\)[^:]*:", + "def update_schema(schema, keys, adapter_type, user):", + body, + flags=re.S, + ) + + class _Adapter: + def __init__(self, adapter_id: str, name: str) -> None: + self.id = adapter_id + self.adapter_name = name + + class _AdapterProcessor: + @staticmethod + def get_adapters_by_type(adapter_type: Any, user: Any = None) -> list[_Adapter]: + return [_Adapter(REAL_ADAPTER_ID, "My LLM")] + + namespace: dict[str, Any] = { + "AdapterProcessor": _AdapterProcessor, + "Spec": SPEC_CLS, + "AdapterTypes": _AdapterTypes, + "User": object, + "Any": Any, + } + exec(compile(body, str(TOOL_PROCESSOR), "exec"), namespace) + return namespace["update_schema"] + + +REAL_ADAPTER_ID = "11111111-2222-3333-4444-555555555555" + + +@dataclass +class _FakeCustomTool: + """Minimal stand-in for ``CustomTool`` -- ``frame_spec`` reads only these.""" + + tool_id: str = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + description: str = "A tool exported from Prompt Studio" + enable_challenge: bool = False + + +def _build_instance_schema(*, enable_challenge: bool) -> dict[str, Any]: + """Compose export-time ``frame_spec`` with deploy-time enum injection. + + This is the exact path a tool instance's metadata is validated against: + ``frame_spec`` output is persisted as ``tool_spec`` and later rehydrated via + ``Spec.from_dict`` in ``get_tool_by_prompt_registry_id``. + """ + frame_spec = _real_frame_spec() + update_schema = _real_update_schema() + + spec = frame_spec(_FakeCustomTool(enable_challenge=enable_challenge)) + llm_keys = spec.get_llm_adapter_properties_keys() + update_schema(spec, llm_keys, _AdapterTypes.LLM, None) + return spec.to_dict() + + +def _validation_errors(schema: dict[str, Any], instance: dict[str, Any]) -> list[Any]: + return list(jsonschema.Draft7Validator(schema).iter_errors(instance)) + + +def _seeded_default(schema: dict[str, Any], prop: str) -> Any: + """Mimic ``ToolUtils.get_default_settings`` for a single property. + + Kept in lockstep with the real seeding rule: use the spec ``default`` when + present, else the zero value for the declared type. + """ + prop_schema = schema["properties"][prop] + if "default" in prop_schema: + return prop_schema["default"] + return {"string": "", "integer": 0, "boolean": False}[prop_schema["type"]] + + +class TestChallengeDisabled: + """With LLMChallenge off, an unset ``challenge_llm`` must deploy cleanly.""" + + def test_seeded_empty_value_validates(self) -> None: + """The reported bug: a tool that never used LLMChallenge 422s on deploy.""" + schema = _build_instance_schema(enable_challenge=False) + seeded = _seeded_default(schema, "challenge_llm") + + errors = _validation_errors(schema, {"challenge_llm": seeded}) + + assert errors == [], ( + "An instance seeded by get_default_settings must validate when " + f"LLMChallenge is off; got {[e.validator for e in errors]}" + ) + + def test_no_enum_is_injected(self) -> None: + """No ``adapterType`` means no enum, which is what makes "unset" legal. + + Guards the actual fix: dropping ``required`` alone would leave the enum + in place and the 422 intact. + """ + schema = _build_instance_schema(enable_challenge=False) + + assert "enum" not in schema["properties"]["challenge_llm"], ( + "challenge_llm must not carry an adapter enum while LLMChallenge is " + "off, otherwise the empty seeded value is unrepresentable" + ) + + def test_challenge_llm_is_not_required(self) -> None: + schema = _build_instance_schema(enable_challenge=False) + assert "challenge_llm" not in schema.get("required", []) + + def test_a_real_adapter_id_still_validates(self) -> None: + """Turning the feature off must not reject a value the user did set.""" + schema = _build_instance_schema(enable_challenge=False) + assert _validation_errors(schema, {"challenge_llm": REAL_ADAPTER_ID}) == [] + + +class TestChallengeEnabled: + """With LLMChallenge on, the adapter constraint must still be enforced.""" + + def test_empty_value_is_rejected(self) -> None: + """The fix must not open a hole: challenge on + no LLM must not pass. + + This is the regression that a careless "just allow empty strings" + implementation would introduce. + """ + schema = _build_instance_schema(enable_challenge=True) + + errors = _validation_errors(schema, {"challenge_llm": ""}) + + assert errors, "An enabled LLMChallenge must not accept an empty adapter" + assert any( + error.validator == "enum" for error in errors + ), f"Expected an enum violation, got {[e.validator for e in errors]}" + + def test_missing_key_is_rejected(self) -> None: + schema = _build_instance_schema(enable_challenge=True) + errors = _validation_errors(schema, {}) + assert any(error.validator == "required" for error in errors) + + def test_real_adapter_id_validates(self) -> None: + schema = _build_instance_schema(enable_challenge=True) + assert _validation_errors(schema, {"challenge_llm": REAL_ADAPTER_ID}) == [] + + def test_enum_holds_real_adapter_ids(self) -> None: + schema = _build_instance_schema(enable_challenge=True) + assert schema["properties"]["challenge_llm"]["enum"] == [REAL_ADAPTER_ID] + + +def test_other_adapter_properties_are_untouched() -> None: + """The fix is scoped to ``challenge_llm``. + + ``_update_schema_for_adapter_type`` runs for LLM/EMBEDDING/VECTOR_DB/X2TEXT/ + OCR on every tool. Broadening the empty-value allowance there would make + ``""`` valid for every optional adapter on every tool -- and then fail at + runtime on ``AdapterInstance.objects.get(id="")``. This asserts the enum is + injected verbatim for a property this fix does not own. + """ + update_schema = _real_update_schema() + spec = SPEC_CLS( + title="unrelated-tool", + description="A tool that is not from Prompt Studio", + required=[], + properties={"some_llm": {"type": "string", "adapterType": "LLM"}}, + ) + + update_schema(spec, ["some_llm"], _AdapterTypes.LLM, None) + + assert spec.properties["some_llm"]["enum"] == [REAL_ADAPTER_ID], ( + "Adapter enums outside challenge_llm must keep listing only real " "adapter IDs" + )