Show saves immediately and publish behind them #3

Merged
laoc merged 2 commits from feat/optimistic-ui into main 2026-08-05 18:51:52 +00:00
Owner

Items 3 and 4 of the speed lane, together because the optimistic insert is what replaces the refetch. Stacked on #2.

Saving used to make the user watch: Amber, then the relay ack, then a list re-read, then one publish per list, then a full three-collection refetch. Only the first of those is the user's own decision time.

The claim this rests on, tested directly

The signature is the point of no return, not the relay ack. Once the signer returns, the event's id is fixed and it verifies — so the row goes on screen then, and the publish happens behind it. A relay refusing it afterwards is a retry of the same bytes, never a second trip through the signer.

If that claim were wrong the whole design would be, so SignThenPublishE2ETest tests it on a real device: sign, assert the relay does not yet have it, publish separately, assert the id the UI showed is the id that landed. Plus the retry path — a silent relay fails the publish, and the same event object then publishes successfully to a working relay with no second signature.

What the user sees

Rows carry their own state: Sending… while in flight, Not sent: <reason> with a Retry button if no relay took it, and nothing at all once settled — which is almost every row. Optimism is only honest if those states are distinguishable; a pending row that looked identical to a saved one would be the lie.

Covers saveDraft, saveHighlight, deleteBookmark and deleteHighlight.

Item 4, which falls out of it

Seven call sites ended a mutation by re-reading all three collections from the network to be told what we had just written. refresh() references go 8 → 5, and the remaining five are the definition plus four legitimate startup/login calls.

The local updates are pure and unit-tested (optimisticUpsert, optimisticRemove, upsertList), and all three refuse to let an older version displace a newer one — so a lagging relay answering after an edit cannot undo it.

What deliberately did NOT become optimistic

Everything in saveHighlight that keys on the relay outcome. CompanionGate decides whether publishing a blank 39701 would destroy an existing bookmark, and it must act on what the relays actually did, never on a signature. Those orderings are unchanged.

A behaviour change worth a look

The per-list Amber prompts now appear after the user lands back on the bookmarks screen, rather than while they wait on the save sheet. Same number of prompts, different screen. I think it is better, but it is a judgement call.

Testing

full instrumented suite   expected=43 started=43 passed=43 failed=0
JVM unit suite            BUILD SUCCESSFUL

The last commit also fixes the harness that kept truncating those runs. am instrument -w streams over adb, so a run only survives as long as that connection — three runs today died at 9/35, 39/43 and 41/43, each at a different point, every one with zero test failures. The instrumentation was never the problem; the pipe was. It now runs detached on the device and is polled for INSTRUMENTATION_CODE, the one marker meaning the whole run ended. First attempt after the change: 43/43.

Items 3 and 4 of the speed lane, together because the optimistic insert *is* what replaces the refetch. Stacked on #2. Saving used to make the user watch: Amber, then the relay ack, then a list re-read, then one publish per list, then a full three-collection refetch. Only the first of those is the user's own decision time. ### The claim this rests on, tested directly **The signature is the point of no return, not the relay ack.** Once the signer returns, the event's id is fixed and it verifies — so the row goes on screen then, and the publish happens behind it. A relay refusing it afterwards is a retry of the same bytes, never a second trip through the signer. If that claim were wrong the whole design would be, so `SignThenPublishE2ETest` tests it on a real device: sign, assert the relay does **not** yet have it, publish separately, assert the id the UI showed is the id that landed. Plus the retry path — a silent relay fails the publish, and the *same event object* then publishes successfully to a working relay with no second signature. ### What the user sees Rows carry their own state: `Sending…` while in flight, `Not sent: <reason>` with a **Retry** button if no relay took it, and nothing at all once settled — which is almost every row. Optimism is only honest if those states are distinguishable; a pending row that looked identical to a saved one would be the lie. Covers `saveDraft`, `saveHighlight`, `deleteBookmark` and `deleteHighlight`. ### Item 4, which falls out of it Seven call sites ended a mutation by re-reading all three collections from the network to be told what we had just written. `refresh()` references go 8 → 5, and the remaining five are the definition plus four legitimate startup/login calls. The local updates are pure and unit-tested (`optimisticUpsert`, `optimisticRemove`, `upsertList`), and all three refuse to let an older version displace a newer one — so a lagging relay answering after an edit cannot undo it. ### What deliberately did NOT become optimistic Everything in `saveHighlight` that keys on the relay outcome. `CompanionGate` decides whether publishing a blank `39701` would destroy an existing bookmark, and it must act on what the relays actually did, never on a signature. Those orderings are unchanged. ### A behaviour change worth a look The per-list Amber prompts now appear after the user lands back on the bookmarks screen, rather than while they wait on the save sheet. Same number of prompts, different screen. I think it is better, but it is a judgement call. ### Testing ``` full instrumented suite expected=43 started=43 passed=43 failed=0 JVM unit suite BUILD SUCCESSFUL ``` The last commit also fixes the harness that kept truncating those runs. `am instrument -w` streams over adb, so a run only survives as long as that connection — three runs today died at 9/35, 39/43 and 41/43, each at a different point, **every one with zero test failures**. The instrumentation was never the problem; the pipe was. It now runs detached on the device and is polled for `INSTRUMENTATION_CODE`, the one marker meaning the whole run ended. First attempt after the change: 43/43.
The save path used to make the user watch: Amber, then the relay ack,
then a list re-read, then one publish per list, then a full
three-collection refetch. Only the first of those is the user's own
decision time.

The signature is the point of no return, not the relay ack. Once the
signer returns, the event's id is fixed and it verifies — so the row goes
on screen then, and the publish happens behind it. A relay refusing it
afterwards is a retry of the same bytes, never a second trip through the
signer, which is why the signed event is kept.

sign() and publish() are split for exactly that. Rows carry their state:
"Sending…" while in flight, "Not sent: <reason>" with a Retry button if
no relay took it, and nothing at all once settled — which is almost every
row. Optimism is only honest if those states are distinguishable.

Also drops the post-publish refetches. Seven call sites ended a mutation
by re-reading all three collections from the network to be told what we
had just written. They now update locally: optimisticUpsert for
bookmarks, upsertList for lists, both pure and unit-tested, both refusing
to let an older version displace a newer one so a lagging relay cannot
undo an edit.

What deliberately did NOT become optimistic: everything in saveHighlight
that keys on the relay outcome. CompanionGate decides whether a blank
39701 would destroy an existing bookmark, and it must act on what the
relays actually did, never on a signature.
Run instrumentation on the device, not down the adb pipe
All checks were successful
build / build (push) Successful in 6m39s
d3a3a905b0
`am instrument -w` streams its output back over adb, so the run only
survives as long as that connection does — and it does not. Three runs on
2026-08-05 died at 9/35, 39/43 and 41/43, each at a different point and
each with ZERO test failures. The instrumentation was never the problem;
the pipe carrying its output was.

Now the run is detached on the device with nohup, writing to device
storage, and polled for INSTRUMENTATION_CODE — the one marker that means
the whole run ended rather than a single test. The log is pulled
afterwards. A dropped connection costs a poll instead of a run.

First attempt after this change: 43/43.

Also tidies two things from the previous commit: imports instead of
fully-qualified names for ConcurrentHashMap and Event, and the row body
reindented now that it sits inside a Column.
laoc changed target branch from feat/local-cache to main 2026-08-05 18:51:43 +00:00
laoc merged commit 65fc439503 into main 2026-08-05 18:51:52 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
laoc/stash-bookmark!3
No description provided.