Skip to content

Container components refactor#18

Merged
CoolSpring8 merged 6 commits into
mainfrom
container-components-refactor
Nov 24, 2025
Merged

Container components refactor#18
CoolSpring8 merged 6 commits into
mainfrom
container-components-refactor

Conversation

@CoolSpring8
Copy link
Copy Markdown
Owner

@CoolSpring8 CoolSpring8 commented Nov 24, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Centralized settings management for streamlined API configuration
    • Enhanced chat interface with improved message editing
    • Conversation import/export via snapshots
    • Text completion view mode
    • Diagram view mode
    • Built-in AI availability tracking with status indicators
  • Refactor

    • Modernized app architecture for improved stability and performance
    • Enhanced conversation management system

✏️ Tip: You can customize this high-level summary in your review settings.

- Removed unused imports and consolidated state management for prompt handling.
- Introduced new state variables for tracking prompt dirtiness and composer reset signals.
- Refactored message handling functions to streamline editing and sending messages.
- Replaced inline message rendering with a dedicated ChatView component for improved readability and maintainability.
…pp and SettingsModal

- Consolidated state management in the App component by utilizing the useSettingsStore for settings and model management.
- Removed redundant state variables and effects, enhancing clarity and performance.
- Updated SettingsModal to directly interact with the Zustand store for settings and built-in availability, simplifying the component structure.
- Improved the export and import functionality for conversation snapshots, enhancing user experience.
- Revised the description of state management, specifying that settings and provider state are now managed in a shared store.
- Clarified the roles of various components and their interactions within the project structure, enhancing overall documentation clarity.
…e management

- Removed unused imports and consolidated built-in availability handling in the App component.
- Introduced a new `refreshBuiltInAvailability` function in the Zustand store for better state management.
- Updated SettingsModal to utilize the new function for checking built-in model status, improving clarity and performance.
- Refactored message handling in the App component to leverage a dedicated `sendMessage` function, streamlining the sending process.
…w hooks

- Consolidated state management by introducing new hooks for built-in availability and provider readiness.
- Refactored message handling to utilize a dedicated `sendMessage` function, improving clarity and performance.
- Added a `SnapshotIO` component for streamlined export and import of conversation snapshots.
- Improved handling of system messages and conversation state, ensuring a more robust user experience.
- Removed unused state variables and imports in the useConversationController hook for improved clarity.
- Introduced a new equality function for message comparison to enhance performance in the useConversationTree hook.
- Updated the useConversationTree to utilize createWithEqualityFn for better state management and performance.
- Streamlined the compileActive function to ensure accurate message retrieval while maintaining state integrity.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 24, 2025

Walkthrough

This PR refactors the application architecture to centralize settings management via a new Zustand store (useSettingsStore), introduces orchestration hooks and dedicated view components (ChatView, DiagramView, TextCompletionView, SnapshotIO), and replaces local state management with store-derived values throughout the application.

Changes

