Skip to content

refactor(chat): clean up the deployed chat surface - #6220

Merged
waleedlatif1 merged 2 commits into
stagingfrom
chore/chat-surface-cleanup
Aug 3, 2026
Merged

refactor(chat): clean up the deployed chat surface#6220
waleedlatif1 merged 2 commits into
stagingfrom
chore/chat-surface-cleanup

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Eight-angle cleanup pass (effects, state, memo, callback, React Query, url-state, emcn, comments) run over the full contents of the deployed chat surface and the speech code that survived the voice-mode removal — not just a diff.

Dead code

  • enforceChatRateLimit — added for the TTS relay in fix(security): meter and throttle the deployed-chat TTS relay #6212, orphaned when chore(chat): remove deployed-chat voice mode #6215 deleted that route. Zero consumers.
  • ChatToolCallStatus, ChatErrorType, and six unused CHAT_ERROR_MESSAGES keys (only GENERIC_ERROR and CHAT_UNAVAILABLE are read).
  • scrollToMessage was declared and destructured by ChatMessageContainer but never used in its body. Removing the prop also made the scrollToShowOnlyMessage branch unreachable, since the one caller passed true.
  • permissionState and the language prop on useSpeechToText — both write-only across the repo.
  • The image branch in ChatFileDownload's renderIcon returned the same DefaultFileIcon at the same size as the fallback.
  • chatKeys.status/detail — aliases nothing imported, and misleading since they root under ['deployments', …] rather than chatKeys.all.

Redundant state

  • password-auth and email-auth each kept a boolean in lockstep with errors.length > 0; email-auth also validated on every keystroke and then immediately hid the result.
  • file-download tracked hover in state to drive one opacity class → group-hover. Verified emcn Button sets no group class of its own.

Memoization

  • ChatMessageContainer's memo() could never bail: chat.tsx passes an inline arrow for scrollToBottom and displayMessages is a fresh array. Four of the five things that re-render ChatClient are its props anyway, so the memo is dropped rather than propped up.
  • ClientChatMessage keeps its memo — it blocks markdown re-parsing during streaming — but loses the custom comparator, which compared proxies (a key:status fingerprint, files by length) and ignored attachments and type entirely. Default shallow compare on its single prop is simpler and stricter.
  • Six useCallbacks whose consumers are native DOM handlers or inline arrows.

Effects

  • The scroll listener attached in an effect keyed on [chatConfig, authRequired] — values it never reads, standing in for "the container has mounted". Now a ref callback, so it no longer re-attaches on every config refetch.

Design system / a11y

  • z-[100]z-[var(--z-dropdown)] (identical value), shadow-lgshadow-medium, inline list styles → Tailwind, hover:hover-hover: on touch-reachable targets, Check sourced from emcn alongside its Duplicate pair.
  • Accessible names on the remove-attachment, stop, and send buttons — all previously announced as just "button".
  • Dropped a keyboard handler on a role='group' div with no tabIndex (so target === currentTarget was unreachable), plus the Tooltip.Provider wrappers and delayDuration, which emcn documents as no-op passthroughs.

Needs a visual check

The scroll-listener change is the one behavioral risk. Please verify on a deployed chat: scroll up mid-conversation → jump-to-bottom button appears; click it → re-pins and hides; stream a reply while scrolled away → does not auto-follow.

shadow-lgshadow-medium is a small deliberate shadow change on the jump-to-bottom pill.

Deliberately not included

  • useSpeechToText fetching server state in an effect into useState — should become a useQuery, but its only consumer is the workspace home input, and the render path is hydration-sensitive. Wants its own PR.
  • A real missing-invalidation bug: useUpdateChat doesn't invalidate deploymentKeys.info/versions/deployedState, though the PATCH can trigger a redeploy (api/chat/manage/[id]/route.ts calls performFullDeploy). useCreateChat invalidates for exactly this reason. Left out because it changes deployment-panel behavior I can't verify here — worth its own PR.
  • Visual-risk items needing design sign-off: border-purple-500 drag state, text-purple-500 audio icon, off-scale text-sm/text-lg, the stop glyph → emcn Square, and consolidating the two chat auth screens onto the shared (auth)/components kit (which would shrink headings 40px → 32px).

Type of Change

  • Refactor / dead code removal

Testing

1153 tests pass across chat, speech, settings, hooks and rate-limiter. Typecheck clean on apps/sim. Lint warnings in scope unchanged from staging's baseline (2). No behavior change intended except the two items called out above.

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)

Eight-angle cleanup pass over the full contents of the chat surface and
the speech code that survived the voice-mode removal.

Dead code
- enforceChatRateLimit: added for the TTS relay in #6212, orphaned when
  #6215 deleted that route. Zero consumers.
- ChatToolCallStatus, ChatErrorType, and six unused CHAT_ERROR_MESSAGES
  keys (only GENERIC_ERROR and CHAT_UNAVAILABLE are read).
- scrollToMessage was declared and destructured by ChatMessageContainer
  but never used in its body; removing the prop also made the
  scrollToShowOnlyMessage branch unreachable, since the sole caller
  passed true.
- permissionState and the language prop on useSpeechToText: both
  write-only across the repo.
- The image branch in ChatFileDownload's renderIcon returned the same
  DefaultFileIcon at the same size as the fallback.
