Various improvements to chat interface by ScriptSmith · Pull Request #18 · ScriptSmith/hadrian

ScriptSmith · 2026-03-21T11:04:16Z

No description provided.

greptile-apps · 2026-03-21T11:15:44Z

Greptile Summary

This PR is a substantial refactor of the chat streaming UI, replacing the old "streaming section outside virtualization" architecture with an in-virtualizer approach that integrates streaming content directly into each message group. It also introduces ContentRound for interleaved per-round rendering of multi-turn tool-calling responses, adds a compact/verbose display toggle with localStorage persistence, removes the now-redundant ToolCallIndicator component, and upgrades the thinking/processing indicator to a unified StreamingPhase model.

Key changes:

Streaming moved inside the virtualizer: The absolute-positioned streaming block is replaced by inline showStreaming logic per message group, using a committedInstanceIds check to determine which model streams are still in-flight.
ContentRound component: Encapsulates one reasoning → content → tool-execution block per iteration, replacing raw <hr> separators in multi-round responses.
CompletedRound data model: New pushCompletedRound / setCompletedRoundToolExecution / resumeStreaming actions on the streaming store allow the UI to render each iteration progressively as it completes.
Compact mode: Global compactMode flag (localStorage-persisted) collapses reasoning sections and tool details to minimal Thinking… / Processing… indicators.
Live tool-execution details are suppressed during multi-round gaps: When completedRounds is set, ToolExecutionBlock is gated off. Between rounds, only the generic "Processing…" indicator is visible — individual tool status is not shown until the round snapshot is committed, which is a regression from the previous behavior.
isToolsStreaming in ContentRound will always be false: toolExecution is attached to completed rounds only after all tools finish, so the pending | running status check never fires.
Compact mode defaults to ON: loadCompactMode() uses !== "false", silently enabling compact view for every new user.

Confidence Score: 3/5

Needs targeted fixes before merge — in-flight tool execution visibility regression and a dead-code inconsistency in the round-accumulation logic warrant resolution.
The architectural direction is solid and several prior concerns from earlier review threads have been addressed (showStreaming fix, completedRounds threaded through conversation store). However, the suppression of live tool execution details during multi-round gaps is a visible UX regression for any tool-calling flow, isToolsStreaming is structurally dead, and the compact-mode-on-by-default choice may surprise users. The completedRounds.length > 1 guard inconsistency between the return value and what actually gets committed adds maintenance confusion. These are not catastrophic but the tool-execution visibility issue affects the primary multi-round tool-calling user path.
Pay close attention to MultiModelResponse.tsx (ToolExecutionBlock suppression and isToolsStreaming), useChat.ts (completedRounds > 1 guard vs stream.completedRounds commit path), and chatUIStore.ts (compact mode default).

Important Files Changed

Filename	Overview
ui/src/components/ChatMessageList/ChatMessageList.tsx	Streaming section moved inside the virtualizer loop with a `showStreaming` guard; removes the 200px height buffer and the absolute-positioned external streaming div. The `showStreaming` logic correctly checks whether any streaming instance IDs are absent from committed responses, but prior thread concerns around single-model regeneration visibility and virtualizer mutation during render remain.
ui/src/components/MultiModelResponse/MultiModelResponse.tsx	Major refactor: replaces TypingIndicator/ToolCallIndicator with a unified StreamingPhase model, adds ContentRound-based multi-round rendering, compact mode toggle, and removes ToolExecutionBlock from the multi-round path. Two issues: (1) in-flight tool execution details are invisible during inter-round gaps when completedRounds is set; (2) isToolsStreaming on ContentRound will always be false since toolExecution is only attached after tools complete.
ui/src/components/MultiModelResponse/ContentRound.tsx	New component encapsulating a single reasoning→content→tool-execution round. Clean memo wrapping, correct useMemo for artifact resolution, handles compact/full modes. Logic is straightforward and well-structured.
ui/src/pages/chat/useChat.ts	streamWithClientSideTools refactored to accumulate CompletedRound objects and push them to the streaming store for interleaved rendering. The `completedRounds.length > 1` guard in the end-of-while return is dead code since committed messages always read completedRounds from the streaming store. The resumeStreaming call correctly re-enables the isStreaming flag between rounds.
ui/src/stores/streamingStore.ts	Adds pushCompletedRound (resets content/reasoning for next round), setCompletedRoundToolExecution, and resumeStreaming. Actions are clean and well-isolated. pushCompletedRound correctly resets ephemeral fields so next-round tokens start fresh.
ui/src/stores/chatUIStore.ts	Adds compactMode state with localStorage persistence. Default logic (`!== "false"`) silently opts all new users into compact mode on first load; may be intentional but inverts the intuitive null-check convention.

