Commit Graph
3 Commits
Author SHA1 Message Date
Jakob WennbergandClaude Opus 5 a43a8b03cf refactor(assistant): one source of truth for the conversation list (#1214)
* refactor(assistant): one source of truth for the conversation list

PR5 of the assistant UI makeover (dev_docs/assistant_redesign_plan.md section 7):
unified history.

The two surfaces that list conversations had drifted apart in BEHAVIOUR, not
just chrome. The in-sheet list rolled a failed rename back and said so; the
/chat sidebar fired pin, archive and rename blind, with no res.ok check, no
rollback and no message. A failed archive there removed a conversation from the
list while it still existed on the server, and a failed rename displayed a title
the server never saved, both until the next reload, with an unhandled promise
rejection on a network error.

State, search, grouping and all three mutations now live in one hook that both
surfaces consume, so they cannot diverge again: every write is optimistic,
reverts to the value captured before the write on failure, and reports it. The
sheet gains pin and archive, which it never had.

Archive resolves whether the row is really gone, so the sidebar only navigates
away from a conversation that was actually archived.

Chrome deliberately stays per-surface: a 320px sidebar that collapses to a rail
and a sheet panel are different shapes, and merging the markup belongs with the
shell work in PR6, where both containers change anyway.

Verified: 9554 unit tests pass (5 new pinning the rollback semantics, including
reverting to the original pin value rather than toggling and restoring a null
title), lint and tsc clean on the touched files, guards pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): test the real mutation code, and make rollback mutation-aware

Review follow-ups on the unified conversation list. Both findings were right.

The tests duplicated the state transforms and called fetch directly, so they
never executed the hook: deleting the rollback entirely would have left them
green. That is test theater. The transforms and the write coordinator now live
in conversation-mutations.ts, React-free, and the tests exercise those. Checked
by deleting the rollback and confirming three tests fail.

Rollback was not mutation-aware. A failed archive restored a render-time
snapshot of the whole list, discarding any pin, rename or archive made while the
request was in flight; and a failing earlier write could roll back over a newer
value for the same row (a double-click on pin). Writes now claim a per-row
revision and only undo while they are still the latest for that row, and a
failed archive re-inserts the single row into the list AS IT STANDS, at its
server-sort position, rather than replacing the list.

Also drops a ref read during render that the React lint rules reject.

Verified: 9560 unit tests pass (11 covering the real coordinator, including the
overlapping-write case and the concurrent-edit-survives-archive-failure case),
lint clean, tsc clean, guards pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(assistant): unstub globals so the fetch stub cannot outlive the file

vi.restoreAllMocks does not undo vi.stubGlobal, and the config sets no
unstubGlobals, so the stubbed fetch survived past the suite that set it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 23:30:01 +02:00
Jakob WennbergandClaude Opus 5 8397452440 feat(assistant): copy, feedback, an interrupt marker and a way back to the latest answer (#1213)
* feat(assistant): copy, feedback, an interrupt marker and a way back to the latest answer

PR4 of the assistant UI makeover (dev_docs/assistant_redesign_plan.md section 7):
message anatomy and actions.

Assistant turns get a hover action row: copy, thumbs up/down and the existing
regenerate, which until now was the only affordance on an answer. gnubok_feedback
exists as a tool with no UI at all, so the thumbs are local-only for the moment;
the point of this row is that the affordances sit where people look for them,
and wiring the vote through is a follow-up that cannot break reading an answer.

Stop used to abort and leave the half-written answer looking finished, which is
worst exactly when it stopped mid-figure. The partial text still stays, now with
a marker saying it was interrupted.

A failed send cleared the composer and left a user bubble that had never reached
the server, so the question vanished on the next reload and had to be retyped.
The text now goes back into the composer and the unsent bubble is dropped, so
the screen matches what was actually sent. startTurn reports whether the request
got out; a mid-stream failure still counts as sent and keeps its content.

Autoscroll already respected a user who had scrolled up, but nothing told them
an answer had landed below the fold. A "Nytt svar" pill now appears in that
case and takes them back.

Verified: 9549 unit tests pass (7 new pinning the stop and failed-send state
rules), lint and tsc clean on the touched file, guards pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): anchor the jump pill to the message area, not the whole panel

