docs: retire the change-signals TODO into the ADRs that own it
Build Debug APK / build (push) Successful in 5m34s
Build Debug APK / build (push) Successful in 5m34s
The work is done: every screen on every modality follows the store, and the polls this started as are gone. What was left in TODO.md was either a fact about the design or a deliberate exception, and both belong where someone reading the design will meet them rather than in a list of things to do. ADR 0041 gains "Where this ended up": the in-memory P2P state no store channel can announce, the players that must never follow one, the record screen in edit mode, the Sync tab's multi-second walk, and the timers that were never polls. Its amendment now names SyncStagingStore as the one store still to be held to the rule. ADR 0031 gains the design that is actually owed — what a *form* needs that a single-value guard doesn't cover: a per-field "keep mine" that doesn't revert four fields nobody disagreed about, and an alert that can say which fields moved. The two stopgap call sites point there now instead of at TODO.md.
This commit is contained in:
@@ -409,81 +409,6 @@ since it isn't worth taking before it shows up in a profile.
|
||||
|
||||
## Core Functionality
|
||||
|
||||
### Change signals: what still isn't announced
|
||||
Every announcing store shares one `ChangeLedger` (`stores::change_bus`), and
|
||||
`DaemonCommand::AwaitChange` carries it to clients with no store of their own —
|
||||
see ADR 0041. The GUI subscribes in-process through `gui-app/src/store_watch.rs`
|
||||
(coalesced, and only for the screen on display); the TUI parks one thread per
|
||||
process and hands each moved channel to the screens that read it; Android
|
||||
collects one shared `StoreChanges` flow through `OnStoreChange`. What's left:
|
||||
- **The P2P node announces nothing**: gossip neighbour count and per-member
|
||||
live reachability are in-memory, not store rows, so the Ensemble screens
|
||||
can't be told when they move and Refresh is still the way to ask. A channel
|
||||
for those would have to come from `P2PNode`, not the store, which is why the
|
||||
policy below doesn't already cover it.
|
||||
- **`SyncStagingStore` is the last store that announces nothing.** Settings has
|
||||
a `ChangeSignal` now, and with it the rule changed — ADR 0041's amendment:
|
||||
every store that writes something a screen shows gets one, rather than
|
||||
waiting for a screen to ask. Staging is what's left to hold to that, and the
|
||||
screen it would serve is a sync in progress showing its own staged count
|
||||
rather than a figure from when it last looked.
|
||||
- **The podcast screens follow the store now on both modalities** — library,
|
||||
subscriptions and detail. `PodcastRepository::reloadPodcasts` is the seam
|
||||
Android's other screens had as a counter, and `Podcast` compares by what it
|
||||
draws rather than by id, so an unrelated sync publishes nothing and
|
||||
recomposes nothing. Both detail screens are settled-or-hold, and they draw
|
||||
the line in different places for a reason worth remembering:
|
||||
`IncomingEpisodes.arrivalOf` can apply most arrivals because Compose keys the
|
||||
list by episode and re-finds the row under the user's finger, while gpui
|
||||
draws its list by index and so falls back to `SyncGuarded` plus a scroll-at-
|
||||
top test. Still open on those screens:
|
||||
- **The players deliberately don't follow the store, and that is now
|
||||
written down** — `docs/audio-player-ui-design-principles.md`'s "What syncs,
|
||||
and what doesn't". Live player state is local to the node playing; only the
|
||||
progress artifact travels. What is left is small and optional: a player
|
||||
screen sitting open and *not* playing could show a position that arrived
|
||||
from another node, so the resume point is current when the user presses
|
||||
play. Nothing may seek under a listener who is playing.
|
||||
- **A held arrival can only be taken whole.** Android's
|
||||
`ChangedElsewhereStrip` says episodes changed and offers Show; it can't say
|
||||
how many, or which. The desktop's line does say how many (it holds the list
|
||||
it would apply, so it can count) — the Documents list on both has the same
|
||||
limitation, so this wants one answer, not three.
|
||||
- **The Settings screens follow the `Settings` channel now**, on the desktop
|
||||
and on Android, for the switches and menus — sync policy, the scheduler row,
|
||||
unverified-deletion. The desktop's *text* fields (node name, the timezone
|
||||
search) are deliberately left out: they hold what the user has typed and not
|
||||
yet saved. Making those follow properly is the same alert-don't-apply design
|
||||
the Humans detail screen wants, and should be done once, for both.
|
||||
- **Record screens need one alert-don't-apply design, and don't have it.** The
|
||||
Humans detail screen on both modalities now follows the store the expedient
|
||||
way: the timeline always (an interaction is its own document, shared with its
|
||||
attendees, and lives outside the form), the profile only while it is being
|
||||
*read*. In edit mode the arrival is dropped on the floor, the local edit
|
||||
stands, and Save wins — which is the thing CLAUDE.md says must not happen by
|
||||
default: the user is never told what they gave up, and the save that
|
||||
overwrites the other node's version is exactly the "silently wins the
|
||||
conflict" case. It is honest as a stopgap because a record edited in two
|
||||
places at once is rare; it is not the answer.
|
||||
- What the answer needs: a marker on the form saying a newer version arrived,
|
||||
a statement of what each choice costs (take theirs and lose the staged
|
||||
edit; keep mine and overwrite theirs), and a Save held until the user
|
||||
chooses. The desktop's `SyncGuarded` is the state half of it and the Notes
|
||||
editor's banner is the shape, but a *form* is not a text buffer: the
|
||||
conflict is per-field, and "keep mine" on a form the user only changed one
|
||||
field of should not overwrite the four fields they didn't touch.
|
||||
- Do it once and apply it to all of them: Humans on both modalities, the
|
||||
desktop Settings screen's text fields (node name, timezone), and whatever
|
||||
record screen comes next. `human.rs`'s `refresh_detail` and
|
||||
`HumanDetailScreen.kt`'s `OnStoreChange` are where the stopgap is, and both
|
||||
say so.
|
||||
- **Settings' Sync tab still reloads only when shown.** Its storage stats are a
|
||||
multi-second store walk, so this one should stay that way unless the walk
|
||||
gets cheaper.
|
||||
- `RemoteFetch::watch` and the pairing card's tick stay timers on purpose — a
|
||||
stall clock and a countdown. Both have to fire on *absence*, which no change
|
||||
signal can do.
|
||||
|
||||
### Keeping track of long-running jobs
|
||||
Model and dedup design: `docs/adr/0011-jobs-first-class-model.md`; product design:
|
||||
`docs/sync-management.md`'s "Jobs" section.
|
||||
|
||||
@@ -254,8 +254,9 @@ fun HumanDetailScreen(
|
||||
//
|
||||
// That is the expedient answer rather than the right one: the rule asks for the arrival to be
|
||||
// announced, for the user to be told what each choice costs, and for the save that would
|
||||
// silently overwrite it to wait until they have chosen. See TODO.md — it wants designing once
|
||||
// for the record screens, not once per screen.
|
||||
// silently overwrite it to wait until they have chosen. ADR 0031's "What a form needs, and
|
||||
// this does not cover yet" is where that design is owed — it wants doing once for the record
|
||||
// screens, not once per screen.
|
||||
OnStoreChange(nativeLib, ChangeChannel.DOCUMENTS) {
|
||||
historyReloadToken++
|
||||
if (!editing) reloadToken++
|
||||
|
||||
@@ -99,6 +99,42 @@ body alike; a future consumer needing a fuzzier "did this meaningfully
|
||||
change" comparison would need its own wrapper around `SyncGuarded`, not a
|
||||
change to it.
|
||||
|
||||
### What a form needs, and this does not cover yet
|
||||
|
||||
*Added 2026-08-26, when the record screens ran into it.*
|
||||
|
||||
Every consumer above holds one value the user reads: a note body, a queue, a
|
||||
list of clippings. A **record screen** — Humans' person detail, the desktop
|
||||
Settings screen's text fields — holds a *form*, and the same rule lands
|
||||
differently there in two ways this type does not answer.
|
||||
|
||||
**The conflict is per-field, so the two choices are not symmetrical.** "Take
|
||||
theirs" is the same act it always was. "Keep mine" on a form the user changed
|
||||
one field of must not write back the four fields they never touched — that
|
||||
would silently revert the other node's edits to fields nobody here disagreed
|
||||
about, which is a worse outcome than the conflict it resolves. A
|
||||
`SyncGuarded<Profile>` compares whole values, so it can say *that* the record
|
||||
moved and hand back both versions; deciding what "mine" means is per-field work
|
||||
above it, and the Humans screen already has the machinery for exactly that in
|
||||
its `diff_staged`.
|
||||
|
||||
**The alert has to be able to say what arrived.** `ChangedElsewhereStrip` and
|
||||
its desktop equivalents say "changed elsewhere" and offer Show; on a form the
|
||||
user needs to know *which* fields moved before they can choose, and CLAUDE.md
|
||||
requires the choice to name what each answer costs. That is an affordance the
|
||||
list screens have never needed and do not have — the Documents lists on both
|
||||
modalities can only offer their held page whole.
|
||||
|
||||
Until that is designed, the record screens take the expedient answer and say so
|
||||
at the call site: they follow the store while the record is being *read*, and in
|
||||
edit mode they drop the arrival and let the local edit stand and win at Save
|
||||
(`agents/human.rs`'s `refresh_detail`, `HumanDetailScreen.kt`'s
|
||||
`OnStoreChange`). That is honest only because a record edited in two places at
|
||||
once is rare; it is the case CLAUDE.md names as forbidden by default, and it
|
||||
should be designed once for every record screen rather than three times. Android
|
||||
is the other half of it: it has no `SyncGuarded` equivalent at all, only the
|
||||
by-hand pairs each screen keeps.
|
||||
|
||||
### Where it lives
|
||||
|
||||
`rust/gui-app/src/components/` — alongside `player_controls.rs` and the
|
||||
@@ -137,9 +173,8 @@ overwrite-the-remote behavior (see above) when mapping it onto
|
||||
`notes/mod.rs`'s equivalent logic today can only be exercised through the
|
||||
view itself.
|
||||
- `notes/mod.rs` keeps its existing, working implementation unchanged for
|
||||
now — this ADR records the intended future migration
|
||||
(see [[project_sync_management_design]]-adjacent tracking, or a
|
||||
`TODO.md` entry) rather than performing it.
|
||||
now — this ADR records the intended future migration rather than performing
|
||||
it, and is the record: there is no tracking entry elsewhere.
|
||||
|
||||
## Open questions (not blocking, but unresolved)
|
||||
|
||||
|
||||
@@ -68,7 +68,9 @@ Sync policy is the worked example: it looks like a local preference, so
|
||||
`p2p::node_admin` let a remote administrator set it on this node while the tab
|
||||
showing it sat there. A store that writes anything a screen shows should have a
|
||||
`ChangeSignal`, and marking the write is the store's job rather than a judgement
|
||||
about which fields are interesting.
|
||||
about which fields are interesting. `SyncStagingStore` is the one store still to
|
||||
be held to that; the screen it would serve is a sync in progress showing its own
|
||||
staged count rather than a figure from when it last looked.
|
||||
|
||||
`JobsCmd::AwaitChange` is replaced by a top-level `DaemonCommand::AwaitChange
|
||||
{ since: ChangeCursor, timeout_ms }`, answering with a `ChangeSnapshot`. The
|
||||
@@ -227,6 +229,28 @@ per-member live reachability are in-memory P2P state; nothing announces them,
|
||||
and the Ensemble screen's `r` remains the way to ask. A channel for them would
|
||||
have to come from the P2P node, not the store.
|
||||
|
||||
**Where this ended up.** Every screen on every modality follows the store now,
|
||||
and the polls this ADR set out to remove are gone. What is *deliberately* not
|
||||
following it, so the next reader doesn't take any of these for an oversight:
|
||||
|
||||
- **In-memory P2P state.** Gossip neighbour count and per-member live
|
||||
reachability are not store rows, so no store channel can announce them; the
|
||||
Ensemble screens' Refresh is still the way to ask. A signal for those would
|
||||
have to come from `P2PNode` — see the paragraph above.
|
||||
- **The media players.** Live player state — playing or paused, the current
|
||||
second, the volume — is local to the node playing and never travels; only the
|
||||
progress artifact does. A player that followed the store would be one step
|
||||
from seeking under whoever is listening. `docs/audio-player-ui-design-
|
||||
principles.md`'s "What syncs, and what doesn't" is the statement of it.
|
||||
- **A record screen in edit mode**, which drops the arrival and lets the local
|
||||
edit win at Save. That is a stopgap and is recorded as one in ADR 0031's
|
||||
"What a form needs, and this does not cover yet", which is where the design
|
||||
it is waiting on belongs.
|
||||
- **Settings' Sync tab storage stats**, which are a multi-second store walk. The
|
||||
tab follows the `Settings` channel for its policies; the stats stay
|
||||
read-on-show until the walk is cheaper.
|
||||
- **Timers that were never polls**, below.
|
||||
|
||||
**The TUI's 50 ms event-loop tick is not a data poll and stays.** It is the
|
||||
terminal's redraw cadence and the ceiling on how often a change is applied,
|
||||
which is also what coalesces a burst: a large import reports every 4 MiB, and
|
||||
|
||||
@@ -845,7 +845,8 @@ impl HumansView {
|
||||
/// alert-don't-apply: the arrival should be announced on the form, the user
|
||||
/// told what each choice costs, and the save that would silently overwrite
|
||||
/// it held until they choose. That wants designing once for the record
|
||||
/// screens rather than three times — see TODO.md.
|
||||
/// screens rather than three times — ADR 0031's "What a form needs, and
|
||||
/// this does not cover yet" is where that design is owed.
|
||||
fn refresh_detail(&mut self, cx: &mut Context<Self>) {
|
||||
if self.detail.is_none() {
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user