Skip to content

Feature/version 6.0.0#25

Open
ginccc wants to merge 9 commits intomasterfrom
feature/version-6.0.0
Open

Feature/version 6.0.0#25
ginccc wants to merge 9 commits intomasterfrom
feature/version-6.0.0

Conversation

@ginccc
Copy link
Copy Markdown
Member

@ginccc ginccc commented Mar 25, 2026

🚀 EDDI Chat UI v6.0.0 — Standalone Chat Widget

Modern React 19 chat widget for EDDI agents with SSE streaming and rich markdown rendering.

✨ Features

  • 🌊 SSE Streaming — Real-time token-by-token responses with thinking indicator
  • 💬 Rich Markdown — Tables, code blocks, bold/italic, links, lists
  • 🎨 Dark/Light Themes — Toggle via UI or ?theme= query param
  • Quick Replies — Pill buttons for suggested responses
  • 🔐 Secret Input — Masked password fields for sensitive data (API keys, credentials)
  • ↩️ Undo/Redo — Step through conversation history
  • 🔧 Configurable — All features togglable via URL query parameters
  • 📱 Responsive — Mobile-first with adaptive breakpoints
  • 🎭 Demo Mode — Full showcase at /chat/demo/showcase without a backend

🛠️ Tech Stack

React 19 · TypeScript 5.7 · Vite 6 · Vanilla CSS (BEM) · Context + useReducer

🧹 Cleanup

  • Untracked .env from git, added to .gitignore

📦 Deployment

Production build deploys into EDDI backend at META-INF/resources/ — served directly by Quarkus.

Summary by CodeRabbit

  • New Features

    • Redesigned chat UI with React 19 + Vite, including a standalone chat HTML entry
    • Dark/light/system theme switching and persisted preference
    • Real-time streaming messages with typing/thinking indicators and scroll-to-bottom control
    • Demo mode, secret/password input, quick replies, undo/redo, and improved markdown/math/code rendering
  • Documentation

    • New README and developer guidelines for setup, embedding, and workflow
  • Chores

    • Migrated from Create React App to Vite and updated core dependencies
  • Tests

    • Added unit and integration tests for key UI components and APIs

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

This PR migrates the project from CRA/React 18 to a Vite/React 19 app, replaces a monolithic Chat component with a composed widget and Context+useReducer store, adds a fetch-based API layer including SSE streaming and a demo-mode mock, and introduces a new styling system and test setup.

Changes

