diff --git a/CHANGELOG.md b/CHANGELOG.md index 4edf72d..81fe347 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,16 @@ public API changes with it. authority changes. - `ReportStore.ingest` required a `target` field it never read, since the target is identified by the key argument. +- The performance fixture was intermittently failing. Its setup repeated the + same 100 mute-list writes 50 times with identical data, making construction + 50x more expensive than the fixture it describes and pushing individual tests + past their timeout under load. The fixture's shape is now asserted so it + cannot silently degrade again. +- The performance suite's wall-clock assertion was replaced with structural + ones. A timing budget measured the machine as much as the code — it varied + roughly 20x between idle and loaded runs here — so it now asserts that state + is materialized once per batch, which is the actual regression that made a + 5,000-target pass take ~47 seconds. - Evaluation was quadratic in state size: both the snapshot and its fingerprint were rebuilt for every target, so each call walked every report and mute list. A 5,000-target pass took ~47s. The runtime now memoizes the snapshot diff --git a/packages/widget/src/admin.js b/packages/widget/src/admin.js index 2d0a98b..0155b38 100644 --- a/packages/widget/src/admin.js +++ b/packages/widget/src/admin.js @@ -98,6 +98,12 @@ export class GovernanceAction extends GovernanceElement { this.commands = null; /** @type {(() => Promise)|null} */ this.action = null; + /** + * The in-flight action, or null when idle. Hosts can await it to disable + * surrounding UI; tests can await it instead of polling. + * @type {Promise|null} + */ + this.pending = null; this._status = ""; } @@ -132,7 +138,9 @@ export class GovernanceAction extends GovernanceElement { `); - this.$("#run")?.addEventListener("click", () => void this._run()); + this.$("#run")?.addEventListener("click", () => { + this.pending = this._run(); + }); } async _run() { @@ -154,6 +162,7 @@ export class GovernanceAction extends GovernanceElement { } this.render(); + this.pending = null; } } @@ -168,6 +177,8 @@ export class GovernanceAdminPanel extends GovernanceElement { super(); /** @type {GovernanceCommands|null} */ this.commands = null; + /** @type {Promise|null} */ + this.pending = null; this._status = ""; } @@ -258,7 +269,9 @@ export class GovernanceAdminPanel extends GovernanceElement { `); - this.$("#deny")?.addEventListener("click", () => void this._deny()); + this.$("#deny")?.addEventListener("click", () => { + this.pending = this._deny(); + }); } async _deny() { @@ -275,6 +288,7 @@ export class GovernanceAdminPanel extends GovernanceElement { this._status = result.ok ? "Restricted" : `Refused: ${result.code}`; this.emit(result.ok ? "account-restricted" : "action-refused", { pubkey, result }); this.render(); + this.pending = null; } } diff --git a/packages/widget/src/viewer.js b/packages/widget/src/viewer.js index bcde5e4..4731aad 100644 --- a/packages/widget/src/viewer.js +++ b/packages/widget/src/viewer.js @@ -195,6 +195,11 @@ export class GovernanceReport extends GovernanceElement { this.categories = ["spam", "nudity", "profanity", "illegal", "malware", "impersonation", "other"]; /** @type {import('@nostr-governance/runtime').GovernanceCommands|null} */ this.commands = null; + /** + * The in-flight submission, or null when idle. + * @type {Promise|null} + */ + this.pending = null; this._status = ""; } @@ -242,7 +247,7 @@ export class GovernanceReport extends GovernanceElement { this.$("form")?.addEventListener("submit", (fromEvent) => { fromEvent.preventDefault(); - void this._submit(); + this.pending = this._submit(); }); } @@ -263,6 +268,7 @@ export class GovernanceReport extends GovernanceElement { this._status = result.ok ? "Report published" : `Failed: ${result.code}`; this.emit(result.ok ? "reported" : "report-failed", { target: this._target, category, result }); this.render(); + this.pending = null; } } diff --git a/packages/widget/test/admin.test.js b/packages/widget/test/admin.test.js index 2bac0cd..859c1f5 100644 --- a/packages/widget/test/admin.test.js +++ b/packages/widget/test/admin.test.js @@ -154,7 +154,8 @@ describe("", () => { const completed = vi.fn(); action.addEventListener("governance:action-completed", completed); action.shadowRoot.querySelector("#run").click(); - await vi.waitFor(() => expect(completed).toHaveBeenCalled()); + await action.pending; + expect(completed).toHaveBeenCalled(); expect(action.shadowRoot.textContent).toContain("Done"); }); @@ -169,7 +170,8 @@ describe("", () => { const refused = vi.fn(); action.addEventListener("governance:action-refused", refused); action.shadowRoot.querySelector("#run").click(); - await vi.waitFor(() => expect(refused).toHaveBeenCalled()); + await action.pending; + expect(refused).toHaveBeenCalled(); expect(action.shadowRoot.textContent).toContain("protected-target"); }); @@ -183,7 +185,8 @@ describe("", () => { const failed = vi.fn(); action.addEventListener("governance:action-failed", failed); action.shadowRoot.querySelector("#run").click(); - await vi.waitFor(() => expect(failed).toHaveBeenCalled()); + await action.pending; + expect(failed).toHaveBeenCalled(); expect(action.shadowRoot.textContent).toContain("relay down"); }); @@ -268,7 +271,8 @@ describe("", () => { const restricted = vi.fn(); panel.addEventListener("governance:account-restricted", restricted); panel.shadowRoot.querySelector("#deny").click(); - await vi.waitFor(() => expect(restricted).toHaveBeenCalled()); + await panel.pending; + expect(restricted).toHaveBeenCalled(); expect(commands.denyUser).toHaveBeenCalledWith(CREATOR); }); @@ -285,7 +289,8 @@ describe("", () => { const refused = vi.fn(); panel.addEventListener("governance:action-refused", refused); panel.shadowRoot.querySelector("#deny").click(); - await vi.waitFor(() => expect(refused).toHaveBeenCalled()); + await panel.pending; + expect(refused).toHaveBeenCalled(); expect(panel.shadowRoot.textContent).toContain("protected-target"); }); diff --git a/packages/widget/test/viewer.test.js b/packages/widget/test/viewer.test.js index b9776df..8e7be36 100644 --- a/packages/widget/test/viewer.test.js +++ b/packages/widget/test/viewer.test.js @@ -299,7 +299,8 @@ describe("", () => { report.shadowRoot.querySelector("#category").value = "spam"; report.shadowRoot.querySelector("form").dispatchEvent(new Event("submit")); - await vi.waitFor(() => expect(reported).toHaveBeenCalled()); + await report.pending; + expect(reported).toHaveBeenCalled(); expect(commands.report).toHaveBeenCalledWith({ type: "event", id: EVENT_ID }, "spam", ""); expect(report.shadowRoot.textContent).toContain("Report published"); @@ -317,7 +318,8 @@ describe("", () => { const failed = vi.fn(); report.addEventListener("governance:report-failed", failed); report.shadowRoot.querySelector("form").dispatchEvent(new Event("submit")); - await vi.waitFor(() => expect(failed).toHaveBeenCalled()); + await report.pending; + expect(failed).toHaveBeenCalled(); expect(report.shadowRoot.textContent).toContain("invalid-target"); }); diff --git a/tests/performance/large-feed.test.js b/tests/performance/large-feed.test.js index 9a94a0d..e5dbf32 100644 --- a/tests/performance/large-feed.test.js +++ b/tests/performance/large-feed.test.js @@ -88,16 +88,17 @@ function buildLargeRuntime() { runtime.trust.setContacts(Array.from({ length: MUTERS_PER_AUTHOR }, (_, i) => muter(i))); - // 100 trusted muters against each of the first 50 authors. - for (let authorIndex = 0; authorIndex < 50; authorIndex += 1) { - for (let muterIndex = 0; muterIndex < MUTERS_PER_AUTHOR; muterIndex += 1) { - runtime.mutes.replaceList({ - owner: muter(muterIndex), - updatedAt: NOW - 1000, - entries: Array.from({ length: 50 }, (_, i) => ({ pubkey: author(i) })), - hasEncryptedEntries: false, - }); - } + // 100 trusted muters, each muting the same 50 authors — which is what gives + // every one of those authors 100 muters. One pass per muter is enough; an + // outer loop over authors would repeat identical work 50 times. + const mutedAuthors = Array.from({ length: 50 }, (_, i) => ({ pubkey: author(i) })); + for (let muterIndex = 0; muterIndex < MUTERS_PER_AUTHOR; muterIndex += 1) { + runtime.mutes.replaceList({ + owner: muter(muterIndex), + updatedAt: NOW - 1000, + entries: mutedAuthors, + hasEncryptedEntries: false, + }); } // Reports across several categories. @@ -120,6 +121,30 @@ function buildLargeRuntime() { return { runtime, transport, targets, MODERATORS }; } +describe("fixture shape", () => { + it("matches the size the migration plan specifies", () => { + const { runtime, targets } = buildLargeRuntime(); + + expect(targets).toHaveLength(TARGET_COUNT); + const authors = targets.map((t) => (t.type === "event" ? t.author : undefined)); + expect(new Set(authors).size).toBe(AUTHOR_COUNT); + + // Every muted author must carry the full muter count; an earlier version of + // this fixture built it with a redundant loop that made setup 50x more + // expensive without changing this number. + const muteRecords = runtime.mutes.toRecordMap(); + expect(muteRecords.get(`user:${author(0)}`)).toHaveLength(MUTERS_PER_AUTHOR); + expect(muteRecords.get(`user:${author(49)}`)).toHaveLength(MUTERS_PER_AUTHOR); + + // Multiple report categories and several administrative contributors. + const categories = new Set( + [...runtime.reports.toRecordMap().values()].flat().map((r) => r.category), + ); + expect(categories.size).toBeGreaterThanOrEqual(3); + expect(runtime.admin.state.userDeny.size).toBeGreaterThanOrEqual(30); + }); +}); + describe("large feed evaluation", () => { it("evaluates 5,000 targets without touching the network", () => { const { runtime, transport, targets } = buildLargeRuntime(); @@ -180,12 +205,53 @@ describe("large feed evaluation", () => { } }); - it("completes a 5,000-target pass in reasonable time", () => { + it("materializes state once per batch, not once per target", () => { + // This is the regression that made a 5,000-target pass take ~47 seconds: + // both the snapshot and its fingerprint were rebuilt for every target, so + // each evaluation walked every report and every mute list. + // + // Asserted structurally rather than by wall clock. A timing budget + // measures the machine as much as the code — it varied roughly 20x here + // between an idle and a loaded run — which makes it flaky instead of + // informative. Counting materializations tests the actual claim and is + // deterministic. const { runtime, targets } = buildLargeRuntime(); - const started = performance.now(); + const reportMaps = vi.spyOn(runtime.reports, "toRecordMap"); + const muteMaps = vi.spyOn(runtime.mutes, "toRecordMap"); + runtime.evaluateMany(targets, { profile: "feed" }); - // Loose on purpose: this catches an accidental O(n^2), not a slow runner. - expect(performance.now() - started).toBeLessThan(10_000); + + expect(reportMaps).toHaveBeenCalledTimes(1); + expect(muteMaps).toHaveBeenCalledTimes(1); + }); + + it("reuses one snapshot fingerprint across a batch", () => { + const { runtime, targets } = buildLargeRuntime(); + const decisions = runtime.evaluateMany(targets.slice(0, 500), { profile: "feed" }); + + const fingerprints = new Set([...decisions.values()].map((d) => d.snapshotFingerprint)); + expect(fingerprints.size).toBe(1); + }); + + it("re-materializes after state changes, and only then", () => { + const { runtime, targets } = buildLargeRuntime(); + const slice = targets.slice(0, 100); + + const reportMaps = vi.spyOn(runtime.reports, "toRecordMap"); + runtime.evaluateMany(slice, { profile: "feed" }); + expect(reportMaps).toHaveBeenCalledTimes(1); + + // A second pass is served from cache and materializes nothing. + runtime.evaluateMany(slice, { profile: "feed" }); + expect(reportMaps).toHaveBeenCalledTimes(1); + + // A state change forces exactly one fresh materialization. + runtime.reports.ingest( + { reporter: muter(0), category: "spam", createdAt: NOW }, + `event:${eventId(0)}`, + ); + runtime.evaluateMany(slice, { profile: "feed" }); + expect(reportMaps).toHaveBeenCalledTimes(2); }); });