Self-review before merge: the pill sat at a fixed offset from the bottom of the
component, but the composer below it grows to 128px as the user types. A long
multi-line draft plus a scrolled-up reader would have slid the pill underneath
the composer, exactly when it is needed. It now positions against the message
area itself, so composer height is irrelevant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 22:40:02 +02:00
Jakob WennbergandClaude Opus 5 4de648fb5d fix(assistant): keep proposals, selections and picks intact across a resume (#1212)
* fix(assistant): keep proposals, selections and picks intact across a resume

PR3 of the assistant UI makeover (dev_docs/assistant_redesign_plan.md section 7):
resume fidelity. Four ways the chat lost state that the user had every reason to
think was still there.

Approval cards ride on streamed staged_operation events, which are never
persisted, so reopening a conversation rendered the tool trace and the answer
but silently dropped the card. The proposal then sat in Granskning for its full
30-day expiry with nothing in the thread pointing at it. run-turn already stamps
agent_metadata.conversation_id on every staged row, so both resume paths (the
sheet's history and the /chat page) now re-attach the still-pending ones to the
last assistant turn.

Regenerate abandoned whatever the discarded turn had staged: the card left the
screen, the operation stayed pending, and the regenerated turn usually staged a
second proposal for the same booking, leaving two live proposals for one action.
It now withdraws them through the same reject path the Avslå button uses, so the
audit trail records why they went away.

The sheet's remount key ignored intentArgs while some callers pass a CONSTANT
contextRef with varying args: bulk-book always uses 'inbox:bulk' and carries the
selected ids. Selecting A+B, collapsing, then selecting C+D reopened the A+B
conversation while the user believed C+D were being booked. The key now includes
a stable serialization of the args.

Picking conversation A (slow) then B (fast) let A's late response overwrite B,
leaving the user typing into a thread they did not choose. A sequence token now
means only the newest pick may write state.

Verified: 9540 unit tests pass (11 new), lint and tsc clean on every touched
file, guards pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: record the resume-fidelity decisions

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): render hydrated proposals with the same preview as live ones

pending_operations.operation_type stores the bare action name
('categorize_transaction'), while the streamed card carries the MCP tool name
('gnubok_categorize_transaction') and ApprovalCard's PreviewBlock dispatches on
that. Hydrated cards therefore fell through to the flat generic preview instead
of the journal-line one, so a resumed proposal looked materially worse than the
same proposal did live: the opposite of what this PR is for.

Found by checking the query against prod rather than trusting the mock, which is
also how the stored value space was confirmed: categorize_transaction,
create_voucher and approve_supplier_invoice are what exist in the wild, and the
four operation types that have a specialized renderer all stage unprefixed.

The test fixture now uses the real stored shape so the mapping is actually
covered rather than assumed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(assistant): await proposal withdrawals, surface staged-query errors, share the type

Review follow-ups on the resume-fidelity batch. All four findings were valid.

The withdrawals were fire-and-forget and raced the replacement turn, so the new
turn could stage a second proposal before the old one was rejected: the exact
double-staging this change exists to prevent. They are now awaited, a 409 counts
as withdrawn (someone else resolved it, which is all we need), and if any
withdrawal genuinely fails the turn stays on screen with an error rather than
hiding a card whose operation is still pending.

Both staged-operation loaders ignored their error result, so a database or
policy failure rendered the conversation as successful with the proposals
silently missing: again the failure this query exists to prevent, reintroduced
through the error path. Both now propagate, matching the sibling message query.

StoredStagedOperation now lives in @/types: it is a persisted API contract that
crosses a server page and three components, not an AgentChat detail.

The unserializable-args fallback used a timestamp, which collides for two
objects created in the same millisecond and changes on every render tick for the
same object, remounting the sheet mid-session. A WeakMap gives each object one
stable id for its lifetime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 21:56:50 +02:00