From 9d1a01eb0cc6c096a3e32b1858ccdd09823a88d5 Mon Sep 17 00:00:00 2001 From: "DESKTOP-RTLN3BA\\$punk" Date: Fri, 29 May 2026 20:27:40 -0700 Subject: [PATCH] refactor(automations): streamline model eligibility handling in automation creation - Removed the eligibility gate for model selection in the automation creation process, allowing users to choose models directly in the builder. - Updated the `AutomationBuilderForm` to incorporate model selection logic, ensuring that selected models are validated and preserved during automation creation and editing. - Simplified the `AutomationsContent` and `AutomationNewContent` components by eliminating unnecessary eligibility checks and alerts. - Enhanced the user experience by integrating model selection directly into the automation approval process, ensuring that only billable models are used. - Refactored related tests to cover new model selection behavior and ensure proper validation of user-selected models. --- .../main_agent/tools/automation/create.py | 28 +- .../app/automations/services/automation.py | 63 ++++- .../test_automation_service_policy.py | 259 +++++++++++++++++- .../automations/automations-content.tsx | 25 +- .../automation-model-gate-alert.tsx | 61 ----- .../components/automations-empty-state.tsx | 46 +--- .../components/automations-header.tsx | 91 +----- .../builder/automation-builder-form.tsx | 76 ++++- .../builder/automation-model-fields.tsx | 198 +++++++++++++ .../new/automation-new-content.tsx | 23 +- .../tool-ui/automation/create-automation.tsx | 83 +++++- .../hooks/use-automation-eligible-models.ts | 150 ++++++++++ .../lib/automations/builder-schema.ts | 47 ++++ 13 files changed, 887 insertions(+), 263 deletions(-) delete mode 100644 surfsense_web/app/dashboard/[search_space_id]/automations/components/automation-model-gate-alert.tsx create mode 100644 surfsense_web/app/dashboard/[search_space_id]/automations/components/builder/automation-model-fields.tsx create mode 100644 surfsense_web/hooks/use-automation-eligible-models.ts diff --git a/surfsense_backend/app/agents/multi_agent_chat/main_agent/tools/automation/create.py b/surfsense_backend/app/agents/multi_agent_chat/main_agent/tools/automation/create.py index 8e841c1e9..62d39fcf2 100644 --- a/surfsense_backend/app/agents/multi_agent_chat/main_agent/tools/automation/create.py +++ b/surfsense_backend/app/agents/multi_agent_chat/main_agent/tools/automation/create.py @@ -32,8 +32,7 @@ from app.agents.multi_agent_chat.subagents.shared.hitl.approvals.self_gated impo ) from app.automations.schemas.api import AutomationCreate from app.automations.services.automation import AutomationService -from app.automations.services.model_policy import get_automation_model_eligibility -from app.db import SearchSpace, User, async_session_maker +from app.db import User, async_session_maker from app.utils.content_utils import extract_text_content from .prompt import build_draft_prompt @@ -99,26 +98,11 @@ def create_create_automation_tool( declined. Acknowledge once and stop — do NOT retry or pitch variants without a fresh user request. """ - # --- 0. Eligibility gate (fail fast, before drafting + HITL) --- - # Automations may only use premium or BYOK models. Check up front so we - # don't make the user draft + approve a card that can't be saved. - async with async_session_maker() as session: - search_space = await session.get(SearchSpace, search_space_id) - if search_space is None: - return { - "status": "error", - "message": "search space not found in this session", - } - eligibility = get_automation_model_eligibility(search_space) - if not eligibility["allowed"]: - reasons = " ".join(v["reason"] for v in eligibility["violations"]) - return { - "status": "error", - "message": ( - f"{reasons} Update the search space's model settings to a " - "premium or your own (BYOK) model, then try again." - ), - } + # Models are chosen per-automation on the approval card (premium/BYOK + # selectors) and validated when persisted by ``AutomationService.create`` + # — so there's no fail-fast search-space eligibility gate here. The + # search space's current chat/role model selection no longer constrains + # whether an automation can be drafted or saved. # --- 1. Draft via sub-LLM --- prompt = build_draft_prompt(search_space_id=search_space_id, intent=intent) diff --git a/surfsense_backend/app/automations/services/automation.py b/surfsense_backend/app/automations/services/automation.py index 6c602d886..23dcd116f 100644 --- a/surfsense_backend/app/automations/services/automation.py +++ b/surfsense_backend/app/automations/services/automation.py @@ -22,6 +22,7 @@ from app.automations.schemas.definition.envelope import AutomationModels from app.automations.services.model_policy import ( AutomationModelPolicyError, assert_automation_models_billable, + assert_models_billable, get_automation_model_eligibility, ) from app.automations.triggers import get_trigger @@ -43,16 +44,23 @@ class AutomationService: await self._authorize( payload.search_space_id, Permission.AUTOMATIONS_CREATE.value ) - search_space = await self._assert_models_billable(payload.search_space_id) - # Snapshot the search space's current (already-validated) model prefs onto - # the definition so runs are insulated from later chat/search-space model - # changes. Captured ids are guaranteed billable by the check above. - payload.definition.models = AutomationModels( - agent_llm_id=search_space.agent_llm_id or 0, - image_generation_config_id=search_space.image_generation_config_id or 0, - vision_llm_config_id=search_space.vision_llm_config_id or 0, - ) + # Capture the model profile onto the definition so runs are insulated + # from later chat/search-space model changes. Two sources: + # 1. Explicit per-automation selection in ``payload.definition.models`` + # (manual builder + chat approval card). Validate the chosen ids. + # 2. Fallback (no selection): snapshot the search space's current prefs. + # Either way the captured ids are guaranteed billable (premium/BYOK). + selected_models = payload.definition.models + if selected_models is not None: + self._assert_selected_models_billable(selected_models) + else: + search_space = await self._assert_models_billable(payload.search_space_id) + payload.definition.models = AutomationModels( + agent_llm_id=search_space.agent_llm_id or 0, + image_generation_config_id=search_space.image_generation_config_id or 0, + vision_llm_config_id=search_space.vision_llm_config_id or 0, + ) automation = Automation( search_space_id=payload.search_space_id, @@ -122,13 +130,22 @@ class AutomationService: automation.status = data["status"] if "definition" in data: new_def = patch.definition.model_dump(mode="json", by_alias=True) - # Preserve the captured model snapshot across edits so a definition - # change never silently re-binds the automation to the current chat - # model selection. Backend-managed; survives whether or not the - # client round-trips ``models``. + # Model snapshot handling on edit: + # * absent in the patch -> preserve the captured snapshot + # (a non-model definition change never silently re-binds the + # automation to the current chat/search-space selection). + # * unchanged from the snapshot -> keep as-is, no re-validation + # (so editing an automation whose captured model later drifted + # out of premium isn't blocked by an unrelated name/schedule edit). + # * genuinely changed -> validate the new selection (422 on a + # non-billable pick), then accept it. existing_models = (automation.definition or {}).get("models") - if existing_models is not None: - new_def["models"] = existing_models + provided_models = new_def.get("models") + if provided_models is None: + if existing_models is not None: + new_def["models"] = existing_models + elif provided_models != existing_models: + self._assert_selected_models_billable(patch.definition.models) automation.definition = new_def automation.version += 1 @@ -199,6 +216,22 @@ class AutomationService: raise HTTPException(status_code=422, detail=str(exc)) from exc return search_space + def _assert_selected_models_billable(self, models: AutomationModels) -> None: + """Reject creation when an explicitly selected model isn't billable. + + Used when the client supplies ``definition.models`` (per-automation + selection from the builder or chat approval card). Same policy as the + search-space path: premium global or BYOK only, no free/Auto. + """ + try: + assert_models_billable( + agent_llm_id=models.agent_llm_id, + image_generation_config_id=models.image_generation_config_id, + vision_llm_config_id=models.vision_llm_config_id, + ) + except AutomationModelPolicyError as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + async def _authorize(self, search_space_id: int, permission: str) -> None: await check_permission( self.session, diff --git a/surfsense_backend/tests/unit/automations/services/test_automation_service_policy.py b/surfsense_backend/tests/unit/automations/services/test_automation_service_policy.py index d81302380..0bbff39dc 100644 --- a/surfsense_backend/tests/unit/automations/services/test_automation_service_policy.py +++ b/surfsense_backend/tests/unit/automations/services/test_automation_service_policy.py @@ -16,7 +16,10 @@ from fastapi import HTTPException import app.automations.services.automation as automation_mod from app.automations.schemas.api import AutomationCreate, AutomationUpdate -from app.automations.schemas.definition.envelope import AutomationDefinition +from app.automations.schemas.definition.envelope import ( + AutomationDefinition, + AutomationModels, +) from app.automations.schemas.definition.plan_step import PlanStep from app.automations.services.automation import AutomationService from app.automations.services.model_policy import AutomationModelPolicyError @@ -175,6 +178,100 @@ async def test_create_treats_unset_prefs_as_auto_zero( } +async def test_create_honors_selected_models_when_provided( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the payload carries ``definition.models`` they are validated + kept. + + The search-space snapshot path is bypassed entirely (no + ``assert_automation_models_billable`` call). + """ + + def _fail_snapshot(_ss): + raise AssertionError("snapshot path should not run when models are provided") + + monkeypatch.setattr( + automation_mod, "assert_automation_models_billable", _fail_snapshot + ) + validated: dict[str, Any] = {} + + def _assert_ok(*, agent_llm_id, image_generation_config_id, vision_llm_config_id): + validated["ids"] = ( + agent_llm_id, + image_generation_config_id, + vision_llm_config_id, + ) + + monkeypatch.setattr(automation_mod, "assert_models_billable", _assert_ok) + + async def _noop_authorize(self, *_a, **_k): + return None + + async def _return_added(self, _aid): + return self.session.added[-1] + + monkeypatch.setattr(AutomationService, "_authorize", _noop_authorize) + monkeypatch.setattr(AutomationService, "_get_with_triggers_or_raise", _return_added) + + service = _service(SimpleNamespace(agent_llm_id=-99)) + payload = AutomationCreate( + search_space_id=1, + name="A", + definition=_definition( + models=AutomationModels( + agent_llm_id=-1, + image_generation_config_id=7, + vision_llm_config_id=-2, + ) + ), + ) + + automation = await service.create(payload) + + assert validated["ids"] == (-1, 7, -2) + assert automation.definition["models"] == { + "agent_llm_id": -1, + "image_generation_config_id": 7, + "vision_llm_config_id": -2, + } + + +async def test_create_rejects_unbillable_selected_models( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A non-billable explicit selection maps the policy error to HTTP 422.""" + + def _raise(*, agent_llm_id, image_generation_config_id, vision_llm_config_id): + raise AutomationModelPolicyError( + [{"kind": "llm", "config_id": -3, "reason": "free model"}] + ) + + monkeypatch.setattr(automation_mod, "assert_models_billable", _raise) + + async def _noop_authorize(self, *_a, **_k): + return None + + monkeypatch.setattr(AutomationService, "_authorize", _noop_authorize) + + service = _service(SimpleNamespace(agent_llm_id=-3)) + payload = AutomationCreate( + search_space_id=1, + name="A", + definition=_definition( + models=AutomationModels( + agent_llm_id=-3, + image_generation_config_id=7, + vision_llm_config_id=-2, + ) + ), + ) + + with pytest.raises(HTTPException) as exc_info: + await service.create(payload) + + assert exc_info.value.status_code == 422 + + async def test_update_preserves_captured_models( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -211,6 +308,166 @@ async def test_update_preserves_captured_models( assert result.version == 4 +async def test_update_honors_changed_models_when_valid( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A definition edit with a *changed* models block validates + keeps it.""" + existing = SimpleNamespace( + search_space_id=1, + definition={ + "name": "A", + "plan": [], + "models": { + "agent_llm_id": -1, + "image_generation_config_id": 5, + "vision_llm_config_id": -1, + }, + }, + version=3, + ) + validated: dict[str, Any] = {} + + def _assert_ok(*, agent_llm_id, image_generation_config_id, vision_llm_config_id): + validated["ids"] = ( + agent_llm_id, + image_generation_config_id, + vision_llm_config_id, + ) + + monkeypatch.setattr(automation_mod, "assert_models_billable", _assert_ok) + + async def _noop_authorize(self, *_a, **_k): + return None + + async def _return_existing(self, _aid): + return existing + + monkeypatch.setattr(AutomationService, "_authorize", _noop_authorize) + monkeypatch.setattr( + AutomationService, "_get_with_triggers_or_raise", _return_existing + ) + + service = _service(SimpleNamespace()) + patch = AutomationUpdate( + definition=_definition( + models=AutomationModels( + agent_llm_id=-2, + image_generation_config_id=9, + vision_llm_config_id=-2, + ) + ) + ) + + result = await service.update(7, patch) + + assert validated["ids"] == (-2, 9, -2) + assert result.definition["models"] == { + "agent_llm_id": -2, + "image_generation_config_id": 9, + "vision_llm_config_id": -2, + } + assert result.version == 4 + + +async def test_update_rejects_changed_unbillable_models( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A *changed* non-billable models block is rejected with HTTP 422.""" + existing = SimpleNamespace( + search_space_id=1, + definition={ + "name": "A", + "plan": [], + "models": { + "agent_llm_id": -1, + "image_generation_config_id": 5, + "vision_llm_config_id": -1, + }, + }, + version=3, + ) + + def _raise(*, agent_llm_id, image_generation_config_id, vision_llm_config_id): + raise AutomationModelPolicyError( + [{"kind": "llm", "config_id": -7, "reason": "free model"}] + ) + + monkeypatch.setattr(automation_mod, "assert_models_billable", _raise) + + async def _noop_authorize(self, *_a, **_k): + return None + + async def _return_existing(self, _aid): + return existing + + monkeypatch.setattr(AutomationService, "_authorize", _noop_authorize) + monkeypatch.setattr( + AutomationService, "_get_with_triggers_or_raise", _return_existing + ) + + service = _service(SimpleNamespace()) + patch = AutomationUpdate( + definition=_definition( + models=AutomationModels( + agent_llm_id=-7, + image_generation_config_id=5, + vision_llm_config_id=-1, + ) + ) + ) + + with pytest.raises(HTTPException) as exc_info: + await service.update(7, patch) + + assert exc_info.value.status_code == 422 + + +async def test_update_keeps_unchanged_models_without_revalidation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unchanged models block is kept as-is and is NOT re-validated. + + Lets users edit an automation whose captured model later drifted out of + premium without an unrelated edit tripping the policy check. + """ + captured = { + "agent_llm_id": -1, + "image_generation_config_id": 5, + "vision_llm_config_id": -1, + } + existing = SimpleNamespace( + search_space_id=1, + definition={"name": "A", "plan": [], "models": captured}, + version=3, + ) + + def _fail(*_a, **_k): + raise AssertionError("unchanged models must not be re-validated") + + monkeypatch.setattr(automation_mod, "assert_models_billable", _fail) + + async def _noop_authorize(self, *_a, **_k): + return None + + async def _return_existing(self, _aid): + return existing + + monkeypatch.setattr(AutomationService, "_authorize", _noop_authorize) + monkeypatch.setattr( + AutomationService, "_get_with_triggers_or_raise", _return_existing + ) + + service = _service(SimpleNamespace()) + patch = AutomationUpdate( + definition=_definition(models=AutomationModels(**captured)) + ) + + result = await service.update(7, patch) + + assert result.definition["models"] == captured + assert result.version == 4 + + async def test_model_eligibility_authorizes_and_returns_payload( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/surfsense_web/app/dashboard/[search_space_id]/automations/automations-content.tsx b/surfsense_web/app/dashboard/[search_space_id]/automations/automations-content.tsx index 6bbe55ec9..756221d38 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/automations/automations-content.tsx +++ b/surfsense_web/app/dashboard/[search_space_id]/automations/automations-content.tsx @@ -1,8 +1,6 @@ "use client"; import { ShieldAlert } from "lucide-react"; -import { useAutomationModelEligibility } from "@/hooks/use-automation-model-eligibility"; import { useAutomations } from "@/hooks/use-automations"; -import { AutomationModelGateAlert } from "./components/automation-model-gate-alert"; import { AutomationsEmptyState } from "./components/automations-empty-state"; import { AutomationsHeader } from "./components/automations-header"; import { AutomationsTable } from "./components/automations-table"; @@ -24,18 +22,6 @@ interface AutomationsContentProps { export function AutomationsContent({ searchSpaceId }: AutomationsContentProps) { const { automations, total, loading, error } = useAutomations(); const perms = useAutomationPermissions(); - // Gate creation on billable models (premium/BYOK). Only meaningful for - // users who can create; the eligibility query loads in parallel. - const { data: eligibility, isLoading: eligibilityLoading } = useAutomationModelEligibility( - perms.canCreate ? searchSpaceId : undefined - ); - const modelViolations = eligibility?.violations ?? []; - // Disable create CTAs while loading (avoid a flash of enabled buttons) and - // when the resolved models aren't billable. - const createDisabled = perms.canCreate && (eligibilityLoading || modelViolations.length > 0); - const disabledReason = eligibilityLoading - ? "Checking model eligibility…" - : modelViolations[0]?.reason; if (perms.loading) { // Permissions gate the entire page; defer everything until we know. @@ -91,11 +77,7 @@ export function AutomationsContent({ searchSpaceId }: AutomationsContentProps) { canCreate={perms.canCreate} showCreateCta={false} /> - + ); } @@ -107,12 +89,7 @@ export function AutomationsContent({ searchSpaceId }: AutomationsContentProps) { total={total} loading={loading} canCreate={perms.canCreate} - createDisabled={createDisabled} - disabledReason={disabledReason} /> - {modelViolations.length > 0 && ( - - )} = { - llm: "Agent model", - image: "Image model", - vision: "Vision model", -}; - -/** - * Warns that the search space's models aren't billable for automations. - * - * Automations may only use premium global models or your own (BYOK) models — - * free models and Auto mode are blocked so every run is metered in premium - * credits. Surfaced wherever a user can start creating an automation. - */ -export function AutomationModelGateAlert({ - searchSpaceId, - violations, - className, -}: AutomationModelGateAlertProps) { - if (violations.length === 0) return null; - - return ( - - - Automations need a premium or your own model - -

- Automations run unattended, so every run must use a premium model or your own (BYOK) - model. Update these in your model settings, then create your automation. -

-
    - {violations.map((violation) => ( -
  • - - {KIND_LABEL[violation.kind]} - - — {violation.reason} -
  • - ))} -
-
-
- ); -} diff --git a/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-empty-state.tsx b/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-empty-state.tsx index ee7dadce6..cc54c5e94 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-empty-state.tsx +++ b/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-empty-state.tsx @@ -2,14 +2,10 @@ import { MessageSquarePlus, SquarePen, Workflow } from "lucide-react"; import Link from "next/link"; import { Button } from "@/components/ui/button"; -import type { ModelEligibilityViolation } from "@/contracts/types/automation.types"; -import { AutomationModelGateAlert } from "./automation-model-gate-alert"; interface AutomationsEmptyStateProps { searchSpaceId: number; canCreate: boolean; - /** Model slots that block creation (free/Auto). Empty when eligible. */ - modelViolations?: ModelEligibilityViolation[]; } /** @@ -18,13 +14,7 @@ interface AutomationsEmptyStateProps { * "new automation" form. We surface the chat path explicitly so users * don't go hunting for an "add" button that doesn't exist. */ -export function AutomationsEmptyState({ - searchSpaceId, - canCreate, - modelViolations = [], -}: AutomationsEmptyStateProps) { - const modelsBlocked = modelViolations.length > 0; - +export function AutomationsEmptyState({ searchSpaceId, canCreate }: AutomationsEmptyStateProps) { return (
@@ -36,26 +26,20 @@ export function AutomationsEmptyState({ SurfSense drafts the automation for your approval.

{canCreate ? ( - modelsBlocked ? ( -
- -
- ) : ( -
- - -
- ) +
+ + +
) : (

You don't have permission to create automations in this search space. diff --git a/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-header.tsx b/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-header.tsx index 308eaccfb..137727f60 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-header.tsx +++ b/surfsense_web/app/dashboard/[search_space_id]/automations/components/automations-header.tsx @@ -1,9 +1,7 @@ "use client"; import { MessageSquarePlus, SquarePen } from "lucide-react"; import Link from "next/link"; -import type { ReactNode } from "react"; import { Button } from "@/components/ui/button"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; interface AutomationsHeaderProps { searchSpaceId: number; @@ -16,22 +14,13 @@ interface AutomationsHeaderProps { * there to avoid a duplicate button. */ showCreateCta?: boolean; - /** - * Disable the create CTAs when the search space's models aren't billable - * for automations (free/Auto). When set, a tooltip explains why and the - * buttons render disabled rather than as links. - */ - createDisabled?: boolean; - disabledReason?: string; } -const DEFAULT_DISABLED_REASON = - "Automations need a premium or your own (BYOK) model. Update your model settings to enable."; - /** * Page header: title + count + "Create via chat" CTA. Creation is intent-driven * (the create_automation tool runs inside chat with a HITL approval card), so - * the CTA links to a new chat rather than opening a form. + * the CTA links to a new chat rather than opening a form. Model eligibility is + * handled per-automation in the builder + approval card, not gated here. */ export function AutomationsHeader({ searchSpaceId, @@ -39,8 +28,6 @@ export function AutomationsHeader({ loading, canCreate, showCreateCta = true, - createDisabled = false, - disabledReason, }: AutomationsHeaderProps) { return (

@@ -54,70 +41,20 @@ export function AutomationsHeader({
{canCreate && showCreateCta && (
- {createDisabled ? ( - <> - } - label="Create manually" - reason={disabledReason ?? DEFAULT_DISABLED_REASON} - /> - } - label="Create via chat" - reason={disabledReason ?? DEFAULT_DISABLED_REASON} - /> - - ) : ( - <> - - - - )} + +
)}
); } - -function DisabledCta({ - icon, - label, - reason, - variant, -}: { - icon: ReactNode; - label: string; - reason: string; - variant?: "outline"; -}) { - return ( - - - {/* aria-disabled (not `disabled`) keeps the button focusable so the - tooltip is reachable by hover and keyboard; onClick is a no-op. */} - - - {reason} - - ); -} diff --git a/surfsense_web/app/dashboard/[search_space_id]/automations/components/builder/automation-builder-form.tsx b/surfsense_web/app/dashboard/[search_space_id]/automations/components/builder/automation-builder-form.tsx index f3323da61..39904dfa0 100644 --- a/surfsense_web/app/dashboard/[search_space_id]/automations/components/builder/automation-builder-form.tsx +++ b/surfsense_web/app/dashboard/[search_space_id]/automations/components/builder/automation-builder-form.tsx @@ -21,8 +21,10 @@ import { automationCreateRequest, automationUpdateRequest, } from "@/contracts/types/automation.types"; +import { useAutomationEligibleModels } from "@/hooks/use-automation-eligible-models"; import { type BuilderForm, + type BuilderModels, buildCreatePayload, builderFormSchema, buildScheduleTrigger, @@ -30,10 +32,12 @@ import { createEmptyForm, formFromAutomation, type HydratableTrigger, + hasResolvedModels, hydrateForm, } from "@/lib/automations/builder-schema"; import { cn } from "@/lib/utils"; import { AdvancedSection } from "./advanced-section"; +import { AutomationModelFields } from "./automation-model-fields"; import { BasicsSection } from "./basics-section"; import { BuilderSummary } from "./builder-summary"; import { JsonModePanel } from "./json-mode-panel"; @@ -47,9 +51,9 @@ interface AutomationBuilderFormProps { /** Required in edit mode; seeds the form and trigger reconciliation. */ automation?: Automation; /** - * When set (create mode only), the search space's models aren't billable - * for automations. Submit is disabled with this reason as the tooltip; the - * orchestrator also renders the full gate alert above the form. + * Optional extra create-mode block reason (composed with the form's own + * model-eligibility gate). Shown as the submit button's tooltip. Model + * eligibility itself is now owned by the in-form pickers. */ submitDisabledReason?: string; } @@ -117,15 +121,46 @@ export function AutomationBuilderForm({ ? `/dashboard/${searchSpaceId}/automations/${automation.id}` : `/dashboard/${searchSpaceId}/automations`; + // Eligible models + the search-space-seeded defaults. Models are chosen per + // automation on create; in edit mode the backend preserves the captured + // snapshot, so the picker is create-only. + const eligibleModels = useAutomationEligibleModels(); + + // Resolve each slot during render: an explicit (non-zero) pick wins, + // otherwise fall back to the eligible default. No effect copies async hook + // data into state, so there's no flicker/loop and the user's pick is sticky. + const resolvedModels = useMemo( + () => ({ + agentLlmId: form.models.agentLlmId || eligibleModels.llm.defaultId || 0, + imageConfigId: form.models.imageConfigId || eligibleModels.image.defaultId || 0, + visionConfigId: form.models.visionConfigId || eligibleModels.vision.defaultId || 0, + }), + [ + form.models, + eligibleModels.llm.defaultId, + eligibleModels.image.defaultId, + eligibleModels.vision.defaultId, + ] + ); + + // The form with resolved models folded in — what every payload builder reads. + const formForPayload = useMemo( + () => ({ ...form, models: resolvedModels }), + [form, resolvedModels] + ); + function patchForm(patch: Partial) { setForm((prev) => ({ ...prev, ...patch })); } function jsonFromCurrentForm(): Record { if (mode === "edit" && automation) { - return { ...buildUpdatePayload(form), status: automation.status }; + return { ...buildUpdatePayload(formForPayload), status: automation.status }; } - const { search_space_id: _ignored, ...rest } = buildCreatePayload(form, searchSpaceId); + const { search_space_id: _ignored, ...rest } = buildCreatePayload( + formForPayload, + searchSpaceId + ); return rest; } @@ -223,7 +258,7 @@ export function AutomationBuilderForm({ setSubmitting(true); try { if (mode === "edit" && automation) { - const payload = buildUpdatePayload(form); + const payload = buildUpdatePayload(formForPayload); const parsed = automationUpdateRequest.safeParse(payload); if (!parsed.success) { setRootError(zodIssueList(parsed.error).join("; ")); @@ -233,7 +268,7 @@ export function AutomationBuilderForm({ await reconcileTriggers(automation.id); router.push(`/dashboard/${searchSpaceId}/automations/${automation.id}`); } else { - const payload = buildCreatePayload(form, searchSpaceId); + const payload = buildCreatePayload(formForPayload, searchSpaceId); const parsed = automationCreateRequest.safeParse(payload); if (!parsed.success) { setRootError(zodIssueList(parsed.error).join("; ")); @@ -281,8 +316,18 @@ export function AutomationBuilderForm({ } const submitLabel = mode === "edit" ? "Save changes" : "Create automation"; + // Block creation until every model slot resolves to an eligible id. The + // per-field Alert already explains *why* a slot is empty; this just guards + // submit. `submitDisabledReason` (from the caller) still composes in. + const modelsUnresolved = + mode === "create" && !eligibleModels.isLoading && !hasResolvedModels(resolvedModels); + const effectiveDisabledReason = + submitDisabledReason ?? + (modelsUnresolved + ? "Set up a premium or your own (BYOK) agent, image, and vision model in role settings before creating an automation." + : undefined); // Only gate creation; editing an existing automation isn't blocked here. - const submitBlocked = mode === "create" && !!submitDisabledReason; + const submitBlocked = mode === "create" && !!effectiveDisabledReason; return (
@@ -364,6 +409,19 @@ export function AutomationBuilderForm({ + + + Models + + + patchForm({ models: { ...form.models, ...patch } })} + /> + + + Settings @@ -416,7 +474,7 @@ export function AutomationBuilderForm({ {submitLabel} - {submitDisabledReason} + {effectiveDisabledReason} ) : ( diff --git a/surfsense_web/hooks/use-automation-eligible-models.ts b/surfsense_web/hooks/use-automation-eligible-models.ts new file mode 100644 index 000000000..e74994221 --- /dev/null +++ b/surfsense_web/hooks/use-automation-eligible-models.ts @@ -0,0 +1,150 @@ +"use client"; + +import { useAtomValue } from "jotai"; +import { useMemo } from "react"; +import { + globalImageGenConfigsAtom, + imageGenConfigsAtom, +} from "@/atoms/image-gen-config/image-gen-config-query.atoms"; +import { + globalNewLLMConfigsAtom, + llmPreferencesAtom, + newLLMConfigsAtom, +} from "@/atoms/new-llm-config/new-llm-config-query.atoms"; +import { + globalVisionLLMConfigsAtom, + visionLLMConfigsAtom, +} from "@/atoms/vision-llm-config/vision-llm-config-query.atoms"; + +/** + * A single model the user may pick for an automation slot. + */ +export interface EligibleModelOption { + id: number; + name: string; + /** Underlying model identifier (e.g. `gpt-4o`); shown as secondary text. */ + modelName: string; + provider: string; + /** `true` for user BYOK configs (positive ids), `false` for premium globals. */ + isBYOK: boolean; +} + +export interface EligibleModelKind { + options: EligibleModelOption[]; + /** Default selection: the search-space pref when eligible, else first option. */ + defaultId: number | null; + /** O(1) id → option lookup for trigger labels (avoids per-render `.find()`). */ + byId: Map; +} + +export interface AutomationEligibleModels { + llm: EligibleModelKind; + image: EligibleModelKind; + vision: EligibleModelKind; + isLoading: boolean; +} + +interface GlobalConfigLike { + id: number; + name: string; + model_name: string; + provider: string; + is_premium?: boolean; + is_auto_mode?: boolean; +} + +interface UserConfigLike { + id: number; + name: string; + model_name: string; + provider: string; +} + +/** + * Build the eligible option list for one model kind: premium globals + * (`is_premium === true`, never Auto mode) followed by all BYOK configs. + */ +function buildKind( + globals: GlobalConfigLike[] | undefined, + byok: UserConfigLike[] | undefined, + prefId: number | null | undefined +): EligibleModelKind { + const premiumGlobals: EligibleModelOption[] = (globals ?? []) + .filter((c) => c.is_premium === true && !c.is_auto_mode) + .map((c) => ({ + id: c.id, + name: c.name, + modelName: c.model_name, + provider: c.provider, + isBYOK: false, + })); + + const byokOptions: EligibleModelOption[] = (byok ?? []).map((c) => ({ + id: c.id, + name: c.name, + modelName: c.model_name, + provider: c.provider, + isBYOK: true, + })); + + const options = [...premiumGlobals, ...byokOptions]; + const byId = new Map(options.map((o) => [o.id, o])); + + let defaultId: number | null = null; + if (prefId != null && byId.has(prefId)) { + defaultId = prefId; + } else if (options.length > 0) { + defaultId = options[0].id; + } + + return { options, defaultId, byId }; +} + +/** + * Lists the LLM / image / vision models that are eligible for automations + * (premium globals + user BYOK — never free globals or Auto mode), with a + * default selection seeded from the search space's role preferences. + * + * Everything is derived during render from the existing config query atoms; + * there are no effects, so option lists/maps keep stable references. + */ +export function useAutomationEligibleModels(): AutomationEligibleModels { + const { data: llmUserConfigs, isLoading: llmUserLoading } = useAtomValue(newLLMConfigsAtom); + const { data: llmGlobalConfigs, isLoading: llmGlobalLoading } = + useAtomValue(globalNewLLMConfigsAtom); + const { data: preferences, isLoading: prefsLoading } = useAtomValue(llmPreferencesAtom); + const { data: imageGlobalConfigs, isLoading: imageGlobalLoading } = + useAtomValue(globalImageGenConfigsAtom); + const { data: imageUserConfigs, isLoading: imageUserLoading } = useAtomValue(imageGenConfigsAtom); + const { data: visionGlobalConfigs, isLoading: visionGlobalLoading } = useAtomValue( + globalVisionLLMConfigsAtom + ); + const { data: visionUserConfigs, isLoading: visionUserLoading } = + useAtomValue(visionLLMConfigsAtom); + + const llm = useMemo( + () => buildKind(llmGlobalConfigs, llmUserConfigs, preferences?.agent_llm_id), + [llmGlobalConfigs, llmUserConfigs, preferences?.agent_llm_id] + ); + + const image = useMemo( + () => buildKind(imageGlobalConfigs, imageUserConfigs, preferences?.image_generation_config_id), + [imageGlobalConfigs, imageUserConfigs, preferences?.image_generation_config_id] + ); + + const vision = useMemo( + () => buildKind(visionGlobalConfigs, visionUserConfigs, preferences?.vision_llm_config_id), + [visionGlobalConfigs, visionUserConfigs, preferences?.vision_llm_config_id] + ); + + const isLoading = + llmUserLoading || + llmGlobalLoading || + prefsLoading || + imageGlobalLoading || + imageUserLoading || + visionGlobalLoading || + visionUserLoading; + + return useMemo(() => ({ llm, image, vision, isLoading }), [llm, image, vision, isLoading]); +} diff --git a/surfsense_web/lib/automations/builder-schema.ts b/surfsense_web/lib/automations/builder-schema.ts index a9349c56f..c2bd69209 100644 --- a/surfsense_web/lib/automations/builder-schema.ts +++ b/surfsense_web/lib/automations/builder-schema.ts @@ -66,6 +66,19 @@ export const builderExecutionSchema = z.object({ }); export type BuilderExecution = z.infer; +/** + * Per-automation model selection. ``0`` means "unset" — the builder resolves it + * to the eligible default during render, and the resolved (non-zero) ids are + * written onto ``definition.models`` at submit so the run is insulated from + * later chat/search-space model changes. + */ +export const builderModelsSchema = z.object({ + agentLlmId: z.number().int(), + imageConfigId: z.number().int(), + visionConfigId: z.number().int(), +}); +export type BuilderModels = z.infer; + export const builderFormSchema = z.object({ name: z.string().trim().min(1, "Give your automation a name").max(200), description: z.string().trim().max(2000).nullable(), @@ -77,6 +90,8 @@ export const builderFormSchema = z.object({ tags: z.array(z.string()), /** Carried through from an edited definition so we don't drop it. */ goal: z.string().nullable(), + /** Selected agent/image/vision models (``0`` = use the eligible default). */ + models: builderModelsSchema, }); export type BuilderForm = z.infer; @@ -132,6 +147,7 @@ export function createEmptyForm(): BuilderForm { }, tags: [], goal: null, + models: { agentLlmId: 0, imageConfigId: 0, visionConfigId: 0 }, }; } @@ -218,9 +234,26 @@ function buildDefinition(form: BuilderForm): AutomationDefinition { on_failure: [], }, metadata: { tags: form.tags }, + // Only emit models when fully resolved (the builder seeds non-zero + // defaults before submit). A zero/unset triple is omitted so the + // backend falls back to the search-space snapshot. + ...(hasResolvedModels(form.models) + ? { + models: { + agent_llm_id: form.models.agentLlmId, + image_generation_config_id: form.models.imageConfigId, + vision_llm_config_id: form.models.visionConfigId, + }, + } + : {}), } as unknown as AutomationDefinition; } +/** True once every model slot holds a concrete (non-zero) id. */ +export function hasResolvedModels(models: BuilderModels): boolean { + return models.agentLlmId !== 0 && models.imageConfigId !== 0 && models.visionConfigId !== 0; +} + /** The desired schedule trigger for this form, or ``null`` if none. */ export function buildScheduleTrigger(form: BuilderForm): TriggerCreateRequest | null { if (!form.schedule) return null; @@ -434,6 +467,8 @@ export function hydrateForm( ? metadata.tags.filter((tag): tag is string => typeof tag === "string") : []; + const models = modelsFromDefinition(d.models); + return { formable: true, form: { @@ -455,10 +490,22 @@ export function hydrateForm( }, tags, goal: typeof d.goal === "string" ? d.goal : null, + models, }, }; } +/** Read a captured ``definition.models`` snapshot into the form's model slots. */ +function modelsFromDefinition(raw: unknown): BuilderModels { + const m = asRecord(raw); + const num = (value: unknown) => (typeof value === "number" ? value : 0); + return { + agentLlmId: num(m.agent_llm_id), + imageConfigId: num(m.image_generation_config_id), + visionConfigId: num(m.vision_llm_config_id), + }; +} + /** * Project an existing automation into the builder form for editing. */