From 60d8eef340d37f7d9a4f3858e5ee7064cf067610 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Thu, 15 Oct 2026 13:12:44 +0700 Subject: [PATCH] update plan --- AGENTS.md | 6 + docs/PROFILE_DIRECTORIES.md | 230 ++++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+) create mode 100644 docs/PROFILE_DIRECTORIES.md diff --git a/AGENTS.md b/AGENTS.md index bddfca0..74c175e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,12 @@ Action handlers can be registered on an element via the event handler `.on_actio When a view's state has changed in a way that may affect its rendering, it should call `cx.notify()`. This will cause the view to be rerendered. It will also cause any observe callbacks registered for the entity with `cx.observe` to be called. +GPUI also tracks the entities a window reads while rendering. When an entity notifies, every view that read that entity during the previous frame is rerendered automatically. + +* A view that reads an entity in render (e.g. `state.read(cx).profile(...)`) does not need `cx.observe(&entity, ...)` to pick up its changes - the read itself is the dependency. Reserve `cx.observe` and `cx.subscribe` for when the view must run logic in response to a change (rebuild derived state, re-query, dedupe), not to trigger a re-render. +* Notify the narrowest entity holding the change. Notifying a global store rerenders every view that ever read it; notifying a per-panel entity rerenders only the panels reading it. +* This works with cached panels (`panel.cached(...)`): a cached subtree repaints when the view it belongs to is invalidated. + ## Entity events While updating an entity (`cx: Context`), it can emit an event using `cx.emit(event)`. Entities register which events they can emit by declaring `impl EventEmitter for EntityType {}`. diff --git a/docs/PROFILE_DIRECTORIES.md b/docs/PROFILE_DIRECTORIES.md new file mode 100644 index 0000000..8ad5cf8 --- /dev/null +++ b/docs/PROFILE_DIRECTORIES.md @@ -0,0 +1,230 @@ +# Per-view profile directories + +Status: proposal +Reference: coop `a365a8f` "bind user metadata to per-view directories" + +## Background + +`ProfileStore` (`crates/signed_state/src/profile.rs`) is a single global entity: + +- Warm-loads the 500 most recent metadata events at startup (`WARM_LIMIT`). +- `get(pk)` returns a cached clone or a placeholder and queues a network fetch + (flume channel, 500 ms batching, chunks of 100, `sync_bootstrap_only`). +- Subscribes to `BackendEvent::ProfileUpdates` to re-query the local database. +- Every view reads through `ProfileStore::global(cx)`. + +Problems: + +- Invalidation is global: every `cx.notify()` on the store invalidates every + panel that read it, so any profile update repaints all profile-consuming + surfaces (including `panel.cached` ones), not just the panel whose data + changed. +- Startup work and memory scale with the local database, not with what is on + screen, and the global map never shrinks. +- Every screen shares one data set, so nothing is cancellable or reclaimable. + +## Goals + +- Profile data is owned per panel/screen. A directory dies with its panel, + cancelling outstanding work and dropping its cache. +- An update re-renders only the panel whose data changed: reads during render + are tracked by the window, and `panel.cached` repaints a panel as soon as + its directory notifies. +- Local database first: query the local cache, fetch from relays only for keys + the directory cannot resolve. +- One shared network pipeline (batching, dedupe) so N panels still produce one + relay sync per key. + +Non-goals: changing `Profile` accessors, changing `Backend`, persistence. + +## Design + +Two pieces replace the fat global: + +1. `ProfileStore` (global, thin): network fetch service + event broadcaster. + Holds no profiles and does no warm load. + - `fetch(keys)`: dedupe via an in-flight set, batch via the existing flume + channel + 500 ms timeout, chunks of 100 through `sync_bootstrap_only`. + - Emits `ProfileEvent::Updated(Vec)` (a) after each synced batch + and (b) when `BackendEvent::ProfileUpdates` arrives from the pump. +2. `ProfileDirectory` (per panel/screen entity): the metadata one surface + renders. + - `track(keys, cx)`: queries the local database for missing keys, inserts + what it finds, queues the rest for a network fetch. + - `profile(key, cx)`: cached value or placeholder; a miss marks the key + pending and queues a fetch (same call shape as today's `get`). + - Subscribes to `ProfileEvent::Updated`, refreshes only keys it tracks or + has pending, notifies itself on change. + - Stores its own tasks and subscription; both drop with the panel. + +``` +panel view ──track(keys)──▶ ProfileDirectory ──DB query──▶ local database + ▲ │ keys not found + │ cx.notify() ▼ + └────refresh(authors)── ProfileEvent::Updated ──── ProfileStore (global) + ▲ │ batch + + │ │ sync_bootstrap_only + BackendEvent::ProfileUpdates ▼ + relay pump ─────────────▶ relays +``` + +Panels own `Entity` and read it directly in render; no +`cx.observe` is needed. gpui-pre records which entities a window reads while +drawing (`App::record_entities_accessed`) and, on `cx.notify`, invalidates +exactly the views that read them (`App::notify` → `Window::invalidate_view`). +This is the mechanism coop relies on — its `Directory` only calls +`cx.notify()` — and it composes with `panel.cached(...)`, which repaints a +panel when the panel view itself is invalidated. Scoping data per directory is +what scopes invalidation: notifying the old global store would invalidate +every panel that ever read it, while notifying one directory invalidates only +the panels reading it. + +## API sketch + +```rust +// signed_state/src/profile.rs + +#[derive(Debug, Clone)] +pub enum ProfileEvent { + /// Metadata for these authors may have changed in the local database. + Updated(Vec), +} + +impl EventEmitter for ProfileStore {} + +/// Thin global: batches metadata requests from directories and syncs them +/// into the local database. Holds no profile data. +pub struct ProfileStore { + in_flight: RefCell>, + sender: Sender, + _subscription: Subscription, +} + +impl ProfileStore { + pub fn global(cx: &App) -> Entity; + pub(crate) fn set_global(entity: Entity, cx: &mut App); + /// Queue a batched metadata request; no-op for keys already in flight. + pub fn fetch(&self, keys: impl IntoIterator); +} + +/// Metadata cache for one panel or screen. +pub struct ProfileDirectory { + profiles: HashMap, + pending: RefCell>, + tasks: SmallVec<[Task<()>; 2]>, + _subscriptions: SmallVec<[Subscription; 1]>, +} + +impl ProfileDirectory { + /// Create a directory, following the profile store's events. + pub fn new(cx: &mut App) -> Entity; + /// Resolve keys from the local database, queue the rest for fetch. + pub fn track(&mut self, keys: impl IntoIterator, cx: &mut Context); + /// The profile for a key, or a placeholder that queues a fetch. + pub fn profile(&self, public_key: &PublicKey, cx: &App) -> Profile; + /// Re-query tracked/pending keys named by a `ProfileEvent`. + fn refresh(&mut self, authors: &[PublicKey], cx: &mut Context); +} +``` + +Panel wiring: + +```rust +pub struct IssuesView { + // ... + directory: Entity, +} + +// constructor — no observe wiring; render-time reads of the directory are +// tracked automatically and `cx.notify()` on it invalidates the panel. +let directory = ProfileDirectory::new(cx); +``` + +Behavior details carried over from coop's `Directory`: + +- The `pending` set guards the window between `track`/`profile` and the + database task finishing, so events arriving mid-query are still accepted. +- Inserts never overwrite data that already arrived from an event + (insert-if-absent on bulk load). +- `refresh` removes a pending key only when the database produced a profile, + so keys that exist on no relay stay pending and never loop into refetches; + `in_flight` in the store is released on every `Updated` emission. + +## Where directories live + +| Surface | File | Keys tracked | When | +| --- | --- | --- | --- | +| `SidebarPanel` | `views/sidebar/mod.rs` | current user, local repo owners | on `SignerChanged`, on local repos update | +| `RepoListView` | `views/repo_list.rs` | announcement owners | when cards rebuild | +| `RepoDetailView` | `views/repo/mod.rs`, `repo/about.rs` | owner, maintainers, binding owners | on announcement/sync; about dialog captures the handle | +| `IssuesView` | `views/issues/mod.rs` | issue authors, repo owner (tab title) | in `rebuild` | +| `IssueDetailView` | `views/issues/detail.rs` | author, discussion participants | on store events; passed to `discussion::` helpers | +| `PullRequestsView` | `views/pull_requests/mod.rs` | PR authors, repo owner | in `rebuild` | +| `PullRequestDetailView` | `views/pull_requests/detail.rs` | author, participants | on store events | +| `NewPullRequestView`, `SendPatchView`, `CommitDiffView` | respective files | repo owner (tab title only) | on construction | +| `InboxView` | `views/inbox.rs` | activity authors | in rows rebuild | + +Helpers that stop reading the global and take `&Entity` +instead — every caller has `&mut self` or captures the handle: + +- `repo_tab_avatar(store, cx)` in `views/mod.rs` (all `Panel::title` impls) +- `discussion::sidebar_section`, `discussion::comments_section` +- `about::maintainers` via `open_about_dialog` (dialog closure captures the + entity handle, same as coop's device-registry notification) +- `bound_repo_label` in `views/repo/mod.rs` +- `sub_activity` in `views/inbox.rs`, `local_avatar` in `views/sidebar/mod.rs` + +## Migration steps + +1. `signed_state`: introduce `ProfileEvent` + `EventEmitter`, extract the + "latest metadata per author" reduction from `apply_authors` into a pure + `fn latest_profiles(events: Vec) -> Vec`, and strip + `ProfileStore` down to the fetch service (drop `profiles`, `seen`, `load`, + `apply_seen`, `apply_authors`, `get`, `WARM_LIMIT`). The batch loop emits + `ProfileEvent::Updated` instead of querying; the `BackendEvent::ProfileUpdates` + subscription re-emits and releases in-flight keys. +2. `signed_state`: add `ProfileDirectory` in the same `profile.rs` and export + it from `lib.rs`. `init` keeps creating the global store. +3. `workspace`: add a `directory` field to each surface in the table, replace + `ProfileStore::global(cx).read(cx).get(...)` with + `self.directory.read(cx).profile(...)`, thread the handle through the free + helpers, and call `track` from each surface's data-load/rebuild path. +4. Delete `ProfileStore::get` last; the compiler walks the remaining call + sites. The workspace crate will not compile between steps 1 and 3, so this + lands as one change. + +## Edge cases + +- **Relay event during a database query**: covered by the pending set. +- **Keys with no profile on relays**: stay pending/in-flight, no retry storm + (strictly no worse than today's permanent `seen` dedupe). +- **Panel closed**: directory drops → subscription, tasks, and cache drop; no + cross-panel notifies. +- **Same person on many panels**: each directory keeps its own `Profile` + clone. Metadata JSON is small and queries are local index lookups; if this + ever shows up, a directory can attach to `RepoStore` later. +- **Own profile edits**: publishing flows through `Backend` → pump → + `ProfileUpdates` → store event → directories refresh, so no extra channel is + needed. +- **Dock tab titles**: `Panel::title` receives `&mut self`, so panels pass + their own directory to `repo_tab_avatar`; no global needed for chrome. + +## Testing + +- Unit: `latest_profiles` keeps the newest event per author (pure, no GPUI). +- Unit: `ProfileDirectory` pending-set dedupe and insert-if-absent merge via + `TestAppContext` (the dock crate already uses GPUI test contexts), seeding + the directory through a `#[cfg(test)]` helper rather than the network. +- Manual: cold start with an empty database, open explore / repo / issues / + inbox, confirm placeholders resolve without interacting with each panel, and + that `panel.cached` tabs still repaint on profile arrival. + +## Alternatives considered + +- **Attach the directory to `RepoStore`** instead of panels: repo surfaces + sharing one store would share one directory, but `RepoStore` instances are + created per open and sidebar/inbox/explore still need their own; revisit + only if duplicated queries ever matter. +- **Keep the global store as-is**: read tracking already propagates updates, + but the warm load, unbounded cache, and all-panels invalidation remain; does + not meet the goals.