Merge pull request #1390 from PR0M3TH3AN/ai/doc-nostr-client-20231026-4334329458905976061

chore(ai): document js/nostr/client.js

docs/nostr-client-overview.md

@@ -1,91 +1,79 @@
 # NostrClient Overview
 
-`js/nostr/client.js` exports the `NostrClient` class, which serves as the central controller for all Nostr network interactions and state management within the application. It acts as a facade over `nostr-tools`, managing connections, subscriptions, caching, and signing.
+`NostrClient` (`js/nostr/client.js`) is the central controller for all Nostr network interactions and state management in the application. It acts as a facade over `nostr-tools`, managing connections to multiple relays, signing events, and maintaining the local application state.
 
-## Key Responsibilities
+## Responsibilities
 
-1.  **Connection Management**: Maintains a pool of connections to multiple relays (`SimplePool`).
-2.  **State Management**: Tracks video events, deduplicates them, and manages deletions via tombstones.
-3.  **Caching**: Implements a "stale-while-revalidate" strategy using IndexedDB (with localStorage fallback) to restore UI state immediately upon load.
-4.  **Event Publishing**: Orchestrates the signing and broadcasting of complex event chains (Video Note -> NIP-94 Mirror -> NIP-71 Metadata).
-5.  **Signer Abstraction**: Unifies NIP-07 (Extension), NIP-46 (Remote/Bunker), and Local (nsec) signing under a single interface.
-6.  **Video History**: Reconstructs the edit history of videos using `videoRootId` and `d` tags.
+1.  **Connection Management**: Maintains a pool of connections to configured Nostr relays. It handles initialization, reconnection, and error logging.
+2.  **Event Publishing**: Handles the creation, signing, and broadcasting of various Nostr event kinds (Video Post: 30078, Reaction: 7, Comment: 1, etc.). It supports NIP-07 (Extension), NIP-46 (Remote/Bunker), and NIP-01 (Local nsec) signers.
+3.  **State Management**:
+    *   `allEvents`: A raw map of all fetched video events (Kind 30078), preserving history.
+    *   `activeMap`: A derived map containing only the *latest* version of each video, deduplicated by `videoRootId` or `d` tag. This is the source of truth for the UI.
+    *   `rawEvents`: A cache of the exact raw JSON events from relays, used for signature verification and republishing.
+    *   `tombstones`: Tracks deletion timestamps to prevent deleted events from reappearing.
+4.  **Caching**: Implements a dual-layer cache (`IndexedDB` + `localStorage`) to persist `activeMap`, `allEvents`, and `tombstones`. This allows the app to restore its state instantly on load ("stale-while-revalidate").
+5.  **Video Lifecycle**: Manages the full lifecycle of a video post: Publish -> Edit (new event with same `d` tag) -> Revert (new event, same `d`, deleted flag) -> Delete (Kind 5).
 
-## Architecture & State
+## Key Concepts
 
-The client maintains several internal maps to manage state:
+### Active Video Logic
+Nostr is an append-only protocol. Edits are new events. `NostrClient` implements "latest-wins" logic to present a coherent view:
+- **Identifier**: Videos are identified by a `videoRootId` (in the content) or a NIP-33 `d` tag.
+- **Resolution**: When a new event arrives, the client checks if it's newer than the current active version in `activeMap`. If so, it replaces the entry.
+- **Deletions**: If an event has `deleted: true` or is a Kind 5 deletion, it is removed from `activeMap`, and a tombstone is recorded.
 
-*   `allEvents`: A raw map of all fetched video events (key: event ID).
-*   `activeMap`: A derived map containing only the *latest* version of each video, keyed by a stable identifier (`videoRootId` or addressable pointer).
-*   `rawEvents`: A cache of the exact raw JSON events from relays, used for signature verification and re-publishing.
-*   `tombstones`: Tracks timestamps of deletion events to prevent "zombie" events (deleted events reappearing from slow relays) from resurfacing.
+### Buffering & Debouncing
+To handle high volumes of events (e.g., during initial relay dump), `subscribeVideos` uses a buffering strategy:
+1.  Incoming events are pushed to `eventBuffer`.
+2.  A flush is scheduled with a short debounce (e.g., 75ms).
+3.  On flush, the batch is processed: validation, state updates, and UI notification.
+This prevents UI thrashing.
 
-### Caching Strategy (`EventsCacheStore`)
+### Tombstones
+Because relays are eventually consistent, a deleted event might arrive *after* a deletion command.
+- `tombstones` map `activeKey` -> `timestamp`.
+- If an incoming event is older than the tombstone for its key, it is ignored (`applyTombstoneGuard`).
 