Sequence Diagram

sequenceDiagram
    participant UC as useChat
    participant SS as streamingStore
    participant MMR as MultiModelResponse
    participant CR as ContentRound

    UC->>SS: initStreaming(model)
    loop Each tool-calling iteration
        UC->>SS: appendContent / appendReasoningContent (tokens)
        UC->>SS: completeStream(usage)
        UC->>SS: pushCompletedRound({reasoning, content})<br/>resets content="" & reasoningContent=""
        UC->>SS: startExecutionRound / updateToolExecution (live tools)
        Note over MMR: completedRounds set → ToolExecutionBlock suppressed<br/>Only "Processing…" shown during tool execution
        UC->>SS: setCompletedRoundToolExecution(round)<br/>(called after tools complete)
        UC->>SS: resumeStreaming()
    end
    UC->>SS: appendContent / completeStream (final round)
    UC->>SS: pushCompletedRound({content: finalAnswer})
    UC->>SS: clearStreams()
    MMR->>CR: render each CompletedRound<br/>(reasoning + content + ExecutionSummaryBar)

Comments Outside Diff (4)

ui/src/stores/chatUIStore.ts, line 2099-2105 (link)

Compact mode ON by default may surprise users

loadCompactMode uses !== "false" as its sentinel, meaning the very first load (when localStorage is empty) returns true — compact mode is on by default, hiding reasoning sections and tool execution details until users explicitly toggle the verbose button.

If the intent is to default to compact, this is fine. But if it's a mistake, swapping the sentinel to === "true" (with a false default) would make the logic less surprising:

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/stores/chatUIStore.ts
Line: 2099-2105

Comment:
**Compact mode ON by default may surprise users**

`loadCompactMode` uses `!== "false"` as its sentinel, meaning the very first load (when localStorage is empty) returns `true` — compact mode is **on** by default, hiding reasoning sections and tool execution details until users explicitly toggle the verbose button.

If the intent is to default to compact, this is fine. But if it's a mistake, swapping the sentinel to `=== "true"` (with a `false` default) would make the logic less surprising:



How can I resolve this? If you propose a fix, please make it concise.

ui/src/pages/chat/useChat.ts, line 1985 (link)

result.completedRounds field is dead code for single-model responses

The final return at the end of the while loop (after break) guards completedRounds on allCompletedRounds.length > 1:

completedRounds: allCompletedRounds.length > 1 ? allCompletedRounds : undefined,

But in every commit site for single-model responses, completedRounds is read directly from the streaming store, not from this return value:

completedRounds: stream?.completedRounds.length ? stream.completedRounds : undefined,

stream.completedRounds is populated by pushCompletedRound, which is called for every round (including single-round responses), so a single-round tool-enabled response always ends up with completedRounds = [{content}] in the committed message — regardless of the > 1 guard here. The guard is effectively a no-op for committed messages.

Consider either:

Removing the > 1 condition here for consistency, or
Resetting stream.completedRounds to [] at the end of a single-round response to prevent inadvertently storing a 1-element list that triggers the multi-round renderer for simple responses.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/pages/chat/useChat.ts
Line: 1985

Comment:
**`result.completedRounds` field is dead code for single-model responses**

The final return at the end of the while loop (after `break`) guards `completedRounds` on `allCompletedRounds.length > 1`:

```ts
completedRounds: allCompletedRounds.length > 1 ? allCompletedRounds : undefined,
```

But in every commit site for single-model responses, `completedRounds` is read directly from the **streaming store**, not from this return value:

```ts
completedRounds: stream?.completedRounds.length ? stream.completedRounds : undefined,
```

`stream.completedRounds` is populated by `pushCompletedRound`, which is called for **every** round (including single-round responses), so a single-round tool-enabled response always ends up with `completedRounds = [{content}]` in the committed message — regardless of the `> 1` guard here. The guard is effectively a no-op for committed messages.

