diff --git a/docs/KNOWN_BUGS.md b/docs/KNOWN_BUGS.md index ba6b6896..ea8d8a5e 100644 --- a/docs/KNOWN_BUGS.md +++ b/docs/KNOWN_BUGS.md @@ -25,15 +25,12 @@ refactor work in that doc — these are defects. decoupled from event ordering. Could not reproduce headlessly (real-relay feed returned nothing in the sandbox); reproduce in a real browser first. -### 2. Residual full-feed reload loop on list-loaded signals -- **Severity:** low–medium (bounded by the 2s throttle, but wasteful — re-fetches - lists ~every 2s while signals keep arriving). -- **Where:** `js/app/authSessionCoordinator.js` `handleBlocksLoaded` (and peers) - call `onVideosShouldRefresh` **unconditionally**; the forced reload re-fetches - the lists, which re-emit "loaded". -- **Fix:** P5 — only trigger a reload when the underlying list data actually - changed (emit-on-change at the list-loaded sources + collapse the refresh - triggers into the Coordinator). +### 2. Residual full-feed reload loop on list-loaded signals — FIXED (P5) +- **Status:** RESOLVED 2026-06-14. `handleBlocksLoaded` now reloads the feed + only when the block-set signature actually changes (emit-on-change guard); + `handleRelaysLoaded` only refreshes the relay UI (never the feed), and + `handleHashtagPreferencesChange` already deduped by signature. Regression test: + `tests/app/blocks-loaded-reload-guard.test.mjs` (cheat-resistant). ### 3. Direct-pool-access lint still misses MULTI-LINE access - **Severity:** low (tooling gap — weakens the L1 chokepoint guarantee). @@ -77,6 +74,12 @@ refactor, but should be triaged. Not in `KNOWN_ISSUES.md`. (no real `localStorage`). Likely environmental — provide a localStorage shim (see `tests/test-helpers/setup-localstorage.mjs`) or guard the test. +### 9. `tests/app/hydrate-sidebar-navigation.test.mjs` +- Fails at the pre-session baseline (`4bd9503c`). Triage. + +### 10. `tests/app/is-user-logged-in.test.mjs` +- Fails at the pre-session baseline (`4bd9503c`). Triage. + --- ## Perf follow-ups (not bugs, but tracked) diff --git a/docs/architecture-refactor.md b/docs/architecture-refactor.md index e6aa6b40..14437763 100644 --- a/docs/architecture-refactor.md +++ b/docs/architecture-refactor.md @@ -269,5 +269,9 @@ The mock harness lied because it used **one perfect relay**. New fixtures: Each keeps a manager-less fallback for tests. (Follow-ups: live profile subscription + comment/reaction events — lower volume; multi-line lint gap KNOWN_BUGS #3.) -- [ ] P5 AppState + Coordinator + unidirectional flow +- [~] P5 AppState + Coordinator + unidirectional flow — IN PROGRESS: emit-on- + change sweep done (handleBlocksLoaded reloads only on block-set change → + KNOWN_BUGS #2 FIXED). Remaining: normalized AppState + render decoupled + from event ordering (fixes disclaimer-empty-grid #1; needs real-browser + repro) — a large app.js rewrite to stage carefully. - [ ] P6 cleanup + config relay-set trim + final docs diff --git a/js/app/authSessionCoordinator.js b/js/app/authSessionCoordinator.js index 0e3aae54..19541530 100644 --- a/js/app/authSessionCoordinator.js +++ b/js/app/authSessionCoordinator.js @@ -775,6 +775,27 @@ export function createAuthSessionCoordinator(deps) { } } + // Only reload the feed when the block set ACTUALLY changed. "blocks-loaded" + // fires on every (re)fetch — including no-op refetches — and a forced + // reload re-fetches the lists, which re-emit "blocks-loaded": an + // unconditional reload here is a self-sustaining loop (KNOWN_BUGS #2). + let nextSignature = ""; + try { + const blocked = + typeof userBlocks?.getBlockedPubkeys === "function" + ? userBlocks.getBlockedPubkeys() + : []; + nextSignature = Array.isArray(blocked) + ? blocked.slice().sort().join(",") + : String(blocked || ""); + } catch (error) { + devLogger.warn("[Application] Failed to read block set signature:", error); + } + if (nextSignature === this._blocksRefreshSignature) { + return; + } + this._blocksRefreshSignature = nextSignature; + try { void this.onVideosShouldRefresh({ reason: "blocks-loaded" }); } catch (error) { diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index f84ccbdf..36b82ae2 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -43,7 +43,7 @@ const GRANDFATHERED = { "js/nostr/nip46Client.js": 2102, "js/state/cache.js": 1827, "js/app/feedCoordinator.js": 1767, - "js/app/authSessionCoordinator.js": 1743, + "js/app/authSessionCoordinator.js": 1803, "js/ui/components/RevertModal.js": 1663, "js/payments/nwcClient.js": 1642, "js/ui/moreMenuController.js": 1601, diff --git a/tests/app/blocks-loaded-reload-guard.test.mjs b/tests/app/blocks-loaded-reload-guard.test.mjs new file mode 100644 index 00000000..70006eb9 --- /dev/null +++ b/tests/app/blocks-loaded-reload-guard.test.mjs @@ -0,0 +1,74 @@ +// Scenario (SCN-blocks-reload-on-change-only): +// Given the app reloads the feed when the block list loads, +// When "blocks-loaded" fires repeatedly with an UNCHANGED block set, +// Then the feed is reloaded only on the FIRST (changed) signal — not on every +// no-op refetch. An unconditional reload re-fetches the lists, which re-emit +// "blocks-loaded", forming a self-sustaining loop (KNOWN_BUGS #2). +// And a genuine block-set change triggers exactly one more reload. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createAuthSessionCoordinator } from "../../js/app/authSessionCoordinator.js"; + +function makeCoordinator(getBlockedPubkeys) { + const noop = () => {}; + const logger = { warn: noop, info: noop, log: noop, error: noop, debug: noop }; + const coord = createAuthSessionCoordinator({ + devLogger: logger, + userLogger: logger, + nostrClient: {}, + accessControl: {}, + userBlocks: { getBlockedPubkeys }, + subscriptions: {}, + hashtagPreferences: {}, + storageService: {}, + relayManager: {}, + torrentClient: {}, + getHashViewName: noop, + setHashView: noop, + DEFAULT_NIP07_PERMISSION_METHODS: [], + RELAY_UI_BATCH_DELAY_MS: 0, + sanitizeRelayList: (x) => x, + buildDmRelayListEvent: noop, + publishEventToRelays: noop, + assertAnyRelayAccepted: noop, + queueSignEvent: noop, + bootstrapTrustedSeeds: noop, + getModerationSettings: () => ({}), + getActiveProfilePubkey: () => null, + }); + coord.profileController = null; + return coord; +} + +test("handleBlocksLoaded reloads the feed only when the block set changes", () => { + let blocked = []; + const coord = makeCoordinator(() => blocked); + let reloads = 0; + coord.onVideosShouldRefresh = async () => { + reloads += 1; + }; + + // First signal (initial set) => one reload. + coord.handleBlocksLoaded({ blocksLoaded: true }); + assert.equal(reloads, 1, "first blocks-loaded reloads once"); + + // Same set, repeated => no further reloads (this is the loop guard). + coord.handleBlocksLoaded({ blocksLoaded: true }); + coord.handleBlocksLoaded({ blocksLoaded: true }); + assert.equal(reloads, 1, "unchanged block set must not reload"); + + // Genuine change => exactly one more reload. + blocked = ["a".repeat(64), "b".repeat(64)]; + coord.handleBlocksLoaded({ blocksLoaded: true }); + assert.equal(reloads, 2, "a changed block set reloads once"); + + // Order-independent: same members, different order => unchanged. + blocked = ["b".repeat(64), "a".repeat(64)]; + coord.handleBlocksLoaded({ blocksLoaded: true }); + assert.equal(reloads, 2, "reordered-but-equal set must not reload"); + + // Non-blocks-loaded details are ignored. + coord.handleBlocksLoaded({ blocksLoaded: false }); + assert.equal(reloads, 2, "ignored when blocksLoaded !== true"); +});