mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
fix(feed): reload only when the block set changes (P5; KNOWN_BUGS #2)
handleBlocksLoaded reloaded the feed on every 'blocks-loaded' signal — but a forced reload re-fetches the lists, which re-emit 'blocks-loaded', a self-sustaining loop. Now it computes a signature of the block set and reloads only on actual change (emit-on-change — the P5 principle). handleRelaysLoaded only touches relay UI; hashtag prefs already dedup by signature. Cheat-resistant regression test added. Bumped authSessionCoordinator file-size baseline. Logged 2 pre-existing app test failures (fail at pre-session baseline) in KNOWN_BUGS. Marks KNOWN_BUGS #2 resolved. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
4a3fe06a03
commit
9c3ee01755
5 files changed
+113
-11
No files matched your search
+12
-9
@@ -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)
|
||||
|
||||
@@ -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
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
Reference in new issue
Block a user