-To ensure instant load times, the client persists its state to IndexedDB (`bitvid-events-cache`).
-*   **Snapshot**: On a debounced timer (or idle callback), the current state (`allEvents`, `tombstones`) is serialized.
-*   **Fingerprinting**: Events are fingerprinted to avoid writing unchanged data to the DB.
-*   **Restore**: On `init()`, the client loads the latest snapshot into memory, allowing the UI to render before any network requests complete.
-
-## Key Flows
-
-### 1. Initialization (`init`)
+## Example Flow: `subscribeVideos`
 
 ```mermaid
-graph TD
-    A[init()] --> B[restoreLocalData]
-    B --> C{IndexedDB Available?}
-    C -- Yes --> D[Load Snapshot]
-    C -- No --> E[Load localStorage]
-    D & E --> F[Render UI with Cached Data]
-    F --> G[ensurePool]
-    G --> H[connectToRelays]
-    H --> I[Ready for Subscriptions]
+sequenceDiagram
+    participant UI
+    participant Client as NostrClient
+    participant Buffer as EventBuffer
+    participant State as activeMap
+    participant Relays
+
+    UI->>Client: subscribeVideos(onVideo)
+    Client->>Relays: REQ Kind 30078
+    Relays-->>Client: EVENT (Video A, v1)
+    Client->>Buffer: Push Event
+    Client->>Buffer: Schedule Flush (75ms)
+    Relays-->>Client: EVENT (Video B, v1)
+    Client->>Buffer: Push Event
+
+    Note over Client, Buffer: 75ms passes...
+
+    Client->>Buffer: Flush()
+    loop For each event
+        Buffer->>State: Is newer?
+        alt Yes
+            State->>State: Update Entry
+            State->>UI: onVideo(Video)
+        else No
+            State->>State: Ignore
+        end
+    end
 ```
 
-### 2. Video Subscription (`subscribeVideos`)
+## When to Modify
 
-To prevent UI thrashing during high-volume event ingress (e.g., initial flood from relays), the client uses a **buffering and debouncing** strategy.
-
-1.  **Buffer**: Incoming events are pushed to `eventBuffer` instead of being processed immediately.
-2.  **Debounce**: A flush timer is scheduled (75ms).
-3.  **Flush**: When the timer fires, the entire buffer is processed:
-    *   Events are validated and converted.
-    *   `activeMap` is updated (latest-wins logic).
-    *   `onVideo` callback is fired once with the batch updates.
-    *   Cache persistence is scheduled.
-
-### 3. Publishing a Video (`publishVideo`)
-
-Publishing is a multi-step process ensuring compatibility and discoverability.
-
-1.  **Video Note (Kind 30078)**: The core content (magnet, infohash, metadata) is signed and published. A unique `videoRootId` is generated for new videos.
-2.  **NIP-94 Mirror (Kind 1063)**: If a direct HTTP URL is provided, a file header event is published to point to the hosted file.
-3.  **NIP-71 Metadata (Kind 22/21)**: Addressable metadata tags are published to support categorization/grouping without modifying the immutable video note.
-
-### 4. Remote Signer Handshake (`connectRemoteSigner`)
-
-For NIP-46 (Nostr Connect), the client implements a full handshake flow:
-
-1.  **Ephemeral Key**: Generates a local key pair for the session.
-2.  **Handshake**: Publishes a Kind 24133 (`NIP46_RPC_KIND`) event to the relay.
-3.  **Wait**: Listens for an acknowledgment or `auth_url` challenge from the signer.
-4.  **Session**: Once connected, stores the session metadata to allow auto-reconnection.
-
-## Why it works this way
-
-*   **Buffering**: essential for performance. React rendering cycles are expensive; updating state for every single event in a 5000-event relay dump would freeze the browser.
-*   **Tombstones**: essential for eventual consistency. Relays are distributed and can be out of sync. If a user deletes a video, we must remember that deletion timestamp so a slow relay doesn't "undelete" it by serving the old event later.
-*   **Dual-Layer Cache**: IndexedDB is asynchronous and large-capacity; localStorage is synchronous but small. We use IndexedDB for the bulk data but keep a fallback for environments where IDB might be flaky or slow to initialize.
-
-## When to change
-
-*   **Refactor `EventsCacheStore`**: If moving to a more robust state management library (like RxDB or dedicated worker), this logic should be extracted.
-*   **New Event Kinds**: If the video data model changes (e.g., V4 schema), `convertEventToVideo` and `publishVideo` will need updates.
-*   **Signer upgrades**: As NIP-46 evolves (e.g., Bunker standards), the handshake and connection logic in `nip46Client.js` (and its orchestration here) will need adjustment.
+- **Adding a new Event Kind**: If the app needs to support a new type of interaction (e.g., playlists), add the handling logic here and in `js/nostrEventSchemas.js`.
+- **Changing Caching Strategy**: Modify `EventsCacheStore` or `persistLocalData` if the caching requirements change.
+- **Updating Signing Logic**: Changes to NIP-07 or NIP-46 integration should be carefully reviewed here.
 
 ## Related Files
 
