Skip to content

fix(audit-sprint-7b): toast consolidation onto sonner (resubmit, was #176) - #188

Merged
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-7b-toast-consolidation
May 23, 2026
Merged

fix(audit-sprint-7b): toast consolidation onto sonner (resubmit, was #176)#188
ejay-dev merged 1 commit into
mainfrom
fix/audit-sprint-7b-toast-consolidation

Conversation

@ejay-dev

@ejay-dev ejay-dev commented May 23, 2026

Copy link
Copy Markdown
Owner

Resubmit after base 4c branch deletion. Rebased onto main.

Summary by CodeRabbit

  • Refactor
    • Consolidated toast notification system to use unified notification framework across compliance alerts, compliance toast messages, and general notifications for consistent presentation and behavior.

Review Change Stack

Modal-audit agent found 4 hand-rolled toast implementations across the
codebase. This PR migrates 3 of them to the shared sonner Toaster
(Sprint 4c). The 4th (motion/InteractionFeedback.ToastItem) has no
external consumers and is left in place for separate cleanup.

Each migration preserves the component's call-site contract — only the
internals change. Visual unification is the win: same neutral default
surface across notifications, automation alerts, and compliance graph
events. Off-brand cyan/teal/violet badges go away (matches stored
"enterprise aesthetic" preference).

Migrated
  - components/notifications/notification-toast.tsx (-60 LoC):
    Realtime Supabase subscription stays (the actual job of the
    component). Hand-rolled portal + queue + ToastItem render replaced
    with toast.error / toast.warning. Click-to-route preserved via
    sonner's action prop. Component now returns null; mount in
    app/app/layout.tsx unchanged.
  - components/automation/ComplianceToastAlerts.tsx (-140 LoC):
    30-second polling against getAutomationHistory() stays. ToastItem
    renderer + dismiss button + slide-in animation deleted. Critical
    triggers (control_failed, risk_score_change) → toast.error;
    others → toast.warning. Same returns-null pattern.
  - components/compliance-system/compliance-toast.tsx (-150 LoC):
    Trickier — this one has external consumers via the
    useComplianceToast() hook in use-compliance-action.tsx. Preserves
    the full public API (ComplianceToastData type, showToast,
    dismissToast). Provider becomes a passthrough; internally
    showToast maps to sonner's typed variants and builds a short
    description from message/nodeType/nodeAction/impactArea/impactDelta.
    No caller changes needed.

Out of scope
  - components/motion/InteractionFeedback.tsx ToastItem export
    (~30 lines of a 437-line utility module). Zero external consumers
    of ToastItem found via grep — left in place. Can be a one-line
    re-export removal later.

Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
shared Toaster.

Validation
  - tsc -p tsconfig.typecheck.json: clean
  - eslint: 0 errors, 18 warnings (baseline)
  - jest: 5319/5334 pass (no test changes)

Test plan
  - Send a critical notification → top-right red sonner toast with
    title + body + View action that routes to data.href
  - Trigger a control_failed automation → bottom-right red toast
  - Call useComplianceToast().showToast({ type:'success', title:'x',
    nodeType:'policy', nodeAction:'created' }) from a React component
    → green toast with description "policy created"

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 23, 2026 16:57
@vercel

vercel Bot commented May 23, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
forma-os Building Building Preview, Comment May 23, 2026 4:57pm

Request Review

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 926a8e29-0318-4143-a412-f7c87206a4f9

📥 Commits

Reviewing files that changed from the base of the PR and between dd8e354 and d38dd70.

📒 Files selected for processing (3)
  • components/automation/ComplianceToastAlerts.tsx
  • components/compliance-system/compliance-toast.tsx
  • components/notifications/notification-toast.tsx

📝 Walkthrough

Walkthrough

Three toast-related components are refactored to remove local state management and rendering logic, delegating toast UI to a shared Sonner-based Toaster. The compliance-toast provider becomes an adapter; ComplianceToastAlerts shifts from rendering to polling and emitting; NotificationToast delegates from a local queue to direct toast API calls from its realtime subscription.

Changes

Toast System Consolidation

Layer / File(s) Summary
Compliance Toast Provider Adapter
components/compliance-system/compliance-toast.tsx
The provider removes custom toast rendering, timers, and UI JSX. ComplianceToastData remains the typed input, but showToast now delegates to Sonner's toast API, composing a single description from message and impact fields. dismissToast forwards to sonnerToast.dismiss. The exported ComplianceToast component is removed.
Compliance Alerts Polling and Toast Emission
components/automation/ComplianceToastAlerts.tsx
Polling logic remains, but the component no longer renders a local toast list or manages auto-dismiss timers. A useRef-based deduplicator tracks the last-seen event id. When a critical trigger is detected, the component emits a single toast directly via the shared toast API with a trigger-specific title and dynamically generated description. The component returns null, delegating all UI to the root Toaster.
Notification Toast Realtime Subscription
components/notifications/notification-toast.tsx
The Supabase realtime subscription remains active, but the INSERT handler now immediately calls toast.error or toast.warning based on notification priority instead of queuing. The handler wires a "View" action to navigate to the notification URL. Local state for the toast queue and setTimeout timers are removed. The component returns null, with UI handled by the root Toaster.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • ejay-dev/FormaOS#167: Introduces the root <Toaster /> component and toast API in components/ui/toaster.tsx and app/app/layout.tsx, which these three components now depend on.
  • ejay-dev/FormaOS#181: Removes the now-obsolete ToastItem toast renderer from components/motion/InteractionFeedback.tsx as part of the same consolidation effort.

Poem

🐰 Three toasts hop toward one shared lane,
No more local state to maintain!
Polling, subscribing, delegating with glee,
A Sonner dream for all to see. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: consolidation of three toast implementations onto the sonner library. It is concise, uses a conventional commit prefix (fix), and references the relevant audit sprint and predecessor PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-sprint-7b-toast-consolidation

Comment @coderabbitai help to get the list of available commands and usage tips.

@ejay-dev
ejay-dev merged commit 0bffd56 into main May 23, 2026
12 of 28 checks passed
@ejay-dev
ejay-dev deleted the fix/audit-sprint-7b-toast-consolidation branch May 23, 2026 16:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d38dd707ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const [toasts, setToasts] = useState<ComplianceToastData[]>([]);
const idCounter = useRef(0);

export function ComplianceToastProvider({

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore ComplianceToast export to keep barrel imports valid

This refactor removes the ComplianceToast named export from compliance-toast.tsx, but the barrel file still re-exports it (components/compliance-system/index.ts exports ComplianceToast, ComplianceToastProvider, useComplianceToast). Re-exporting a missing binding breaks module linking/builds when consumers import @/components/compliance-system, so this change can fail app compilation/runtime import resolution until a compatibility export is restored or the barrel is updated in the same commit.

Useful? React with 👍 / 👎.

ejay-dev added a commit that referenced this pull request May 23, 2026
… by local tsc (#191)

Every Vercel production deploy since #167 (modal primitives) errored
because two re-exports referenced names that the audit sprints had
deleted from their source files. Local `tsc -p tsconfig.typecheck.json`
didn't catch them because its include list misses the barrel +
default-export bundle below.

What was broken
  1. components/compliance-system/index.ts:47 re-exported
     `ComplianceToast` (the bespoke render component). Sprint 7b
     (PR #188) rewrote compliance-toast.tsx as a sonner shim and
     deleted that component — only ComplianceToastProvider and
     useComplianceToast remain.
  2. components/motion/InteractionFeedback.tsx:386 default-export
     bundle still listed `ToastItem` as a shorthand property after
     Sprint 8b (PR #181) deleted the ToastItem function.

Vercel error excerpt (from dpl_98NHQyUnwGo7CoSxKGTuWbfhoros):
  Type error: '"./compliance-toast"' has no exported member named
  'ComplianceToast'. Did you mean 'useComplianceToast'?

Fix
  - Drop ComplianceToast from the barrel re-export. No external
    importers — confirmed via grep.
  - Drop ToastItem from the default-export bundle. Same — zero
    importers of the default object reach for ToastItem.

Validation
  - npm run build: green end-to-end (Compiled successfully →
    TypeScript pass → page tree generated, 0 errors)

Why local tsc didn't catch this
  - tsconfig.typecheck.json's include list scopes type-checking to a
    subset of the tree. Sprint 4c-onwards needed full-app type
    coverage to catch downstream re-export drift. Out of scope here
    but worth tightening tsconfig.typecheck.json in a follow-up so
    Vercel doesn't have to be the integration test.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ejay-dev
ejay-dev removed the request for review from Copilot May 23, 2026 17:19
@github-actions

Copy link
Copy Markdown

♿ Accessibility Test Results

⚠️ ERROR - Unable to complete accessibility testing

Tests Performed:

  • WCAG 2.1 AA compliance validation
  • Cross-browser accessibility testing
  • Keyboard navigation testing
  • Screen reader compatibility
  • Color contrast validation

Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results.

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