From 4921adb4e687638a398dbd2d432288e6279074ef Mon Sep 17 00:00:00 2001 From: Tim Disney Date: Wed, 29 Jul 2026 18:33:28 +0000 Subject: [PATCH] Make UI writes update locally (#21) Co-authored-by: codexbot.disnetdev.com (did:plc:hbonvqr5ysrscg5wdyb5klie) --- packages/ui/src/lib/session.svelte.ts | 28 ++++++++++++++++++++-------- packages/ui/src/lib/write.test.ts | 29 +++++++++++++++-------------- packages/ui/src/lib/write.ts | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------ packages/ui/src/lib/components/Composer.svelte | 2 +- packages/ui/src/lib/components/NewGoal.svelte | 4 ++-- packages/ui/src/lib/components/NewRequest.svelte | 4 ++-- packages/ui/src/lib/components/UnitDetail.svelte | 2 +- 7 file(s) changed, 100 insertion(s)(+), 40 deletion(s)(-) diff --git a/packages/ui/src/lib/session.svelte.ts b/packages/ui/src/lib/session.svelte.ts --- a/packages/ui/src/lib/session.svelte.ts +++ b/packages/ui/src/lib/session.svelte.ts @@ -17,7 +17,7 @@ // - **Opening a space is the only await.** Hydration, the cold scan, and handle resolution all // finish before `status` becomes `ready`, so no view ever renders a half-loaded space. -import { COLLECTIONS, materialize, type MaterializedIndex } from '@radial/core' +import { COLLECTIONS, materialize, type MaterializedIndex, type StoredRecord } from '@radial/core' import { FetchRepoTransport, parseAtUri, type RepoReadTransport } from '@radial/atproto' import { RepoPoller, SpaceIngestor } from '@radial/ingest' import { buildDirectory } from './directory.js' @@ -248,14 +248,26 @@ export const spaceTransport = (): RepoReadTransport | undefined => live?.ingestor.poller.transport /** - * Re-read one repo and refold. This is what a write is followed by: the record is in the author's - * own repo, so re-listing theirs is the whole of the news, and doing it before the screen changes - * means the human never sees the gap between "written" and "there". One head check and one repo - * scan, against the ten-second tick's one head check per member. + * Fold records the PDS has just acknowledged without re-reading that repo first. * - * The alternative — inserting the record locally — was considered and rejected: the store's - * earliest-revision and edit-annotation rules key off the repo revision a record was observed at, - * and a synthetic one would collide with the real one on the next poll. + * The response to createRecord/putRecord already gives the durable URI and CID, while the writer + * still has the exact validated value it sent. The only field it does not know is the observer-local + * repo revision. A high placeholder makes an acknowledged edit newer than the version currently in + * the store; the next ordinary poll sees the same CID and replaces this provenance with the real, + * lower revision. Append-only records do not depend on the placeholder at all. + */ +export function publishWritten(records: StoredRecord[]): void { + const current = live + if (!current || current.stopped || records.length === 0) return + for (const record of records) current.stores.records.put(record) + publish(materialize(current.stores.records, { spaceUri: current.uri })) + session.error = '' +} + +/** + * Re-read one repo and refold. The acknowledged-write path above avoids this extra round trip; this + * remains useful when a caller explicitly needs everything else that moved in one author's repo. + * One head check and one repo scan, against the ten-second tick's one head check per member. * * It reports a failure the way a poll does — `session.error`, the pane bar's "stale" — rather than * throwing. By the time this runs the record is written and public; saying the write failed because diff --git a/packages/ui/src/lib/write.test.ts b/packages/ui/src/lib/write.test.ts --- a/packages/ui/src/lib/write.test.ts +++ b/packages/ui/src/lib/write.test.ts @@ -47,8 +47,8 @@ // The write path, end to end, against the same fake PDS the read tests use. Everything between the // stub `fetch` and the screen is the code a browser runs: `RepoWriter` signing with a session it -// cannot read the key of, `runCli` from `@radial/sidecar` building the record, `RepoPoller` -// re-reading the author's repo, `materialize()` folding it. Nothing here is a browser-only +// cannot read the key of, `runCli` from `@radial/sidecar` building the record, and `materialize()` +// folding the acknowledged value. Nothing here is a browser-only // reimplementation of anything — that is the claim 6.3 is making, and this is where it is checked. const fixture = fixtureSpace() @@ -149,8 +149,8 @@ expect(created?.did).toBe(FIXTURE_DIDS.tim) expect(created?.authorization).toBe(`DPoP ${TOKEN}`) - // …and `write` re-read that repo, so the index a route renders from already holds it. Nothing - // waits for the ten-second tick, and nothing was inserted locally to fake it. + // …and `write` folded the PDS-acknowledged URI, CID and value immediately, so the index a route + // renders from already holds it without a second network round trip. const goals = goalsOf(space().index, project()) expect(goals).toHaveLength(before + 1) expect(goals.some((goal) => goal.target.uri === result.primary.uri)).toBe(true) @@ -158,7 +158,7 @@ expect(space().index.edits).toEqual([]) }) - it('re-reads only the repo that moved', async () => { + it('does not re-read the repo after the PDS acknowledges the write', async () => { const harness = await open() signInAs(FIXTURE_DIDS.tim, harness.network) harness.network.reset() @@ -171,12 +171,12 @@ '--title', 'Only one repo moved', '--body', - 'So only one repo is re-read.', + 'So no repo re-read is needed.', ]) - const listed = harness.network.calls.filter((call) => call.method === 'listRecords') - expect(listed.length).toBeGreaterThan(0) - expect(listed.every((call) => call.did === FIXTURE_DIDS.tim)).toBe(true) + expect(harness.network.counts('createRecord')).toBe(1) + expect(harness.network.counts('getLatestCommit')).toBe(0) + expect(harness.network.counts('listRecords')).toBe(0) }) it('pins the project to the version it read, rather than to whatever the URI names later', async () => { @@ -1035,9 +1035,9 @@ expect(harness.network.counts('createRecord')).toBe(0) }) - it('does not report a written record as a failed write when the re-read cannot reach the repo', async () => { + it('shows an acknowledged write even when the PDS goes away immediately afterwards', async () => { const harness = await open() - // The PDS accepts the write and then goes away before the tab can re-read the repo. + // The PDS accepts the write and then goes away. There is no second read in the action path. adopt(FIXTURE_DIDS.tim, async (path, init) => { const response = await harness.network.fetch(new URL(path, FAKE_PDS), init) harness.network.offline = true @@ -1055,10 +1055,11 @@ 'The record exists whether or not this tab could re-read it.', ]) - // The record is real and public. What failed is this tab's view of it, which is what the pane - // bar's "stale" says — claiming the write failed would be claiming something untrue. + // The acknowledged record is real, public and already in this tab's fold. The next background + // poll may report the repo stale, but the action itself needs no further network response. expect(parseAtUri(result.primary.uri).collection).toBe(COLLECTIONS.goal) - expect(session.error).toBe('offline') + expect(space().index.goals.some((goal) => goal.target.uri === result.primary.uri)).toBe(true) + expect(session.error).toBe('') expect(session.status).toBe('ready') }) diff --git a/packages/ui/src/lib/write.ts b/packages/ui/src/lib/write.ts --- a/packages/ui/src/lib/write.ts +++ b/packages/ui/src/lib/write.ts @@ -38,20 +38,25 @@ // Both are answered out of the fold this tab is already holding rather than off the network: the // CLI's versions of them go and look because a shell has nothing to look in. -import { FetchRepoTransport, StrongRefResolver, resolveHandleDid, type RepoReadTransport } from '@radial/atproto' -import { runCli, type CliDependencies, type CommandResult } from '@radial/sidecar' +import { FetchRepoTransport, StrongRefResolver, parseAtUri, resolveHandleDid, type RepoReadTransport } from '@radial/atproto' +import { type Collection, type RecordByCollection, type StoredRecord, type StrongRef } from '@radial/core' +import { runCli, type CliDependencies, type CommandResult, type RecordWriter } from '@radial/sidecar' import { signedIn, type Credentials } from './auth.svelte.js' import { Identities } from './identity.js' -import { refreshRepo, session, spaceTransport } from './session.svelte.js' +import { publishWritten, session, spaceTransport } from './session.svelte.js' import { toast } from './ui.svelte.js' /** * The seams every command runs through, whichever transport is behind them. Split out because * `createSpace` is the one write with no space open, and so with no space transport to borrow. */ -function dependencies(credentials: Credentials, transport: RepoReadTransport): CliDependencies { +function dependencies( + credentials: Credentials, + transport: RepoReadTransport, + writer: RecordWriter = credentials.writer, +): CliDependencies { return { - writer: credentials.writer, + writer, resolver: new StrongRefResolver(transport), // A handle is rebindable and a DID is not, so every actor argument is resolved at write time. // The directory asked is the writer's own PDS: a browser has no DNS, and the identity we are @@ -76,11 +81,52 @@ } } +const OPTIMISTIC_REV = '\uffff-ui-write' + /** - * Run one command and leave the screen showing its result. The refresh is part of the write, not an - * optimisation on top of it: a caller that navigates after this returns is navigating to a view that - * already holds the record. It cannot fail the write, though — by then the record exists and is - * public, so a refresh that could not reach the repo makes the view stale, not the write wrong. + * Keep the validated values sent by runCli beside the durable refs returned by the PDS. That pair + * is everything the fold needs except the repo revision, which is observer-local and reconciled by + * the next poll. + */ +function recordingWriter(writer: RecordWriter, did: string, records: StoredRecord[]): RecordWriter { + const remember = ( + collection: K, + value: RecordByCollection[K], + ref: StrongRef, + ): StrongRef => { + const parts = parseAtUri(ref.uri) + records.push({ + did, + collection, + rkey: parts.rkey, + uri: ref.uri, + cid: ref.cid, + rev: OPTIMISTIC_REV, + firstSeenAt: new Date().toISOString(), + value: structuredClone(value), + } as StoredRecord) + return ref + } + return { + create: async (collection, value, options) => + remember(collection, value, await writer.create(collection, value, options)), + ...(writer.put + ? { + put: async ( + collection: K, + uri: string, + value: RecordByCollection[K], + options?: { swapRecord?: string }, + ) => remember(collection, value, await writer.put!(collection, uri, value, options)), + } + : {}), + } +} + +/** + * Run one command and leave the screen showing its result. Once the PDS acknowledges each write, + * fold that exact URI, CID and value locally instead of waiting for a second network round trip to + * re-read it. The normal poll remains the authority for the repo revision and reconciles it later. */ export async function write(args: string[]): Promise { const credentials = signedIn() @@ -90,9 +136,10 @@ throw new Error('This space is not live — the fixture is synthetic records, with no repo to write to.') } - const result = await runCli(args, dependencies(credentials, transport)) - - await refreshRepo(credentials.did) + const records: StoredRecord[] = [] + const writer = recordingWriter(credentials.writer, credentials.did, records) + const result = await runCli(args, dependencies(credentials, transport, writer)) + publishWritten(records) return result } diff --git a/packages/ui/src/lib/components/Composer.svelte b/packages/ui/src/lib/components/Composer.svelte --- a/packages/ui/src/lib/components/Composer.svelte +++ b/packages/ui/src/lib/components/Composer.svelte @@ -19,7 +19,7 @@ // The first thing this app ever wrote. A message is the smallest record with a target — one goal, // one body — so it is where the whole write path gets proved: the OAuth session signs it, the - // sidecar's `runCli` builds it, the author's own repo takes it, and the poll that follows puts it + // sidecar's `runCli` builds it, the author's own repo takes it, and the acknowledged value is folded // on screen through the same fold that drew everything above it. // // It now writes two more things, and both are the same record with one more field on it: a REPLY diff --git a/packages/ui/src/lib/components/NewGoal.svelte b/packages/ui/src/lib/components/NewGoal.svelte --- a/packages/ui/src/lib/components/NewGoal.svelte +++ b/packages/ui/src/lib/components/NewGoal.svelte @@ -94,8 +94,8 @@ // the next ⊕ pressed on a smart list starts from where the reader last actually wrote. rememberGoalProject(project) closeDraft() - // `write` has already re-read the author's repo, so the goal is in the index this navigates - // into — no empty page waiting for the next tick. + // `write` has already folded the PDS-acknowledged record, so the goal is in the index this + // navigates into — no empty page waiting for the next tick. await goto(goalUriHref(result.primary.uri)) } catch (failure) { error = failure instanceof Error ? failure.message : String(failure) diff --git a/packages/ui/src/lib/components/NewRequest.svelte b/packages/ui/src/lib/components/NewRequest.svelte --- a/packages/ui/src/lib/components/NewRequest.svelte +++ b/packages/ui/src/lib/components/NewRequest.svelte @@ -110,13 +110,13 @@ if (system) { // The one case that navigates: a capture's row lands in System, not in the goal this card // was opened from, and a toast over an unchanged list would be the app saying something - // happened somewhere the reader cannot see. `write` has already re-read the author's repo, + // happened somewhere the reader cannot see. `write` has already folded the acknowledged record, // so the row is there — and a unit with nothing landed is keyed by its request (§3.7). toast(wroteCapture(requested)) await goto(unitHref(systemHref(system), { key: result.primary.uri })) } else { // No navigation: the request is a new row in the list this card is sitting on top of, and - // `write` has already re-read the author's repo, so it is there by the time the card closes. + // `write` has already folded the acknowledged record, so it is there by the time the card closes. toast(wroteRequest(requested, named)) } } catch (failure) { diff --git a/packages/ui/src/lib/components/UnitDetail.svelte b/packages/ui/src/lib/components/UnitDetail.svelte --- a/packages/ui/src/lib/components/UnitDetail.svelte +++ b/packages/ui/src/lib/components/UnitDetail.svelte @@ -145,7 +145,7 @@ asking = false judging = verdict } - // A written review re-reads the author's repo, so the version this drawer is showing already + // A written review is folded from the PDS acknowledgement, so the version this drawer is showing already // holds it by the time the card closes. const closeCards = (): void => { judging = null -- tangled.sh