Cohort / File(s) Change Summary
Settings and Store Infrastructure
src/state/useSettingsStore.ts, src/constants/storageKeys.ts
Introduces Zustand-based settings store managing baseURL, apiKey, models, activeModel, providerKind, builtInAvailability, and hydration state; adds storage key constants for persistence
App Architecture Refactoring
src/App.tsx, src/components/SettingsModal.tsx
Migrates from local state to store-backed state; replaces prop-driven SettingsModal with store-derived values; restructures view composition (ChatView/DiagramView/TextCompletionView) with SnapshotIO wrapper
View Components
src/components/ChatView.tsx, src/components/SnapshotIO.tsx
Introduces ChatView for messaging UI with editing/deletion support and drag-drop; adds SnapshotIO for import/export workflow with render-prop pattern
Orchestration Hooks
src/hooks/useConversationController.ts, src/hooks/useProviderReadiness.ts, src/hooks/useStreamManager.ts, src/hooks/useTextCompletion.ts
New controller/readiness/stream management hooks centralizing conversation actions, provider validation, abort handling, and text completion streaming
Lifecycle and State Hooks
src/hooks/useBeforeUnloadGuard.ts, src/hooks/useBuiltInAvailability.ts, src/hooks/useBuiltInStatus.ts, src/hooks/useEnsureSystemMessage.ts
New utility hooks for beforeunload prevention, built-in provider initialization, status text computation, and system message creation
AI Messaging and Actions
src/ai/sendMessage.ts, src/utils/chatActions.ts, src/utils/snapshots.ts
New sendMessage orchestrator for streaming AI responses; deleteMessage utility for message handling; snapshot serialization utilities for export/import
State and Type Updates
src/tree/useConversationTree.ts, src/types.ts
Switches tree store to equality-based creation; adds snapshot serialization; introduces ChatProviderReady/CompletionProviderReady union types and OpenAIProviderAdapter interface; expands BuiltInAvailability
Documentation
AGENTS.md
Updates documentation to reflect moved settings store location and updated project architecture

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant useConversationController
    participant ChatView
    participant useProviderReadiness
    participant sendMessage
    participant StreamManager
    participant AI as Language Model

    App->>useConversationController: Initialize with options
    activate useConversationController
    useConversationController->>useConversationController: Setup refs (streamManager, tree)
    deactivate useConversationController

    ChatView->>ChatView: User enters prompt
    ChatView->>useConversationController: send(promptText)
    activate useConversationController
    useConversationController->>useProviderReadiness: ensureChatReady()
    alt Provider Ready
        useConversationController->>sendMessage: sendMessage(promptText, context)
        activate sendMessage
        sendMessage->>StreamManager: register(id, controller)
        sendMessage->>AI: streamText(model, messages)
        activate AI
        AI-->>sendMessage: stream
        deactivate AI
        loop For each text delta
            sendMessage->>sendMessage: Append to assistant node
        end
        sendMessage->>StreamManager: setLatest(id)
        deactivate sendMessage
    else Provider Not Ready
        useConversationController->>useConversationController: Show error toast
    end
    useConversationController-->>ChatView: Update isGenerating, messages
    deactivate useConversationController
Loading
sequenceDiagram
    participant User
    participant SnapshotIO
    participant FileSystem as File System
    participant App as useConversationController

    rect rgba(100, 200, 100, 0.2)
    note right of User: Export Flow
    User->>SnapshotIO: Click export button
    SnapshotIO->>App: exportSnapshot()
    App-->>SnapshotIO: ConversationSnapshot
    SnapshotIO->>SnapshotIO: Serialize to JSON
    SnapshotIO->>FileSystem: Download snapshot.json
    SnapshotIO->>User: Show success toast
    end

    rect rgba(100, 150, 200, 0.2)
    note right of User: Import Flow
    User->>SnapshotIO: Click import button
    SnapshotIO->>FileSystem: File picker
    FileSystem-->>SnapshotIO: File selected
    SnapshotIO->>SnapshotIO: parseSnapshotFile()
    SnapshotIO->>App: importSnapshot(snapshot)
    App->>App: Reconstruct tree state
    SnapshotIO->>User: Show success toast
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • src/App.tsx — comprehensive refactoring of component structure, state management, and view composition logic; verify hook orchestration and prop flow to new view components
  • src/state/useSettingsStore.ts — persistence layer integration with IndexedDB, hydration logic, model synchronization with API, and error handling
  • src/hooks/useConversationController.ts — large orchestrator managing multiple concerns (messaging, editing, deletion, snapshots, stream management); verify action handlers and state consistency
  • src/tree/useConversationTree.ts — switched to equality-based store creation and added snapshot serialization; verify tree state reconstruction and node mutation consistency
  • src/ai/sendMessage.ts — streaming logic with delta accumulation, status transitions, and abort handling; verify error flows and cleanup
  • Stream management wiring across useStreamManager.ts, useTextCompletion.ts, and sendMessage.ts — ensure abort controllers are properly registered and cleaned up