Consider either:
- Removing the `> 1` condition here for consistency, or  
- Resetting `stream.completedRounds` to `[]` at the end of a single-round response to prevent inadvertently storing a 1-element list that triggers the multi-round renderer for simple responses.

How can I resolve this? If you propose a fix, please make it concise.

ui/src/components/MultiModelResponse/MultiModelResponse.tsx, line 1213-1230 (link)

In-flight tool execution details are invisible between multi-round iterations

The ToolExecutionBlock (which shows live per-tool status) is now suppressed whenever completedRounds is present:

hasToolExecutionRounds
  ? !response.completedRounds && (
      <div className="mt-4 pt-4 border-t">
        <ToolExecutionBlock ... />
      </div>
    )

During multi-round tool execution, the sequence is:

pushCompletedRound is called → stream.completedRounds is non-empty
resumeStreaming is called → isStreaming = true
Tools begin executing in toolExecutionRounds (real-time updates)
ToolExecutionBlock is suppressed because response.completedRounds is truthy
Per-round ContentRound only shows ExecutionSummaryBar after setCompletedRoundToolExecution is called (i.e., after tools finish)

During step 3–4 the user sees only the generic StreamingStatusIndicator ("Processing…") with no indication of which tools are actually running. This is a regression from the previous behaviour where ToolExecutionBlock provided live per-tool status throughout execution.

Consider rendering a live tool block for the in-progress (not yet in completedRounds) execution round:

{hasToolExecutionRounds &&
  !response.completedRounds &&
  !compactMode && (
    <div className="mt-4 pt-4 border-t">
      <ToolExecutionBlock
        rounds={toolExecutionRounds}
        isStreaming={response.isStreaming}
        onArtifactClick={handleArtifactClick}
        displaySelection={displaySelection}
      />
    </div>
  )}

Or alternatively, show the in-progress toolExecutionRounds entries that are not yet captured in any completedRounds[i].toolExecution.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/MultiModelResponse/MultiModelResponse.tsx
Line: 1213-1230

Comment:
**In-flight tool execution details are invisible between multi-round iterations**

The `ToolExecutionBlock` (which shows live per-tool status) is now suppressed whenever `completedRounds` is present:

```tsx
hasToolExecutionRounds
  ? !response.completedRounds && (
      <div className="mt-4 pt-4 border-t">
        <ToolExecutionBlock ... />
      </div>
    )
```

During multi-round tool execution, the sequence is:

1. `pushCompletedRound` is called → `stream.completedRounds` is non-empty  
2. `resumeStreaming` is called → `isStreaming = true`  
3. Tools begin executing in `toolExecutionRounds` (real-time updates)  
4. **`ToolExecutionBlock` is suppressed because `response.completedRounds` is truthy**  
5. Per-round `ContentRound` only shows `ExecutionSummaryBar` after `setCompletedRoundToolExecution` is called (i.e., after tools finish)

During step 3–4 the user sees only the generic `StreamingStatusIndicator` ("Processing…") with no indication of which tools are actually running. This is a regression from the previous behaviour where `ToolExecutionBlock` provided live per-tool status throughout execution.

Consider rendering a live tool block for the in-progress (not yet in `completedRounds`) execution round:

```tsx
{hasToolExecutionRounds &&
  !response.completedRounds &&
  !compactMode && (
    <div className="mt-4 pt-4 border-t">
      <ToolExecutionBlock
        rounds={toolExecutionRounds}
        isStreaming={response.isStreaming}
        onArtifactClick={handleArtifactClick}
        displaySelection={displaySelection}
      />
    </div>
  )}
```

Or alternatively, show the in-progress `toolExecutionRounds` entries that are not yet captured in any `completedRounds[i].toolExecution`.

How can I resolve this? If you propose a fix, please make it concise.

ui/src/components/MultiModelResponse/MultiModelResponse.tsx, line 1127-1133 (link)

isToolsStreaming will always be false for items in completedRounds

The flag is computed as:

isToolsStreaming={
  response.isStreaming &&
  i === rounds.length - 1 &&
  !!round.toolExecution?.executions.some(
    (e) => e.status === "pending" || e.status === "running"
  )
}

round.toolExecution is attached via setCompletedRoundToolExecution, which is called after executeTools resolves — meaning every execution in round.toolExecution already has a terminal status (success, failed, or error) by the time it is set. The pending | running guard will therefore never match.

