Architecture seams: chat client interface, message filter, fragments, settings fields - #1293
Merged
Conversation
- Add chat.Client interface (Connect/Disconnect) to internal/chat/client.go - Define chat.Platform type with PlatformTwitch/PlatformYouTube/PlatformKick constants as the single source of truth - Remove ChangeChannel from Twitch client (subsumed by Connect reconnect) - Replace six per-platform App fields with a platform-keyed registry - Inject emit func on App; production wires to Wails, tests use recorder - Replace ConnectTwitch/DisconnectTwitch etc. with Connect/Disconnect(platform) - Add app_chat_test.go: fake client + recording emitter prove the full chain (connect → message → chat:message emitted) without network or Wails runtime Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…enum Regenerated with `wails3 generate bindings -ts -clean`. Six per-platform methods replaced with Connect(platform, input)/Disconnect(platform). The chat.Platform Go type now emits as a TypeScript enum (PlatformTwitch/PlatformYouTube/PlatformKick). Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- connection store: keyed maps (connected/inputs) replace three named fields; setChannel → setInput - Home: single handleToggle(platform) handler replaces three copy-pasted ones - App.tsx: type casts use Platform instead of hand-written union - types/chat.ts: ChatMessage.platform typed as Platform from bindings - Chat.tsx / ChatMessage.tsx: platform comparisons use Platform enum constants; getFade/getFadeTimeout/platformClass/PlatformIcon take Platform instead of string - ThemePreview.tsx: sample data uses Platform enum values No hand-written 'twitch' | 'youtube' | 'kick' union remains in source. Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Kick Connect now closes the old conn before dialing a new one, preventing the old readLoop from blocking indefinitely - readLoop, heartbeatLoop, and reconnect now accept ctx by parameter so each goroutine exits with the context it was started with, not a stale struct field replaced by a later Connect - Twitch, YouTube, and Kick OnEvent payloads now use chat.Platform* constants instead of hand-written strings - connection.ts initialises state with computed Platform enum keys and drops the as-cast - Extends fakeClient in app_chat_test.go with call-order recording and adds TestConnectWhileConnectedDisconnectsFirst Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Twitch: pass conn and ctx as parameters to readLoop and reconnect so each goroutine holds the context it was started with. After a read error, check ctx.Done() before spawning reconnect; if cancelled, exit silently. Rewrote reconnect to dial directly (matching the Kick pattern) instead of calling Connect — calling Connect cancelled the live ctx, causing readLoop to error, which re-spawned reconnect indefinitely at ~1 Hz. YouTube: pass ctx as a parameter through pollLoop and rebootstrap so each poll goroutine reads the context value it was started with, not the struct field written by a concurrent Connect call. Eliminates the unsynchronised read described by the Go memory model as a data race. Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finding A: change sendHandshake(conn, channel) to receive locals instead of reading struct fields unlocked, eliminating the unsynchronised read vs locked writes on reconnect. Finding B: check ctx.Err() under the lock after dialing so a concurrent Disconnect cannot clobber a new live session; drop the redundant c.channel write-back in Twitch reconnect. Apply the same ctx re-check to Kick reconnect and snapshot c.chatroomID under the lock before the dial loop. Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Remove dead ctx struct fields from twitch, kick, and youtube clients; only cancel is retained after ctx-parameterization of loops - Fix twitch Connect to close conn and cancel ctx on sendHandshake failure, preventing a resource leak - Reorder readLoop params to (ctx, conn) per Go convention in twitch and kick; update all call sites - Replace hardcoded platform strings in parsers with string(chat.PlatformX) to track the canonical constant - Delete self-verifying TestConnectWhileConnectedDisconnectsFirst and remove the disconnect-mirroring shim from fakeClient.Connect - Extract makeHandlers from SetApp/wireClients so TestEmitSeam uses the production message-handler path instead of duplicating it Refs #1288 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extracts shouldDisplay, fadePolicy, and classifyEvent into a pure src/filter/messageFilter.ts module with no React/DOM/Wails dependencies. Adds vitest 3.2.6 as the first frontend test runner; 46 specs cover blacklist case-insensitivity, command hiding, bot visibility toggle, event-gate categories, unknown event types, and fade defaults. The test script runs via pnpm test (vitest run). Refs #1289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chat.tsx: removes inline SUB_EVENTS, RAID_EVENTS, ANNOUNCEMENT_EVENTS, KNOWN_BOTS, isEventAllowed, and per-platform fade variables. The message listener now calls shouldDisplay(msg, cfg) for intake-drop, and render calls fadePolicy(msg.platform, config) for fade props. EventMessage.tsx: removes the local eventAccentClass switch; derives the CSS accent class via classifyEvent from the Message Filter, using a lookup map so the taxonomy lives in exactly one place. Refs #1289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extracts the case-insensitive username comparison from three parallel platform branches into a single private isBlacklisted helper so the comparison logic exists exactly once in the frontend source. Refs #1289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace `|| 30` with `?? 30` in fadePolicy so a configured value of 0 is not coerced to the default. Update the three specs that codified the wrong coercion to assert 0 passes through, and apply the project blank- line spacing rule between assignment blocks and expect calls throughout the test file. Refs #1289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…seams - vite.config.ts: import defineConfig from vitest/config to fix TS2769 on test key; add target+skipLibCheck to tsconfig.node.json - Chat.tsx: destructure fadePolicy once per map callback instead of calling it twice per branch - EventMessage.tsx: type EVENT_ACCENT_CLASS as Record<ReturnType<typeof classifyEvent>, string> and remove unreachable ?? fallback - messageFilter.test.ts: add specs for null-config default-allow and unknown-platform fallthrough - messageFilter.ts: replace inline anonymous events type with Partial<TwitchEvents>; move isEventAllowed above its caller Refs #1289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds Fragmentize(text, emotes) to the shared chat package. Uses Unicode code points for indexing (matching Twitch IRC), fills emote URLs, and drops overlapping or out-of-bounds ranges defensively. Tests cover all required cases: plain text, single/adjacent emotes, emote at start/end, third-party emotes, emoji before an emote, and overlapping/out-of-bounds ranges. Refs #1290 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes the Emotes field and offset-based Emote type from ChatMessage. The Twitch client now calls fillNativeEmoteURLs (filling CDN URLs on the Go side) then Fragmentize after emote resolution on both PRIVMSG and USERNOTICE messages. Text field is retained for Message Filter use. Parser tests updated to check Tags instead of the removed Emotes field. Refs #1290 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extracts renderFragments into a shared module consumed by both ChatMessage and EventMessage. Removes the two offset-walking renderers (renderTextWithEmotes and renderUserMessage) and the TWITCH_EMOTE_CDN constant from both components. Updates ChatMessage types to remove Emote interface and emotes field. ThemePreview sample data and messageFilter tests updated to use fragments only. Refs #1290 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Refs #1290 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ead CSS Extract a buildFragments method on the Twitch Client to eliminate the copy-pasted ParseEmotes → fillNativeEmoteURLs → ResolveEmotes → Fragmentize pipeline from both PRIVMSG and USERNOTICE paths, and drop the redundant empty-text guard on the USERNOTICE side since Fragmentize already handles empty input. Rename fragmentize.go's `deduped` variable to `nonOverlapping` to accurately describe its purpose (the loop skips overlapping ranges, not duplicates). Remove the dead `.emote` rule from EventMessage.module.css which was never referenced by the component. Refs #1290 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d modules Pull shared validation and save logic out of each platform settings component into three focused field modules. The split/trim/filter blacklist parsing and fade-timeout validation each now exist exactly once. Locale fade and blacklist strings consolidate under settings.platform.*; per-platform duplicates are removed. Twitch event toggle handlers collapse to single-field partials, relying on the config store's existing deep merge to preserve sibling event fields. Refs #1291 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Refs #1291 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nline styles
- Add explicit `normalize` prop to ChannelField; Twitch and Kick pass
`(v) => v.toLowerCase()` (byte-identical behaviour), YouTube omits it
so case-sensitive channel IDs are preserved
- Add `footer` prop rendered inside the `.field` div so resolve-status
spans in YouTubeSettings sit within the flex-column gap, matching
the pre-refactor placement
- Replace `style={{ display:'flex', gap:'6px' }}` with `.field-input-row`
and the resolved-URL inline styles with `.field-success` in index.css
Refs #1291
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds bindings/** to oxfmt ignorePatterns and updates oxfmt to 0.54.0. The bindings are committed in their raw generated form, so a bare wails3 bindings regeneration no longer leaves a formatting-only diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dshake failure The reconnect path no longer routes through Connect, which previously emitted chat:connected; restore that emit so the frontend stays in sync if a disconnect is ever surfaced during an outage. Connect also reset c.conn/c.cancel when the initial handshake fails instead of leaving them pointing at a closed connection and cancelled context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ChatMessage.Platform becomes chat.Platform, dropping the string() conversions in the parsers. The connection store uses partial records so the generated Platform.$zero enum member no longer needs seeding, and uses the create callback set. The frontend fragments type admits null to match Fragmentize returning a nil slice for empty text, and the settings field components document their captured-once initialValue contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying ghost-chat with
|
| Latest commit: |
45809cc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c2ed0c29.ghost-chat-4c1.pages.dev |
| Branch Preview URL: | https://architecture-seams.ghost-chat-4c1.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the four architecture issues from the 2026-06-10 review, each developed from its agent brief with separate spec-compliance and code-quality review passes.
Closes #1288
Closes #1289
Closes #1290
Closes #1291
#1288 — Chat client seam
chat.Clientinterface andPlatformconstants ininternal/chat/client.go; App holds a platform-keyed registryConnect(platform, input)/Disconnect(platform); unknown platform errorsfunc(event string, data any)) — the connect → message →chat:messagechain is covered by plain Go tests with a fake client and recording emitterPlatformenum replaces every hand-written'twitch' | 'youtube' | 'kick'union in the frontend#1289 — Message Filter + vitest
frontend/src/filter/messageFilter.ts:shouldDisplay,fadePolicy,classifyEvent— blacklist, command/bot hiding, event gating, and fade policy live in one moduleshouldDisplay→ append; EventMessage derives its accent fromclassifyEventpnpm test, 48 specs#1290 — Fragmentize
chat.Fragmentize(text, emotes)with Unicode code-point indexing (fixes emoji-before-emote misrendering), defensive overlap/out-of-bounds handling, Go-side CDN fill for native Twitch emotesEmotesfield removed from ChatMessage; fragments are the only content encoding; USERNOTICE user text gets fragments too#1291 — Settings field modules
ChannelField,FadeControls,BlacklistFieldown validation/save behavior; no store imports, no platform config knowledge (parents bind the writes)settings.platform.*locale keys in en-US and de-DE; duplicate per-platform strings removedVerification
go vet ./...,go test ./...clean;go test -raceclean except a pre-existing data race in the twitchTestConnectDisconnectfakeIRC test harness (reproduces on main)cd frontend && pnpm fix && pnpm build && pnpm testclean; bindings regenerated withwails3 generate bindings -ts -cleanand in syncFollow-up candidates (pre-existing, not addressed here): the twitch test-harness race blocks CI
-race; a bare bindings regeneration leaves a formatting-only diff untilpnpm fixruns.🤖 Generated with Claude Code