optimistic-files.md22.5 KBView on GitHub # Optimistic files — every file action lands instantly, and stays landed
## 1) Introduction — goal, present state, future state
Every mutation in the conversation Files tab — create a note, create a folder, rename, delete,
drag into a folder, add a Drive file, unlink a Drive file — currently waits for a server round
trip **and then a refetch** before anything moves on screen. Nothing is optimistic. On a slow
conversation the delete of a note is a click, a pause, and then a list that redraws whole; with
the External drives section open, renaming a local note blocks on a Google Drive API call,
because `invalidateFiles` awaits four query families in sequence and one of them is Drive.
This is the same complaint heyTelo filed as **"17 AUG — Tasks not optimistically rendering"**
(#cedar-heytelo, in Jesse's 2026-08-20 roll-up), one surface over. The task version got a design
([TASK_OPTIMISTIC_RENDERING_DESIGN.md](../modules/userTasks/TASK_OPTIMISTIC_RENDERING_DESIGN.md),
2026-08-02) whose four rules generalise, and which was never implemented (0 of 61 boxes):
1. **Never withhold the write.** Dispatch on click; a mutation delayed so Undo can cancel it is
a mutation any refetch inside the window can legitimately contradict.
2. **Undo is a real inverse mutation**, against the soft delete the server already has.
3. **Slow destructive side effects move off the request path**, behind a grace period a restore
cancels.
4. **Nothing but an explicit post-mutation invalidation may contradict local state.**
Future state: every action paints in the same frame as the click, the server write goes out
immediately, a failure rolls the row back and says why, and a destructive action offers an Undo
that is a real `documents.restore` call. No action waits for a refetch to become visible, and no
in-flight refetch can undo one.
The Files tab is a better-shaped problem than tasks were: it has **no Zustand mirror of the
tree**. The rows come straight off `files.listChildren`, so the optimistic layer can own the
query cache and there is no second writer to reconcile against — with one exception this design
handles explicitly (`batchSetDocuments`, §2.3).
## 2) Present state
### 2.1 Architecture diagram
```text
┌──────────────────────────── Files tab (apps/mail) ─────────────────────────────┐
│ │
│ FilesTab │
│ ├─ handleCreateDoc ──── await files.createFile ─────┐ │
│ ├─ handleCreateFolder ─ await files.createFolder ───┤ │
│ ├─ handleRename ─────── await files.renameNode ─────┤ │
│ ├─ handleDeleteDoc ──── crm.deleteConversationDoc ──┤ nothing moves on screen │
│ ├─ handleMove ───────── files.moveNode ─────────────┤ until this resolves │
│ ├─ onPick (Add file) ── await files.pinDriveItem ×N ┤ │
│ └─ DriveSection ─────── await drive.unlinkFile ─────┘ │
│ │ │
│ ▼ │
│ invalidateFiles() ── awaits FOUR in sequence: │
│ 1. files.listChildren (every expanded folder) │
│ 2. drive.listMount (Google round trip) │
│ 3. drive.listForConversation (Google round trip) │
│ 4. crm.listConversationDocs │
│ │ │
│ ┌──────────────────────────┼───────────────────────────┐ │
│ ▼ ▼ ▼ │
│ files.listChildren crm.listConversationDocs drive.list* │
│ (one entry per (flat list) (Drive rows) │
│ expanded folder) │ │
│ │ ├──► batchSetDocuments ──► CedarStore │
│ ▼ │ documents slice │
│ ConversationFileTree └──► nonAgentDocs ──► open-doc resolver, │
│ rows note_N counter, │
│ agentDocs grouping │
└─────────────────────────────────────────────────────────────────────────────────┘
GLOBAL QUERY DEFAULTS (providers/query-provider.tsx:54)
staleTime 60s · refetchOnWindowFocus TRUE · refetchOnMount TRUE
⚠️ a focus refetch in flight when a patch lands is exactly what "pops back in"
```
### 2.2 Step-by-step walkthrough
Deleting `note_2` from a conversation whose External drives section is open.
1. **Click** — the row's trash icon calls `handleDeleteDoc(documentId)` at
[FilesTab.tsx:243](../modules/conversations/components/files/FilesTab.tsx).
- Receives: `documentId = 'd41c…'`. Screen state: unchanged. The row is still there.
2. **Mutation dispatched** — `deleteDocMutation.mutate` at
[use-conversation-body.ts:135](../modules/conversations/hooks/use-conversation-body.ts),
i.e. `crm.deleteConversationDoc`. Nothing is rendered differently. On a 400 ms API the row
sits under the cursor for 400 ms looking exactly as it did.
3. **`onSuccess` → `invalidateFiles`** at
[ConversationFileTree.tsx:403](../modules/conversations/components/files/ConversationFileTree.tsx).
Four `await`ed invalidations, in order. Every *active* query refetches:
```text
files.listChildren ({parentId:null}) ~120 ms
files.listChildren ({parentId:'f-89ab'}) ~120 ms (per expanded folder)
drive.listMount (if a mount is expanded) ~900 ms ← Google
drive.listForConversation (section open) ~1400 ms ← Google
crm.listConversationDocs ~180 ms
```
- Total before the row disappears: **~700 ms** with Drive closed, **~2.6 s** with it open.
4. **`batchSetDocuments` re-hydrates the store** —
[use-conversation-body.ts:85](../modules/conversations/hooks/use-conversation-body.ts) writes
the fresh `listConversationDocs` array into the CedarStore documents slice. This is the one
second writer, and it is downstream of the same query, so it is consistent — but it means a
patch to `listConversationDocs` must be made *through the cache*, not around it.
5. **Row disappears.** ~700 ms–2.6 s after the click.
The other five actions have the same shape. `handleCreateDoc`
([:199](../modules/conversations/components/files/FilesTab.tsx)) additionally counts existing
`note*` files out of `nonAgentDocs` to pick `note_N`, so a created note is not even *named*
until the flat list refetches; two fast creates in a row both compute `note_2`.
### 2.3 The three readers a patch must satisfy
| Reader | Query | What breaks if it is not patched |
| --- | --- | --- |
| `ConversationFileTree` rows | `files.listChildren` per expanded parent | The row itself doesn't move |
| open-doc resolver, `note_N` counter, `agentDocs` | `crm.listConversationDocs` | Deep-link resolves a deleted doc; next note takes a used name |
| CedarStore documents slice | hydrated from ↑ by `batchSetDocuments` | A deleted doc stays addressable in the store |
| External drives section | `drive.listForConversation` | Pin/unlink don't move |
| Drive mount rows in the tree | `drive.listMount` | Pin into a mount doesn't move |
### 2.4 What is already right, and must not be lost
- **`documents` deletes are already soft** — `deleteDocument` stamps `deleted_at`, and
`restoreDocument` ([documents/index.ts:988](../../server/src/services/documents/index.ts))
already exists and already refuses a restore whose path is re-occupied. Undo has a server-side
home; it has no tRPC route yet.
- **The tree has no Zustand mirror.** Rows are query-owned. Do not add one.
- **`files.*` is already headlessly drivable** — `crm-admin` CLI covers `listChildren`,
`createFile`, `createFolder`, `renameNode`, `moveNode`, `deleteNode`, `pinDriveItem`,
`markOpened`. `drive.unlinkFile`/`relinkFile` and a new `documents.restore` need verbs.
## 3) Designed state
### 3.1 Architecture diagram
```text
┌──────────────────────────── Files tab (apps/mail) ─────────────────────────────┐
│ FilesTab / DriveSection │
│ │ every action calls ONE layer │
│ ▼ │
│ useOptimisticFileActions() ◄── new: optimistic-files.ts │
│ create · createFolder · rename · remove · move · pinDrive · unlinkDrive │
│ │ │
│ │ 1. cancelQueries(affected keys) ← kills the in-flight refetch that │
│ │ would otherwise land on top │
│ │ 2. snapshot(affected keys) │
│ │ 3. setQueryData × N ── SAME TICK, row moves now │
│ │ 4. mutate() immediately ── never withheld (rule 1) │
│ │ 5. onError → restore snapshot + toast │
│ │ 6. onSuccess → reconcile temp id → real id, in place │
│ │ 7. onSettled → ONE batched invalidation (not four awaits) │
│ ▼ │
│ ┌──────────────────────────────────────────────────────────────────┐ │
│ │ filesCachePatch.ts — pure functions over the cache shapes │ │
│ │ insertNode · removeNode · renameNodeIn · reparentNode │ │
│ │ applied to: listChildren(parent) · listConversationDocs │ │
│ │ drive.listForConversation · drive.listMount │ │
│ └──────────────────────────────────────────────────────────────────┘ │
│ │
│ Destructive actions push an undo: │
│ toast "Deleted note_2" [Undo] ──► documents.restore (real inverse, rule 2) │
└─────────────────────────────────────────────────────────────────────────────────┘
SERVER (new)
documents.restore ──► restoreDocument() (service already exists)
files.createFile/createFolder already return the full node → reconcile needs no refetch
```
### 3.2 Step-by-step walkthrough
The same delete, redesigned.
1. **Click** — `remove(documentId)` from `useOptimisticFileActions`.
2. **Cancel in-flight reads** — `await queryClient.cancelQueries` for the affected keys. This is
the step that makes the patch stick: with `refetchOnWindowFocus: true` globally, a refetch
started when the window regained focus can otherwise resolve *after* the patch and rewrite
the row back in. Cancelling is what the task design meant by "not reliant on cache timers".
3. **Snapshot** — `getQueryData` for every key about to be written, kept in a closure for
rollback.
```ts
{ 'files.listChildren|f-89ab': [ {id:'d41c…',title:'note_2'}, {id:'d99f…'} ],
'crm.listConversationDocs': [ …, {id:'d41c…'} ] }
```
4. **Patch, synchronously** — `removeNode('d41c…')` applied to both:
```ts
'files.listChildren|f-89ab': [ {id:'d99f…'} ]
'crm.listConversationDocs': [ … ] // d41c… gone
```
The row is gone in this frame. `batchSetDocuments` re-runs off the patched array, so the
store mirror drops it too — no separate store write.
5. **Mutate now** — `crm.deleteConversationDoc({documentId})` dispatched in the same handler.
6. **Undo toast** — `toast.success('Deleted note_2', { action: Undo, duration: 5s })`. Undo
calls `documents.restore({id})` — a real inverse mutation against the existing soft delete,
optimistically re-inserting the snapshot row at its old index. `documents.restore` rather
than `documents.restore` so the undo asks the same ownership question the delete asked:
`crm.deleteConversationDoc` and `documents.restore` share one `callerOwnsDocument`, whereas
the files-layer route authorizes on scope write and would revive rows this caller was never
allowed to delete. `documents.restore` remains the `crm-admin` CLI's verb.
7. **`onError`** — restore the snapshot verbatim, `toast.error(message)`. The row reappears
*with an explanation*, which is the one outcome that must never be silent.
8. **`onSettled`** — one invalidation pass, **not awaited by the handler** and scoped: the tree
and the flat list always; the Drive queries **only for Drive actions**. Renaming a note stops
costing a Google round trip.
### 3.3 The API
```ts
// apps/mail/modules/conversations/components/files/optimistic-files.ts
export function useOptimisticFileActions(scope: ConversationScope, conversationId: string): {
createFile: (input: { parentId: string | null; name: string; documentType?: string }) => Promise<string | null>;
createFolder: (input: { parentId: string | null; name: string }) => Promise<string | null>;
rename: (documentId: string, newName: string) => Promise<void>;
remove: (documentId: string, opts?: { undo?: boolean }) => Promise<void>;
removeMany: (documentIds: string[]) => Promise<void>;
move: (documentId: string, newParentId: string | null) => Promise<void>;
pinDrive: (items: PickedDriveItem[]) => Promise<void>;
unlinkDrive: (node: DriveNode) => Promise<void>;
};
```
**Temp ids.** A create paints a node with `id: 'optimistic:<uuid>'` at the correct position, and
`onSuccess` swaps it **in place** for the server node — no refetch, no flash, no re-sort. A temp
row is deliberately inert: it cannot be opened, renamed, dragged, or selected (it has no server
identity to address), and it renders at 60 % opacity so the state is visible rather than a lie.
`isTempNode(id)` is the single predicate.
**`note_N` naming** is computed from the *patched* flat list, so two fast creates produce
`note_2` and `note_3` rather than two `note_2`s.
**Failure is never silent.** Every rollback raises a toast naming the file and the reason. A
refused delete (protected playbook path) already throws from the route; today it leaves the row
in place with no explanation, which reads as a dead button.
### 3.4 Schema
No schema change. The whole design rests on `documents.deleted_at` (already there, already
partial-unique-indexed on `deleted_at IS NULL`) and on `restoreDocument`, which already exists
and already refuses a path collision.
One new tRPC route:
```ts
documents.restore: privateProcedure
.input(z.object({ documentId: z.string().uuid() }))
.mutation(→ restoreDocument({ orgId, documentId, userId }))
```
## 4) Implementation phases
Each phase ends green: typecheck, its own tests, and a headless CLI drive of any server surface
it touched.
### Phase 1 — The cache-patch primitives
- [x] `filesCachePatch.ts`: pure `insertNode`, `removeNode`, `renameNodeIn`, `reparentNode`,
`replaceTempNode` over `TreeNode[]` and the flat doc list. No React, no query client.
- [x] `isTempNode` / `newTempId` helpers.
- [x] Unit tests: each function on a populated list, an empty list, and a miss (id not present
→ unchanged array identity, so React does not re-render siblings).
### Phase 2 — `useOptimisticFileActions`, delete first
- [x] The hook, with the cancel → snapshot → patch → mutate → rollback → settle pipeline.
- [x] Wire `handleDeleteDoc` and `handleDeleteSelected` to it.
- [x] Tests: with a **deferred** mutation promise, the row is gone before the promise resolves;
on rejection it comes back *and* a toast names the file; a `listChildren` refetch that was
in flight at click time does not resurrect it.
### Phase 3 — Undo as a real inverse mutation
- [x] `documents.restore` route + `crm-admin` CLI verb `files restore --id <docId>`.
- [x] Undo action on the delete toast, optimistic re-insert at the old index.
- [x] Tests: undo re-inserts at the same position; a restore refused by a path collision rolls
the re-insert back and says why.
- [x] Headless: create → delete → restore through the CLI, asserting `listChildren` each step.
### Phase 4 — Create, rename, move
- [x] `createFile` / `createFolder` with temp ids and in-place reconcile; `note_N` off the
patched list.
- [x] `rename` and `move` (move patches two parents: remove from old, insert into new).
- [x] Tests: temp row is inert (not openable/renamable/selectable); two fast creates name
themselves `note_2`/`note_3`; a failed rename restores the old title; a move that the
server rejects returns the row to its original parent.
### Phase 5 — Drive rows
- [x] `pinDrive` inserts pinned rows into `drive.listForConversation` before the round trip;
partial failure rolls back only the items that failed.
- [x] `unlinkDrive` removes the row optimistically; the existing undo becomes an optimistic
re-insert rather than a refetch.
- [x] Tests: pinning three files paints three rows immediately; one failing pin leaves the other
two and names the failure.
### Phase 6 — Kill the four-await invalidation
- [x] Split `invalidateFiles` into `invalidateTree` (listChildren + flat list) and
`invalidateDrive`, fired only by Drive actions, and never awaited by a click handler.
- [x] Audit the files queries for rule 4: nothing may contradict a patch except an explicit
post-mutation invalidation.
- [x] Test: a rename issues no Drive query at all.
### Phase 7 — Headless verification pass
- [x] CLI verbs for `drive.unlinkFile` / `drive.relinkFile` (the two routes with no driver).
- [x] End-to-end headless script: create folder → create file inside → rename → move → delete →
restore → pin Drive file → unlink → relink, asserting server state after each step.
- [x] Full `filesTab*` jest suite + `@zero/mail` and `@zero/server` typechecks green.
## 5) Verification
- **Instant**: with the network throttled to 3 s, every action paints in the same frame as the
click. This is the acceptance test; anything that waits on a promise fails it.
- **Sticky**: blur and refocus the window (a focus refetch fires) immediately after a delete —
the row stays gone.
- **Honest**: kill the server, act, and confirm every action rolls back with a toast that names
the file and the reason.
- **Reversible**: delete → Undo restores the file at its original position, and the server
agrees (`files listChildren` through the CLI).
- **Cheap**: with External drives open, a rename issues zero Drive requests.
## 6) Out of scope
The other surfaces heyTelo's feedback touches — tasks (the 2026-08-02 design, still unbuilt),
threads, and the timeline — keep their current behaviour. This design deliberately fixes one
surface completely rather than four halfway; the `filesCachePatch` + `cancel → snapshot → patch
→ mutate → rollback` shape is written to be liftable to them, and the task doc's rules 1–4 are
restated here so the second surface has a precedent to copy rather than a second opinion.
## 7) What the implementation changed from this plan
Three amendments, all found by running the thing rather than reading it:
1. **Inserts are targeted, removals are broadcast** (§3.1 said only "patch the cache").
`patchTree` walks every cached page — correct for a removal, since the id is on exactly one
page — but using it to insert put the new row into the root list *and* inside every expanded
folder simultaneously. Inserts go through `patchTreeParent(parentId, …)`.
2. **A move cannot infer its target page.** The first cut chose the page to insert into by asking
which cached page already held rows with that `parentId`. An EMPTY folder has no row to infer
from, so a file moved into one left its old parent and appeared nowhere until the next
refetch.
3. **`cancelQueries` reverts a mid-flight query to its last successful data**, which is
`undefined` for a section opened moments ago — so `patchDrive` had nothing to attach rows to
and dropped them all. It now seeds a minimal listing object. Reproduce by opening External
drives and clicking Add file before the listing lands.
Also: `note_N` counts every cached tree page, not only the flat list. The flat list's cache entry
is garbage-collected while nothing observes it, so the count silently reset to zero and two
creates in a row both chose `note_2` — the exact bug §2.2 predicted, arriving through a different
door than expected.
Headless proof (worktree API on **8798**, as <email>, conversation `86313104…`):
`mkdir → touch → rename → mv → list (moved) → rm-guarded → list (empty) → restore → list (same
id, same parent, same path) → drive unlink → drive relink → cleanup`. Note the port: **8797 is
occupied by a stale server from the primary checkout**, so the worktree `.env` repoint does not
take effect and driving 8797 would have proved nothing about this branch.