The isToolsStreaming prop on ContentRound and the downstream ExecutionSummaryBar is always false for items in completedRounds. If ContentRound is intended to ever show a streaming/in-progress state for its ExecutionSummaryBar, the live toolExecutionRounds from the streaming store (not the snapshot on the completed round) would be needed here.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/components/MultiModelResponse/MultiModelResponse.tsx
Line: 1127-1133

Comment:
**`isToolsStreaming` will always be `false` for items in `completedRounds`**

The flag is computed as:

```tsx
isToolsStreaming={
  response.isStreaming &&
  i === rounds.length - 1 &&
  !!round.toolExecution?.executions.some(
    (e) => e.status === "pending" || e.status === "running"
  )
}
```

`round.toolExecution` is attached via `setCompletedRoundToolExecution`, which is called *after* `executeTools` resolves — meaning every execution in `round.toolExecution` already has a terminal status (`success`, `failed`, or `error`) by the time it is set. The `pending | running` guard will therefore never match.

The `isToolsStreaming` prop on `ContentRound` and the downstream `ExecutionSummaryBar` is always `false` for items in `completedRounds`. If `ContentRound` is intended to ever show a streaming/in-progress state for its `ExecutionSummaryBar`, the live `toolExecutionRounds` from the streaming store (not the snapshot on the completed round) would be needed here.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ui/src/stores/chatUIStore.ts
Line: 2099-2105

Comment:
**Compact mode ON by default may surprise users**

`loadCompactMode` uses `!== "false"` as its sentinel, meaning the very first load (when localStorage is empty) returns `true` — compact mode is **on** by default, hiding reasoning sections and tool execution details until users explicitly toggle the verbose button.

If the intent is to default to compact, this is fine. But if it's a mistake, swapping the sentinel to `=== "true"` (with a `false` default) would make the logic less surprising:

```suggestion
function loadCompactMode(): boolean {
  try {
    return localStorage.getItem("hadrian:compactMode") === "true";
  } catch {
    return false;
  }
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/pages/chat/useChat.ts
Line: 1985

Comment:
**`result.completedRounds` field is dead code for single-model responses**

The final return at the end of the while loop (after `break`) guards `completedRounds` on `allCompletedRounds.length > 1`:

```ts
completedRounds: allCompletedRounds.length > 1 ? allCompletedRounds : undefined,
```

But in every commit site for single-model responses, `completedRounds` is read directly from the **streaming store**, not from this return value:

```ts
completedRounds: stream?.completedRounds.length ? stream.completedRounds : undefined,
```

`stream.completedRounds` is populated by `pushCompletedRound`, which is called for **every** round (including single-round responses), so a single-round tool-enabled response always ends up with `completedRounds = [{content}]` in the committed message — regardless of the `> 1` guard here. The guard is effectively a no-op for committed messages.

Consider either:
- Removing the `> 1` condition here for consistency, or  
- Resetting `stream.completedRounds` to `[]` at the end of a single-round response to prevent inadvertently storing a 1-element list that triggers the multi-round renderer for simple responses.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/MultiModelResponse/MultiModelResponse.tsx
Line: 1213-1230

Comment:
**In-flight tool execution details are invisible between multi-round iterations**

The `ToolExecutionBlock` (which shows live per-tool status) is now suppressed whenever `completedRounds` is present:

```tsx
hasToolExecutionRounds
  ? !response.completedRounds && (
      <div className="mt-4 pt-4 border-t">
        <ToolExecutionBlock ... />
      </div>
    )
```

During multi-round tool execution, the sequence is:

1. `pushCompletedRound` is called → `stream.completedRounds` is non-empty  
2. `resumeStreaming` is called → `isStreaming = true`  
3. Tools begin executing in `toolExecutionRounds` (real-time updates)  
4. **`ToolExecutionBlock` is suppressed because `response.completedRounds` is truthy**  
5. Per-round `ContentRound` only shows `ExecutionSummaryBar` after `setCompletedRoundToolExecution` is called (i.e., after tools finish)

During step 3–4 the user sees only the generic `StreamingStatusIndicator` ("Processing…") with no indication of which tools are actually running. This is a regression from the previous behaviour where `ToolExecutionBlock` provided live per-tool status throughout execution.

Consider rendering a live tool block for the in-progress (not yet in `completedRounds`) execution round:

```tsx
{hasToolExecutionRounds &&
  !response.completedRounds &&
  !compactMode && (
    <div className="mt-4 pt-4 border-t">
      <ToolExecutionBlock
        rounds={toolExecutionRounds}
        isStreaming={response.isStreaming}
        onArtifactClick={handleArtifactClick}
        displaySelection={displaySelection}
      />
    </div>
  )}
