Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/fix-chat-thinking-order/intent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Intent: Preserve Chat thinking block order
Author: SpireCode maintainer. Status: approved.

## Problem

Chat often renders an agent's thinking section after the final answer even though Pi stores and streams thinking before the answer. SpireCode currently flattens an assistant message into one text item first and appends thinking items afterward, losing Pi's content-block order in both restored and live conversations.

## Proposed outcome

Render assistant thinking, text, and tool-call blocks in the same order supplied by Pi. Live streaming and restored session snapshots must produce the same timeline order, without duplicate blocks or position jumps as partial messages update.

## Affected users and systems

- Chat transcript rendering for all reasoning-capable providers, including TraeX.
- Electron Main's Pi event normalization.
- Renderer Chat timeline state and process grouping.

## Constraints

- Preserve the sandbox boundary and existing narrow Chat IPC.
- Treat Pi's assistant content array and stream `contentIndex` ordering as authoritative.
- Preserve tool execution updates and process grouping.
- Do not reorder unrelated user messages, notices, or todo snapshots.

## Open questions

None.
35 changes: 35 additions & 0 deletions docs/fix-chat-thinking-order/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Plan: Preserve Chat thinking block order (from docs/fix-chat-thinking-order/spec.md 2026-09-23)

## Files that change

- `electron/domains/chat/wire.ts` — preserve assistant content-block order in snapshot and event normalization.
- `electron/domains/chat/wire.test.ts` — add restored and live ordering regressions.
- `src/features/chat/sessionReducer.test.ts` — prove keyed streaming updates retain block positions.
- `docs/fix-chat-thinking-order/intent.md` — capture the approved user-visible correction.
- `docs/fix-chat-thinking-order/spec.md` — define block ordering and compatibility requirements.
- `docs/fix-chat-thinking-order/plan.md` — record implementation order, risks, and proof.

## Order of work

1. Add regression assertions showing that `thinking -> text -> tool -> thinking -> text` is currently flattened out of order in restored and live normalization.
2. Run the focused tests and retain the expected failure as red evidence.
3. Refactor assistant normalization around stable per-content-index block IDs, emitting timeline items/events in Pi source order.
4. Ensure tool block IDs correlate with execution events and preserve error fallback behavior.
5. Add reducer coverage proving later partial/final updates replace blocks in place rather than appending them.
6. Run focused Chat normalization/reducer/display tests.
7. Run `pnpm check`, inspect the diff, commit, push, and open a pull request.

## Risks

- The most dangerous change is replacing one aggregated assistant text item with block-level text items; React keys and reducer IDs must remain stable across stream updates.
- Tool cards can arrive from assistant content and tool execution lifecycle events; mismatched IDs would create duplicate cards.
- Providers may expose sparse content arrays during streaming. Iteration must preserve defined indices without inventing unstable IDs.
- A renderer-only reorder is rejected because the original content index is already discarded by the backend DTO.
- A simple “put thinking before message” swap is rejected because it still fails interleaved multi-block responses.

## Proof

- Red: focused wire test fails because current output places aggregated text before thinking.
- Green: `pnpm test -- electron/domains/chat/wire.test.ts src/features/chat/sessionReducer.test.ts src/features/chat/chatDisplayItems.test.tsx` (or the matching existing `.ts` path) exits 0.
- `pnpm check` exits 0.
- `git diff --check` exits 0.
28 changes: 28 additions & 0 deletions docs/fix-chat-thinking-order/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Spec: Preserve Chat thinking block order

## Requirements

1. Restored assistant messages must project `thinking`, `text`, and `toolCall` blocks in source-array order.
2. Live assistant events must create and update timeline blocks according to Pi's authoritative assistant message content order.
3. Repeated stream updates must replace existing blocks without moving them to the end or duplicating them.
4. Multiple text blocks in one assistant turn must remain independently ordered around thinking and tool blocks.
5. Empty completed assistant text placeholders must remain hidden where the display projection currently uses them only to join adjacent tool operations.
6. Existing tool result correlation, todo ordering, and notice behavior must remain unchanged.