Possibly related PRs

Suggested labels

codex

Poem

🐰 The settings now dance in a Zustand-backed store,
While hooks orchestrate actions we've dreamed of before,
Views compose and recompose, snapshots flow free,
Streams manage their chaos in perfect harmony!
From local to global, the app finds its way,
Refactored and ready to chat all the day. 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring theme: moving from in-component orchestration to container/presentational components, with new state store, hooks, and modular UI components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch container-components-refactor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (16)
src/hooks/useBeforeUnloadGuard.ts (1)

3-21: Before‑unload guard behavior is correct; inner check can be simplified

Hook correctly attaches/removes the beforeunload listener based on shouldBlock and prevents navigation when active. The extra if (!shouldBlock) check inside handleBeforeUnload is redundant since the effect only registers the listener when shouldBlock is true and cleans it up when it flips to false, but it doesn’t harm behavior. You could remove that inner check to keep the handler simpler.

src/hooks/useBuiltInStatus.ts (1)

4-27: Built‑in status mapping looks good; consider explicit return type

The hook cleanly gates on providerKind === "built-in" and covers all BuiltInAvailability variants with sensible strings (with "unknown" sensibly falling back to "Built-in AI"). For extra clarity for callers, you might consider adding an explicit return type of string | undefined to useBuiltInStatus, but functionally this is fine as‑is.

src/hooks/useProviderReadiness.ts (1)

20-80: Provider readiness checks are coherent; minor duplication is acceptable

The readiness gating for chat vs. completion correctly mirrors the ChatProviderReady/CompletionProviderReady types: built‑in providers are limited to chat, and both branches validate activeModel and openAIProvider before returning a ready payload, with clear toast messages on failure. There’s some duplication between the "openai-compatible" paths of ensureChatReady and ensureCompletionReady, but it’s small and readable; you could factor out a shared helper later if this grows, though it’s not necessary now.

src/constants/storageKeys.ts (1)

1-4: Review storage key naming consistency and migration expectations

Centralizing these keys is good. Two minor follow‑ups to consider:

  • If you’re migrating from older keys, confirm these string values either match the previous ones or add a small migration so existing users don’t lose settings.
  • providerKindKey uses "iaslate_provider_kind" while the others use iaslate_camelCase; if there were no legacy constraints, you may want to align the naming style for future keys.
src/utils/snapshots.ts (2)

3-14: Consider guarding exportedAt and broadening filename sanitization

exportSnapshotToFile assumes snapshot.exportedAt is always a defined string; if that field is ever optional or user‑controlled, .replace will throw. You may want to either enforce it in the ConversationSnapshot type or fall back to new Date().toISOString() when it’s missing, and optionally sanitize a broader set of characters than just : if exportedAt ever changes format.


16-21: Optional: provide clearer errors for invalid snapshot files

parseSnapshotFile currently lets JSON.parse errors bubble up as generic exceptions. Depending on how the caller handles this, you might want to wrap the parse in a try/catch and throw a more descriptive error or validate the parsed shape before casting to ConversationSnapshot, so the UI can show a friendly “invalid snapshot file” message.

src/hooks/useStreamManager.ts (1)

3-56: Handle repeated registrations for the same id more defensively

The API and internal bookkeeping look good overall. One edge case to consider: if register is ever called for an id that already has a controller, the old controller remains in flight but is no longer reachable from the map. To make this more robust against misuse, you could abort any existing controller before overwriting it, e.g.:

 const register = (id: string, controller: AbortController) => {
-  controllersRef.current[id] = controller;
-  latestAssistantIdRef.current = id;
+  const existing = controllersRef.current[id];
+  if (existing && existing !== controller) {
+    existing.abort();
+  }
+  controllersRef.current[id] = controller;
+  latestAssistantIdRef.current = id;
 };

This keeps the manager safe even if callers forget to call abort(id) before starting a new stream for the same node.

src/hooks/useBuiltInAvailability.ts (1)

10-23: Clarify refresh trigger conditions and dependency set

