Skip to content

UN-2896 [MISC] Deprecate and remove the LLMWhisperer V1 adapter - #2260

Open
Deepak-Kesavan wants to merge 7 commits into
mainfrom
UN-2896-remove-llmwhisperer-v1
Open

UN-2896 [MISC] Deprecate and remove the LLMWhisperer V1 adapter#2260
Deepak-Kesavan wants to merge 7 commits into
mainfrom
UN-2896-remove-llmwhisperer-v1

Conversation

@Deepak-Kesavan

Copy link
Copy Markdown
Contributor

What

Retires the LLMWhisperer V1 text extractor and adds the machinery that makes retiring an adapter actually stick.

  • New DEPRECATED_ADAPTERS registry (backend/adapter_processor_v2/deprecated_adapters.py) — a single source that drives every guard, seeded with LLMWhisperer V1 only.
  • Deletes the V1 adapter package, its icon, its dead env vars, and the plumbing that forwarded them into tool containers.
  • Data migration marks existing V1 instances deprecated so they render as such rather than erroring on an unknown adapter id.

Why

UN-2896 was closed once and reopened — V1 was still present in 0.173.1. The reason it came back is that the previous attempt had no enforcement: is_available / deprecation_metadata and the frontend deprecated-badge UI already landed in #1677, but nothing stopped a deprecated adapter from being offered, created, selected, or executed. This PR wires up the half that was missing, then removes the adapter it retires.

The existing seeding also had a latent bug: migration 0003 used .filter(...).first(), so it marked at most one row per adapter id — every other org's instance stayed is_available=True.

How

A registry entry is the whole deprecation. Four guards read it, all adapter-type agnostic, so LLM / embedding / vector-DB deprecations later are a one-line entry with no new code:

Guard Location
Not offered for creation adapter_processor.get_all_supported_adapters + get_json_schema
Cannot be created or tested AdapterInstanceSerializer.validate (covers create and update/partial_update, since adapter_id is writable) + AdapterViewSet.test
Cannot be selected ProfileManagerSerializer.validate, set_default_triad, default-profile creation
Cannot be executed platform-service get_adapter_instance_from_db — the single choke point every SDK adapter lookup passes through

is_adapter_selectable() states the rule once: usable, available, and not deprecated.

Deliberately still visible: Settings › Adapters keeps listing deprecated instances (badged, edit/share disabled) so users can find and delete them. Only the four selection surfaces filter them. Editing a profile already on a deprecated adapter still works — validate skips unchanged values — so users aren't locked out of their other fields.

Also fixed while in here: the adapter_instance route's blanket except Exception re-wrapped every APIError as a 500, so both the new deprecation error and the pre-existing "adapter not found" surfaced as server errors with a logged traceback. Now re-raised untouched, matching the neighbouring route.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

Intended, breaking by design: an org still using LLMWhisperer V1 can no longer run it. That is the ticket. The failure is now a clear message ("LLMWhisperer has been deprecated. Please switch to the LLMWhisperer V2 text extractor") instead of an opaque SDK registry miss. Existing V1 adapter rows, profiles and workflows are not deleted — they stay visible so users can see what to migrate.

