mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
docs: add data-flow refactor dev plan & spec
Incremental, phased plan (P0-P6) to put every subsystem on a shared SubscriptionManager + RelayHealth foundation with unidirectional data flow, fixing the O(videos x relays) REQ storms and emit-loop feedback identified from real-environment logs. No feature removed; each phase shippable. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
thePR0M3TH3ANandClaude Opus 4.8 committed
1 parent
35c4e68cde
commit
57902e5cb5
2 files changed
+264
No files matched your search
@@ -5,6 +5,7 @@ Welcome to the bitvid documentation. This directory contains detailed architectu
|
||||
## Core Systems
|
||||
|
||||
- **[Application Coordinators](./app-coordinators.md)**: Overview of the coordinator pattern used to manage app-wide state and services.
|
||||
- **[Data-Flow Refactor — Dev Plan & Spec](./architecture-refactor.md)**: The incremental plan to put all subsystems on a shared, governed relay/data foundation (SubscriptionManager, stores, unidirectional flow).
|
||||
- **[Authentication Architecture](./auth-architecture.md)**: How session management, login flows, and signer integration work.
|
||||
- **[Feed Engine](./feed-engine.md)**: The pipeline for fetching, filtering, scoring, and sorting video feeds.
|
||||
- **[Playback & Fallback](./playback-fallback.md)**: The URL-first playback strategy with WebTorrent fallback orchestration.
|
||||
|
||||
@@ -0,0 +1,263 @@
|
||||
# bitvid Data-Flow Refactor — Dev Plan & Spec
|
||||
|
||||
> **Status:** Active plan. Target branch: `unstable`. Incremental — every phase
|
||||
> is independently shippable, testable, and revertable. No feature removed.
|
||||
>
|
||||
> **Goal:** Take the existing, individually-working subsystems (videos,
|
||||
> moderation, view counts, watch history, follows, hashtags, blocks, DMs,
|
||||
> profiles) and put them on a shared, governed data-flow foundation so they stop
|
||||
> fighting each other for the relay pool and the nip-07 signer.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why "each part works, but together it's a mess"
|
||||
|
||||
Captured from a **real** logged-in session (the `[req-monitor]` + `ConnectionManager`
|
||||
console output), not the mock harness:
|
||||
|
||||
- `RELAY STORM 1035 REQ/s → kind 1984=2024` (reports), `kind 30079=1760` (views).
|
||||
- ~20 relays, most failing: endless `WebSocket connection failed` /
|
||||
`Circuit breaker opened`, each reconnect re-issuing **every** subscription
|
||||
(`refreshTrustedMuteSubscriptions`, `moderationService.js` report subs,
|
||||
`viewEvents.js` count subs, `watchHistory.js` fetch).
|
||||
- `Decryption timed out after 6s` for hashtag prefs and the block list — the
|
||||
nip-07 signer is drowning, so encrypted lists never resolve.
|
||||
|
||||
### Two structural root causes
|
||||
|
||||
1. **Uncoordinated relay access.** Every subsystem calls `pool.sub` / `legacyList`
|
||||
directly, per-item, on the *full* relay list. "Show 30 videos" becomes
|
||||
`~30 ids × 2 data types × ~20 relays ≈ 1,200 subscriptions`, and every flaky
|
||||
reconnect re-fires all of them. There is no batching, no dedup, no concurrency
|
||||
budget, and no relay-health gating.
|
||||
|
||||
2. **Non-idempotent events + bidirectional flow.** Emitters fire "changed"
|
||||
unconditionally; listeners react by re-fetching/re-rendering, which emits
|
||||
again → feedback loops (already fixed cases: watch-history `fingerprint`,
|
||||
moderation `contacts`; structural cases remain). Rendering is a *side-effect*
|
||||
of these cascades and of load ordering (hence "logged-out grid is empty until
|
||||
refresh").
|
||||
|
||||
Each subsystem is locally fine. There is no governor and no idempotency, so in
|
||||
aggregate they resonate.
|
||||
|
||||
---
|
||||
|
||||
## 2. Target architecture (4 layers, 5 invariants)
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────┐
|
||||
L4 │ View (renders from AppState; never fetches) │
|
||||
└───────────────▲─────────────────────────────┘
|
||||
│ state change (diffed)
|
||||
┌───────────────┴─────────────────────────────┐
|
||||
L3 │ AppState (normalized source of truth) + │
|
||||
│ Coordinator (debounced orchestration) │
|
||||
└───────────────▲─────────────────────────────┘
|
||||
│ patches (on real change only)
|
||||
┌───────────────┴─────────────────────────────┐
|
||||
L2 │ Domain Stores: videos, profiles, moderation, │
|
||||
│ views, watchHistory, follows, hashtags, │
|
||||
│ blocks, dms — normalized cache + decrypt-once │
|
||||
└───────────────▲─────────────────────────────┘
|
||||
│ subscribe()/list() (batched filters)
|
||||
┌───────────────┴─────────────────────────────┐
|
||||
L1 │ SubscriptionManager + RelayHealth │
|
||||
│ (the ONLY caller of pool.sub / pool.list) │
|
||||
└───────────────▲─────────────────────────────┘
|
||||
│
|
||||
relay pool (vendored nostr pool)
|
||||
```
|
||||
|
||||
### The 5 invariants (the rules that keep it from re-rotting)
|
||||
|
||||
1. **Single transport chokepoint.** Nothing outside L1 calls `pool.sub` /
|
||||
`pool.list` / `legacySub` / `legacyList`. (Enforceable by lint.)
|
||||
2. **Batch by filter, never per-item.** N ids → one `{kinds, "#e":[...]}`.
|
||||
3. **Health-ranked, bounded relay set.** Subscribe on a small healthy core;
|
||||
never target circuit-broken relays; cap concurrent REQs per relay.
|
||||
4. **Emit-on-change only.** Every store/emitter diffs before emitting.
|
||||
(Enforceable by a shared `emitIfChanged` helper + review.)
|
||||
5. **Unidirectional flow.** relay → store → AppState → view. The view never
|
||||
triggers a fetch; stores never read the DOM.
|
||||
|
||||
---
|
||||
|
||||
## 3. Component specs
|
||||
|
||||
### 3.1 RelayHealth (L1)
|
||||
Owns relay status; the SubscriptionManager asks it which relays to use.
|
||||
|
||||
- `getCoreRelays(n = 5)` → top-N currently-healthy relays (read set).
|
||||
- `isUsable(url)` → not circuit-broken / not in backoff.
|
||||
- `report(url, ok|fail, reason)` → updates rolling health + circuit breaker.
|
||||
- Absorbs the existing `ConnectionManager` backoff/circuit-breaker logic
|
||||
(already present at `js/nostr/managers/ConnectionManager.js`); the new part is
|
||||
exposing a **ranked usable set** so we stop fanning to all ~20 relays.
|
||||
|
||||
### 3.2 SubscriptionManager (L1) — the foundation
|
||||
The only module that talks to the pool. Absorbs and generalizes the existing
|
||||
`relaySubscriptionService.js` + `relayBatchFetcher.js`.
|
||||
|
||||
```js
|
||||
// Live subscription (deduped + batched). Returns a handle.
|
||||
const handle = subscriptionManager.subscribe({
|
||||
key, // logical identity for dedup, e.g. "moderation:reports"
|
||||
filters, // nostr filters; same-key calls are merged into one REQ
|
||||
relays, // optional; defaults to RelayHealth.getCoreRelays()
|
||||
onEvent, // (event, relayUrl) => void
|
||||
onEose, // optional
|
||||
priority, // budget/ordering hint
|
||||
});
|
||||
handle.update({ filters }); // diff vs current; re-REQ only if changed
|
||||
handle.close();
|
||||
|
||||
// One-shot fetch (EOSE → resolve), batched + deduped + budgeted.
|
||||
const events = await subscriptionManager.list({ key, filters, relays, timeoutMs });
|
||||
```
|
||||
|
||||
Responsibilities:
|
||||
- **Dedup** by `key` (+ normalized filter signature): identical in-flight work
|
||||
shares one subscription/list.
|
||||
- **Batch**: coalesce same-kind filters enqueued within a short window into one
|
||||
REQ with merged `authors` / `#e` / `#p` / `#d` arrays (chunk if a relay caps
|
||||
array size).
|
||||
- **Budget**: per-relay max concurrent REQs (default ~8, below the ~20 limit) +
|
||||
a global rate limiter; overflow queues (priority-ordered).
|
||||
- **Relay gating**: only target `RelayHealth.getUsable()`.
|
||||
- **Reconnect-safe**: maintain a registry of *active logical subscriptions*; on a
|
||||
relay (re)connect, re-issue only those, batched — never the historical pile.
|
||||
|
||||
### 3.3 Store contract (L2)
|
||||
Each domain store is a thin object with a uniform shape:
|
||||
|
||||
```js
|
||||
{
|
||||
// read-through cache; returns immediately, refreshes via SubscriptionManager
|
||||
get(id) / getAll(),
|
||||
ensure(idsOrActor, opts), // request data (batched) through L1
|
||||
subscribe(listener), // listener invoked ONLY on real change (diffed)
|
||||
clear(), // on logout
|
||||
}
|
||||
```
|
||||
|
||||
Rules: normalized cache keyed by id; **decrypt-once** (persisted plaintext cache,
|
||||
already exists at `js/nostr/persistedPlaintextCache.js`); `emitIfChanged` for all
|
||||
notifications; no DOM access; no direct pool access.
|
||||
|
||||
### 3.4 AppState + Coordinator (L3)
|
||||
- **AppState**: normalized maps (videos, profiles, summaries, viewCounts,
|
||||
follows, blocks, hashtagPrefs, watchHistory). Stores patch it; patches are
|
||||
diffed.
|
||||
- **Coordinator**: the single place that turns "inputs changed" into "recompute
|
||||
the derived feed" — debounced/coalesced. Replaces the scattered
|
||||
`onVideosShouldRefresh` callers and the per-emitter refresh wiring. The feed
|
||||
engine reads AppState and produces the derived feed; it never fetches.
|
||||
|
||||
### 3.5 View (L4)
|
||||
Renders from AppState; subscribes to AppState slices; mounting is independent of
|
||||
data arrival (grid paints whatever is present whenever state changes). Fixes the
|
||||
logged-out "empty grid until refresh".
|
||||
|
||||
---
|
||||
|
||||
## 4. Phased migration plan
|
||||
|
||||
Each phase: **shippable**, **behind the existing behavior or a flag**, with
|
||||
acceptance criteria + verification. Worst offenders (from the real logs) first.
|
||||
|
||||
### P0 — Stabilize & instrument ✅ (done this session)
|
||||
- Circuit-broke per-video report (1984) + view (30079) subs; contact emit-loop
|
||||
fixed; reload throttle; nip-07 fair scheduling.
|
||||
- Diagnostics landed: `scripts/perf/*` + `js/devReqMonitor.js`.
|
||||
- **Acceptance:** real-env `[req-monitor]` no longer shows the 1000 REQ/s storm
|
||||
for reports/views. (User to confirm fan calms on build `9745232c`.)
|
||||
|
||||
### P1 — SubscriptionManager + RelayHealth (the foundation)
|
||||
- Build L1 as described; **route nothing through it yet** except a pilot.
|
||||
- Add a lint rule (`scripts/check-direct-pool-access.mjs`) that flags new direct
|
||||
`pool.sub`/`legacyList` calls outside L1 (allowlist the not-yet-migrated ones).
|
||||
- Pilot: migrate the **public video feed** (`client.js subscribeVideos`,
|
||||
`nostrService.loadVideos`) through L1.
|
||||
- **Acceptance:** logged-out feed loads with ≤ (core-relay-count) REQs for video
|
||||
kind; reconnect does not multiply REQs. New unit tests for batching/dedup/
|
||||
budget/reconnect using a **multi-relay flaky mock** (see §5).
|
||||
- **Rollback:** L1 is additive; revert the pilot wiring only.
|
||||
|
||||
### P2 — Moderation reports (kind 1984), batched → re-enable
|
||||
- Replace per-video `subscribeToReports` with one L1 live subscription over the
|
||||
active video-id set (`{kinds:[1984], "#e":[...all visible ids]}`); `handle.update`
|
||||
as the visible set changes. Same for trusted-mute subs (`refreshTrustedMuteSubscriptions`).
|
||||
- Flip `MODERATION_REPORT_SUBS_ENABLED` back on.
|
||||
- **Acceptance:** report aggregation works; `[req-monitor]` stays < threshold
|
||||
with 30+ videos across multiple flaky relays; no churn on reconnect.
|
||||
|
||||
### P3 — View counts (kind 30079), batched → re-enable
|
||||
- Replace per-card `ensureViewCountSubscription` with one batched count/list per
|
||||
visible set through L1. Re-enable.
|
||||
- **Acceptance:** counts render; steady-state REQ ≈ 0 after load.
|
||||
|
||||
### P4 — Lists through stores (follows, hashtags, blocks, watch history, profiles)
|
||||
- Wrap each in the Store contract; route their fetches/subs through L1 (batched,
|
||||
health-gated). Decrypt-once via persisted plaintext cache.
|
||||
- **Acceptance:** no list re-fetch storm on reconnect; decrypt timeouts gone for
|
||||
a healthy core relay set; lists populate on login without manual refresh.
|
||||
|
||||
### P5 — AppState + Coordinator; unidirectional flow
|
||||
- Introduce normalized AppState; stores patch it; collapse refresh triggers into
|
||||
the Coordinator; feed engine reads AppState only.
|
||||
- Sweep remaining emitters for emit-on-change.
|
||||
- **Acceptance:** no feedback loops under `[req-monitor]`; logged-out grid paints
|
||||
without refresh (disclaimer bug fixed); CPU idle at steady state.
|
||||
|
||||
### P6 — Cleanup
|
||||
- Remove `devReqMonitor.js` import (keep file for future use), retire dead
|
||||
per-item subscription code, document the final architecture, fold the relay set
|
||||
to a health-ranked core in config.
|
||||
|
||||
---
|
||||
|
||||
## 5. Verification strategy (close the harness fidelity gap)
|
||||
|
||||
The mock harness lied because it used **one perfect relay**. New fixtures:
|
||||
|
||||
- **Flaky multi-relay mock**: spin up N mock relays (extend
|
||||
`scripts/agent/simple-relay.mjs`), some that refuse connections / drop / delay,
|
||||
to reproduce the real reconnect-resubscribe amplifier.
|
||||
- **REQ-rate assertions**: every migrated phase has a headless test asserting
|
||||
steady-state REQ/s ≈ 0 and load-burst REQ count is **O(relays), not
|
||||
O(items×relays)** — reuse `scripts/perf/nip07-heat.mjs` patterns.
|
||||
- **Real-env smoke**: `js/devReqMonitor.js` stays until P6; user confirms no
|
||||
`RELAY STORM` line after each phase that touches subscriptions.
|
||||
- Keep all changes cheat-resistant per the repo's Dark-Factory rules
|
||||
(scenario tests, fail-without-fix proven).
|
||||
|
||||
---
|
||||
|
||||
## 6. Invariants as guardrails (so it can't re-rot)
|
||||
- Lint: no direct pool access outside L1 (allowlist shrinks each phase).
|
||||
- Shared `emitIfChanged(prev, next, emit)` helper; code review rejects unguarded
|
||||
emits.
|
||||
- Store contract review checklist: normalized cache? decrypt-once? no DOM? no
|
||||
direct pool?
|
||||
|
||||
---
|
||||
|
||||
## 7. Risks & rollback
|
||||
- **Risk:** batching changes event-delivery timing for moderation/views.
|
||||
*Mitigation:* keep ingestion logic identical; only the transport changes.
|
||||
- **Risk:** L1 budget too aggressive → slow loads. *Mitigation:* tune per-relay
|
||||
cap; it's a constant.
|
||||
- **Rollback:** each phase is isolated; L1 is additive; feature flags
|
||||
(`MODERATION_REPORT_SUBS_ENABLED`, etc.) gate re-enables.
|
||||
|
||||
---
|
||||
|
||||
## 8. Tracking checklist
|
||||
- [x] P0 stabilize + instrument
|
||||
- [ ] P1 SubscriptionManager + RelayHealth + lint guard + video-feed pilot
|
||||
- [ ] P2 moderation reports batched + re-enabled
|
||||
- [ ] P3 view counts batched + re-enabled
|
||||
- [ ] P4 lists via stores (follows/hashtags/blocks/watch-history/profiles)
|
||||
- [ ] P5 AppState + Coordinator + unidirectional flow
|
||||
- [ ] P6 cleanup + config relay-set trim + final docs
|
||||
Reference in new issue
Block a user