-*   `js/nostrEventSchemas.js`: Defines the structure of the events being built/parsed.
-*   `js/nostr/nip46Client.js`: Handles the low-level RPC for remote signers.
-*   `js/state/profileCache.js`: Handles user profile metadata (Kind 0), which is separate from video data.
+- `js/nostrEventSchemas.js`: Definitions of event structures.
+- `js/nostr/nip71.js`: Helper logic for video categorization (NIP-71).
+- `js/nostr/watchHistory.js`: Manages user watch history state.

js/nostr/client.js

@@ -421,6 +421,10 @@ function waitForTransaction(tx) {
  * - If the TTL expires, the cache is cleared on restore.
  */
 class EventsCacheStore {
+  /**
+   * Initializes the EventsCacheStore.
+   * Tracks fingerprints of persisted items to avoid redundant writes.
+   */
   constructor() {
     this.dbPromise = null;
     this.persistedEventFingerprints = new Map();
@@ -515,6 +519,12 @@ class EventsCacheStore {
     return meta;
   }
 
+  /**
+   * Restores the full state from IndexedDB.
+   *
+   * @returns {Promise<{version: number, savedAt: number, events: Map, tombstones: Map}|null>}
+   * The restored snapshot or null if cache is missing/expired.
+   */
   async restoreSnapshot() {
     const db = await this.getDb();
     if (!db) {
@@ -577,6 +587,14 @@ class EventsCacheStore {
     };
   }
 
+  /**
+   * Persists the current state to IndexedDB.
+   * Only writes changes (diffs against fingerprints).
+   *
+   * @param {{events: Map, tombstones: Map, savedAt: number}} payload - The state to save.
+   * @returns {Promise<{persisted: boolean, eventWrites: number, eventDeletes: number, tombstoneWrites: number, tombstoneDeletes: number}>}
+   * Stats about the persistence operation.
+   */
   async persistSnapshot(payload) {
     const db = await this.getDb();
     if (!db) {
@@ -1038,6 +1056,10 @@ export {
 };
 
 export class NostrClient {
+  /**
+   * Initializes the NostrClient.
+   * Sets up connection pools, state maps, and default relays.
+   */
   constructor() {
     this.pool = null;
     this.poolPromise = null;
@@ -1046,16 +1068,32 @@ export class NostrClient {
     this.readRelays = Array.from(this.relays);
     this.writeRelays = Array.from(this.relays);
 
-    // Store all events so older links still work
+    /**
+     * @type {Map<string, object>}
+     * Maps event ID to the converted Video object.
+     * Stores ALL fetched versions to ensure old links resolve.
+     */
     this.allEvents = new Map();
 
-    // Keep a separate cache of raw events so we can republish the exact payload
+    /**
+     * @type {Map<string, object>}
+     * Maps event ID to the raw Nostr event object.
+     * Kept for signature verification and republishing (e.g. NIP-94 mirror).
+     */
     this.rawEvents = new Map();
 
-    // “activeMap” holds only the newest version for each root
+    /**
+     * @type {Map<string, object>}
+     * Maps `videoRootId` (or `pubkey:dTag`) to the latest valid Video object.
+     * This is the "materialized view" used by the UI.
+     */
     this.activeMap = new Map();
 
-    // Track the newest deletion timestamp for each active key
+    /**
+     * @type {Map<string, number>}
+     * Maps `activeKey` to the timestamp of its latest deletion.
+     * Prevents older events from reappearing after a delete.
+     */
     this.tombstones = new Map();
 
     this.rootCreatedAtByRoot = new Map();
@@ -3659,6 +3697,8 @@ export class NostrClient {
    * 1. Restores local data (IndexedDB/localStorage) to render the UI immediately ("stale-while-revalidate").
    * 2. Schedules the restoration of any stored NIP-46 remote signer session.
    * 3. Initializes the NostrTools `SimplePool` and connects to the configured relays.
+   *
+   * @returns {Promise<void>} Resolves when relays are connected (or at least attempted).
    */
   async init() {
     devLogger.log("Connecting to relays...");
@@ -4984,6 +5024,11 @@ export class NostrClient {
    * - `url`: The direct HTTP URL (WebSeed).
    * - NIP-94 Mirror: Optionally publishes a Kind 1063 file header event if supported.
    * - NIP-71: Optionally publishes a Kind 22 (Video Wrapper) for categorization if metadata is provided.
+   *
+   * @param {object} videoPayload - The video metadata and form data.
+   * @param {string} pubkey - The public key of the publisher.
+   * @returns {Promise<import("nostr-tools").Event>} The signed and published event.
+   * @throws {Error} If not logged in or publishing fails.
    */
   async publishVideo(videoPayload, pubkey) {
     if (!pubkey) throw new Error("Not logged in to publish video.");
@@ -5344,6 +5389,12 @@ export class NostrClient {
    *
    * This version forces version=2 for the original note and uses
    * lowercase comparison for public keys.
+   *
+   * @param {object} originalEventStub - The original video event (must have `id`).
+   * @param {object} updatedData - The new metadata to apply.
+   * @param {string} userPubkey - The public key of the editor (must match owner).
+   * @returns {Promise<import("nostr-tools").Event>} The signed and published edit event.
+   * @throws {Error} If permission denied, ownership mismatch, or publish failure.
    */
   async editVideo(originalEventStub, updatedData, userPubkey) {
     if (!userPubkey) {
@@ -5579,7 +5630,13 @@ export class NostrClient {
   }
 
   /**
-   * revertVideo => old style
+   * Reverts a video (soft delete).
+   * Publishes a new version with the same `d` tag but `deleted: true`.
+   * The content is replaced with a placeholder.
+   *
+   * @param {object} originalEvent - The video event to revert.
+   * @param {string} pubkey - The public key of the owner.
+   * @returns {Promise<{event: object, publishResults: object[], summary: object}>} Result of the operation.
    */
   async revertVideo(originalEvent, pubkey) {
     if (!pubkey) {
@@ -5750,10 +5807,14 @@ export class NostrClient {
   }
 
   /**
-   * "Deleting" => Mark all content with the same videoRootId as {deleted:true}
-   * and blank out magnet/desc.
+   * Deletes all versions of a video.
+   * 1. Reverts every known version (soft delete) by publishing a `deleted: true` update for each `d` tag or root.
+   * 2. Publishes Kind 5 (NIP-09) deletion events for all event IDs and NIP-33 addresses.
    *
-   * This version now asks for confirmation before proceeding.
+   * @param {string} videoRootId - The root ID of the video series.
+   * @param {string} pubkey - The owner's public key.
+   * @param {{confirm?: boolean, video?: object}} [options] - Options (confirm dialog, target video hint).
+   * @returns {Promise<{reverts: object[], deletes: object[]}|null>} Summary of actions taken, or null if cancelled.
    */
   async deleteAllVersions(videoRootId, pubkey, options = {}) {
     if (!pubkey) {
@@ -6233,8 +6294,9 @@ export class NostrClient {
    *    - Persist to local cache.
    *    - Notify the UI (`onVideo`).
    *
-   * @param {Function} onVideo - Callback fired when new valid videos are processed.
-   * @param {{ since?: number, until?: number, limit?: number }} [options]
+   * @param {Function} onVideo - Callback fired when new valid videos are processed. Receives the `Video` object.
+   * @param {{ since?: number, until?: number, limit?: number }} [options] - Filter options.
+   * @returns {import("nostr-tools").Sub} The subscription object. Call `unsub()` to stop.
    */
   subscribeVideos(onVideo, options = {}) {
     const { since, until, limit } = options;
@@ -7171,6 +7233,9 @@ export class NostrClient {
    * - If the root event is missing locally, fetch it by ID.
    * - If we suspect missing history (e.g., only 1 version found), perform a relay query for the specific `d` tag.
    * - Merge and sort all findings to present a linear history.
+   *
+   * @param {object} video - The video object to find history for.
+   * @returns {Promise<object[]>} Sorted array of video objects (newest first).
    */
   async hydrateVideoHistory(video) {
     if (!video || typeof video !== "object") {

tests/nostr-delete-flow.test.mjs

@@ -257,10 +257,10 @@ await (async function testTombstonePersistenceAcrossSaveRestore() {
 
   client.allEvents.set(legacyVideo.id, { ...legacyVideo });
   client.recordTombstone(activeKey, legacyVideo.created_at + 50);
-  client.saveLocalData();
+  await client.saveLocalData("test", { immediate: true });
 
   const restored = new NostrClient();
-  restored.restoreLocalData();
+  await restored.restoreLocalData();
 
   assert.equal(
     restored.tombstones.get(activeKey),