Checked and not broken:

  • Other adapters marked is_available=False. The new execution gate keys off that column, so it was worth confirming nothing legitimate is caught. All five ids seeded by migration 0003 (noOpLlm, noOpEmbedding, palm ×2, qdrantfastembed) are already absent from SDK1 — verified against the live registry. The noOp adapters still shipping (noOpX2text, noOpVectorDb) have different ids and are not in that list.
  • The is_available column predates this PR ([FEAT] - Handle Deprecate Adapters Not Supported In SDK1 #1677) and ships in the same release train as platform-service, so the new SELECT cannot hit a schema without it.
  • llmwhisperer-client stays in sdk1/pyproject.toml — V2 imports it as unstract.llmwhisperer.client_v2.
  • Env vars removed are V1-only. POLL_INTERVAL / MAX_POLLS / STATUS_RETRIES were read only by V1; V2 uses WAIT_TIMEOUT / MAX_RETRIES / RETRY_MIN_WAIT / RETRY_MAX_WAIT, all untouched. STATUS_RETRIES had no consumer anywhere.
  • Profile edit path. adapterOptions() already re-appends a currently-selected adapter missing from the list as a disabled option, so a profile on V1 renders its name, greyed out — not a bare UUID.
  • Default Triad. Deprecated entries are disabled, not filtered out, so the current-defaults fetch still fires and an existing default still renders.

Database Migrations

adapter_processor_v2/0007_deprecate_llmwhisperer_v1.py — data migration, reversible. Marks all LLMWhisperer V1 instances is_available=False with deprecation metadata, using .update() across every org. No schema change.

Env Config

Removed (V1-only, now unread):

  • backend/sample.envADAPTER_LLMW_POLL_INTERVAL, ADAPTER_LLMW_MAX_POLLS, ADAPTER_LLMW_STATUS_RETRIES
  • workers/sample.envADAPTER_LLMW_POLL_INTERVAL, ADAPTER_LLMW_MAX_POLLS
  • unstract/workflow-execution — dropped from ToolRuntimeVariable and from the vars forwarded into tool containers

Follow-up: unstract-cloud charts/unstract-platform/values.yaml still sets these three. They are inert once nothing reads them; a companion PR can drop them.

Relevant Docs

Related Issues or PRs

Dependencies Versions

No dependency changes. llmwhisperer-client>=2.8.1 retained for V2.

Notes on Testing

Automated — new backend/adapter_processor_v2/tests/test_deprecated_adapters.py (13 tests) asserts, for every registry entry, that it is absent from the SDK registry, excluded from the supported-adapter listing, refused a JSON schema, and rejected by the serializer; plus the is_adapter_selectable truth table. The SDK-registry assertion is the regression guard that fails if V1 is ever re-added — the specific failure this ticket hit.

  • adapter_processor_v2 + prompt_studio: 137 passed
  • workers/tests/test_legacy_executor_extract.py: 18 passed
  • Biome clean at CI's pinned 2.3.13; all pre-commit hooks pass.

Manually verified the SDK registry now resolves only V2:

llmwhisperer|a5e6b8af-3e1f-4a80-b006-d017e8e67f93   ← V2, present
llmwhisperer|0a1647f0-f65f-410d-843b-3d979c78350e   ← V1, gone

Not yet done: no dev-cluster deploy, so the UI surfaces (Default Triad disabled option, profile dropdowns, the deprecated badge on an actual V1 row) have not been exercised against a live org that has a V1 adapter configured. Worth a pass before merge if a test org can be pointed at one.

Screenshots

n/a — no new UI; existing deprecated-adapter styling from #1677 is reused.

Checklist

I have read and understood the Contribution Guidelines.

Adds a single DEPRECATED_ADAPTERS registry that drives every guard, seeded
with LLMWhisperer V1, and deletes the V1 adapter package it retires.

Guards (adapter-type agnostic, so future deprecations are a one-line entry):
- excluded from supported_adapters, so it cannot be picked for creation
- POST /adapter/ and /test_adapters/ reject a deprecated adapter_id
- profile manager rejects pointing a profile at one; existing profiles on a
  deprecated adapter stay editable in their other fields
- platform-service rejects execution off the is_available column, which is
  every SDK adapter lookup's single choke point

Backfill migration marks existing V1 instances unavailable across all orgs
(0003 used .first(), which marked only one row per adapter).

Removes the V1 package, its icon, its dead env vars (POLL_INTERVAL,
MAX_POLLS, STATUS_RETRIES -- V2 uses WAIT_TIMEOUT/MAX_RETRIES/RETRY_*) and
the workflow-execution plumbing that forwarded them into tool containers.

Claude-Session: https://claude.ai/code/session_01DXuiGyUwXyU1EVQBeMHppe
- platform-service: the adapter_instance route's blanket `except Exception`
  re-wrapped every APIError as a 500, so both the new deprecation error and
  the pre-existing "not found" reported as server errors and logged a
  traceback. Re-raise APIError untouched, as the neighbouring route does.
- Move the adapter_id check into AdapterInstanceSerializer.validate:
  adapter_id is writable, so update/partial_update could set a deprecated id
  that create rejected.
- Default profile creation and the project-import warning gated on is_usable
  alone, letting a deprecated default land in a new profile without passing
  through the serializer.
- set_default_triad accepted a deprecated adapter as a user default straight
  from the API.
- New is_adapter_selectable() states the rule once: usable, available, and
  not deprecated.
- DefaultTriad: disable deprecated options instead of dropping them. Filtering
  emptied adapterList for an org whose adapters are all deprecated, which
  gated the effect that loads the current defaults, and left the current
  default rendering as a bare UUID.

Claude-Session: https://claude.ai/code/session_01DXuiGyUwXyU1EVQBeMHppe
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fully retires the LLMWhisperer V1 adapter and adds centralized enforcement to prevent deprecated adapters from being offered, created, newly selected, or executed.

  • Adds a deprecation registry and consistent availability metadata.
  • Migrates every existing V1 adapter instance to an unavailable, visibly deprecated state.
  • Preserves existing profile and default-triad references for migration while blocking new selections.
  • Removes the V1 SDK package, icon, environment variables, and worker plumbing.
  • Returns explicit client errors when deprecated adapter execution is attempted.
  • Adds regression coverage for deprecation enforcement and editing defaults that contain an unchanged deprecated adapter.

Confidence Score: 5/5

The PR appears safe to merge; the follow-up changes correctly preserve editing of default triads containing unchanged deprecated references without weakening validation of new selections.

No actionable new defects or outstanding rule violations were identified in the changes since the previous review.

Important Files Changed

Filename Overview
backend/adapter_processor_v2/deprecated_adapters.py Introduces the centralized deprecation registry, metadata helpers, and adapter-selectability predicate.
backend/adapter_processor_v2/adapter_processor.py Excludes deprecated adapters from discovery and validates changed default-triad selections while tolerating unchanged legacy references.
backend/adapter_processor_v2/migrations/0007_deprecate_llmwhisperer_v1.py Marks all existing LLMWhisperer V1 instances unavailable with reversible deprecation metadata.
backend/adapter_processor_v2/serializers.py Rejects deprecated adapter IDs on writes and consistently exposes effective deprecation state.
backend/prompt_studio/prompt_profile_manager_v2/serializers.py Prevents profiles from being newly pointed to unavailable or registry-deprecated adapters.
platform-service/src/unstract/platform_service/helper/adapter_instance.py Blocks execution of unavailable adapter instances at the shared database lookup boundary.
frontend/src/components/settings/default-triad/DefaultTriad.jsx Keeps deprecated defaults visible while disabling them as selectable options.
unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer/src/llm_whisperer.py Removes the retired LLMWhisperer V1 SDK adapter implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    R[Deprecated adapter registry] --> L[Supported-adapter listing]
    R --> C[Creation and test validation]
    R --> P[Profile and default selection]
    R --> M[Data migration]
    M --> DB[(AdapterInstance is_available=false)]
    DB --> E[Platform-service execution gate]
    L -->|Excluded| U[User interfaces]
    C -->|Rejected| X[Deprecated adapter request]
    P -->|New selection rejected| X
    E -->|Execution rejected with client error| X
    DB -->|Existing rows retained| V[Visible with deprecated metadata]
Loading

Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/UN-..." | Re-trigger Greptile

Matches the repo's existing convention for this rule in data migrations.
@praveen-formido

Copy link
Copy Markdown
Contributor

Code review

Found 1 issue:

  1. set_default_triad re-validates unchanged triad members, so a user whose stored default is already deprecated cannot change any other default. _resolve_selectable_adapter is called for every non-empty key in the payload with no skip for values that match what is already stored, and DefaultTriad.jsx always POSTs all four defaults (it filters only null/blank), sourced from the user's existing defaults. So once migration 0007 marks a user's default X2TEXT (LLMWhisperer V1) is_available=False, changing the Default LLM resubmits the deprecated X2TEXT id and the whole save fails with DeprecatedAdapter. The deprecated option is disabled in the dropdown, so the stale value is still what gets submitted. This also hits users whose defaults point at adapters marked unavailable by the earlier 0003_mark_deprecated_adapters migration (palm, noOpLlm, noOpEmbedding, qdrantfastembed). Before this PR set_default_triad did a bare AdapterInstance.objects.get(pk=...) with no availability check, so this is a new regression.

@staticmethod
def _resolve_selectable_adapter(adapter_pk: str) -> AdapterInstance:
"""Adapter for ``adapter_pk``, refusing one that can no longer be chosen."""
adapter = AdapterInstance.objects.get(pk=adapter_pk)
if not is_adapter_selectable(adapter):
raise DeprecatedAdapter(get_deprecation_message(adapter.adapter_id))
return adapter
@staticmethod
def set_default_triad(default_triad: dict[str, str], user: User) -> None:
try:
organization_member = OrganizationMemberService.get_user_by_id(user.id)
(
user_default_adapter,
created,
) = UserDefaultAdapter.objects.get_or_create(
organization_member=organization_member
)

The same PR already solves this exact hazard in the sibling serializer, which skips unchanged values so "a profile already on a deprecated adapter stays editable in its other fields":

for field, _ in ADAPTER_LABELS:
adapter = attrs.get(field)
if not adapter or adapter == getattr(self.instance, field, None):
continue
if not accessible.filter(id=adapter.id).exists():
raise ValidationError({field: "No access to the selected adapter."})
if not adapter.is_available or is_adapter_deprecated(adapter.adapter_id):
raise ValidationError(
{field: get_deprecation_message(adapter.adapter_id)}

It also contradicts is_adapter_selectable's own docstring, which states existing selections are not re-validated:

Covers the three ways an adapter stops being a valid choice: usage
exhausted (``is_usable``), withdrawn from the SDK (``is_available``), and
deprecated here. Existing selections are not re-validated against this —
they stay readable so users can see what to migrate off.
"""

Frontend submitting all four values regardless of which one changed:

// Handler for form submission
const handleSubmit = async () => {
let body = {
llm_default: selectedValues[getKeyByValue(labelMap.LLM)],
embedding_default: selectedValues[getKeyByValue(labelMap.EMBEDDING)],
vector_db_default: selectedValues[getKeyByValue(labelMap.VECTOR_DB)],
x2text_default: selectedValues[getKeyByValue(labelMap.X2TEXT)],
};
// Filter out null or blank values
body = Object.fromEntries(
Object.entries(body).filter(
([key, value]) => value !== null && value !== "",
),
);

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

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.

@Deepak-Kesavan have we also discussed on whether we will rename the LLMW v2 adapter since this v1 will no longer be visible?

Comment thread backend/sample.env

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.

@Deepak-Kesavan remember to raise a cloud PR for these

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Raised: Zipstack/unstract-cloud#1760.

It drops ADAPTER_LLMW_POLL_INTERVAL, ADAPTER_LLMW_MAX_POLLS and ADAPTER_LLMW_STATUS_RETRIES from charts/unstract-platform/values.yaml and from docker (deprecated)/sample.backend.env. ADAPTER_LLMW_WAIT_TIMEOUT stays — V2 still resolves it, along with MAX_RETRIES, RETRY_MIN_WAIT and RETRY_MAX_WAIT.

Not a merge blocker for this PR either way: until that one lands the three values just sit in the pod env with no reader, and if it happens to merge first, V1's in-code defaults are identical to the values being removed.

@harini-venkataraman

Copy link
Copy Markdown
Contributor

Critical Issues (2 found)

  1. set_default_triad re-validates unchanged defaults — confirmed regression

File: adapter_processor.py, set_default_triad method

pk-zipstack's review comment is correct and this is not yet fixed in the PR. The frontend submits all four defaults on every save. When migration 0007 marks a user's X2TEXT default as deprecated, attempting to change any
other default (e.g., LLM) fails because _resolve_selectable_adapter rejects the stale, unchanged X2TEXT reference.

The ProfileManagerSerializer.validate() correctly handles this with:
if not adapter or adapter == getattr(self.instance, field, None):
continue # unchanged → skip

But set_default_triad blindly validates every submitted value. Fix: compare each submitted PK against the currently stored default and skip unchanged ones:

if default_triad.get(AdapterKeys.LLM_DEFAULT, None):
new_pk = default_triad[AdapterKeys.LLM_DEFAULT]
current = user_default_adapter.default_llm_adapter_id
if str(new_pk) != str(current):
user_default_adapter.default_llm_adapter = (
AdapterProcessor._resolve_selectable_adapter(new_pk)
)

This also violates the is_adapter_selectable() docstring which explicitly states "Existing selections are not re-validated."

  1. IS_DEPRECATED conflates "unavailable" with "deprecated"

File: serializers.py, _add_deprecation_info()

rep[AdapterKeys.IS_DEPRECATED] = not is_available

An adapter with is_available=False for non-deprecation reasons (e.g., manually disabled, usage-exhausted) will be flagged as IS_DEPRECATED=True in API responses. This is semantically wrong and could confuse frontend logic
or users who see a "deprecated" badge on an adapter that was simply disabled.

Fix: use the actual deprecation check:
rep[AdapterKeys.IS_DEPRECATED] = is_adapter_deprecated(instance.adapter_id)


Important Issues (3 found)

  1. Redundant deprecation message

File: deprecated_adapters.py, get_deprecation_message()

The message format is:
"{adapter_name} has been deprecated. {reason}"
With the current LLMWhisperer entry, this produces:
"LLMWhisperer has been deprecated. LLMWhisperer V1 is retired. Please switch to the LLMWhisperer V2 text extractor."

The name appears twice and "deprecated" + "retired" are redundant. Consider simplifying to just return the reason field directly, or restructuring the message template.

  1. Inline import logging inside to_representation

File: serializers.py, AdapterInstanceSerializer.to_representation()

except Exception as e:
import logging
logger = logging.getLogger(name)

This should use a module-level logger. Every other module in the PR declares logger = logging.getLogger(name) at the top. The serializers module lacks this. Add it at module level and remove the inline import.

  1. Platform-service error message is generic

File: platform-service/.../adapter_instance.py

The platform-service rejection message is:
"Adapter '{name}' has been deprecated and can no longer be used. Please reconfigure with a supported adapter."

Unlike the Django side, it doesn't mention the specific replacement (LLMWhisperer V2). Since platform-service can't access the Django registry, consider storing the replacement name in deprecation_metadata (which migration
0007 already writes to the DB) and reading it from the row data.


Suggestions (4 found)

  1. Icon inconsistency

AdapterProcessor.get_icon() returns AdapterKeys.UNAVAILABLE_ICON ("⚠️ ") for deprecated adapters, but AdapterInstanceSerializer.to_representation() hardcodes "🚫". Pick one.

  1. Missing test for the set_default_triad regression

The test suite thoroughly covers the registry, schema rejection, and serializer validation, but doesn't test the set_default_triad path. Add a test that sets up a UserDefaultAdapter with a deprecated adapter as one
default, then attempts to change a different default, confirming it succeeds without re-validating the deprecated one (once the fix is in).

  1. _FakeAdapter in tests doesn't test None adapter fields

is_adapter_selectable checks adapter.is_usable and adapter.is_available, but _FakeAdapter.init always sets these. Consider adding a case where the adapter object exists but one of these attributes is None to verify the
bool() wrapping handles it.

  1. Helm chart env var cleanup

As noted in the PR review, the three removed env vars (ADAPTER_LLMW_POLL_INTERVAL, ADAPTER_LLMW_MAX_POLLS, ADAPTER_LLMW_STATUS_RETRIES) still exist in the unstract-cloud Helm charts. A companion PR should be filed.

harini-venkataraman and others added 4 commits September 1, 2026 17:06
The Default Triad UI submits all four defaults on every save, so validating
every submitted value locked a user out of changing any default once one of
theirs had been deprecated underneath them. Skip unchanged values, matching
ProfileManagerSerializer.validate, and cover it with a regression test.

Also drops the duplicated adapter name from the deprecation message and
promotes the inline logger in serializers to module level.
The README still listed ADAPTER_LLMW_POLL_INTERVAL and ADAPTER_LLMW_MAX_POLLS,
which only V1 ever read, and the WhispererEnv docstring quoted a 300s default
where the code uses 900. Document the four vars V2 actually resolves.
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Frontend Lint Report (Biome)

All checks passed! No linting or formatting issues found.

@sonarqubecloud

sonarqubecloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
e2e-api-deployment e2e 3 0 0 0 20.5
e2e-coowners e2e 1 0 0 0 1.5
e2e-etl e2e 1 0 0 0 12.3
e2e-login e2e 2 0 0 0 1.4
e2e-prompt-studio e2e 1 0 0 0 13.3
e2e-smoke e2e 2 0 0 0 1.1
e2e-workflow e2e 1 0 0 0 16.2
frontend unit 0 1 0 0 0.0
integration-backend integration 312 0 0 26 36.0
integration-connectors integration 1 0 0 7 5.9
integration-workers integration 157 0 0 1 41.5
ui e2e 0 1 0 0 0.0
unit-backend unit 1138 0 0 1 44.6
unit-connectors unit 63 0 0 0 10.3
unit-core unit 33 0 0 0 1.4
unit-platform-service unit 15 0 0 0 2.8
unit-rig unit 120 0 0 0 4.8
unit-runner unit 5 0 0 0 3.0
unit-sdk1 unit 563 0 0 0 30.0
unit-workers unit 1385 0 0 1 128.4
TOTAL 3803 2 0 36 375.0

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

@Deepak-Kesavan

Copy link
Copy Markdown
Contributor Author

@chandrasekharan-zipstack on the rename — not in this PR. It isn't in the ticket's scope and there's no real value in it, so let's keep it separate and decide on it later if we want to.

For whenever we do pick it up, I checked what it would actually cost, and it's small:

  • Only get_name() and get_description() in llm_whisperer_v2.py would change. Display strings.
  • get_id() (llmwhisperer|a5e6b8af-…) stays, so no migration and no existing instance breaks.
  • Every reference to the V2 name outside the adapter's own directory is a Python class reference (isinstance, imports) in legacy_executor.py, index.py and the tests. Nothing keys off the display string.
  • AdapterInstance.adapter_name is the name the user typed for their instance, not the SDK type name, so no stored row is affected.

The one argument against doing it at all: LLMWhisperer is externally versioned as a product — the v2 API, the client_v2 import path we still depend on — so dropping "V2" from the UI while the docs and API keep it could confuse more than it tidies.

@Deepak-Kesavan

Copy link
Copy Markdown
Contributor Author

@praveen-formido @harini-venkataraman thanks — the set_default_triad finding was correct and it was a regression introduced in this PR. Fixed, along with a few of the others.

Fixed

1. set_default_triad re-validated unchanged defaults — confirmed and fixed.

DefaultTriad.jsx submits all four defaults on every save (it filters only null/blank), and _resolve_selectable_adapter ran on every non-empty key. So once migration 0007 marked a user's X2TEXT default deprecated, changing their LLM default resubmitted the stale, disabled X2TEXT id and 400'd the whole save. It also hit anyone whose defaults pointed at adapters marked unavailable by the older 0003 migration (palm, noOpLlm, noOpEmbedding, qdrantfastembed).

Fixed by skipping values that match what's already stored, mirroring ProfileManagerSerializer.validate(), and collapsing the four duplicated blocks into one loop over a _DEFAULT_TRIAD_FIELDS map.

Covered by a new test_default_triad_deprecation.py with two cases — changing one default tolerates a stale deprecated one, and newly selecting a deprecated adapter is still rejected, so the fix can't quietly weaken the guard. Both pass in CI.

3. Redundant deprecation message — fixed. It read "LLMWhisperer has been deprecated. LLMWhisperer V1 is retired…". get_deprecation_message now returns reason verbatim, and the registry docstring says each entry must phrase reason as a complete sentence.

4. Inline import logging — fixed, promoted to a module-level logger. (Pre-existing from #1677 rather than new here, but it was cheap to tidy while in the file.)

7. Missing set_default_triad test — added, see above.

Also found while re-checking this: the V2 adapter's README documented ADAPTER_LLMW_POLL_INTERVAL and ADAPTER_LLMW_MAX_POLLS, which only V1 ever read, and the WhispererEnv docstring quoted a 300s WAIT_TIMEOUT default where the code uses 900. Both corrected.

Not changing here

2. IS_DEPRECATED conflating "unavailable" with "deprecated". The semantic point is fair, but the suggested fix would make things worse in practice: the frontend's usableAdapters() filters on is_deprecated, so narrowing it to the registry check alone would let every adapter marked unavailable by migration 0003 back into the selection dropdowns. The conflation is also pre-existing — main already has rep[IS_DEPRECATED] = not instance.is_available. Untangling it properly means moving the frontend filters onto is_available and leaving is_deprecated as the narrow signal, which is worth doing but not as a drive-by in this PR.

5. Platform-service message not naming the replacement. Reasonable, but it'd mean widening the SELECT to pull deprecation_metadata and parsing it in a service that deliberately knows nothing about the registry. The generic message plus the Django-side detail seems like the right trade for now.

6. ⚠️ vs 🚫 icon inconsistency. Real, but both sides are pre-existing from #1677 and neither is touched by this PR.

8. _FakeAdapter with None fields. is_adapter_selectable wraps in bool() precisely so falsy values work; a None case wouldn't exercise anything the existing False cases don't.

9. Helm chart cleanup. Raised as Zipstack/unstract-cloud#1760.

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

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.

I'd suggest to follow up the rename as a stacked PR on top of this if its low effort as you mentioned. The adapter names in certain parts would just say V2 and sound confusing - this rename might be an activity we'll do in the future anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants