record §5.5 and the close of tier a
This commit is contained in:
@@ -18,12 +18,12 @@ before deciding what to pick up next. Its conclusions in one paragraph:
|
||||
resize-debounce lost-update items (§5.5), the never-invalidated cache (§5.5), and tests for
|
||||
`layout-utils.ts` (§9). Five of the seven are one defect — _a write that silently did not land_ — and
|
||||
they matter because panel identity now lives in the layout jsonb.
|
||||
**Done as of 2026-08-07:** §1's `.catch(() => {})` (`70c2f08`), all of §2 and §3's missing `userId`
|
||||
predicate (`f4ed740`), §9 (`85452d1`), and all three Tier A items in §4 — the error boundary
|
||||
(`64961d4`), validate-on-read and the `'[]'` layout default (`ef036df`).
|
||||
**Left in Tier A: §5.5's two debounce items and the never-invalidated cache.** Everything the re-rank
|
||||
put on the critical path for "the human opens the dashboard in the morning and sees the result" has
|
||||
landed; what remains is a write that can still be lost between a resize and its debounce.
|
||||
**Tier A is empty as of 2026-08-07.** §1's `.catch(() => {})` (`70c2f08`), all of §2 and §3's missing
|
||||
`userId` predicate (`f4ed740`), §9 (`85452d1`), all three Tier A items in §4 — the error boundary
|
||||
(`64961d4`), validate-on-read and the `'[]'` layout default (`ef036df`) — and §5.5's three: the two
|
||||
resize-debounce lost updates (`b0a32ae`) and the never-invalidated cache (`81ad3ef`).
|
||||
Everything the re-rank put on the critical path for "the human opens the dashboard in the morning and
|
||||
sees the result" has landed. **Pick up from Tier B.**
|
||||
- **Tier B:** `dashboardId`-as-a-bag (§5.9) — the agent address book is keyed on it; panel lifecycle
|
||||
(§5.1), but inverted: the requirement is that closing a chat panel must _not_ destroy the agent;
|
||||
`normalizeLayout` as framework (§5.4); and the mobile-collapse decision (§6), which swings on one
|
||||
@@ -74,10 +74,14 @@ Both are two-line fixes, and without them you cannot tell whether any later fix
|
||||
roll the cache back. The rollback is a **compare-and-swap**: it only reverts if the cache still
|
||||
holds exactly what that call wrote. Writes to one key overlap freely (a window resize fires one
|
||||
per group), and a blind rollback over a later successful write would turn one failure into two.
|
||||
**Still swallowed, and not through this hook:** the three direct `client.patch('/dashboards', …)`
|
||||
**The other three, not through this hook, resolved `81ad3ef`:** the direct `client.patch('/dashboards', …)`
|
||||
calls in `apps/Dashboards/DashboardListApp.tsx:93` and `DashboardPreview.tsx:323,326,356` — dashboard
|
||||
create, rename and delete. Same defect, different call site; they bypass `useDashboardState`
|
||||
entirely. A failed delete there leaves a dashboard that reappears on reload.
|
||||
create, rename and delete, which build multi-key patches and so bypass `useDashboardState`. They now
|
||||
call `persistDashboardState`, which shares the hook's in-flight bookkeeping and, on failure,
|
||||
**invalidates rather than reverts**: once the roster has been rewritten and keys dropped from the
|
||||
blob there is no single previous value to swap back, and a refetch is the only thing that makes the
|
||||
list agree with the server. A failed delete used to leave the dashboard gone from the list and alive
|
||||
on the server, reappearing at the next reload with no hint why.
|
||||
|
||||
---
|
||||
|
||||
@@ -355,20 +359,36 @@ does not just resurrect a deleted panel, it **re-writes an older `config`**, so
|
||||
named reverts to anonymous. And the never-invalidated cache means two windows onto the same server-side
|
||||
work disagree permanently about the roster, with neither told — a direct contradiction of §5 Q1._
|
||||
|
||||
- [ ] **The resize debounce can resurrect a deleted panel.** `WorkspaceRenderer.tsx:108-124` holds a
|
||||
500 ms timer in a ref with **no `useEffect`, therefore no cleanup**, and its callback closes over
|
||||
- [x] **The resize debounce can resurrect a deleted panel.** `WorkspaceRenderer.tsx:108-124` held a
|
||||
500 ms timer in a ref with **no `useEffect`, therefore no cleanup**, and its callback closed over
|
||||
the layout as it was when the drag began. Drag a splitter, remove a panel within 500 ms → the timer
|
||||
PATCHes the pre-deletion tree and the panel comes back. Same shape for a split.
|
||||
Fix: clean up on unmount, and make `setValue` accept an updater so it composes against current
|
||||
state instead of a captured one.
|
||||
- [ ] **Concurrent same-key writes clobber.** A window resize fires `onLayout` on every group at once;
|
||||
each schedules its own timer against the same base tree and the last wins
|
||||
(`WorkspaceView.tsx:76-81`). Same fix as above.
|
||||
- [ ] **The cache is never invalidated.** `staleTime: Infinity` and no `invalidateQueries` anywhere in
|
||||
the repo. Meanwhile every PATCH already computes and returns a full fresh state blob
|
||||
(`servers/api/dashboards/dashboards.ts:90`) which the client **discards** — 3 SELECTs per splitter
|
||||
release, thrown away. Consume the response, or stop computing it. Two tabs currently diverge
|
||||
permanently and neither is told.
|
||||
PATCHed the pre-deletion tree and the panel came back. Same shape for a split.
|
||||
**Resolved `b0a32ae`** — the timer now lives behind a `useEffect` that **flushes** it on unmount
|
||||
rather than dropping it: with the write expressed as an updater the early write is correct, and
|
||||
dropping would lose a splitter drag made just before navigating away, which the no-cleanup version
|
||||
did at least persist.
|
||||
- [x] **Concurrent same-key writes clobber.** A window resize fires `onLayout` on every group at once;
|
||||
each scheduled its own timer against the same base tree and the last won
|
||||
(`WorkspaceView.tsx:76-81`).
|
||||
**Resolved `b0a32ae`** — all eight mutations in `WorkspaceView` pass an **updater** to `setValue`
|
||||
(`onLayoutChange((prev) => …)`) instead of a tree computed from the `layout` their callback closed
|
||||
over, so each composes against the current cache. The no-op guards (`countPanels`, the `=== layout`
|
||||
identity checks) still test the rendered `layout`: they only decide whether a write is worth making,
|
||||
and being one render stale there costs a redundant no-op write at worst.
|
||||
Worth keeping in mind for anything new that writes the layout: **compute from `prev`, always.** The
|
||||
cost of getting it wrong is no longer a size that snaps back — it is an older `config`, so a panel
|
||||
that was just given an agent's name reverts to anonymous and the agent stops being addressable.
|
||||
- [x] **The cache is never invalidated.** `staleTime: Infinity` and no `invalidateQueries` anywhere in
|
||||
the repo. Meanwhile every PATCH computed and returned a full fresh state blob which the client
|
||||
**discarded** — 3 SELECTs per splitter release, thrown away.
|
||||
**Resolved `81ad3ef`** — both halves. The PATCH returns `{ok: true}`; nothing had ever read that
|
||||
body, and a caller that did would be reading state assembled *before* whatever concurrent write it
|
||||
raced. The client refetches **on focus**, with three non-default guards, because this cache is
|
||||
optimistic: a refetch that started before an in-flight PATCH landed would overwrite the value
|
||||
already on screen — the same lost-update shape as the two items above, and self-healing only until
|
||||
the next mutation composes on top of the stale tree. So `refetchOnMount: false` (splitting a panel
|
||||
mounts a fresh consumer, which is exactly when a write is in flight), `refetchOnReconnect: false`,
|
||||
and `refetchOnWindowFocus` gated on a module-level in-flight count plus a 2 s quiet period.
|
||||
- [ ] **Preserve sibling sizes on split.** `splitInner`/`insertPanel` redistribute evenly
|
||||
(`100 / newChildren.length`), so one split discards carefully tuned proportions.
|
||||
- [ ] **`tpl-N` panel ids collide across dashboards.** `DashboardPreview.tsx:25-26` —
|
||||
@@ -551,8 +571,8 @@ _(move items here with the commit and a one-line resolution)_
|
||||
404 rather than an INSERT for an unknown dashboard id, and a null that means "forget" rather than
|
||||
"write NULL into a NOT NULL column". Framework: one rule for deriving a terminal's state key
|
||||
(`apps/Terminal/state-key.ts`) where there had been three.
|
||||
The remaining member of that family is the three direct `client.patch('/dashboards', …)` calls in
|
||||
`apps/Dashboards/` — see §1.
|
||||
The remaining member of that family — the three direct `client.patch('/dashboards', …)` calls in
|
||||
`apps/Dashboards/` — followed in `81ad3ef`. See §1.
|
||||
|
||||
- [x] **A bad stored value is no longer a white screen.** _(`64961d4` + `ef036df`, branch
|
||||
`agent-coordination-mvp`)_ — Tier A3 and A4, and with them the first three items of §4. Three
|
||||
@@ -562,6 +582,20 @@ _(move items here with the commit and a one-line resolution)_
|
||||
What this does **not** do is validate on write — §4's last open item. The database will still accept
|
||||
any jsonb at all; the two new guards mean it can no longer reach a renderer.
|
||||
|
||||
- [x] **A layout write can no longer be quietly undone by an older one.** _(`b0a32ae` + `81ad3ef`, branch
|
||||
`agent-coordination-mvp`)_ — Tier A5 and A7, and with them the whole of §5.5, which empties Tier A.
|
||||
One defect in three places, all of it the same shape: a write computed from a snapshot that a later
|
||||
write had already superseded. `WorkspaceView`'s eight mutations pass **updaters** rather than trees;
|
||||
`WorkspaceRenderer`'s resize debounce gained the `useEffect` cleanup it never had, flushing on
|
||||
unmount rather than firing against a captured layout; and the query no longer sits on
|
||||
`staleTime: Infinity` with nothing to converge it — it refetches on focus, and deliberately _not_ on
|
||||
mount or reconnect, because an optimistic cache makes a badly-timed refetch the same lost update
|
||||
again. The PATCH stopped assembling a full state blob nobody read.
|
||||
Worth remembering for anything new here: the cost of getting this wrong is no longer a panel size
|
||||
that snaps back. Panel identity lives in the layout jsonb, so a resurrected tree carries an older
|
||||
`config` — the panel that was just given an agent's name goes anonymous, and the agent stops being
|
||||
addressable through it.
|
||||
|
||||
---
|
||||
|
||||
## 8. Dead code sweep
|
||||
|
||||
Reference in New Issue
Block a user