Skip to content

improvement(knowledge): make KB modals toast-only for errors#5349

Closed
TheodoreSpeaks wants to merge 3 commits into
stagingfrom
kb-modal-toast-errors
Closed

improvement(knowledge): make KB modals toast-only for errors#5349
TheodoreSpeaks wants to merge 3 commits into
stagingfrom
kb-modal-toast-errors

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(knowledge): surface KB description validation errors and raise limit to 10k #5347. Standardize both KB modals on a single error-surfacing mechanism: toast.
  • Create modal: validation errors already toast via onInvalid; removed the inline description/regexPattern field messages and the submitStatus state + <ChipModalError> block. Upload failures now toast in the catch.
  • Add onError: toast.error to useCreateKnowledgeBase (matching useUpdateKnowledgeBase) so server errors surface from the hook — the create modal no longer hand-rolls server-error handling.
  • Edit modal: dropped the redundant inline nameError/descriptionError/error state and <ChipModalError> (server errors already toast via the update hook's onError); validation now toasts.

Type of Change

  • Improvement

Testing

Tested manually. bun run lint, bun run check:api-validation:strict, and tsc --noEmit all pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

🤖 Generated with Claude Code

@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 3, 2026 6:11pm

Request Review

@cursor

cursor Bot commented Jul 2, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
UI-only error presentation changes in knowledge modals; create/upload failure behavior is slightly softer (toast + stay open) but no auth or data-model changes.

Overview
Standardizes create and edit knowledge base modals on toasts for API/upload failures and inline field errors for validation, removing modal footers/banners via ChipModalError and related state.

Create modal: Drops submitStatus, the upload error banner, and onInvalid validation toasts. Zod messages are wired through ChipModalField error props (including chunk size fields). Upload failures toast and return early after orphan KB cleanup instead of rethrowing. Create API errors are handled by new useCreateKnowledgeBase onError toast (no local server-error UI).

Edit modal: Removes generic error state and ChipModalError; validation stays on the name/description fields without validation toasts. Save failures rely on useUpdateKnowledgeBase’s existing onError toast.

Hook: useCreateKnowledgeBase gains onError: toast.error to mirror useUpdateKnowledgeBase.

Reviewed by Cursor Bugbot for commit 5fe6348. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR standardizes error-surfacing in the two KB modals to use toasts exclusively, removing inline ChipModalError blocks and field-level error state. It also aligns useCreateKnowledgeBase with useUpdateKnowledgeBase by adding an onError toast handler to the hook.

  • knowledge.ts: Adds onError: (error) => toast.error(error.message, { duration: 5000 }) to useCreateKnowledgeBase, matching the pattern already present in useUpdateKnowledgeBase.
  • create-base-modal.tsx: Removes SubmitStatus state, ChipModalError, and the errors destructure from useForm; upload failures now call toast.error directly and return instead of throw; server errors toast via the hook's onError.
  • edit-knowledge-base-modal.tsx: Removes nameError/descriptionError/error state and ChipModalError; validation errors toast via toast.error(validationError) in handleSubmit; server errors remain handled by useUpdateKnowledgeBase.onError.

Confidence Score: 5/5

Safe to merge — the changes remove redundant inline error state and wire server errors through the mutation hooks' onError callbacks, which is the established pattern in this codebase.

All three error paths (validation, upload failure, server error) have clear toast coverage. The upload-failure path was the most surgical change — swapping throw uploadError for toast.error + return cleanly avoids re-entering the outer catch. The hook-owns-errors pattern for server errors is already live on the update side; adding it to the create hook is a direct mirror. No logic was removed that was load-bearing.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/hooks/queries/kb/knowledge.ts Adds onError toast handler to useCreateKnowledgeBase, mirroring the existing pattern in useUpdateKnowledgeBase. Straightforward and consistent.
apps/sim/app/workspace/[workspaceId]/knowledge/components/create-base-modal/create-base-modal.tsx Removes SubmitStatus state, ChipModalError, and field-level error props. Upload failures now toast-and-return instead of re-throwing. Server errors are handled entirely by the hook's onError. Field error highlighting is gone (toast-only), which matches the PR's stated intent.
apps/sim/app/workspace/[workspaceId]/knowledge/components/edit-knowledge-base-modal/edit-knowledge-base-modal.tsx Removes nameError/descriptionError/error state and ChipModalError. Validation toasts directly; server errors remain handled by useUpdateKnowledgeBase.onError. Simplified validate() returns early with a message string.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant CreateModal
    participant useCreateKB as useCreateKnowledgeBase
    participant useUpload as useKnowledgeUpload
    participant useDeleteKB as useDeleteKnowledgeBase

    User->>CreateModal: Submit form

    alt Validation fails (onInvalid)
        CreateModal-->>User: toast.error(first field error)
    else Validation passes (onSubmit)
        CreateModal->>useCreateKB: mutateAsync(data)
        alt Server error
            useCreateKB-->>User: toast.error(error.message) via onError
            useCreateKB-->>CreateModal: throws (caught, logged only)
        else KB created
            alt Files selected
                CreateModal->>useUpload: uploadFiles(files, kbId)
                alt Upload fails
                    CreateModal->>useDeleteKB: mutateAsync(kbId) [cleanup]
                    CreateModal-->>User: toast.error("Failed to upload files")
                else Upload succeeds
                    CreateModal-->>User: Modal closes
                end
            else No files
                CreateModal-->>User: Modal closes
            end
        end
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant CreateModal
    participant useCreateKB as useCreateKnowledgeBase
    participant useUpload as useKnowledgeUpload
    participant useDeleteKB as useDeleteKnowledgeBase

    User->>CreateModal: Submit form

    alt Validation fails (onInvalid)
        CreateModal-->>User: toast.error(first field error)
    else Validation passes (onSubmit)
        CreateModal->>useCreateKB: mutateAsync(data)
        alt Server error
            useCreateKB-->>User: toast.error(error.message) via onError
            useCreateKB-->>CreateModal: throws (caught, logged only)
        else KB created
            alt Files selected
                CreateModal->>useUpload: uploadFiles(files, kbId)
                alt Upload fails
                    CreateModal->>useDeleteKB: mutateAsync(kbId) [cleanup]
                    CreateModal-->>User: toast.error("Failed to upload files")
                else Upload succeeds
                    CreateModal-->>User: Modal closes
                end
            else No files
                CreateModal-->>User: Modal closes
            end
        end
    end
Loading

Reviews (2): Last reviewed commit: "improvement(knowledge): drop field highl..." | Re-trigger Greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

Addressed the field-highlighting inconsistency (P2) in 815278f — instead of adding highlighting to the edit modal (its ChipModalField exposes a ReactNode error, so it can only show the inline message text we intentionally removed), I dropped the create modal's boolean error={Boolean(errors.x)} highlighting so both modals are now uniformly toast-only. Both surface the error message via toast; neither adds inline field chrome.

Re: the catch-block note (P1) — replied inline. Short version: server-error toasts are owned by the mutation hook's onError (the shared surface for the edit modal and the [id]/base.tsx inline rename), so a catch-level toast would double-fire. By design.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

Update (5fe6348): reversed the earlier toast-only decision. Surveyed the codebase — inline field-level validation is the dominant pattern (~60 files pass an error= prop to Chip controls; there's a dedicated <FieldError> component; forms like column-config-sidebar do inline validation + toast only for server errors). Toast-only-without-field-feedback was the outlier.

So both KB modals now follow the norm:

  • Validation errors → inline at the field (highlight + message via the error prop).
  • Server/mutation errors → toast (from the mutation hook's onError).

This actually resolves the P2 field-highlighting concern properly (both modals highlight + message the offending field) rather than leveling down to toast-only.

@waleedlatif1 waleedlatif1 deleted the kb-modal-toast-errors branch July 3, 2026 21:39
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.

1 participant