[FIX] Resolve Prompt Studio default LLM profile and unblock deploy when LLMChallenge is off#2203
[FIX] Resolve Prompt Studio default LLM profile and unblock deploy when LLMChallenge is off#2203hari-kuriakose wants to merge 2 commits into
Conversation
…en 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': <uuid>} -> VALID
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughPrompt Studio now falls back to a project-default LLM profile when prompt-specific resolution is unavailable. Registry schemas conditionally validate and require ChangesPrompt Studio profile resolution
Conditional challenge schema
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| backend/prompt_studio/prompt_studio_core_v2/exceptions.py | Clarifies that profile resolution can use either a prompt attachment or the project default. |
| backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py | Adds project-default profile fallback to both fetch-response implementations while preserving existing validation. |
| backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py | Makes challenge LLM requirements and adapter constraints conditional on whether LLMChallenge is enabled. |
| backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py | Exercises the exported schema and adapter-enum injection for enabled, disabled, empty, and populated challenge LLM states. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Prompt response requested] --> B{Prompt has LLM profile?}
B -->|Yes| C[Use prompt profile]
B -->|No| D[Resolve project default profile]
C --> E[Validate adapters and fetch response]
D --> E
F[Export Prompt Studio tool] --> G{LLMChallenge enabled?}
G -->|Yes| H[Require challenge_llm and constrain to LLM adapter IDs]
G -->|No| I[Allow empty challenge_llm without adapter enum]
H --> J[Create and validate tool instance]
I --> J
Reviews (2): Last reviewed commit: "[TEST] Pin challenge_llm schema behaviou..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py (1)
50-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression tests for the complete validation matrix.
requiredonly rejects a missing key; it does not rejectchallenge_llm=""when challenges are enabled. Test the actual deployment validator to confirm that:
- Disabled: omitted, empty, and arbitrary string values are accepted, with no adapter constraint.
- Enabled: the field is required, valid LLM profiles are accepted, and
""is rejected.This also verifies that leaving
adapterIdKeypresent does not retain adapter-specific validation whenadapterTypeis removed.Also applies to: 101-109
🤖 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 50 - 68, Add regression tests covering the deployment validator’s full challenge_llm matrix, using the existing helper and validator test symbols: when challenges are disabled, verify omitted, empty, and arbitrary values are accepted without adapter validation; when enabled, verify the field is required, valid LLM profiles pass, and an empty string fails. Explicitly confirm removing adapterType while retaining adapterIdKey does not impose adapter-specific validation.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/prompt_studio/prompt_studio_core_v2/exceptions.py`:
- Around line 47-49: Update the DefaultProfileError message and its callers so
it accurately reflects tool-level failures from monitor_llm or challenge_llm
when the prompt already has a valid profile. Use context-specific guidance for
unresolved tool profiles, or ensure those fallbacks reuse the already-resolved
prompt profile; retain the existing guidance for genuinely missing prompt and
project profiles.
In `@backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py`:
- Around line 724-728: Update the fallback branch around
ProfileManager.get_default_llm_profile so callback metadata records the resolved
profile rather than the original argument: after resolving the default, set
cb_kwargs["profile_manager_id"] to profile_manager.profile_id. Preserve the
existing profile selection behavior for explicitly provided profile managers.
---
Nitpick comments:
In
`@backend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py`:
- Around line 50-68: Add regression tests covering the deployment validator’s
full challenge_llm matrix, using the existing helper and validator test symbols:
when challenges are disabled, verify omitted, empty, and arbitrary values are
accepted without adapter validation; when enabled, verify the field is required,
valid LLM profiles pass, and an empty string fails. Explicitly confirm removing
adapterType while retaining adapterIdKey does not impose adapter-specific
validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dc89667-a25a-40aa-a931-6116ba38103f
📒 Files selected for processing (3)
backend/prompt_studio/prompt_studio_core_v2/exceptions.pybackend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.pybackend/prompt_studio/prompt_studio_registry_v2/prompt_studio_registry_helper.py
| "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." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the error message accurate for tool-level LLM failures.
DefaultProfileError is also raised when monitor_llm or challenge_llm is unset and the project default cannot be resolved, even when the prompt already has a valid profile. In that case, “attach an LLM profile to the prompt” does not fix the failure; use a context-specific message or make those fallbacks use the already-resolved profile.
🤖 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_core_v2/exceptions.py` around lines 47 -
49, Update the DefaultProfileError message and its callers so it accurately
reflects tool-level failures from monitor_llm or challenge_llm when the prompt
already has a valid profile. Use context-specific guidance for unresolved tool
profiles, or ensure those fallbacks reuse the already-resolved prompt profile;
retain the existing guidance for genuinely missing prompt and project profiles.
| # 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) | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate the selected fallback profile to callback handling.
When this path resolves a project default, cb_kwargs["profile_manager_id"] later still contains the original argument, often None. OutputManagerHelper then resolves the project default again during callback processing, so a default-profile change between execution and callback can make output bookkeeping use a different profile. Pass the resolved profile_manager.profile_id in the callback metadata.
🤖 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_core_v2/prompt_studio_helper.py` around
lines 724 - 728, Update the fallback branch around
ProfileManager.get_default_llm_profile so callback metadata records the resolved
profile rather than the original argument: after resolving the default, set
cb_kwargs["profile_manager_id"] to profile_manager.profile_id. Preserve the
existing profile selection behavior for explicitly provided profile managers.
… 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) <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py (2)
141-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
exec/compilestatic-analysis flags (S102/no-exec/no-compile) are expected here.Ruff and ast-grep flag the
exec(compile(...))calls used to run the extractedframe_specand_update_schema_for_adapter_typebodies. Given the extensive rationale in the module docstring (testing the real function bodies rather than reimplementing them, since Django isn't importable in this checkout), this is a deliberate, non-security-sensitive use on trusted local source files rather than untrusted input — a reasonable false positive. If the project enforces these Ruff/ast-greprules as CI-blocking, consider adding# noqa: S102(and any equivalentast-grepsuppression) at theexec/compilecall sites to avoid recurring lint failures without weakening the rule elsewhere.🤖 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/tests/test_frame_spec_challenge_llm.py` around lines 141 - 186, Add targeted lint suppressions for the intentional exec/compile calls in _real_frame_spec and _real_update_schema, including Ruff S102 and any configured ast-grep equivalent. Keep the suppression limited to these trusted-source test helpers and do not weaken the rules globally.Source: Linters/SAST tools
87-106: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
sys.modulesstubs mutate global interpreter state with no teardown.
_load_spec_class()overwritessys.modules["unstract"],"unstract.sdk1","unstract.sdk1.constants", and"unstract.tool_registry"at module-import time (line 128, executed once at collection) and never restores them. If any other test in the same pytest session (run in the same process) subsequently does a realimport unstract...on one of these exact dotted names, it will get this stub instead of the real module, sincesys.modulesis a process-wide cache. This is order-dependent and can cause hard-to-diagnose cross-test contamination.Consider scoping the stubbing to a fixture using
monkeypatch.setitem(sys.modules, ...)so pytest restores the original registry after the test/session, instead of mutatingsys.modulesat bare module scope.Also applies to: 128-128
🤖 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/tests/test_frame_spec_challenge_llm.py` around lines 87 - 106, Scope the module stubs used by _load_spec_class to pytest-managed setup rather than mutating sys.modules during collection. Use a fixture with monkeypatch.setitem for the unstract, sdk1, constants, and tool_registry entries, and ensure the dynamically loaded modules are likewise restored after use so other tests can import the real packages without contamination.
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py`:
- Around line 53-55: Add jsonschema to the backend test/dev dependency group in
pyproject.toml alongside pytest, using the project's existing dependency
declaration style, so test environments install it and
test_frame_spec_challenge_llm.py exercises the real validator instead of
skipping.
---
Nitpick comments:
In
`@backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py`:
- Around line 141-186: Add targeted lint suppressions for the intentional
exec/compile calls in _real_frame_spec and _real_update_schema, including Ruff
S102 and any configured ast-grep equivalent. Keep the suppression limited to
these trusted-source test helpers and do not weaken the rules globally.
- Around line 87-106: Scope the module stubs used by _load_spec_class to
pytest-managed setup rather than mutating sys.modules during collection. Use a
fixture with monkeypatch.setitem for the unstract, sdk1, constants, and
tool_registry entries, and ensure the dynamically loaded modules are likewise
restored after use so other tests can import the real packages without
contamination.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca646d51-aad2-4f64-8b10-c2b83b4fab02
📒 Files selected for processing (1)
backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py
| jsonschema = pytest.importorskip( | ||
| "jsonschema", reason="jsonschema is required to exercise the real validator" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify jsonschema is declared as a backend dependency
fd pyproject.toml backend
rg -n 'jsonschema' backend/pyproject.toml backend/poetry.lock backend/requirements*.txt 2>/dev/nullRepository: Zipstack/unstract
Length of output: 178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend pyproject files =="
fd '^pyproject\.toml$' backend -x echo "{}"
echo "== jsonschema occurrences in backend dependency/config files =="
rg -n --hidden --glob 'pyproject.toml|requirements*.txt|poetry.lock|setup.py|setup.cfg|uv.lock|pipfile.lock|Pipfile.lock' 'jsonschema' backend 2>/dev/null || true
echo "== relevant backend pyproject sections =="
sed -n '1,240p' backend/pyproject.toml | nl -ba
echo "== test file imports and jsonschema usage =="
sed -n '1,140p' backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py | nl -ba
echo "== top-level dependency declarations mentioning jsonschema =="
rg -n --hidden --glob 'pyproject.toml|requirements*.txt|poetry.lock|uv.lock|setup.py|setup.cfg|Pipfile.lock|pipfile.lock' 'jsonschema' . 2>/dev/null || trueRepository: Zipstack/unstract
Length of output: 355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend pyproject files =="
fd '^pyproject\.toml$' backend
echo "== jsonschema occurrences in backend dependency/config files =="
rg -n --hidden --glob 'pyproject.toml|requirements*.txt|poetry.lock|setup.py|setup.cfg|uv.lock|Pipfile.lock|pipfile.lock' 'jsonschema' backend 2>/dev/null || true
echo "== relevant backend/pyproject.toml sections =="
awk 'NR>=1 && NR<=260 { printf "%6d\t%s\n", NR, $0 }' backend/pyproject.toml
echo "== test file imports and jsonschema usage =="
awk 'NR>=1 && NR<=140 { printf "%6d\t%s\n", NR, $0 }' backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py
echo "== top-level and frontend occurrence search for jsonschema =="
rg -n --hidden --glob 'pyproject.toml|requirements*.txt|poetry.lock|uv.lock|setup.py|setup.cfg|Pipfile.lock|pipfile.lock' 'jsonschema' . 2>/dev/null || trueRepository: Zipstack/unstract
Length of output: 11454
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend pyproject files =="
fd '^pyproject\.toml$' backend
echo "== jsonschema occurrences in backend dependency/config files =="
rg -n --hidden --glob 'pyproject.toml|req[urement]*s*.txt|poetry.lock|setup.py|setup.cfg|uv.lock|Pipfile.lock|pipfile.lock|pyproject_tomls.json' 'jsonschema' backend 2>/dev/null || true
echo "== relevant backend/pyproject.toml lines 1-260 =="
awk '{ printf "%6d\t%s\n", NR, $0 }' backend/pyproject.toml | sed -n '1,260p'
echo "== test file imports and jsonschema usage lines 1-140 =="
awk '{ printf "%6d\t%s\n", NR, $0 }' backend/prompt_studio/prompt_studio_registry_v2/tests/test_frame_spec_challenge_llm.py | sed -n '1,140p'
echo "== jsonschema in all declared/lock dependency-like files in repo =="
rg -n --hidden --glob 'pyproject.toml|requirements*.txt|poetry.lock|uv.lock|setup.py|setup.cfg|Pipfile.lock|pipfile.lock' 'jsonschema' . 2>/dev/null || true
echo "== lock package lines for jsonschema =="
rg -n --hidden --glob 'poetry.lock|uv.lock|requirements*.txt' -E '^\[?Package|jsonschema|json-schema|schema' . 2>/dev/null || trueRepository: Zipstack/unstract
Length of output: 11515
Add jsonschema to the backend test/dev dependency set.
backend/pyproject.toml only declares pytest>=8.0.1 in the test group, so pytest.importorskip("jsonschema", ...) will skip these regression tests in environments without jsonschema installed instead of surface the missing test tooling. Add it to the backend dev/test dependencies to guarantee this suite runs under CI/local test installs.
🤖 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/tests/test_frame_spec_challenge_llm.py`
around lines 53 - 55, Add jsonschema to the backend test/dev dependency group in
pyproject.toml alongside pytest, using the project's existing dependency
declaration style, so test environments install it and
test_frame_spec_challenge_llm.py exercises the real validator instead of
skipping.
Unstract test resultsPer-group results
Critical paths
|



Purpose
Fixes two independent server-side failures in the Prompt Studio → export → deploy path.
1.
fetch_responseignores the project default LLM profilebuild_fetch_response_payload(and the synchronous_fetch_response) resolve the profile from the prompt's own FK and raiseDefaultProfileErrorwhen it is null. Both sibling paths already fall back to the project default:index_documentProfileManager.get_default_llm_profile(tool)single_passProfileManager.get_default_llm_profile(tool)fetch_responseprompt.profile_manageronlyfetch_responsewas the lone omission. Both call sites now fall back to the project default.DefaultProfileErrorpreviously read "Default LLM profile is not configured. Please set an LLM profile as default to continue." — naming a setting the code never read, so setting the project default did not resolve the error. The message now names both places a profile can come from (prompt-level and project-level).2. An exported tool fails deployment validation when LLMChallenge is off
deployment runends in pipeline statusERRORwith422 Unprocessable Entity: Tool validation failed, at the final step after everything else succeeds.The failure is an
enumviolation, not arequiredone — a present-but-empty key satisfiesrequired. The chain:challenge_llmcorrectly, falling back to the default profile's LLM.get_default_settingsseeds a"type": "string"property with no specdefaultas"".challenge_llmdeclaresadapterType: "LLM",_update_schema_for_adapter_typeinjectsenum: [<real adapter ids>].DefaultsGeneratingValidator.validate(tool_meta)rejects""— not in the enum.This fires regardless of
enable_challenge, andvalidate_adapter_accessdoes not catch it first: it filtersAdapterInstance.objects.filter(id__in=adapter_ids), so""matches no row and passes silently.Fix:
frame_specdeclareschallenge_llmas a plain string with a""default whenenable_challengeis off, and only marks it required/adapter-typed when the feature is on. Droppingrequiredalone does not help (the instance stores"", not a missing key), so theadapterTypedeclaration is what has to be conditional.Scoped to the Prompt Studio spec rather than changing
_update_schema_for_adapter_type, which runs for LLM/EMBEDDING/VECTOR_DB/X2TEXT/OCR on every tool — broadening it there would make""valid for every optional adapter property on every tool, risking a runtime failure onAdapterInstance.objects.get(id="").This fixes the challenge-off case. The challenge-on case (where the enum is present and the seeded
""still fails) is a separate seeding fix, handled in #2209.Verification
Composing the real
frame_specwith the real_update_schema_for_adapter_type:With challenge enabled the enum is still injected and
""is still rejected, so an enabled challenge cannot silently run without an LLM.ruffandruff-formatpass at the version pinned in.pre-commit-config.yaml(v0.3.4).Impact
challenge_llm: ""are fixed on re-export; no migration.Related
🤖 Generated with Claude Code