```

Or alternatively, show the in-progress `toolExecutionRounds` entries that are not yet captured in any `completedRounds[i].toolExecution`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/MultiModelResponse/MultiModelResponse.tsx
Line: 1127-1133

Comment:
**`isToolsStreaming` will always be `false` for items in `completedRounds`**

The flag is computed as:

```tsx
isToolsStreaming={
  response.isStreaming &&
  i === rounds.length - 1 &&
  !!round.toolExecution?.executions.some(
    (e) => e.status === "pending" || e.status === "running"
  )
}
```

`round.toolExecution` is attached via `setCompletedRoundToolExecution`, which is called *after* `executeTools` resolves — meaning every execution in `round.toolExecution` already has a terminal status (`success`, `failed`, or `error`) by the time it is set. The `pending | running` guard will therefore never match.

The `isToolsStreaming` prop on `ContentRound` and the downstream `ExecutionSummaryBar` is always `false` for items in `completedRounds`. If `ContentRound` is intended to ever show a streaming/in-progress state for its `ExecutionSummaryBar`, the live `toolExecutionRounds` from the streaming store (not the snapshot on the completed round) would be needed here.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "Update compact UI"}

ui/src/components/ChatMessageList/ChatMessageList.tsx

ui/src/pages/chat/useChat.ts

ScriptSmith · 2026-03-22T01:41:48Z

@greptile-apps

ui/src/pages/chat/useChat.ts

ui/src/components/MultiModelResponse/MultiModelResponse.tsx

ScriptSmith · 2026-03-22T03:45:35Z

@greptile-apps

greptile-apps bot reviewed Mar 21, 2026

View reviewed changes

ui/src/components/ChatMessageList/ChatMessageList.tsx Show resolved Hide resolved

ui/src/components/ChatMessageList/ChatMessageList.tsx Show resolved Hide resolved

ui/src/pages/chat/useChat.ts Outdated Show resolved Hide resolved

ScriptSmith added 12 commits March 22, 2026 10:41

Fix overwriting final round text

592df5e

Separate reasoning blocks

a54706b

Fix double spacing

9436b8f

Separate ContentRound component

97ab202

Vertical border per-round

112fe82

Display artifacts within rounds

79f1399

Don't jump to bottom at end

9dbc160

Fix virtualizer pushing scroll down

de05d7f

Remove tool call indicator

408d3d3

Add compact mode

3bf7049

Show rounds on hover

25bcb21

Review fixes

793e574

ScriptSmith force-pushed the chat-fixes branch from 64807de to 793e574 Compare March 22, 2026 00:41

Storybook fixes

87e5e03

greptile-apps bot reviewed Mar 22, 2026

View reviewed changes

ui/src/pages/chat/useChat.ts Outdated Show resolved Hide resolved

ui/src/components/MultiModelResponse/MultiModelResponse.tsx Show resolved Hide resolved

ui/src/components/MultiModelResponse/MultiModelResponse.tsx Outdated Show resolved Hide resolved

ScriptSmith added 7 commits March 22, 2026 12:35

Better constructs for rounds

3b41e25

Show thinking / processing interstitials between rounds

923967a

Higher-level management of streaming status indications

fd98901

Chat UI details as two buttons

49db7ab

Make thinking indicator nicer

1a43f95

Fix missing reasoning indicator

1e9762f

Update compact UI

a5450a9

Fix storybook

99ddb1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various improvements to chat interface#18

Various improvements to chat interface#18
ScriptSmith wants to merge 21 commits intomainfrom
chat-fixes

ScriptSmith commented Mar 21, 2026

Uh oh!

greptile-apps bot commented Mar 21, 2026 •

edited

Loading

Comments Outside Diff (4)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ScriptSmith commented Mar 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ScriptSmith commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ScriptSmith commented Mar 21, 2026

Uh oh!

greptile-apps bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (4)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ScriptSmith commented Mar 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ScriptSmith commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps bot commented Mar 21, 2026 •

edited

Loading