## Design

Introduce stable block-level IDs derived from the assistant message ID and content index. Normalize each assistant block into its own timeline item:

- `thinking` becomes a thinking item.
- `text` becomes an assistant/error message item.
- `toolCall` becomes a tool item when restoring history; live tool cards continue to receive authoritative execution lifecycle events.

For live `message_start`, `message_update`, and `message_end`, emit block events in source order rather than emitting one aggregate text message before all thinking blocks. The reducer's existing keyed upsert then preserves the first observed position while replacing partial content. The final `message_end` snapshot supplies the canonical status and content for every block.

User messages remain one message item because they are not mixed with assistant reasoning/tool blocks in this UI contract.

## Concerns

- Some providers may emit a provisional empty assistant content array at `message_start`; no visible item should be created until a block exists.
- Tool execution may start after a tool-call block has already been projected. Both paths must use the same `toolCallId` so the reducer updates rather than duplicates the tool.
- Error responses with no text block still need one visible error item carrying `errorMessage`.
123 changes: 116 additions & 7 deletions electron/domains/chat/wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ describe("chat wire normalization", () => {
name: "read",
arguments: { path: "a.ts" },
},
{ type: "thinking", thinking: "verify" },
{ type: "text", text: "confirmed" },
],
timestamp: 2,
},
Expand All @@ -57,20 +59,20 @@ describe("chat wire normalization", () => {
status: "complete",
createdAt: 1,
},
{
type: "thinking",
id: "message:assistant:2:thinking:0",
content: "inspect",
status: "complete",
},
{
type: "message",
id: "message:assistant:2",
id: "message:assistant:2:text:1",
role: "assistant",
content: "done",
status: "complete",
createdAt: 2,
},
{
type: "thinking",
id: "message:assistant:2:thinking:0",
content: "inspect",
status: "complete",
},
{
type: "tool",
toolCallId: "call-1",
Expand All @@ -79,6 +81,113 @@ describe("chat wire normalization", () => {
result: "file",
status: "done",
},
{
type: "thinking",
id: "message:assistant:2:thinking:3",
content: "verify",
status: "complete",
},
{
type: "message",
id: "message:assistant:2:text:4",
role: "assistant",
content: "confirmed",
status: "complete",
createdAt: 2,
},
]);
});

it("preserves interleaved assistant block order in live events", () => {
expect(
normalizeEvent({
type: "message_update",
message: {
role: "assistant",
timestamp: 2,
content: [
{ type: "thinking", thinking: "inspect" },
{ type: "text", text: "before" },
{
type: "toolCall",
id: "call-1",
name: "read",
arguments: { path: "a.ts" },
},
{ type: "thinking", thinking: "verify" },
{ type: "text", text: "after" },
],
},
}),
).toEqual([
{
type: "thinking_update",
thinking: {
id: "message:assistant:2:thinking:0",
content: "inspect",
status: "streaming",
},
},
{
type: "message_update",
message: {
id: "message:assistant:2:text:1",
role: "assistant",
content: "before",
status: "streaming",
createdAt: 2,
},
},
{
type: "tool_execution_start",
toolCallId: "call-1",
toolName: "read",
arguments: { path: "a.ts" },
},
{
type: "thinking_update",
thinking: {
id: "message:assistant:2:thinking:3",
content: "verify",
status: "streaming",
},
},
{
type: "message_update",
message: {
id: "message:assistant:2:text:4",
role: "assistant",
content: "after",
status: "streaming",
createdAt: 2,
},
},
]);
});

it("preserves a visible error when an assistant failure has no content blocks", () => {
expect(
normalizeEvent({
type: "message_end",
message: {
role: "assistant",
timestamp: 3,
content: [],
stopReason: "error",
errorMessage: "provider failed",
},
}),
).toEqual([
{
type: "message_end",
message: {
id: "message:assistant:3:error",
role: "error",
content: "provider failed",
status: "error",
createdAt: 3,
},
},
]);
});

Expand Down
Loading
Loading