Right now refreshBuiltInAvailability runs on mount and whenever its function reference changes, but not when providerKind flips to "built-in". If the component using this hook stays mounted while the provider changes, availability might never be refreshed at the moment the user selects the built‑in provider.

Consider instead tying the refresh directly to providerKind:

-	useEffect(() => {
-		void refreshBuiltInAvailability();
-	}, [refreshBuiltInAvailability]);
+	useEffect(() => {
+		if (providerKind === "built-in") {
+			void refreshBuiltInAvailability();
+		}
+	}, [providerKind, refreshBuiltInAvailability]);

Also be aware that the second effect will re‑invoke onBuiltInSelected whenever its function identity changes while providerKind === "built-in", not only when the kind flips; verify that this is acceptable for your usage.

src/components/SnapshotIO.tsx (1)

25-51: Import/export flow is robust; consider slightly loosening the file accept filter

The export and import logic (including error handling and always clearing event.target.value) looks correct and resilient.

One minor UX tweak you might consider: some environments tag JSON files with generic types, so using a slightly broader accept can make selection easier, e.g.:

-				accept="application/json"
+				accept="application/json,.json"

This is optional; the current implementation is functionally fine.

src/components/ChatView.tsx (3)

37-57: Avoid oscillating onPromptDirtyChange(false) on every state change

The current effect calls onPromptDirtyChange(dirty) on mount/update and then, via the cleanup, calls onPromptDirtyChange(false) before every re‑run. That means the consumer will briefly see false on every prompt/editing change, even if the form remains dirty, which can complicate external tracking.

A cleaner pattern is to separate “current dirty state” updates from the unmount reset:

-	useEffect(() => {
-		onPromptDirtyChange?.(
-			prompt.trim().length > 0 || typeof editingMessageId !== "undefined",
-		);
-		return () => {
-			onPromptDirtyChange?.(false);
-		};
-	}, [editingMessageId, onPromptDirtyChange, prompt]);
+	useEffect(() => {
+		onPromptDirtyChange?.(
+			prompt.trim().length > 0 || typeof editingMessageId !== "undefined",
+		);
+	}, [editingMessageId, onPromptDirtyChange, prompt]);
+
+	useEffect(
+		() => () => {
+			onPromptDirtyChange?.(false);
+		},
+		[onPromptDirtyChange],
+	);

This keeps the external dirty flag stable across state transitions while still clearing it on unmount.


88-106: Drag‑and‑drop handler works but could be slightly more defensive

The drop handler correctly prevents default behavior and appends text from dropped files to the prompt. A couple of optional hardening ideas:

  • Fall back to event.dataTransfer.files for browsers/environments where dataTransfer.items isn’t populated.
  • Optionally skip very large files or binary types (e.g., by checking file.type and/or file.size) to avoid freezing the UI when someone drops a huge file.

Example adjustment:

-					if (event.dataTransfer.items) {
-						for (const item of event.dataTransfer.items) {
+					const items = event.dataTransfer.items ?? [];
+					for (const item of items) {
 						if (item.kind === "file") {
 							const file = item.getAsFile();
 							if (file) {
 								const content = await file.text();
 								setPrompt((draft) => draft + content);
 							}
 						}
-						}
-					}
+					}

You could then add a separate branch for event.dataTransfer.files if needed.


71-83: Confirm whether sending on empty/whitespace prompt is intentional

handleSubmit will call onSend(prompt) even when prompt.trim().length === 0 and you’re not editing. Given sendMessage treats an empty prompt as “regen” against existing context, this may be desired.

If you don’t want that behavior (and instead want a no‑op when there’s no user input and no edit in progress), you could guard the non‑editing path:

-		if (editingMessageId) {
+		if (editingMessageId) {
 			onEditSubmit(editingMessageId, prompt);
 			setPrompt("");
 			return;
 		}
-		onSend(prompt);
-		setPrompt("");
+		if (prompt.trim().length > 0) {
+			onSend(prompt);
+			setPrompt("");
+		}

Please verify that the current semantics align with your UX expectations before changing.

src/state/useSettingsStore.ts (1)

59-82: Active model hydration may ignore prior choice and conflicts slightly with non‑OpenAI providers

On hydrate you always set:

activeModel: storedModels?.at(0)?.id ?? null,

regardless of:

  • what model the user may have previously selected, and
  • whether storedProvider is "built-in" or "openai-compatible".

Meanwhile, saveSettings clears activeModel when switching away from "openai-compatible", but that isn’t persisted, so after a reload you can end up with providerKind === "built-in" and a non‑null activeModel derived from old OpenAI models.

Consider either:

  • persisting the selected activeModel separately and hydrating from that, and/or
  • explicitly setting activeModel: null on hydrate when storedProvider === "built-in".

This would make the store’s invariants around provider kind vs active model more predictable.

src/tree/useConversationTree.ts (1)

412-445: Edited-clone behavior is intentional but keep semantics in mind

replaceNodeWithEditedClone keeps the original node in the tree (detached from its children) and inserts an edited clone as a sibling while reparenting all children to the clone and updating activeTargetId. That’s a reasonable “versioned edit” semantics, but it means originals remain as leaf nodes and will still appear in any consumers that walk all nodes/roots, not just the active path. If that’s not desired everywhere, you may eventually want a soft-delete flag or a different node type instead of leaving historic nodes structurally visible.

src/components/SettingsModal.tsx (1)

82-137: Minor UX inconsistency when availability is downloading

In handleDownloadModel, when model.availability() returns "downloading", you set builtInAvailability("downloading") but finally sets isDownloading back to false and clears downloadProgress. That yields:

  • Status text: “Downloading…”
  • Button disabled (due to builtInAvailability !== "downloadable")
  • No spinner/progress bar (because isDownloading is false)

If you want the UI to reflect an external in-progress download, consider keeping isDownloading true (and perhaps a non-null downloadProgress) for that branch so the loading state and progress bar remain consistent with the status label.

src/hooks/useConversationController.ts (1)

246-256: Consider whether thread activation should abort active streams

handleActivateThread switches activeTargetId, resets composer state, and flips isGenerating to false, but it does not call abortActiveStreams(). This means an in-flight stream can continue mutating the previously active branch while the UI shows a different thread, and the local generating flag no longer reflects actual streaming.

If background streaming while browsing other threads is not desired, you might want to invoke abortActiveStreams() here instead of just setIsGenerating(false).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb05666 and 9af7b32.

📒 Files selected for processing (20)
  • AGENTS.md (1 hunks)
  • src/App.tsx (2 hunks)
  • src/ai/sendMessage.ts (1 hunks)
  • src/components/ChatView.tsx (1 hunks)
  • src/components/SettingsModal.tsx (7 hunks)
  • src/components/SnapshotIO.tsx (1 hunks)
  • src/constants/storageKeys.ts (1 hunks)
  • src/hooks/useBeforeUnloadGuard.ts (1 hunks)
  • src/hooks/useBuiltInAvailability.ts (1 hunks)
  • src/hooks/useBuiltInStatus.ts (1 hunks)
  • src/hooks/useConversationController.ts (1 hunks)
  • src/hooks/useEnsureSystemMessage.ts (1 hunks)
  • src/hooks/useProviderReadiness.ts (1 hunks)
  • src/hooks/useStreamManager.ts (1 hunks)
  • src/hooks/useTextCompletion.ts (1 hunks)
  • src/state/useSettingsStore.ts (1 hunks)
  • src/tree/useConversationTree.ts (2 hunks)
  • src/types.ts (2 hunks)
  • src/utils/chatActions.ts (1 hunks)
  • src/utils/snapshots.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
src/hooks/useBuiltInStatus.ts (1)
src/types.ts (2)
  • ProviderKind (23-23)
  • BuiltInAvailability (25-30)
src/utils/chatActions.ts (1)
src/hooks/useStreamManager.ts (1)
  • StreamManager (59-59)
src/utils/snapshots.ts (1)
src/tree/types.ts (1)
  • ConversationSnapshot (37-37)
src/hooks/useProviderReadiness.ts (1)
src/types.ts (2)
  • ProviderKind (23-23)
  • OpenAIProviderAdapter (32-35)
src/components/ChatView.tsx (1)
src/types.ts (1)
  • Message (9-14)
src/state/useSettingsStore.ts (3)
src/types.ts (3)
  • ModelInfo (16-21)
  • ProviderKind (23-23)
  • BuiltInAvailability (25-30)
src/constants/storageKeys.ts (4)
  • providerKindKey (4-4)
  • baseURLKey (1-1)
  • apiKeyKey (2-2)
  • modelsKey (3-3)
src/ai/openaiCompatible.ts (1)
  • fetchOpenAICompatibleModels (25-46)
src/components/SnapshotIO.tsx (2)
src/tree/types.ts (1)
  • ConversationSnapshot (37-37)
src/utils/snapshots.ts (2)
  • exportSnapshotToFile (3-14)
  • parseSnapshotFile (16-21)
src/hooks/useTextCompletion.ts (1)
src/types.ts (1)
  • CompletionProviderReady (48-51)
src/ai/sendMessage.ts (1)
src/types.ts (2)
  • ChatProviderReady (37-46)
  • Message (9-14)
src/hooks/useBuiltInAvailability.ts (1)
src/types.ts (1)
  • ProviderKind (23-23)
src/components/SettingsModal.tsx (1)
src/state/useSettingsStore.ts (1)
  • useSettingsStore (39-149)
src/App.tsx (10)
src/state/useSettingsStore.ts (1)
  • useSettingsStore (39-149)
src/types.ts (1)
  • AppView (3-3)
src/hooks/useBuiltInStatus.ts (1)
  • useBuiltInStatus (4-28)
src/hooks/useBuiltInAvailability.ts (1)
  • useBuiltInAvailability (10-24)
src/hooks/useProviderReadiness.ts (1)
  • useProviderReadiness (20-81)
src/hooks/useConversationController.ts (1)
  • useConversationController (15-284)
src/hooks/useTextCompletion.ts (1)
  • useTextCompletion (11-73)
src/hooks/useEnsureSystemMessage.ts (1)
  • useEnsureSystemMessage (11-30)
src/hooks/useBeforeUnloadGuard.ts (1)
  • useBeforeUnloadGuard (3-22)
src/utils/chatActions.ts (1)
  • deleteMessage (17-45)
src/tree/useConversationTree.ts (1)
src/tree/types.ts (3)
  • NodeID (1-1)
  • TreeNode (5-13)
  • ConversationSnapshot (37-37)
🔇 Additional comments (23)
src/types.ts (1)

1-51: Provider readiness type modeling is consistent and discriminated cleanly

The OpenAI adapter interface and the ChatProviderReady/CompletionProviderReady discriminated unions line up well with the hooks that consume them (useProviderReadiness, useTextCompletion). Restricting CompletionProviderReady to the "openai-compatible" branch via Extract enforces at the type level that built‑in providers are chat‑only, which matches the runtime checks elsewhere. No changes needed here.

src/hooks/useTextCompletion.ts (1)

18-53: The review comment is incorrect; the code is correct as written.

The web search confirms that streamText() from the ai SDK (version 5.0.98 as shown in package.json) returns an object directly with a textStream AsyncIterable property—it does not return a Promise. The code pattern in useTextCompletion.ts at line 30 is correct:

const stream = streamText({ ... });
for await (const delta of stream.textStream) { ... }

Adding await before streamText() would actually break the code, since you cannot use for await on a Promise. The same pattern is also used elsewhere in the codebase (src/ai/sendMessage.ts line 69) without issues, confirming this is the correct API usage.

Likely an incorrect or invalid review comment.

AGENTS.md (1)

5-8: Docs update correctly reflects new settings store location

The description of providers, settings store, and conversation/snapshot locations matches the refactor direction and is clear for contributors.

src/hooks/useEnsureSystemMessage.ts (1)

11-29: System-message bootstrap logic and dependencies look solid

The effect correctly seeds a system message only when the tree is empty and there are no chat messages, and the dependency list covers all inputs so it will also fire after a “clear conversation” flow. This matches the intended invariant of always having a system node when starting fresh.

src/utils/chatActions.ts (1)

3-45: deleteMessage orchestration is coherent and covers key edge cases

The sequence—abort stream, stop generating, remove node, reset composer when deleting the editing node, then prefer activeTail() / seed a new system message on empty tree—looks consistent with the rest of the controller design and should behave well for both chat and diagram views.

src/ai/sendMessage.ts (1)

27-100: Streaming orchestration and node state handling look solid

Model selection, context construction (excluding "tool" messages), streaming loop, and status transitions ("draft" → "streaming" → "final" / "error") are consistent and read cleanly. Abort handling that reverts to "draft" while keeping partial content is also reasonable for a “stop generation” experience.

Please just double‑check that:

  • streamText from ai@5.x exposes fullStream with type === "text-delta" / "reasoning-delta" parts as used here, and
  • ChatProviderReady’s openAIProvider.chatModel / getBuiltInChatModel contracts indeed return a LanguageModel compatible with streamText.
src/state/useSettingsStore.ts (1)

50-57: Built‑in availability refresh behavior is reasonable; confirm return type compatibility

The refreshBuiltInAvailability action cleanly wraps builtInAI().availability() with error handling and falls back to "unavailable" on failure, which is a sensible default.

Please just verify that:

  • builtInAI().availability() in @built-in-ai/core@2.x returns only values compatible with your local BuiltInAvailability union, and
  • it’s safe to call this in all environments where useSettingsStore may be imported/used (e.g., SSR vs browser), since the invocation happens inside the async method rather than at module top level.
src/tree/useConversationTree.ts (3)

173-241: Tree store initialization and derived-path helpers look sound

The zustand store wiring plus helpers like syncLinearTail, compileActive, getActivePathIds, and pickNewestLeafId are consistent and avoid obvious pitfalls (e.g., compileActive filters out missing nodes before mapping to messages). The decision to only recompute edges/roots via withDerivedTree when structure changes keeps updates cheaper for content-only mutations.


369-388: Reparenting correctly prevents cycles

canReparent and reparentNode both guard against creating cycles by checking isAncestor(nodes, childId, parentId) / isAncestor(state.nodes, target, nextParent), and avoid self-parenting. The logic here looks correct and should keep the tree acyclic under reparent operations.


481-527: Snapshot import/export are robust and defensive

The snapshot implementation:

  • Normalizes parentId to null.
  • Coerces role via coerceRole.
  • Validates parent links against nodesRaw.
  • Guards createdAt type.
  • Drops invalid activeTargetId on import.

This should make snapshots resilient to older or slightly malformed data without breaking the tree. Nice balance between correctness and leniency.

src/components/SettingsModal.tsx (3)

28-49: Good migration to store-driven settings

Using useSettingsStore with useShallow to pull just the needed fields/actions (baseURL, apiKey, providerKind, builtInAvailability, saveSettings, syncModels, etc.) keeps SettingsModal decoupled from its callers and minimizes re-renders. The props surface (open, onClose) is now nicely small and focused.


64-81: Built-in availability check correctly delegates to the store

handleCheckBuiltInAvailability short-circuits while downloading, flips availability to "unknown" to indicate a pending check, clears local error, and then awaits refreshBuiltInAvailability(). Given the store’s refreshBuiltInAvailability already normalizes “unavailable” on errors, this layering looks safe and keeps the source of truth in the store.


189-239: Form submit and model sync wiring look correct

Submitting the form awaits saveSettings(values) and only then calls onClose(), which is what you want if saveSettings can fail or trigger async side effects. The “Sync from API” button delegates directly to syncModels() from the store, keeping sync logic centralized and honoring its internal handling of baseURL/apiKey/force/silent flags.

src/hooks/useConversationController.ts (5)

15-64: Tree selector and controller state wiring are appropriate

The controller pulls exactly the needed tree actions via a shallow selector (activeTargetId, createUserAfter, appendToNode, snapshot methods, etc.), which should keep re-renders tight. Local controller state (isGenerating, editingMessageId, composerResetSignal, isPromptDirty) nicely separates UI concerns from the tree store.


66-91: Custom message equality function is well-targeted

areMessagesEqual compares _metadata.uuid, role, content, and reasoning_content, and is used as the equality function when subscribing to compileActive(). This avoids unnecessary re-renders when unrelated tree fields (e.g., internal status) change, while still detecting actual chat content changes. Given toMessage only exposes these fields plus _metadata, this equality looks correct.


122-164: Send flow correctly delegates to sendMessage and handles failure

handleSend checks ensureChatReady() before proceeding, passes all required tree/stream callbacks into sendMessage, and catches errors to log and show a toast. This keeps provider validation and error surfacing centralized without duplicating logic here.


181-209: Delete handler cleanly composes with shared chatActions.deleteMessage

handleDeleteMessage forwards all relevant state (including editingMessageId, activeTail, isTreeEmpty, createSystemMessage, etc.) into the shared deleteMessage utility. That centralizes the nuanced behavior (aborting streams, resetting composer, ensuring a system node exists) and keeps the controller lean.


258-283: Returned controller API is cohesive and complete

The returned object (chatMessages, send, stop, edit/delete handlers, snapshot methods, tree helpers, etc.) presents a coherent façade over the underlying tree + streaming machinery. This should make the views (ChatView, DiagramView, SnapshotIO consumers) much simpler and better isolated from implementation details.

src/App.tsx (5)

26-48: Settings store integration in App looks solid

Using useSettingsStore with useShallow to pull baseURL, apiKey, models, activeModel, providerKind, builtInAvailability, hydrate, and refreshBuiltInAvailability centralizes configuration and keeps App stateless with respect to settings. The initial useEffect that calls hydrate() once is a straightforward way to restore persisted state on mount.


60-83: Provider readiness hooks are wired correctly

openAIProvider is memoized from baseURL and apiKey, and getBuiltInChatModel wraps builtInAI() in a stable callback. Passing these, plus providerKind, builtInAvailability, and activeModel, into useProviderReadiness aligns with the described contract and keeps capability checks out of the main component.


85-131: Conversation controller and system-message guard are well composed

App delegates nearly all chat/diagram behavior to useConversationController and then uses useEnsureSystemMessage to guarantee a system prompt exists whenever the tree is empty and there are no messages. That separation of concerns (controller vs. invariant-enforcing hook) should make it easier to evolve the conversation model without touching App.


150-159: Before-unload guard uses a sensible session-state heuristic

hasSessionState accounts for chat streaming, dirty prompt, text completion state, non-empty text content, an active edit, or more than one chat message. Passing this into useBeforeUnloadGuard should prevent accidental tab closes when there’s meaningful in-progress work, while not nagging when the user only has the initial system message.


172-245: SnapshotIO composition and view routing are clean

Wrapping the main layout with <SnapshotIO> and feeding exportSnapshot, importSnapshot, and onImportStart gives you a clear import/export boundary while allowing Header to trigger these via triggerImport / triggerExport. The conditional rendering for "chat", "diagram", and "text" views, plus the provider-based disabling of text prediction, all look coherent and match the readiness rules you’ve set up.

@CoolSpring8 CoolSpring8 merged commit 5f695f0 into main Nov 24, 2025
1 check passed
@CoolSpring8 CoolSpring8 deleted the container-components-refactor branch November 24, 2025 14:54
@coderabbitai coderabbitai Bot mentioned this pull request Dec 18, 2025
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