Cohort / File(s) Summary
Build & Tooling
package.json, vite.config.ts, vitest.config.ts, tsconfig.json, tsconfig.app.json, vite-env.d.ts
Switched from CRA to Vite (React 19), updated scripts, test config (Vitest/jsdom), TypeScript project refs, path alias, and build output targeting EDDI backend resources.
HTML Entrypoints
index.html, chat.html, public/index.html
Added index.html and chat.html for Vite entry; removed legacy public/index.html. New HTML includes metadata and Noto Sans font-face rules.
App Entry & Routing
src/main.tsx, src/index.js (deleted)
Added Vite TypeScript entrypoint with ChatProvider and React Router routes to ChatWidget; removed old CRA src/index.js.
State & Types
src/store/chat-store.tsx, src/store/chat-store.test.tsx, src/types.ts
Introduced Context + useReducer chat state, actions (messages, processing/thinking, undo/redo, secret input, config), tests for reducer, and shared TypeScript models/interfaces.
API Layer
src/api/chat-api.ts, src/api/demo-api.ts, src/api/demo-api.test.ts
Added fetch-based API with base URL management, conversation lifecycle endpoints, SSE streaming as an async generator, managed-agent routing, and a demo-mode mock with streaming simulation and tests.
Widget & Components
src/components/ChatWidget.tsx, src/components/ChatHeader.tsx, src/components/ChatInput.tsx, src/components/MessageBubble.tsx, src/components/QuickReplies.tsx, src/components/SecretInput.tsx, src/components/ScrollToBottom.tsx, src/components/Indicators.tsx, src/components/*test.tsx
Replaced monolithic Chat with ChatWidget orchestrator and many smaller components (header, input/secret input, message bubble with markdown, quick replies, indicators, scroll button); added tests for components.
Styling & Variables
src/styles/variables.css, src/styles/chat.css, src/index.css (deleted)
Removed old src/index.css; added design tokens (variables.css) and a comprehensive BEM-based chat.css with theme support, responsive rules, and animations.
Theme Hook & Test Setup
src/hooks/useTheme.ts, src/test-setup.ts
Added useTheme hook (persisted theme + system resolution) and Vitest test setup including a matchMedia polyfill.
Docs & Config Cleanup
AGENTS.md, README.md, .env, .gitignore, .whitesource (deleted), renovate.json (deleted)
Added AGENTS.md; rewrote README for Vite/EDDI integration; removed GENERATE_SOURCEMAP from .env; updated .gitignore to ignore /dist and .env; removed Whitesource and Renovate files.
Deleted Public Files
public/manifest.json, public/robots.txt
Removed web manifest and robots directives.
Tests & Misc
src/test-setup.ts, src/components/*.test.tsx, src/api/demo-api.test.ts
Added test setup and multiple component/unit tests (Vitest + React Testing Library).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant U as User
participant W as ChatWidget
participant S as Chat Store
participant API as Chat API
participant SR as Server/Stream

U->>W: submit message (or quick reply)
W->>S: dispatch add user message; set processing/thinking
W->>API: start send (POST or streaming)
API->>SR: HTTP request / SSE handshake
SR-->>API: SSE frames / final snapshot
API-->>W: yield SSEEvent / return snapshot
W->>S: append token events / update conversation state; clear processing

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 A tiny hop from old to new,
Vite and Context stitched in view.
Streams that trickle, tokens that play,
Themes that shift from night to day.
My whiskers twitch — the chat's brand new! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'Feature/version 6.0.0' is vague and uses a generic format that does not clearly describe the main changes. While it references a version bump, it lacks meaningful detail about what this release introduces. Consider revising the title to be more descriptive, such as 'Add EDDI Chat UI v6.0.0 with React 19, Vite, and SSE streaming support' to clearly communicate the major features and technology updates.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 feature/version-6.0.0

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: 14

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (9)
AGENTS.md-41-41 (1)

41-41: ⚠️ Potential issue | 🟡 Minor

Typo in filename: "ScrollToAgenttom.tsx" should be "ScrollToBottom.tsx".

This documentation is used by AI coding assistants, so accuracy is important.

📝 Proposed fix
-│   └── ScrollToAgenttom.tsx  # Floating scroll button
+│   └── ScrollToBottom.tsx    # Floating scroll button
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AGENTS.md` at line 41, Update the documentation entry that lists the
component filename: rename the documented filename "ScrollToAgenttom.tsx" to the
correct "ScrollToBottom.tsx" so the doc matches the actual component name;
locate the string "ScrollToAgenttom.tsx" in AGENTS.md and replace it with
"ScrollToBottom.tsx".
src/components/ChatHeader.test.tsx-48-54 (1)

48-54: ⚠️ Potential issue | 🟡 Minor

Test assertion does not verify theme change behavior.

The test is named "clicking theme toggle calls setTheme" but only asserts that the toggle button exists after clicking. This will always pass and doesn't verify the actual theme change. Assert that document.documentElement has data-theme="light" after the click.

🧪 Proposed fix to verify theme change
   it("clicking theme toggle calls setTheme", () => {
     renderHeader();
     const toggle = screen.getByTestId("theme-toggle");
+    // Default theme is "dark", clicking should switch to "light"
     fireEvent.click(toggle);
-    // Theme changes are reflected on the document element
-    expect(toggle).toBeInTheDocument();
+    expect(document.documentElement.getAttribute("data-theme")).toBe("light");
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatHeader.test.tsx` around lines 48 - 54, The test "clicking
theme toggle calls setTheme" only checks the toggle exists after click; change
it to assert the actual theme change by clicking the element with test id
"theme-toggle" (from renderHeader()) and then asserting that
document.documentElement has data-theme="light" (or use
expect(document.documentElement).toHaveAttribute("data-theme", "light")) to
verify setTheme behavior.
src/components/MessageBubble.test.tsx-42-45 (1)

42-45: ⚠️ Potential issue | 🟡 Minor

Test name claims to verify both avatars but only checks user.

The test title says "shows avatar with U for user and E for agent" but only verifies the user case. Add a separate assertion for the agent avatar.

🧪 Proposed fix to test both avatars
   it("shows avatar with U for user and E for agent", () => {
     const { container } = renderBubble({ id: "6", role: "user", content: "Hi", timestamp: 0 });
     expect(container.querySelector(".message__avatar")?.textContent).toBe("U");
+  });
+
+  it("shows avatar with E for agent", () => {
+    const { container } = renderBubble({ id: "7", role: "agent", content: "Hi", timestamp: 0 });
+    expect(container.querySelector(".message__avatar")?.textContent).toBe("E");
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/MessageBubble.test.tsx` around lines 42 - 45, The test "shows
avatar with U for user and E for agent" only checks the user avatar; update it
to also render the agent case by calling renderBubble with role set to the agent
value (e.g., renderBubble({ id: "7", role: "agent", content: "Hi", timestamp: 0
})) and add an assertion that
container.querySelector(".message__avatar")?.textContent equals "E"; keep the
original user assertion and ensure you reference the renderBubble helper and the
.message__avatar selector when adding the new agent assertion.
package.json-3-3 (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Version mismatch with PR title.

package.json specifies "version": "3.0.0" but the PR is titled "Feature/version 6.0.0". Please verify the intended version number.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 3, The package.json "version" field currently reads
"3.0.0" and conflicts with the PR title "Feature/version 6.0.0"; update the
"version" value in package.json to the intended release number (e.g., "6.0.0")
or change the PR title to match the existing version, ensuring the package.json
"version" entry is the single source of truth and consistent with the PR intent.
README.md-66-66 (1)

66-66: ⚠️ Potential issue | 🟡 Minor

Typo in project structure.

ScrollToAgenttom.tsx should be ScrollToBottom.tsx.

📝 Proposed fix
-│   └── ScrollToAgenttom.tsx
+│   └── ScrollToBottom.tsx
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 66, The README contains a typo in the project structure:
the file listed as "ScrollToAgenttom.tsx" should be "ScrollToBottom.tsx"; rename
the file to ScrollToBottom.tsx and update any references (imports/usages) that
mention ScrollToAgenttom.tsx to use ScrollToBottom.tsx instead (search for
"ScrollToAgenttom" and replace with "ScrollToBottom" in README and source code).
src/components/ChatWidget.tsx-451-455 (1)

451-455: ⚠️ Potential issue | 🟡 Minor

Default missing undo/redo flags to false.

The ?? true fallbacks here make undo/redo look available when the backend omits the flags, so the UI can advertise actions that do not exist.

🔁 Suggested fix
       dispatch({
         type: "SET_UNDO_REDO",
         undoAvailable: snapshot.undoAvailable ?? false,
-        redoAvailable: snapshot.redoAvailable ?? true,
+        redoAvailable: snapshot.redoAvailable ?? false,
       });
@@
       dispatch({
         type: "SET_UNDO_REDO",
-        undoAvailable: snapshot.undoAvailable ?? true,
+        undoAvailable: snapshot.undoAvailable ?? false,
         redoAvailable: snapshot.redoAvailable ?? false,
       });

Also applies to: 496-500

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 451 - 455, The undo/redo
availability fallback is inverted: change the dispatch in the SET_UNDO_REDO
action to default both snapshot.undoAvailable and snapshot.redoAvailable to
false (use snapshot.undoAvailable ?? false and snapshot.redoAvailable ?? false)
so the UI doesn't show unavailable actions; apply the same change to the other
occurrence around the SET_UNDO_REDO dispatch at the later block (the one
referenced at lines ~496-500).
src/styles/chat.css-189-195 (1)

189-195: ⚠️ Potential issue | 🟡 Minor

Resolve the current stylelint blockers before merge.

msgIn/fadeIn violate the configured kebab-case rule, word-break: break-word is deprecated, and currentColor fails the keyword-case rule. This stylesheet will keep failing lint until those are normalized.

Also applies to: 234-234, 323-323, 387-387, 488-488, 581-581, 670-681

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/styles/chat.css` around lines 189 - 195, Rename the keyframes and their
references to kebab-case (e.g., change `@keyframes` msgIn and any animation: msgIn
to `@keyframes` msg-in and animation: msg-in; do the same for fadeIn → fade-in
wherever used), replace deprecated word-break: break-word with a supported
property such as overflow-wrap: anywhere (or word-break: break-all if you need
that behavior) in the rules that currently use it, and normalize keyword casing
by lowercasing CSS keywords like currentColor to currentcolor so they pass the
keyword-case rule; update the corresponding selectors/rules that reference
msgIn, fadeIn, word-break, and currentColor accordingly.
src/components/ChatWidget.tsx-323-381 (1)

323-381: ⚠️ Potential issue | 🟡 Minor

Finalize the streaming placeholder on send failures.

The streaming branches create an empty agent message with isStreaming: true, but the catch path only clears the global flags. If the request throws before a done event, that last bubble stays stuck in the streaming state.

🩹 Suggested fix
       } catch (err) {
         console.error("Failed to send message:", err);
-        dispatch({ type: "SET_PROCESSING", value: false });
-        dispatch({ type: "SET_THINKING", value: false });
+        dispatch({ type: "FINISH_STREAMING" });
       }

Also applies to: 396-400

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 323 - 381, When a send fails in
the streaming branches (where you call demoSendMessageStreaming or
sendMessageStreaming and dispatch ADD_MESSAGE with isStreaming: true), ensure
the catch/failure path both dispatches FINISH_STREAMING and updates the
placeholder agent message created by ADD_MESSAGE to clear isStreaming (and
optionally set an error/content like "Failed to send"). Concretely: in the catch
blocks for the demo and real streaming paths (surrounding sendMessageStreaming
and demoSendMessageStreaming/for-await loops), dispatch { type:
"FINISH_STREAMING" } and then dispatch an update to the same message id created
(e.g., dispatch { type: "UPDATE_MESSAGE", id: <same-id>, message: { isStreaming:
false, content: "<error text>" } } or whatever reducer action your code uses to
update messages) so the bubble is finalized instead of stuck streaming;
reference ADD_MESSAGE, FINISH_STREAMING, sendMessageStreaming,
demoSendMessageStreaming and handleSSEEvent to locate the spots to change.
src/api/chat-api.ts-189-197 (1)

189-197: ⚠️ Potential issue | 🟡 Minor

Branch on undefined, not truthiness, for message.

Line 189 treats "" as “load conversation” and sends a GET instead of a POST. If blank prompts are invalid, reject them explicitly; don’t let them silently change the endpoint.

Proposed fix
-  if (message) {
+  if (message !== undefined) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/chat-api.ts` around lines 189 - 197, The current conditional uses
truthiness for the `message` check (in src/api/chat-api.ts) which treats an
empty string "" as falsy and falls into the GET/“load conversation” branch;
change the branch to explicitly check for undefined (e.g., `message ===
undefined`) so only missing messages trigger loading, and add an explicit
validation that rejects empty-string prompts (throw a clear Error or return a
4xx-style rejection) before attempting the POST; update the block around the
existing fetch call so the POST path only runs when `message` is non-empty and
defined.
🧹 Nitpick comments (10)
chat.html (1)

13-38: Consider moving @font-face declarations to a shared stylesheet.

This full block is duplicated in another HTML entry file, which increases drift risk over time.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@chat.html` around lines 13 - 38, The duplicated `@font-face` block should be
extracted into a single shared stylesheet and referenced by both HTML entry
files: create a shared CSS (e.g., fonts stylesheet) containing the three
`@font-face` rules (preserving font-family 'Noto Sans', font-display, font-style
and weight variants 200/400/500 and the src URLs), remove the duplicate blocks
from chat.html and the other entry file, and add a single <link
rel="stylesheet"> include in each HTML entry that needs the fonts so both pages
load the same centralized definitions.
src/components/ChatHeader.tsx (1)

13-17: Consider: Theme cycle excludes "system" mode.

The cycleTheme function only toggles between "dark" and "light", excluding the "system" option that ThemeMode supports. If "system" theme preference is desired, consider a three-way cycle. Otherwise, this two-way toggle is fine for simplicity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatHeader.tsx` around lines 13 - 17, The cycleTheme function
currently toggles only between "dark" and "light", omitting the "system"
ThemeMode; update cycleTheme to implement a three-way cycle (e.g., dark -> light
-> system -> dark) by reading current via
document.documentElement.getAttribute("data-theme"), mapping to the next
ThemeMode value, and calling setTheme(next); ensure the logic handles
unknown/currently-null values and uses the ThemeMode union and setTheme to apply
the new mode.
src/components/Indicators.tsx (1)

10-24: Extract duplicated avatar styles to CSS or a shared constant.

The same 14-line inline style object is repeated in both TypingIndicator and ThinkingIndicator. This violates DRY and makes maintenance harder. Consider extracting to a CSS class (.indicator__avatar) or a shared constant.

♻️ Option 1: Use a shared style constant
+const indicatorAvatarStyle: React.CSSProperties = {
+  background: "var(--chat-surface-raised)",
+  color: "var(--chat-text-accent)",
+  border: "1px solid var(--chat-agent-border)",
+  width: 32,
+  height: 32,
+  borderRadius: "50%",
+  display: "flex",
+  alignItems: "center",
+  justifyContent: "center",
+  fontSize: "0.75rem",
+  fontWeight: 600,
+  flexShrink: 0,
+};
+
 export function TypingIndicator() {
   return (
     <div className="indicator" data-testid="typing-indicator">
-      <div className="message__avatar" style={{
-        background: "var(--chat-surface-raised)",
-        ...
-      }}>
+      <div className="message__avatar" style={indicatorAvatarStyle}>
         E
       </div>
       ...
♻️ Option 2: Add CSS class in chat.css

Add .indicator__avatar class in src/styles/chat.css and replace inline styles with the class name.

Also applies to: 41-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Indicators.tsx` around lines 10 - 24, The avatar inline style
block duplicated in TypingIndicator and ThinkingIndicator should be extracted to
a single source: either create a shared constant (e.g., INDICATOR_AVATAR_STYLE)
and replace the inline style props in TypingIndicator and ThinkingIndicator with
that constant, or add a CSS rule .indicator__avatar in your styles (e.g.,
src/styles/chat.css) and replace the style object with
className="message__avatar indicator__avatar" in both components; update imports
if needed and remove the duplicate style objects from both components.
AGENTS.md (1)

29-50: Add language specifier to the code block.

The project structure code block lacks a language identifier. While it's a directory tree, adding a language hint (e.g., text or plaintext) satisfies markdown linters.

📝 Proposed fix
-```
+```text
 src/
 ├── api/            # API layer
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AGENTS.md` around lines 29 - 50, Add a language specifier to the fenced code
block that shows the directory tree (the block starting with ``` and containing
"src/") so markdown linters accept it; update the opening fence from ``` to
```text (or ```plaintext) in AGENTS.md where the "src/" directory tree is
rendered.
src/hooks/useTheme.ts (1)

24-27: Unsafe cast of localStorage value.

The stored value is cast directly to ThemeMode without validation. If localStorage contains an invalid value (e.g., from manual tampering or old versions), applyTheme would set an invalid data-theme attribute.

💡 Add validation
+function isValidTheme(value: string | null): value is ThemeMode {
+  return value === "dark" || value === "light" || value === "system";
+}
+
 export function useTheme(initial: ThemeMode = "dark") {
   // Apply on mount
   useEffect(() => {
-    const stored = localStorage.getItem(STORAGE_KEY) as ThemeMode | null;
-    const mode = stored ?? initial;
+    const stored = localStorage.getItem(STORAGE_KEY);
+    const mode = isValidTheme(stored) ? stored : initial;
     applyTheme(mode);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useTheme.ts` around lines 24 - 27, The code unsafely casts the
localStorage string to ThemeMode inside the useEffect; instead read the raw
string via localStorage.getItem(STORAGE_KEY), validate it against the allowed
ThemeMode values (e.g., an array/enum of modes) and only pass a validated value
to applyTheme (otherwise fall back to the initial value). Update the logic in
useEffect to perform this validation (and optionally cleanse localStorage if the
value is invalid) before calling applyTheme, referencing STORAGE_KEY, ThemeMode,
applyTheme, initial and the useEffect block.
src/components/ChatInput.test.tsx (1)

54-60: Consider verifying onSend was called before asserting input is cleared.

The test assumes the input clears after send but doesn't verify onSend was actually invoked. If the send logic fails silently, this test could pass incorrectly.

💡 Suggested improvement
 it("clears input after send", () => {
-  renderInput();
+  const { onSend } = renderInput();
   const textarea = screen.getByTestId("chat-input") as HTMLTextAreaElement;
   fireEvent.change(textarea, { target: { value: "Hello" } });
   fireEvent.keyDown(textarea, { key: "Enter", shiftKey: false });
+  expect(onSend).toHaveBeenCalled();
   expect(textarea.value).toBe("");
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatInput.test.tsx` around lines 54 - 60, The test "clears
input after send" should verify that the send handler was invoked before
checking the textarea is empty: update the test setup in renderInput to pass a
jest mock for onSend (or spy on the existing onSend prop), trigger the Enter key
as before on the element queried with test id "chat-input", then assert
expect(onSend).toHaveBeenCalled() (optionally with the expected message) prior
to asserting textarea.value === ""; this ensures send logic ran rather than the
input clearing by other means.
src/store/chat-store.test.tsx (1)

5-11: Consider using stable test IDs.

makeMsg uses Date.now() for id and timestamp, which could cause issues if tests ever need to compare message identity or if parallel test execution causes timing conflicts. For unit tests, static or sequential IDs are more predictable.

💡 Alternative with counter
+let msgCounter = 0;
 const makeMsg = (overrides: Partial<ChatMessage> = {}): ChatMessage => ({
-  id: `msg-${Date.now()}`,
+  id: `msg-${++msgCounter}`,
   role: "agent",
   content: "Hello",
-  timestamp: Date.now(),
+  timestamp: 1000000000000,
   ...overrides,
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/chat-store.test.tsx` around lines 5 - 11, The test helper makeMsg
currently uses Date.now() for id and timestamp which yields non-deterministic
values; change makeMsg to produce stable IDs/timestamps (for example, introduce
a module-scoped counter like testMsgCounter and increment it for each created
ChatMessage, using that counter to construct id and a deterministic timestamp or
a fixed base timestamp) and ensure tests can reset the counter if needed; update
references in tests to rely on the deterministic id/timestamp and keep the
function signature makeMsg(overrides: Partial<ChatMessage> = {}) so callers can
still override values.
README.md (1)

48-48: Hardcoded test count will become stale.

The documentation states "42 tests" which will need manual updates as tests are added or removed.

💡 Suggested change
-npx vitest run       # Run tests (42 tests)
+npx vitest run       # Run tests
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 48, Update the README line containing "npx vitest run     
# Run tests (42 tests)" to remove the hardcoded count; replace the trailing
comment with a generic description like "# Run tests" or "# Run all tests" (or
dynamically mention that vitest will report the test count) so the documentation
doesn't require manual updates when tests change.
vite.config.ts (2)

13-25: Asset filename pattern may misplace non-CSS assets.

The assetFileNames pattern routes all assets to scripts/css/ with the chat-ui.[hash][extname] naming. This works for CSS but would also catch images, fonts, or other assets, placing them in a css/ directory with potentially confusing names.

💡 Suggested conditional asset naming
       output: {
         // Put JS/CSS into scripts/ to match existing EDDI structure
         entryFileNames: "scripts/js/chat-ui.[hash].js",
         chunkFileNames: "scripts/js/chat-ui-[name].[hash].js",
-        assetFileNames: "scripts/css/chat-ui.[hash][extname]",
+        assetFileNames: (assetInfo) => {
+          if (assetInfo.name?.endsWith('.css')) {
+            return "scripts/css/chat-ui.[hash][extname]";
+          }
+          return "scripts/assets/[name].[hash][extname]";
+        },
       },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 13 - 25, The assetFileNames pattern currently
sends all assets into scripts/css which will misplace images/fonts; update the
rollupOptions.output.assetFileNames in vite.config.ts to discriminate by asset
type (use an assetFileNames function or conditional pattern) so CSS files use
"scripts/css/..." while images/fonts/media use a separate path like
"scripts/assets/" or "scripts/media/..."—modify the assetFileNames option
(referencing rollupOptions.output.assetFileNames) to inspect assetInfo.extname
or assetInfo.name and return the correct directory and naming scheme for each
asset type.

13-15: Document or make the EDDI sibling directory requirement explicit.

The hardcoded outDir path assumes EDDI is checked out as a sibling directory. The README mentions this is the deployment target but doesn't state this as a requirement for local development. Add a setup section explaining that npm run build requires the EDDI repo at ../EDDI, or make the path conditional based on an environment variable with a clear error message if not set.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 13 - 15, The build config in vite.config.ts
hardcodes outDir to resolve(__dirname,
"../EDDI/src/main/resources/META-INF/resources"), which assumes the EDDI repo is
a sibling; update to either document this requirement in README setup (add a
"Local development and build" section stating npm run build requires EDDI
checked out at ../EDDI) or make the path configurable and fail fast: change the
build.outDir to use an environment variable (e.g. process.env.EDDI_OUTDIR or
process.env.EDDI_DIR resolved into the same path) and add a preflight check in
vite.config.ts that verifies the resolved directory exists and throws a clear
error message if not set/found, referencing outDir and the config/build object
so reviewers can find the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@chat.html`:
- Line 5: The meta viewport tag currently includes accessibility-blocking
attributes (maximum-scale=1.0 and user-scalable=no); edit the meta tag that sets
the viewport to remove those attributes (or set them to allow zoom, e.g., omit
maximum-scale and user-scalable or set user-scalable=yes and a reasonable
maximum-scale) so mobile users can pinch-zoom — locate the meta name="viewport"
tag in chat.html and update its attributes accordingly.

In `@index.html`:
- Line 5: Update the viewport meta tag (the <meta name="viewport" ...> element)
so it no longer disables pinch-zoom: remove the maximum-scale and user-scalable
attributes (or set user-scalable to yes and a reasonable maximum-scale) and keep
width=device-width, initial-scale=1.0 to restore accessibility-friendly zooming.

In `@src/api/chat-api.ts`:
- Line 37: URLs interpolate raw path segments (agentId, conversationId, intent,
userId) which can break if they contain "/", "?", or "#" — create a small helper
(e.g., encodePathSegment(value: string) => encodeURIComponent(value)) and use it
when building path templates passed to buildUrl: replace occurrences like
`/agents/${agentId}/start${params}` with
`/agents/${encodePathSegment(agentId)}/start${params}` and likewise encode
conversationId, intent, and userId in all buildUrl/template-string usages (the
ones flagged: the calls that interpolate agentId, conversationId, intent,
userId) while leaving query strings/params unchanged.
- Around line 42-45: The code currently falls back to an empty string when
reading res.headers.get("Location"), which can produce an empty conversation id;
instead, validate the Location header and fail fast: after reading const
location = res.headers.get("Location"), if it's null or not a non-empty string
that contains at least one "/" and a final segment, throw a descriptive error
(e.g., "Missing or malformed Location header in POST response") rather than
returning an empty id; update the logic around location, segments and last so
malformed values are detected and rejected before splitting query params.
- Around line 146-163: Normalize CRLF and support multi-line SSE `data:` fields
in the parser: before splitting, replace CRLF ("\r\n") with "\n" (operate on the
`buffer` used in the loop) and then split on "\n\n" so `\r\n\r\n` doesn't stall;
when parsing each `part` inside the `for (const line of part.split("\n"))` loop,
append subsequent `data:` lines to `eventData` (e.g., join with "\n") instead of
overwriting, and only trim the `data:` prefix (don't drop internal newlines).
Keep the existing `eventType: SSEEventType` logic but ensure you yield using
`yield { type: eventType, data: eventData }` after handling concatenated
multi-line data.

In `@src/components/ChatInput.tsx`:
- Around line 21-24: The send logic currently calls value.trim() in handleSend
(and the other send point around line 57), which mutates secret-mode inputs;
change the behavior so that when isSecretMode is true you forward the raw value
(no trim) to onSend, otherwise continue to trim/normalize as before—update both
handleSend and the other send handler to conditionally use either value (raw) or
value.trim() and ensure canSend/validation uses the same conditional
normalization.

In `@src/components/ChatWidget.tsx`:
- Around line 262-265: The ChatInput is becoming enabled on managed-agent routes
before the initial managed bootstrap completes; modify the logic around
sendManagedAgentMessage and processSnapshot to block input until the bootstrap
finishes by adding a bootstrapping flag (e.g., isBootstrappingManagedAgent) or
by requiring a valid conversationId before enabling ChatInput. Specifically, set
isBootstrappingManagedAgent = true before calling
sendManagedAgentMessage(intent, userId), set it false in the finally/after
processSnapshot (and when snapshot contains/sets conversationId), and change the
ChatInput enable condition to check !isBootstrappingManagedAgent (or require
conversationId) so users cannot submit while the managed snapshot is in-flight
(also apply the same guard where similar managed bootstrap logic appears).
- Around line 249-290: The effect that bootstraps conversations (useEffect with
initializedRef and init) is pinned to mount and doesn't react to route/context
changes; update it to depend on the route-related values (at least isDemo,
isManagedAgent, intent, userId, environment, agentId) and ensure initializedRef
is reset so the init flow runs again when those change (either remove the
initializedRef guard or set initializedRef.current = false before
re-initializing); keep the same bootstrap calls (demoStartConversation,
sendManagedAgentMessage, startConversation, readConversation, processSnapshot,
dispatch) but trigger them whenever the above dependencies change.
- Around line 517-536: handleRestart can let in-flight async work
(handleSend/SSE/processSnapshot) append stale tokens into the new conversation;
fix by creating a restart-fence (e.g., bump a sessionId or create an
AbortController token inside handleRestart and store it in component state/ref)
and have all async producers/consumers (handleSend, SSE handlers,
processSnapshot, startConversation/demoStartConversation follow-ups) check the
current sessionId or abort signal before dispatching state (or cancel their
work). Ensure handleRestart increments/creates the fence before any async calls
and that processSnapshot/handleSend/readConversation early-return if the fence
changed or signal.aborted so no stale messages/quick replies are dispatched into
the new conversation.
- Around line 521-532: The restart logic misses the managed-agent route (intent
+ userId); add an else-if branch (similar to the demo and direct-agent branches
near demoStartConversation and startConversation) that detects the managed route
(e.g., route.type === "intent" && userId) and calls the managed restart API (use
your existing managed restart helper, e.g., restartManagedAgentConversation or
startManagedConversation with intent and userId), then dispatch
SET_CONVERSATION_ID with the returned id and load the conversation via
readConversation and processSnapshot (and set conversation state to READY) so
the "Start New Conversation" button works for managed agents too.

In `@src/components/MessageBubble.tsx`:
- Around line 33-38: The ReactMarkdown usage in MessageBubble.tsx currently
passes rehypeRaw which renders unsanitized HTML from message.content; remove
rehypeRaw from the rehypePlugins array OR replace it with a sanitizing plugin
(import rehypeSanitize from 'rehype-sanitize' and add it to rehypePlugins) and
ensure you install rehype-sanitize if choosing the sanitization route; update
the ReactMarkdown props (where ReactMarkdown, remarkGfm, rehypeRaw, and
message.content are referenced) so raw HTML is not rendered unsafely.

In `@src/components/SecretInput.tsx`:
- Around line 31-37: The submit handler handleSubmit is incorrectly normalizing
secrets by calling value.trim(), which strips valid leading/trailing whitespace;
remove the trim and pass the raw value to onSend (use value directly) and keep
clearing the input via setValue("") and dispatch({ type: "CLEAR_INPUT_FIELD" })
as before; apply the same change to the other submit handler referenced (the
block around lines 85-87) so neither uses trim() before calling onSend, and
ensure any button enabled/disabled checks use a test that preserves
whitespace-only secrets if that is intended (or explicitly check for zero-length
with value.length === 0).
- Around line 54-89: Give the input an explicit id and bind the visible label to
it (set id on the <input> and add htmlFor on secret-input__label), and add
accessible names to both icon-only buttons: give the eye toggle button
(secret-input__eye-toggle) an aria-label like "Show secret" / "Hide secret"
(toggle based on visible) and aria-pressed={visible}; give the send button
(chat-input__send) an aria-label like "Submit secret" (or include the trimmed
value context if desired). Ensure these attributes are applied alongside
existing title/disabled props and keep handleSubmit/handleKeyDown/value/visible
logic unchanged.

In `@tsconfig.json`:
- Around line 2-5: Add a new tsconfig.node.json to cover Node/tooling files and
update the root tsconfig.json references: create tsconfig.node.json with
appropriate compilerOptions (e.g., target ES2022, module ESNext,
moduleResolution "bundler", strict true, skipLibCheck true, esModuleInterop
true, resolveJsonModule true) and an include array containing "vite.config.ts"
and "vitest.config.ts", then modify the root tsconfig.json's "references" array
to include { "path": "./tsconfig.node.json" } alongside the existing { "path":
"./tsconfig.app.json" } so the tooling files inherit the project's strict
settings.

---

Minor comments:
In `@AGENTS.md`:
- Line 41: Update the documentation entry that lists the component filename:
rename the documented filename "ScrollToAgenttom.tsx" to the correct
"ScrollToBottom.tsx" so the doc matches the actual component name; locate the
string "ScrollToAgenttom.tsx" in AGENTS.md and replace it with
"ScrollToBottom.tsx".

In `@package.json`:
- Line 3: The package.json "version" field currently reads "3.0.0" and conflicts
with the PR title "Feature/version 6.0.0"; update the "version" value in
package.json to the intended release number (e.g., "6.0.0") or change the PR
title to match the existing version, ensuring the package.json "version" entry
is the single source of truth and consistent with the PR intent.

In `@README.md`:
- Line 66: The README contains a typo in the project structure: the file listed
as "ScrollToAgenttom.tsx" should be "ScrollToBottom.tsx"; rename the file to
ScrollToBottom.tsx and update any references (imports/usages) that mention
ScrollToAgenttom.tsx to use ScrollToBottom.tsx instead (search for
"ScrollToAgenttom" and replace with "ScrollToBottom" in README and source code).

In `@src/api/chat-api.ts`:
- Around line 189-197: The current conditional uses truthiness for the `message`
check (in src/api/chat-api.ts) which treats an empty string "" as falsy and
falls into the GET/“load conversation” branch; change the branch to explicitly
check for undefined (e.g., `message === undefined`) so only missing messages
trigger loading, and add an explicit validation that rejects empty-string
prompts (throw a clear Error or return a 4xx-style rejection) before attempting
the POST; update the block around the existing fetch call so the POST path only
runs when `message` is non-empty and defined.

In `@src/components/ChatHeader.test.tsx`:
- Around line 48-54: The test "clicking theme toggle calls setTheme" only checks
the toggle exists after click; change it to assert the actual theme change by
clicking the element with test id "theme-toggle" (from renderHeader()) and then
asserting that document.documentElement has data-theme="light" (or use
expect(document.documentElement).toHaveAttribute("data-theme", "light")) to
verify setTheme behavior.

In `@src/components/ChatWidget.tsx`:
- Around line 451-455: The undo/redo availability fallback is inverted: change
the dispatch in the SET_UNDO_REDO action to default both snapshot.undoAvailable
and snapshot.redoAvailable to false (use snapshot.undoAvailable ?? false and
snapshot.redoAvailable ?? false) so the UI doesn't show unavailable actions;
apply the same change to the other occurrence around the SET_UNDO_REDO dispatch
at the later block (the one referenced at lines ~496-500).
- Around line 323-381: When a send fails in the streaming branches (where you
call demoSendMessageStreaming or sendMessageStreaming and dispatch ADD_MESSAGE
with isStreaming: true), ensure the catch/failure path both dispatches
FINISH_STREAMING and updates the placeholder agent message created by
ADD_MESSAGE to clear isStreaming (and optionally set an error/content like
"Failed to send"). Concretely: in the catch blocks for the demo and real
streaming paths (surrounding sendMessageStreaming and
demoSendMessageStreaming/for-await loops), dispatch { type: "FINISH_STREAMING" }
and then dispatch an update to the same message id created (e.g., dispatch {
type: "UPDATE_MESSAGE", id: <same-id>, message: { isStreaming: false, content:
"<error text>" } } or whatever reducer action your code uses to update messages)
so the bubble is finalized instead of stuck streaming; reference ADD_MESSAGE,
FINISH_STREAMING, sendMessageStreaming, demoSendMessageStreaming and
handleSSEEvent to locate the spots to change.

In `@src/components/MessageBubble.test.tsx`:
- Around line 42-45: The test "shows avatar with U for user and E for agent"
only checks the user avatar; update it to also render the agent case by calling
renderBubble with role set to the agent value (e.g., renderBubble({ id: "7",
role: "agent", content: "Hi", timestamp: 0 })) and add an assertion that
container.querySelector(".message__avatar")?.textContent equals "E"; keep the
original user assertion and ensure you reference the renderBubble helper and the
.message__avatar selector when adding the new agent assertion.

In `@src/styles/chat.css`:
- Around line 189-195: Rename the keyframes and their references to kebab-case
(e.g., change `@keyframes` msgIn and any animation: msgIn to `@keyframes` msg-in and
animation: msg-in; do the same for fadeIn → fade-in wherever used), replace
deprecated word-break: break-word with a supported property such as
overflow-wrap: anywhere (or word-break: break-all if you need that behavior) in
the rules that currently use it, and normalize keyword casing by lowercasing CSS
keywords like currentColor to currentcolor so they pass the keyword-case rule;
update the corresponding selectors/rules that reference msgIn, fadeIn,
word-break, and currentColor accordingly.

---

Nitpick comments:
In `@AGENTS.md`:
- Around line 29-50: Add a language specifier to the fenced code block that
shows the directory tree (the block starting with ``` and containing "src/") so
markdown linters accept it; update the opening fence from ``` to ```text (or
```plaintext) in AGENTS.md where the "src/" directory tree is rendered.

In `@chat.html`:
- Around line 13-38: The duplicated `@font-face` block should be extracted into a
single shared stylesheet and referenced by both HTML entry files: create a
shared CSS (e.g., fonts stylesheet) containing the three `@font-face` rules
(preserving font-family 'Noto Sans', font-display, font-style and weight
variants 200/400/500 and the src URLs), remove the duplicate blocks from
chat.html and the other entry file, and add a single <link rel="stylesheet">
include in each HTML entry that needs the fonts so both pages load the same
centralized definitions.

In `@README.md`:
- Line 48: Update the README line containing "npx vitest run       # Run tests
(42 tests)" to remove the hardcoded count; replace the trailing comment with a
generic description like "# Run tests" or "# Run all tests" (or dynamically
mention that vitest will report the test count) so the documentation doesn't
require manual updates when tests change.

In `@src/components/ChatHeader.tsx`:
- Around line 13-17: The cycleTheme function currently toggles only between
"dark" and "light", omitting the "system" ThemeMode; update cycleTheme to
implement a three-way cycle (e.g., dark -> light -> system -> dark) by reading
current via document.documentElement.getAttribute("data-theme"), mapping to the
next ThemeMode value, and calling setTheme(next); ensure the logic handles
unknown/currently-null values and uses the ThemeMode union and setTheme to apply
the new mode.

In `@src/components/ChatInput.test.tsx`:
- Around line 54-60: The test "clears input after send" should verify that the
send handler was invoked before checking the textarea is empty: update the test
setup in renderInput to pass a jest mock for onSend (or spy on the existing
onSend prop), trigger the Enter key as before on the element queried with test
id "chat-input", then assert expect(onSend).toHaveBeenCalled() (optionally with
the expected message) prior to asserting textarea.value === ""; this ensures
send logic ran rather than the input clearing by other means.

In `@src/components/Indicators.tsx`:
- Around line 10-24: The avatar inline style block duplicated in TypingIndicator
and ThinkingIndicator should be extracted to a single source: either create a
shared constant (e.g., INDICATOR_AVATAR_STYLE) and replace the inline style
props in TypingIndicator and ThinkingIndicator with that constant, or add a CSS
rule .indicator__avatar in your styles (e.g., src/styles/chat.css) and replace
the style object with className="message__avatar indicator__avatar" in both
components; update imports if needed and remove the duplicate style objects from
both components.

In `@src/hooks/useTheme.ts`:
- Around line 24-27: The code unsafely casts the localStorage string to
ThemeMode inside the useEffect; instead read the raw string via
localStorage.getItem(STORAGE_KEY), validate it against the allowed ThemeMode
values (e.g., an array/enum of modes) and only pass a validated value to
applyTheme (otherwise fall back to the initial value). Update the logic in
useEffect to perform this validation (and optionally cleanse localStorage if the
value is invalid) before calling applyTheme, referencing STORAGE_KEY, ThemeMode,
applyTheme, initial and the useEffect block.

In `@src/store/chat-store.test.tsx`:
- Around line 5-11: The test helper makeMsg currently uses Date.now() for id and
timestamp which yields non-deterministic values; change makeMsg to produce
stable IDs/timestamps (for example, introduce a module-scoped counter like
testMsgCounter and increment it for each created ChatMessage, using that counter
to construct id and a deterministic timestamp or a fixed base timestamp) and
ensure tests can reset the counter if needed; update references in tests to rely
on the deterministic id/timestamp and keep the function signature
makeMsg(overrides: Partial<ChatMessage> = {}) so callers can still override
values.

In `@vite.config.ts`:
- Around line 13-25: The assetFileNames pattern currently sends all assets into
scripts/css which will misplace images/fonts; update the
rollupOptions.output.assetFileNames in vite.config.ts to discriminate by asset
type (use an assetFileNames function or conditional pattern) so CSS files use
"scripts/css/..." while images/fonts/media use a separate path like
"scripts/assets/" or "scripts/media/..."—modify the assetFileNames option
(referencing rollupOptions.output.assetFileNames) to inspect assetInfo.extname
or assetInfo.name and return the correct directory and naming scheme for each
asset type.
- Around line 13-15: The build config in vite.config.ts hardcodes outDir to
resolve(__dirname, "../EDDI/src/main/resources/META-INF/resources"), which
assumes the EDDI repo is a sibling; update to either document this requirement
in README setup (add a "Local development and build" section stating npm run
build requires EDDI checked out at ../EDDI) or make the path configurable and
fail fast: change the build.outDir to use an environment variable (e.g.
process.env.EDDI_OUTDIR or process.env.EDDI_DIR resolved into the same path) and
add a preflight check in vite.config.ts that verifies the resolved directory
exists and throws a clear error message if not set/found, referencing outDir and
the config/build object so reviewers can find the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 90337052-0f65-4c28-a05c-221c090e5517

📥 Commits

Reviewing files that changed from the base of the PR and between 85ac895 and ac4e08d.

⛔ Files ignored due to path filters (13)
  • dist/assets/index-BCEfKpWy.css is excluded by !**/dist/**
  • dist/assets/index-CSxR_bJz.js is excluded by !**/dist/**
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-200.ttf is excluded by !**/dist/**, !**/*.ttf
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-200.woff2 is excluded by !**/dist/**, !**/*.woff2
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-500.ttf is excluded by !**/dist/**, !**/*.ttf
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-500.woff2 is excluded by !**/dist/**, !**/*.woff2
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-regular.ttf is excluded by !**/dist/**, !**/*.ttf
  • dist/fonts/noto-sans-v36-cyrillic_cyrillic-ext_devanagari_greek_greek-ext_latin_latin-ext_vietnamese-regular.woff2 is excluded by !**/dist/**, !**/*.woff2
  • dist/img/favicon.ico is excluded by !**/dist/**, !**/*.ico
  • dist/img/logo_eddi.png is excluded by !**/dist/**, !**/*.png
  • dist/index.html is excluded by !**/dist/**
  • package-lock.json is excluded by !**/package-lock.json
  • public/img/loading-indicator.svg is excluded by !**/*.svg
📒 Files selected for processing (43)
  • .env
  • .gitignore
  • .whitesource
  • AGENTS.md
  • README.md
  • chat.html
  • index.html
  • package.json
  • public/index.html
  • public/manifest.json
  • public/robots.txt
  • renovate.json
  • src/Chat.tsx
  • src/api/chat-api.ts
  • src/api/demo-api.test.ts
  • src/api/demo-api.ts
  • src/components/ChatHeader.test.tsx
  • src/components/ChatHeader.tsx
  • src/components/ChatInput.test.tsx
  • src/components/ChatInput.tsx
  • src/components/ChatWidget.tsx
  • src/components/Indicators.tsx
  • src/components/MessageBubble.test.tsx
  • src/components/MessageBubble.tsx
  • src/components/QuickReplies.test.tsx
  • src/components/QuickReplies.tsx
  • src/components/ScrollToBottom.tsx
  • src/components/SecretInput.tsx
  • src/hooks/useTheme.ts
  • src/index.css
  • src/index.js
  • src/main.tsx
  • src/store/chat-store.test.tsx
  • src/store/chat-store.tsx
  • src/styles/chat.css
  • src/styles/variables.css
  • src/test-setup.ts
  • src/types.ts
  • tsconfig.app.json
  • tsconfig.json
  • vite-env.d.ts
  • vite.config.ts
  • vitest.config.ts
💤 Files with no reviewable changes (9)
  • .env
  • renovate.json
  • .whitesource
  • public/manifest.json
  • public/robots.txt
  • src/index.js
  • src/Chat.tsx
  • src/index.css
  • public/index.html

<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove zoom-blocking viewport restrictions.

On Line 5, maximum-scale=1.0 and user-scalable=no prevent zooming, which is an accessibility blocker on mobile.

♿ Suggested fix
-  <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
+  <meta name="viewport" content="width=device-width, initial-scale=1.0" />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@chat.html` at line 5, The meta viewport tag currently includes
accessibility-blocking attributes (maximum-scale=1.0 and user-scalable=no); edit
the meta tag that sets the viewport to remove those attributes (or set them to
allow zoom, e.g., omit maximum-scale and user-scalable or set user-scalable=yes
and a reasonable maximum-scale) so mobile users can pinch-zoom — locate the meta
name="viewport" tag in chat.html and update its attributes accordingly.

<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove zoom-blocking viewport restrictions here as well.

On Line 5, disabling user scaling blocks pinch-zoom and harms accessibility.

♿ Suggested fix
-  <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
+  <meta name="viewport" content="width=device-width, initial-scale=1.0" />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@index.html` at line 5, Update the viewport meta tag (the <meta
name="viewport" ...> element) so it no longer disables pinch-zoom: remove the
maximum-scale and user-scalable attributes (or set user-scalable to yes and a
reasonable maximum-scale) and keep width=device-width, initial-scale=1.0 to
restore accessibility-friendly zooming.

): Promise<string> {
const params = userId ? `?userId=${encodeURIComponent(userId)}` : "";
const res = await fetch(
buildUrl(`/agents/${agentId}/start${params}`),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Encode path segments before interpolating them into URLs.

Line 187 is the riskiest case: if intent or userId ever contain /, ?, or #, the browser treats them as route/query delimiters instead of data. The same assumption exists for the raw agentId and conversationId interpolations on the other listed lines.

Apply the same helper to all listed URLs.

Proposed fix
+const encodePathSegment = (value: string) => encodeURIComponent(value);
+
-    buildUrl(`/agents/${agentId}/start${params}`),
+    buildUrl(`/agents/${encodePathSegment(agentId)}/start${params}`),
-  const url = buildUrl(`/agents/managed/${intent}/${userId}?${params}`);
+  const url = buildUrl(
+    `/agents/managed/${encodePathSegment(intent)}/${encodePathSegment(userId)}?${params}`,
+  );

Also applies to: 63-63, 91-91, 123-123, 187-187, 211-211, 228-228, 244-244, 261-261

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/chat-api.ts` at line 37, URLs interpolate raw path segments (agentId,
conversationId, intent, userId) which can break if they contain "/", "?", or "#"
— create a small helper (e.g., encodePathSegment(value: string) =>
encodeURIComponent(value)) and use it when building path templates passed to
buildUrl: replace occurrences like `/agents/${agentId}/start${params}` with
`/agents/${encodePathSegment(agentId)}/start${params}` and likewise encode
conversationId, intent, and userId in all buildUrl/template-string usages (the
ones flagged: the calls that interpolate agentId, conversationId, intent,
userId) while leaving query strings/params unchanged.

Comment on lines +42 to +45
const location = res.headers.get("Location") ?? "";
const segments = location.split("/");
const last = segments[segments.length - 1] || location;
return last.split("?")[0];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fail fast when the Location header is missing or malformed.

Line 42 currently falls back to "", so a successful POST can still return an empty conversation id and the next request becomes /agents/?..., which is much harder to diagnose than surfacing the contract failure here.

Proposed fix
-  const location = res.headers.get("Location") ?? "";
-  const segments = location.split("/");
-  const last = segments[segments.length - 1] || location;
-  return last.split("?")[0];
+  const location = res.headers.get("Location");
+  if (!location) {
+    throw new Error("Failed to start conversation: missing Location header");
+  }
+
+  const conversationId = location.split("/").filter(Boolean).pop()?.split("?")[0];
+  if (!conversationId) {
+    throw new Error(`Failed to start conversation: invalid Location header "${location}"`);
+  }
+
+  return decodeURIComponent(conversationId);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const location = res.headers.get("Location") ?? "";
const segments = location.split("/");
const last = segments[segments.length - 1] || location;
return last.split("?")[0];
const location = res.headers.get("Location");
if (!location) {
throw new Error("Failed to start conversation: missing Location header");
}
const conversationId = location.split("/").filter(Boolean).pop()?.split("?")[0];
if (!conversationId) {
throw new Error(`Failed to start conversation: invalid Location header "${location}"`);
}
return decodeURIComponent(conversationId);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/chat-api.ts` around lines 42 - 45, The code currently falls back to
an empty string when reading res.headers.get("Location"), which can produce an
empty conversation id; instead, validate the Location header and fail fast:
after reading const location = res.headers.get("Location"), if it's null or not
a non-empty string that contains at least one "/" and a final segment, throw a
descriptive error (e.g., "Missing or malformed Location header in POST
response") rather than returning an empty id; update the logic around location,
segments and last so malformed values are detected and rejected before splitting
query params.

Comment on lines +146 to +163
const parts = buffer.split("\n\n");
buffer = parts.pop() ?? "";

for (const part of parts) {
if (!part.trim()) continue;
let eventType: SSEEventType = "token";
let eventData = "";

for (const line of part.split("\n")) {
if (line.startsWith("event:")) {
eventType = line.slice(6).trim() as SSEEventType;
} else if (line.startsWith("data:")) {
eventData = line.slice(5).trim();
}
}

if (eventData || eventType) {
yield { type: eventType, data: eventData };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make the SSE parser tolerant of CRLF and multi-line data: fields.

Line 146 only splits on \n\n, so a valid \r\n\r\n stream can sit in buffer forever. Lines 154-158 also overwrite repeated data: lines instead of joining them, which truncates multi-line payloads.

Proposed fix
-      const parts = buffer.split("\n\n");
+      const parts = buffer.split(/\r?\n\r?\n/);
       buffer = parts.pop() ?? "";
@@
-        let eventType: SSEEventType = "token";
-        let eventData = "";
+        let eventType: SSEEventType = "token";
+        const dataLines: string[] = [];
+        let sawField = false;
 
-        for (const line of part.split("\n")) {
+        for (const line of part.split(/\r?\n/)) {
+          if (!line || line.startsWith(":")) continue;
+          sawField = true;
           if (line.startsWith("event:")) {
             eventType = line.slice(6).trim() as SSEEventType;
           } else if (line.startsWith("data:")) {
-            eventData = line.slice(5).trim();
+            dataLines.push(line.slice(5).replace(/^ /, ""));
           }
         }
 
-        if (eventData || eventType) {
-          yield { type: eventType, data: eventData };
+        if (sawField) {
+          yield { type: eventType, data: dataLines.join("\n") };
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/chat-api.ts` around lines 146 - 163, Normalize CRLF and support
multi-line SSE `data:` fields in the parser: before splitting, replace CRLF
("\r\n") with "\n" (operate on the `buffer` used in the loop) and then split on
"\n\n" so `\r\n\r\n` doesn't stall; when parsing each `part` inside the `for
(const line of part.split("\n"))` loop, append subsequent `data:` lines to
`eventData` (e.g., join with "\n") instead of overwriting, and only trim the
`data:` prefix (don't drop internal newlines). Keep the existing `eventType:
SSEEventType` logic but ensure you yield using `yield { type: eventType, data:
eventData }` after handling concatenated multi-line data.

Comment on lines +521 to +532
if (isDemo) {
const result = await demoStartConversation();
dispatch({ type: "SET_CONVERSATION_ID", id: result.conversationId });
dispatch({ type: "ADD_MESSAGE", message: result.welcomeMessage });
dispatch({ type: "SET_QUICK_REPLIES", replies: result.quickReplies });
dispatch({ type: "SET_CONVERSATION_STATE", state: "READY" });
} else if (environment && agentId) {
const convId = await startConversation(environment, agentId, userId);
dispatch({ type: "SET_CONVERSATION_ID", id: convId });
const snapshot = await readConversation(environment, agentId, convId);
processSnapshot(snapshot);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add the managed-agent restart path here.

This handler only reboots demo and direct-agent conversations. On managed-agent routes (intent + userId), the ended-state “Start New Conversation” button becomes a no-op because there is no matching branch.

🔄 Suggested fix
     try {
       if (isDemo) {
         const result = await demoStartConversation();
         dispatch({ type: "SET_CONVERSATION_ID", id: result.conversationId });
         dispatch({ type: "ADD_MESSAGE", message: result.welcomeMessage });
         dispatch({ type: "SET_QUICK_REPLIES", replies: result.quickReplies });
         dispatch({ type: "SET_CONVERSATION_STATE", state: "READY" });
+      } else if (isManagedAgent && intent && userId) {
+        const snapshot = await sendManagedAgentMessage(intent, userId);
+        processSnapshot(snapshot);
       } else if (environment && agentId) {
         const convId = await startConversation(environment, agentId, userId);
         dispatch({ type: "SET_CONVERSATION_ID", id: convId });
         const snapshot = await readConversation(environment, agentId, convId);
         processSnapshot(snapshot);

Also applies to: 602-607

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 521 - 532, The restart logic
misses the managed-agent route (intent + userId); add an else-if branch (similar
to the demo and direct-agent branches near demoStartConversation and
startConversation) that detects the managed route (e.g., route.type === "intent"
&& userId) and calls the managed restart API (use your existing managed restart
helper, e.g., restartManagedAgentConversation or startManagedConversation with
intent and userId), then dispatch SET_CONVERSATION_ID with the returned id and
load the conversation via readConversation and processSnapshot (and set
conversation state to READY) so the "Start New Conversation" button works for
managed agents too.

Comment on lines +33 to +38
<ReactMarkdown
remarkPlugins={[remarkGfm]}
rehypePlugins={[rehypeRaw]}
>
{message.content}
</ReactMarkdown>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

XSS vulnerability: rehypeRaw renders unsanitized HTML.

Using rehypeRaw allows raw HTML in markdown to render directly. Per the context snippets, message content from APPEND_TO_LAST_AGENT and ADD_MESSAGE is stored without sanitization. If the backend or SSE stream includes malicious HTML/scripts, they will execute in the user's browser.

Either remove rehypeRaw or add rehype-sanitize to the plugin chain.

🔒 Option 1: Remove rehypeRaw (safest)
 import ReactMarkdown from "react-markdown";
 import remarkGfm from "remark-gfm";
-import rehypeRaw from "rehype-raw";
 
 ...
 
 <ReactMarkdown
   remarkPlugins={[remarkGfm]}
-  rehypePlugins={[rehypeRaw]}
 >
🔒 Option 2: Add sanitization
 import rehypeRaw from "rehype-raw";
+import rehypeSanitize from "rehype-sanitize";
 
 ...
 
 <ReactMarkdown
   remarkPlugins={[remarkGfm]}
-  rehypePlugins={[rehypeRaw]}
+  rehypePlugins={[rehypeRaw, rehypeSanitize]}
 >

This requires adding rehype-sanitize to dependencies.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ReactMarkdown
remarkPlugins={[remarkGfm]}
rehypePlugins={[rehypeRaw]}
>
{message.content}
</ReactMarkdown>
<ReactMarkdown
remarkPlugins={[remarkGfm]}
>
{message.content}
</ReactMarkdown>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/MessageBubble.tsx` around lines 33 - 38, The ReactMarkdown
usage in MessageBubble.tsx currently passes rehypeRaw which renders unsanitized
HTML from message.content; remove rehypeRaw from the rehypePlugins array OR
replace it with a sanitizing plugin (import rehypeSanitize from
'rehype-sanitize' and add it to rehypePlugins) and ensure you install
rehype-sanitize if choosing the sanitization route; update the ReactMarkdown
props (where ReactMarkdown, remarkGfm, rehypeRaw, and message.content are
referenced) so raw HTML is not rendered unsafely.

Comment on lines +31 to +37
const handleSubmit = useCallback(() => {
const trimmed = value.trim();
if (!trimmed || disabled) return;
onSend(trimmed, true);
setValue("");
dispatch({ type: "CLEAR_INPUT_FIELD" });
}, [value, disabled, onSend, dispatch]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Don't normalize secret input before submitting it.

trim() changes the secret itself, and the button state enforces the same loss. Leading/trailing whitespace is valid data for passwords, tokens, and passphrases.

🔐 Suggested fix
   const handleSubmit = useCallback(() => {
-    const trimmed = value.trim();
-    if (!trimmed || disabled) return;
-    onSend(trimmed, true);
+    if (value.length === 0 || disabled) return;
+    onSend(value, true);
     setValue("");
     dispatch({ type: "CLEAR_INPUT_FIELD" });
   }, [value, disabled, onSend, dispatch]);
@@
-          className={`chat-input__send ${value.trim() && !disabled ? "chat-input__send--active" : "chat-input__send--disabled"}`}
+          className={`chat-input__send ${value.length > 0 && !disabled ? "chat-input__send--active" : "chat-input__send--disabled"}`}
           onClick={handleSubmit}
-          disabled={!value.trim() || disabled}
+          disabled={value.length === 0 || disabled}
           data-testid="secret-input-send"

Also applies to: 85-87

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SecretInput.tsx` around lines 31 - 37, The submit handler
handleSubmit is incorrectly normalizing secrets by calling value.trim(), which
strips valid leading/trailing whitespace; remove the trim and pass the raw value
to onSend (use value directly) and keep clearing the input via setValue("") and
dispatch({ type: "CLEAR_INPUT_FIELD" }) as before; apply the same change to the
other submit handler referenced (the block around lines 85-87) so neither uses
trim() before calling onSend, and ensure any button enabled/disabled checks use
a test that preserves whitespace-only secrets if that is intended (or explicitly
check for zero-length with value.length === 0).

Comment on lines +54 to +89
{label && (
<label className="secret-input__label" data-testid="secret-input-label">
🔒 {label}
</label>
)}
<div className="secret-input__row">
<div className="secret-input__field-wrapper">
<input
type={inputType}
className="secret-input__field"
value={value}
onChange={(e) => setValue(e.target.value)}
onKeyDown={handleKeyDown}
placeholder={placeholder || "Enter secret value..."}
disabled={disabled}
autoFocus
autoComplete="off"
data-testid="secret-input-field"
/>
<button
type="button"
className="secret-input__eye-toggle"
onClick={() => setVisible((v) => !v)}
title={visible ? "Hide" : "Show"}
data-testid="secret-input-eye"
>
{visible ? "👁" : "👁‍🗨"}
</button>
</div>
<button
type="button"
className={`chat-input__send ${value.trim() && !disabled ? "chat-input__send--active" : "chat-input__send--disabled"}`}
onClick={handleSubmit}
disabled={!value.trim() || disabled}
data-testid="secret-input-send"
>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Name and associate the secret controls for assistive tech.

The label is not bound to the input, and both icon-only buttons are unnamed. Screen readers will typically announce generic “button” controls here, which makes the secret flow hard to complete.

♿ Suggested fix
-import { useState, useCallback, type KeyboardEvent } from "react";
+import { useState, useCallback, useId, type KeyboardEvent } from "react";
@@
   const dispatch = useChatDispatch();
   const [value, setValue] = useState(defaultValue);
   const [visible, setVisible] = useState(false);
+  const inputId = useId();
@@
       {label && (
-        <label className="secret-input__label" data-testid="secret-input-label">
+        <label
+          htmlFor={inputId}
+          className="secret-input__label"
+          data-testid="secret-input-label"
+        >
           🔒 {label}
         </label>
       )}
@@
           <input
+            id={inputId}
+            aria-label={label ?? placeholder ?? "Secret value"}
             type={inputType}
             className="secret-input__field"
@@
           <button
             type="button"
             className="secret-input__eye-toggle"
             onClick={() => setVisible((v) => !v)}
             title={visible ? "Hide" : "Show"}
+            aria-label={visible ? "Hide secret value" : "Show secret value"}
             data-testid="secret-input-eye"
           >
@@
         <button
           type="button"
+          aria-label="Send secret value"
           className={`chat-input__send ${value.length > 0 && !disabled ? "chat-input__send--active" : "chat-input__send--disabled"}`}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SecretInput.tsx` around lines 54 - 89, Give the input an
explicit id and bind the visible label to it (set id on the <input> and add
htmlFor on secret-input__label), and add accessible names to both icon-only
buttons: give the eye toggle button (secret-input__eye-toggle) an aria-label
like "Show secret" / "Hide secret" (toggle based on visible) and
aria-pressed={visible}; give the send button (chat-input__send) an aria-label
like "Submit secret" (or include the trimmed value context if desired). Ensure
these attributes are applied alongside existing title/disabled props and keep
handleSubmit/handleKeyDown/value/visible logic unchanged.

Comment on lines +2 to +5
"files": [],
"references": [
{ "path": "./tsconfig.app.json" }
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== TS config files =="
fd '^tsconfig.*\.json$' -t f

echo
echo "== Root tsconfig.json =="
cat tsconfig.json

echo
echo "== Check for node/tooling tsconfig =="
fd '^tsconfig\.node\.json$' -t f || true

echo
echo "== Locate tooling config files =="
fd '^(vite|vitest)\.config\.ts$' -t f

echo
echo "== References/includes mentioning tooling configs =="
rg -n --glob 'tsconfig*.json' 'vite\.config\.ts|vitest\.config\.ts|references|include|files'

Repository: labsai/EDDI-Chat-UI

Length of output: 505


🏁 Script executed:

#!/bin/bash

echo "== vite.config.ts (first 30 lines) =="
head -30 vite.config.ts

echo
echo "== vitest.config.ts (first 30 lines) =="
head -30 vitest.config.ts

echo
echo "== Check tsconfig.app.json for any nested references =="
cat tsconfig.app.json

echo
echo "== Look for any build/tool related tsconfig patterns =="
find . -maxdepth 1 -name 'tsconfig*.json' -type f | xargs ls -la

echo
echo "== Check if vite/vitest have inline type annotations =="
rg -n 'defineConfig|as const' vite.config.ts vitest.config.ts || echo "No type annotations found"

Repository: labsai/EDDI-Chat-UI

Length of output: 2613


Add a separate TypeScript configuration for tooling files.

The root tsconfig.json currently references only tsconfig.app.json, leaving vite.config.ts and vitest.config.ts outside the project reference setup. These files are at the repository root but not covered by any include or files declaration, causing them to fall back to default TypeScript settings rather than the project's strict configuration.

Create tsconfig.node.json (standard convention for Node tooling) with appropriate settings and reference it in the root tsconfig.json:

Example structure
{
  "compilerOptions": {
    "target": "ES2022",
    "lib": ["ES2022"],
    "module": "ESNext",
    "moduleResolution": "bundler",
    "strict": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "resolveJsonModule": true
  },
  "include": ["vite.config.ts", "vitest.config.ts"]
}

Then update root tsconfig.json to reference both configs:

{
  "files": [],
  "references": [
    { "path": "./tsconfig.app.json" },
    { "path": "./tsconfig.node.json" }
  ]
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tsconfig.json` around lines 2 - 5, Add a new tsconfig.node.json to cover
Node/tooling files and update the root tsconfig.json references: create
tsconfig.node.json with appropriate compilerOptions (e.g., target ES2022, module
ESNext, moduleResolution "bundler", strict true, skipLibCheck true,
esModuleInterop true, resolveJsonModule true) and an include array containing
"vite.config.ts" and "vitest.config.ts", then modify the root tsconfig.json's
"references" array to include { "path": "./tsconfig.node.json" } alongside the
existing { "path": "./tsconfig.app.json" } so the tooling files inherit the
project's strict settings.

Extract quickReplies from the done event snapshot conversationOutputs
so they render in structured JSON output mode (addToOutput=false).
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: 1

🧹 Nitpick comments (3)
src/components/ChatWidget.tsx (3)

456-474: Extract duplicated message-rebuild logic into a helper.

The message reconstruction from conversationSteps is nearly identical in both handleUndo and handleRedo. Consider extracting this into a shared helper function to reduce duplication and simplify future maintenance.

♻️ Suggested helper extraction
function buildMessagesFromSteps(
  steps: Array<{ input?: string; output?: string }>
): ChatMessage[] {
  const msgs: ChatMessage[] = [];
  for (const step of steps) {
    if (step.input) {
      msgs.push({
        id: `user-${msgs.length}-${Date.now()}`,
        role: "user",
        content: step.input,
        timestamp: Date.now(),
      });
    }
    if (step.output) {
      msgs.push({
        id: `agent-${msgs.length}-${Date.now()}`,
        role: "agent",
        content: step.output,
        timestamp: Date.now(),
      });
    }
  }
  return msgs;
}

Then in both handlers:

-     const msgs: ChatMessage[] = [];
-     for (const step of snapshot.conversationSteps ?? []) {
-       // ... duplicate logic
-     }
+     const msgs = buildMessagesFromSteps(snapshot.conversationSteps ?? []);
      dispatch({ type: "REPLACE_MESSAGES", messages: msgs });

Also applies to: 501-519

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 456 - 474, Duplicate logic
building ChatMessage[] from snapshot.conversationSteps appears in handleUndo and
handleRedo; extract it into a shared helper (e.g., buildMessagesFromSteps) that
accepts steps: Array<{ input?: string; output?: string }> and returns
ChatMessage[] using the same id/role/content/timestamp construction, then
replace the inlined loops in handleUndo and handleRedo to call
buildMessagesFromSteps(snapshot.conversationSteps ?? []) so both handlers reuse
the single function and eliminate duplication.

204-240: Consider whether only processing the first conversationOutputs entry is intentional.

Line 206 only processes conversationOutputs[0], ignoring any additional outputs. If the backend ever returns multiple outputs in a single response (e.g., multi-turn or batched replies), they would be silently dropped.

If this is intentional based on the API contract, a brief comment would clarify. Otherwise, consider iterating over all outputs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 204 - 240, The code only
processes the first entry of snapshot.conversationOutputs (output =
snapshot.conversationOutputs[0]) which drops any additional outputs; update the
logic in the handler that iterates over snapshot.conversationOutputs so each
entry is processed (extract output.output into agentReplies, handle inputField
replies and ADD_MESSAGE dispatches, and collect/merge quickReplies) rather than
only using the first element; ensure you keep the same dispatch actions
(SET_INPUT_FIELD, ADD_MESSAGE, SET_QUICK_REPLIES) and preserve existing behavior
for each OutputItem so multi-entry responses are handled correctly.

129-132: Changing apiServer or apiBaseUrl won't affect in-flight requests.

The setBaseUrl call updates a module-level singleton (_baseUrl in chat-api.ts). If this effect re-runs due to prop/config changes, any requests already initiated will continue using the previous URL. In practice this is unlikely to cause issues during normal use, but be aware during HMR or rapid config changes in development.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 129 - 132, The effect in
ChatWidget calling setBaseUrl updates a module-level singleton (_baseUrl in
chat-api.ts) so changing apiServer or state.config.apiBaseUrl won't affect
requests already in-flight; fix by making request functions read the current
base URL per call or by passing the baseUrl explicitly into API functions
instead of relying on the singleton. Modify chat-api.ts to export request
helpers that accept a baseUrl parameter (or a getBaseUrl() accessor) and update
all callers in ChatWidget (and any uses of setBaseUrl) to pass the current
apiServer || state.config.apiBaseUrl value when initiating requests, or recreate
the API client when baseUrl changes to ensure new requests use the updated URL.
Ensure references to setBaseUrl, _baseUrl, and ChatWidget are updated
accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/ChatWidget.tsx`:
- Around line 362-366: The FINISH_STREAMING action may be dispatched twice
because handleSSEEvent emits FINISH_STREAMING when it receives the "done" SSE
event and the post-loop code in the send-streaming path (where
demoSendMessageStreaming is iterated) also dispatches FINISH_STREAMING; fix by
removing the redundant post-loop dispatch or adding a guard flag in the
component state (e.g., isStreaming or hasFinishedStreaming) that handleSSEEvent
sets when it dispatches FINISH_STREAMING and check that flag before dispatching
again after the for-await loop; update references in the streaming flow
(demoSendMessageStreaming loop and any post-loop dispatch sites) to consult the
guard or delete the extra dispatch so FINISH_STREAMING is only emitted once.

---

Nitpick comments:
In `@src/components/ChatWidget.tsx`:
- Around line 456-474: Duplicate logic building ChatMessage[] from
snapshot.conversationSteps appears in handleUndo and handleRedo; extract it into
a shared helper (e.g., buildMessagesFromSteps) that accepts steps: Array<{
input?: string; output?: string }> and returns ChatMessage[] using the same
id/role/content/timestamp construction, then replace the inlined loops in
handleUndo and handleRedo to call
buildMessagesFromSteps(snapshot.conversationSteps ?? []) so both handlers reuse
the single function and eliminate duplication.
- Around line 204-240: The code only processes the first entry of
snapshot.conversationOutputs (output = snapshot.conversationOutputs[0]) which
drops any additional outputs; update the logic in the handler that iterates over
snapshot.conversationOutputs so each entry is processed (extract output.output
into agentReplies, handle inputField replies and ADD_MESSAGE dispatches, and
collect/merge quickReplies) rather than only using the first element; ensure you
keep the same dispatch actions (SET_INPUT_FIELD, ADD_MESSAGE, SET_QUICK_REPLIES)
and preserve existing behavior for each OutputItem so multi-entry responses are
handled correctly.
- Around line 129-132: The effect in ChatWidget calling setBaseUrl updates a
module-level singleton (_baseUrl in chat-api.ts) so changing apiServer or
state.config.apiBaseUrl won't affect requests already in-flight; fix by making
request functions read the current base URL per call or by passing the baseUrl
explicitly into API functions instead of relying on the singleton. Modify
chat-api.ts to export request helpers that accept a baseUrl parameter (or a
getBaseUrl() accessor) and update all callers in ChatWidget (and any uses of
setBaseUrl) to pass the current apiServer || state.config.apiBaseUrl value when
initiating requests, or recreate the API client when baseUrl changes to ensure
new requests use the updated URL. Ensure references to setBaseUrl, _baseUrl, and
ChatWidget are updated accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f696fc8b-0517-4801-9818-0eafe1055cf9

📥 Commits

Reviewing files that changed from the base of the PR and between ac4e08d and 311a46a.

📒 Files selected for processing (1)
  • src/components/ChatWidget.tsx

Comment on lines +362 to +366
const events = demoSendMessageStreaming(text);
for await (const event of events) {
handleSSEEvent(event);
}
dispatch({ type: "FINISH_STREAMING" });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

FINISH_STREAMING may be dispatched twice.

When the SSE stream completes normally with a "done" event, handleSSEEvent (line 146) dispatches FINISH_STREAMING. Then, after the for await loop exits, lines 366 and 406 dispatch FINISH_STREAMING again.

This is likely harmless if the reducer is idempotent, but it's unnecessary work. Consider removing the post-loop dispatch if the "done" event is guaranteed, or add a guard to skip if streaming already finished.

🔧 Suggested fix (streaming path example)
          for await (const event of events) {
            handleSSEEvent(event);
          }
-         dispatch({ type: "FINISH_STREAMING" });
+         // FINISH_STREAMING is dispatched by handleSSEEvent on "done" event

Also applies to: 403-406

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ChatWidget.tsx` around lines 362 - 366, The FINISH_STREAMING
action may be dispatched twice because handleSSEEvent emits FINISH_STREAMING
when it receives the "done" SSE event and the post-loop code in the
send-streaming path (where demoSendMessageStreaming is iterated) also dispatches
FINISH_STREAMING; fix by removing the redundant post-loop dispatch or adding a
guard flag in the component state (e.g., isStreaming or hasFinishedStreaming)
that handleSSEEvent sets when it dispatches FINISH_STREAMING and check that flag
before dispatching again after the for-await loop; update references in the
streaming flow (demoSendMessageStreaming loop and any post-loop dispatch sites)
to consult the guard or delete the extra dispatch so FINISH_STREAMING is only
emitted once.

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