This is a comment left during a code review.
Path: ui/src/ui/controllers/chat.ts
Line: 391-406

Comment:
**Pending message not cleared on send failure**

When `requestChatSend` throws, `pendingChatMessagesById` still holds the unsent message. If `loadChatHistory` runs afterward (e.g. on reconnect), it will see the entry as "not yet in server history," append it to `chatMessages` via `pendingToKeep`, and simultaneously overwrite `chatMessages` with `filteredMessages`, silently dropping the `"Error: …"` assistant message that the catch block added. The user ends up seeing their message as still-pending even though it was never delivered, with no error visible.

Add a cleanup step in the catch block:

```suggestion
  } catch (err) {
    const error = formatConnectError(err);
    state.chatRunId = null;
    state.chatStream = null;
    state.chatStreamStartedAt = null;
    state.lastError = error;
    // Remove from pending tracking — send failed, so the message will never
    // arrive in server history. Leaving it here would cause loadChatHistory
    // to re-surface the message as "still pending" without the error context.
    state.pendingChatMessagesById?.delete(tempId);
    state.chatMessages = [
      ...state.chatMessages,
      {
        role: "assistant",
        content: [{ type: "text", text: "Error: " + error }],
        timestamp: Date.now(),
      },
    ];
    return null;
  }
```

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/ui/controllers/chat.ts
Line: 198-209

Comment:
**`_tempId` not stripped in success path**

The error path correctly strips `_tempId` via object spread (`{ _tempId: _1, ...msg }`), but in the success path the pending message objects are pushed into `pendingToKeep` — and then into `chatMessages` — with `_tempId` still present. This is inconsistent and leaks the internal field into the display layer until the next `loadChatHistory` call overwrites the array.

```suggestion
      const pendingToKeep: unknown[] = [];
      for (const pendingMsg of state.pendingChatMessagesById.values()) {
        const { _tempId: _ignored, ...pending } = pendingMsg as Record<string, unknown>;
        const content = pending.content;
        const fingerprint = Array.isArray(content) ? JSON.stringify(content) : String(content);
        if (!serverFingerprints.has(fingerprint)) {
          pendingToKeep.push(pending);
        }
      }
```

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/ui/controllers/chat.ts
Line: 191-196

Comment:
**Content-only fingerprint can misfire on repeated identical messages**

The fingerprint is built solely from `content` with no position or timestamp. If the user sends the same text twice (e.g. "retry" or a short acknowledgement), the second pending message will match the first one already confirmed in server history and be silently dropped from `pendingToKeep`, causing a brief visual disappearance of the optimistic message. Including at least `timestamp` in the fingerprint would avoid this:

```suggestion
      for (const m of filteredMessages) {
        const msg = m as Record<string, unknown>;
        const content = msg.content;
        const ts = typeof msg.timestamp === "number" ? String(msg.timestamp) : "";
        serverFingerprints.add(
          (Array.isArray(content) ? JSON.stringify(content) : String(content)) + "|" + ts,
        );
      }
```

And the corresponding pending-side fingerprint:
```typescript
const ts = typeof pending.timestamp === "number" ? String(pending.timestamp) : "";
const fingerprint =
  (Array.isArray(content) ? JSON.stringify(content) : String(content)) + "|" + ts;
```

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/ui/controllers/chat.ts
Line: 119

Comment:
**Type vs. runtime defensive checks are inconsistent**

`pendingChatMessagesById` is declared as the required (non-optional) type `Map<string, unknown>`, yet `sendChatMessage` guards with `if (!state.pendingChatMessagesById)` and lazily initialises it, and both `loadChatHistory` paths also guard with `if (state.pendingChatMessagesById && ...)`. This implies some `ChatState` initialisers don't set the field, so the declared type is incorrect. Either make the type optional (`pendingChatMessagesById?: Map<string, unknown>`) or ensure every `ChatState` factory initialises it to `new Map()` and drop the defensive checks.

```suggestion
  pendingChatMessagesById?: Map<string, unknown>;
```

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