- chatKeys.status/detail: aliases of deploymentKeys nothing imported,
  and misleading since they root under a different key namespace.

Redundant state
- password-auth and email-auth each kept a boolean in lockstep with
  `errors.length > 0`; email-auth also validated on every keystroke and
  then immediately hid the result.
- file-download tracked hover in state to drive one opacity class; now
  group-hover. Verified emcn Button sets no `group` class of its own.

Memoization
- ChatMessageContainer's memo() could never bail: chat.tsx passes an
  inline arrow for scrollToBottom and displayMessages is a fresh array.
  Four of the five things that re-render ChatClient are its props
  anyway, so the memo is dropped rather than propped up.
- ClientChatMessage keeps its memo — it blocks markdown re-parsing —
  but loses the custom comparator, which compared proxies (a
  key:status fingerprint, files by length) and ignored attachments and
  type entirely. Default shallow compare on its single prop is both
  simpler and stricter.
- Six useCallbacks whose consumers are native DOM handlers or inline
  arrows, so nothing observed their identity.

Effects
- The scroll listener attached in an effect keyed on [chatConfig,
  authRequired] — values it never reads, standing in for "the container
  has mounted". It now attaches via a ref callback, so it no longer
  re-attaches on every config refetch.

Design system and a11y
- z-[100] -> z-[var(--z-dropdown)] (same value), shadow-lg ->
  shadow-medium, list styles from inline style to Tailwind classes,
  hover: -> hover-hover: on touch-reachable targets, Check sourced from
  emcn alongside its Duplicate pair.
- Accessible names on the remove-attachment, stop, and send buttons,
  which announced only as "button".
- Dropped a keyboard handler on a role='group' div with no tabIndex,
  where target === currentTarget was unreachable, and the Tooltip
  Provider wrappers and delayDuration, which emcn documents as
  no-op passthroughs.
@vercel

vercel Bot commented Aug 3, 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 Aug 3, 2026 7:48pm

Request Review

@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
The ref-callback scroll listener is the main behavioral change—verify jump-to-bottom and stick-to-bottom while streaming. Removing language from STT WebSocket params may affect non-default language transcription if anything relied on it.

Overview
Refactors the deployed chat UI and related speech/query helpers: mostly dead-code removal, simpler React patterns, and design-system alignment—not new product behavior (aside from scroll wiring and a few visual tokens).

Scroll and layout (chat.tsx) — Replaces a scroll useEffect keyed on unrelated chatConfig/authRequired with a ref callback (attachMessagesContainer) so the listener attaches when the message container actually mounts after auth/loading gates. Simplifies scrollToMessage (drops unused offset branch and container prop). Drops several useCallbacks where nothing memoized depended on them.

Components — Removes memo from ChatMessageContainer (props were unstable); keeps ClientChatMessage memo but drops the custom comparator. Removes unused scrollToMessage prop from the container. Auth forms derive error visibility from errors.length instead of duplicate booleans; email no longer validates on every keystroke.

Design / a11yz-[100]z-[var(--z-dropdown)], shadow-lgshadow-medium, list styles → Tailwind utilities, hover:hover-hover: on touch targets. Adds aria-label on remove/stop/send; uses handleKeyboardActivation for attachment previews. Drops redundant Tooltip.Provider wrappers in chat input/messages.

Dead code — Removes enforceChatRateLimit, unused CHAT_ERROR_MESSAGES keys and ChatErrorType, chatKeys.status/detail aliases, ChatToolCallStatus, permissionState and language from useSpeechToText, hover state in file download (group-hover instead).

SpeechuseSpeechToText no longer tracks mic permission or passes language_code to ElevenLabs WS (if that was intentional cleanup vs regression, worth a quick product check).

Reviewed by Cursor Bugbot for commit 0f1f509. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR simplifies the deployed chat surface without intending broader behavioral changes.

  • Replaces redundant state, effects, callbacks, and memoization.
  • Removes unused chat, speech, query-key, and rate-limiter code.
  • Updates chat styling and accessible control labels.
  • Restores Markdown list markers using Tailwind list utilities.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported Markdown list-marker regression is corrected by effective Tailwind list utilities.

Important Files Changed

Filename Overview
apps/sim/app/(interfaces)/chat/components/message/components/markdown-renderer.tsx Replaces inline list styles with equivalent Tailwind utilities; the previously reported missing-marker regression is fixed.
apps/sim/app/(interfaces)/chat/[identifier]/chat.tsx Refactors scroll-listener attachment to a callback ref and simplifies scrolling callbacks.
apps/sim/app/(interfaces)/chat/components/input/input.tsx Removes redundant wrappers and callback memoization while adding accessible labels to icon-only controls.
apps/sim/app/(interfaces)/chat/components/message/message.tsx Simplifies message memoization and adopts the shared keyboard-activation helper.
apps/sim/hooks/use-speech-to-text.ts Removes unused speech-to-text state and parameters without changing the retained transcription flow.

Reviews (2): Last reviewed commit: "fix(chat): restore markdown list markers" | Re-trigger Greptile

The design-system pass swapped inline `listStyleType` for Tailwind
classes, but the edit that added `list-disc`/`list-decimal` silently did
not apply while the one removing the inline style did. With Preflight
setting `list-style: none`, every bullet and number in an assistant
response disappeared. `list-item` on the `li` sets display only, not the
marker type.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0f1f509. Configure here.

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