Skip to content

Commit 44f3853

Browse files
pmaxhoganclaude
andauthored
fix(ui): keep the activity screen smooth during uploads (#56)
## Summary Issue #45: the Activity screen became laggy while an upload was running (repro: configure a folder, click "run now", scroll). Two root causes, both fixed UI-only (no Rust / event-emission changes): - **Coalesced live ingestion.** `activity:new` events used to mutate the reactive `liveEntries` array (and bump the reactive `total`) once per event. Each event arrives in its own event-loop task, so a high-rate burst re-rendered/diffed the whole table once per event. Events are now appended to a non-reactive buffer and a single coalesced flush per animation frame applies the whole burst in **one** reactive update (one `liveEntries` assignment + one `total` write). Newest-first ordering, the `seenIds` dedup, the `activity:lagged` reconcile path, and the `LIVE_TAIL_CAP` eviction are all preserved; no event is dropped or duplicated. The buffer drains on reconcile/unsubscribe and eager-flushes when it reaches the cap, so memory stays bounded even when frames are throttled (window backgrounded). - **Bounded rendered DOM.** The table renders only the newest `slice(0, ACTIVITY_RENDER_WINDOW)` (200) of the accumulated entries and grows the window on demand via a unified "load more" control, so the mounted row count never grows with the live tail (which can reach ~1000). Table columns, `:key="entry.id"`, filters, and history pagination are unchanged. The render window collapses back on a filter change. ## Testing All run from the worktree (`ui/`): - `pnpm install --frozen-lockfile` - ok - `pnpm lint` - ok - `pnpm format:check` - ok (ran `pnpm format` once on `Activity.vue`, then clean) - `pnpm test:unit` - 216 passed (24 files). `activity-store.test.ts` now has 34 tests (27 original + 7 new); new `activity-window.test.ts` mounts the real `Activity.vue` and proves the mounted row count is capped at the window while the store holds more. - `pnpm build` (`vue-tsc --noEmit && vite build`) - ok New tests prove: a burst of N live events is exactly **one** reactive update each to `entries` and `total` (via `flush: "sync"` watchers); dedup, newest-first order, and the cap are preserved across a buffered burst; the scheduler auto-flushes on the next frame; the buffer drains on unsubscribe; and the windowed slice is bounded. No Rust touched, so cargo gates were not required. Closes #45 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MZQh3ZfwtZsM6c5qnTuWZP --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b7513b5 commit 44f3853

4 files changed

Lines changed: 556 additions & 43 deletions

File tree

ui/src/__tests__/activity-store.test.ts

Lines changed: 211 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { createPinia, setActivePinia } from "pinia";
3+
import { watch } from "vue";
34

45
// Activity store tests (SPEC s11.4; DESIGN s8.3). The seams are
56
// `@tauri-apps/api/core`'s `invoke` (every typed IPC wrapper routes through it)
@@ -50,7 +51,12 @@ vi.mock("@tauri-apps/api/event", () => ({
5051
}),
5152
}));
5253

53-
import { useActivityStore, ACTIVITY_PAGE_SIZE, LIVE_TAIL_CAP } from "../stores/activity";
54+
import {
55+
useActivityStore,
56+
ACTIVITY_PAGE_SIZE,
57+
ACTIVITY_RENDER_WINDOW,
58+
LIVE_TAIL_CAP,
59+
} from "../stores/activity";
5460
import type { ActivityEntry } from "../ipc/types";
5561

5662
function makeEntry(over: Partial<ActivityEntry> = {}): ActivityEntry {
@@ -164,8 +170,11 @@ describe("activity store: pagination", () => {
164170
await store.subscribeLive();
165171
await store.loadInitial();
166172

167-
// A live event arrives for id 150 before it is paged in.
173+
// A live event arrives for id 150 before it is paged in. Issue #45: live
174+
// events are buffered + coalesced, so flush to apply the burst before
175+
// asserting the rendered tail.
168176
liveHandler?.(makeEntry({ id: 150, ts: 900 }));
177+
store.flushLive();
169178
expect(store.entries.map((e) => e.id)).toEqual([150, 200]);
170179

171180
// Page 1 includes id 150 again - it must NOT be duplicated.
@@ -188,6 +197,8 @@ describe("activity store: live tail", () => {
188197
expect(store.total).toBe(1);
189198

190199
liveHandler?.(makeEntry({ id: 2, ts: 2000 }));
200+
// Issue #45: the burst is buffered; flush to apply it in one update.
201+
store.flushLive();
191202
expect(store.entries[0].id).toBe(2);
192203
expect(store.entries.map((e) => e.id)).toEqual([2, 1]);
193204
expect(store.total).toBe(2);
@@ -209,11 +220,13 @@ describe("activity store: live tail", () => {
209220
const store = useActivityStore();
210221
await store.subscribeLive();
211222
await store.applyFilter({ minLevel: "error" });
212-
// An info-level live event must be dropped under a min-level=error filter.
223+
// An info-level live event must be dropped under a min-level=error filter
224+
// (filtered out at ingest, never buffered).
213225
liveHandler?.(makeEntry({ id: 9, level: "info" }));
214226
expect(store.entries).toHaveLength(0);
215-
// A matching error-level event is kept.
227+
// A matching error-level event is kept (buffered, applied on flush).
216228
liveHandler?.(makeEntry({ id: 10, level: "error" }));
229+
store.flushLive();
217230
expect(store.entries.map((e) => e.id)).toEqual([10]);
218231
});
219232

@@ -343,6 +356,7 @@ describe("activity store: lag reconcile (M7-P1-1)", () => {
343356
for (const row of allRows.slice(0, ACTIVITY_PAGE_SIZE)) {
344357
liveHandler?.(row);
345358
}
359+
store.flushLive();
346360
expect(store.entries).toHaveLength(ACTIVITY_PAGE_SIZE);
347361

348362
const pageOf = (p: number) =>
@@ -385,6 +399,8 @@ describe("activity store: live-tail cap (M7-P2-2)", () => {
385399
for (let i = 1; i <= LIVE_TAIL_CAP + overflow; i++) {
386400
liveHandler?.(makeEntry({ id: i, ts: i }));
387401
}
402+
// Issue #45: apply the buffered burst, then assert the bound holds.
403+
store.flushLive();
388404
// The store is bounded to the cap (oldest live entries evicted).
389405
expect(store.entries).toHaveLength(LIVE_TAIL_CAP);
390406
// Newest is the last pushed; the oldest retained is id overflow+1.
@@ -402,6 +418,7 @@ describe("activity store: live-tail cap (M7-P2-2)", () => {
402418
for (let i = 2; i <= LIVE_TAIL_CAP + 100; i++) {
403419
liveHandler?.(makeEntry({ id: i, ts: i }));
404420
}
421+
store.flushLive();
405422
// Live tail capped at CAP, but the loaded history row survives at the tail.
406423
expect(store.entries.length).toBe(LIVE_TAIL_CAP + 1);
407424
expect(store.entries[store.entries.length - 1].id).toBe(1);
@@ -489,6 +506,37 @@ describe("activity store: request token (M7-P2-1)", () => {
489506
expect(store.total).toBe(1);
490507
expect(store.loadedPage).toBe(0);
491508
});
509+
510+
it("does NOT double-count total when a live event arrives during loadInitial (issue #45 codex P2)", async () => {
511+
const store = useActivityStore();
512+
await store.subscribeLive();
513+
514+
// loadInitial's query is slow; capture its resolver so we can inject a live
515+
// event while the page is still in flight.
516+
let resolvePage: (v: unknown) => void = () => {};
517+
invokeMock.mockImplementationOnce(
518+
() =>
519+
new Promise((res) => {
520+
resolvePage = res;
521+
})
522+
);
523+
const load = store.loadInitial();
524+
525+
// A live `activity:new` lands while the page query is awaiting; its durable
526+
// row is already part of the backend's authoritative total below.
527+
liveHandler!(makeEntry({ id: 7, ts: 5000 }));
528+
529+
// The page shows 2 of 10 total rows; the live row (id 7) is one of the other
530+
// 8 the backend already counted in `total: 10`.
531+
resolvePage(makePage([makeEntry({ id: 6, ts: 600 }), makeEntry({ id: 5, ts: 500 })], 0, 10));
532+
await load;
533+
store.flushLive();
534+
535+
// The authoritative server total must win - NOT total + the live delta (11).
536+
expect(store.total).toBe(10);
537+
// ...and the buffered live row is not lost from the tail.
538+
expect(store.entries.map((e) => e.id)).toContain(7);
539+
});
492540
});
493541

494542
describe("activity store: backend facets + summary (M7-P2-4, P2-5)", () => {
@@ -665,3 +713,162 @@ describe("activity store: recheck-3 polish (M7-R3-P2)", () => {
665713
expect(store.entries.map((e) => e.id)).toEqual([200, 100]);
666714
});
667715
});
716+
717+
describe("activity store: batched live ingestion (issue #45)", () => {
718+
it("buffers a burst and applies it in ONE reactive update to entries AND total", async () => {
719+
invokeMock.mockResolvedValueOnce(makePage([], 0, 0));
720+
const store = useActivityStore();
721+
await store.subscribeLive();
722+
await store.loadInitial();
723+
724+
// Count how many times the rendered list and the total actually change.
725+
// `flush: "sync"` fires the watcher on every reactive mutation, so a per-event
726+
// mutation (the pre-fix behavior) would push N entries here, not 1.
727+
const entriesUpdates: number[] = [];
728+
const totalUpdates: number[] = [];
729+
const stopEntries = watch(
730+
() => store.entries.length,
731+
(len) => entriesUpdates.push(len),
732+
{ flush: "sync" }
733+
);
734+
const stopTotal = watch(
735+
() => store.total,
736+
(t) => totalUpdates.push(t),
737+
{ flush: "sync" }
738+
);
739+
740+
const N = 50;
741+
for (let i = 1; i <= N; i++) {
742+
liveHandler?.(makeEntry({ id: i, ts: i, eventType: "upload_done", bytes: null }));
743+
}
744+
745+
// The whole burst is buffered: NOT yet reflected in the reactive state.
746+
expect(entriesUpdates).toHaveLength(0);
747+
expect(totalUpdates).toHaveLength(0);
748+
expect(store.entries).toHaveLength(0);
749+
750+
// A single coalesced flush applies the burst as exactly ONE update each.
751+
store.flushLive();
752+
expect(entriesUpdates).toEqual([N]);
753+
expect(totalUpdates).toEqual([N]);
754+
755+
// Ordering preserved (newest-first) and no rows dropped.
756+
expect(store.entries).toHaveLength(N);
757+
expect(store.entries[0].id).toBe(N);
758+
expect(store.entries[N - 1].id).toBe(1);
759+
expect(store.total).toBe(N);
760+
761+
stopEntries();
762+
stopTotal();
763+
});
764+
765+
it("dedups within a buffered burst (no duplicate rows, total counts once)", async () => {
766+
invokeMock.mockResolvedValueOnce(makePage([], 0, 0));
767+
const store = useActivityStore();
768+
await store.subscribeLive();
769+
await store.loadInitial();
770+
771+
// id 7 arrives twice in the same burst; the second is dropped at ingest.
772+
liveHandler?.(makeEntry({ id: 7, ts: 700 }));
773+
liveHandler?.(makeEntry({ id: 8, ts: 800 }));
774+
liveHandler?.(makeEntry({ id: 7, ts: 700 }));
775+
store.flushLive();
776+
777+
expect(store.entries.map((e) => e.id)).toEqual([8, 7]);
778+
expect(store.total).toBe(2);
779+
});
780+
781+
it("auto-flushes the buffer on the next frame via the scheduler", async () => {
782+
vi.useFakeTimers();
783+
try {
784+
invokeMock.mockResolvedValue(undefined);
785+
const store = useActivityStore();
786+
await store.subscribeLive();
787+
788+
for (let i = 1; i <= 10; i++) {
789+
liveHandler?.(makeEntry({ id: i, ts: i, bytes: null }));
790+
}
791+
// No frame has elapsed yet: still buffered.
792+
expect(store.entries).toHaveLength(0);
793+
794+
// The node test env has no requestAnimationFrame, so the store falls back to
795+
// a ~1-frame setTimeout; advancing past it flushes the burst once.
796+
await vi.advanceTimersByTimeAsync(20);
797+
expect(store.entries).toHaveLength(10);
798+
expect(store.entries[0].id).toBe(10);
799+
expect(store.total).toBe(10);
800+
} finally {
801+
vi.useRealTimers();
802+
}
803+
});
804+
805+
it("eagerly flushes when the buffer reaches the cap, keeping newest-first + bound", async () => {
806+
invokeMock.mockResolvedValueOnce(makePage([], 0, 0));
807+
const store = useActivityStore();
808+
await store.subscribeLive();
809+
await store.loadInitial();
810+
811+
// Fire a burst LARGER than the cap WITHOUT any manual flush: the eager
812+
// at-cap flush keeps memory bounded even if no frame fires mid-burst.
813+
const overflow = 25;
814+
for (let i = 1; i <= LIVE_TAIL_CAP + overflow; i++) {
815+
liveHandler?.(makeEntry({ id: i, ts: i, bytes: null }));
816+
}
817+
// Drain the trailing partial buffer (the last < cap events).
818+
store.flushLive();
819+
820+
expect(store.entries).toHaveLength(LIVE_TAIL_CAP);
821+
// Newest retained at the front, oldest `overflow` evicted.
822+
expect(store.entries[0].id).toBe(LIVE_TAIL_CAP + overflow);
823+
expect(store.entries[store.entries.length - 1].id).toBe(overflow + 1);
824+
// total counts every ingested event (eviction does not decrement it).
825+
expect(store.total).toBe(LIVE_TAIL_CAP + overflow);
826+
});
827+
828+
it("flushes the buffer on unsubscribe so state stays consistent", async () => {
829+
invokeMock.mockResolvedValueOnce(makePage([], 0, 0));
830+
const store = useActivityStore();
831+
await store.subscribeLive();
832+
await store.loadInitial();
833+
834+
liveHandler?.(makeEntry({ id: 1, ts: 1, bytes: null }));
835+
liveHandler?.(makeEntry({ id: 2, ts: 2, bytes: null }));
836+
// Not yet applied (buffered).
837+
expect(store.entries).toHaveLength(0);
838+
839+
store.unsubscribeLive();
840+
// Teardown drains the buffer so entries / total stay in sync.
841+
expect(store.entries.map((e) => e.id)).toEqual([2, 1]);
842+
expect(store.total).toBe(2);
843+
});
844+
});
845+
846+
describe("activity store: render window (issue #45)", () => {
847+
it("ACTIVITY_RENDER_WINDOW bounds the rendered slice below the live-tail cap", () => {
848+
// The view renders entries.slice(0, ACTIVITY_RENDER_WINDOW); that window must
849+
// be well under the live-tail cap so the mounted DOM never grows to ~1000.
850+
expect(ACTIVITY_RENDER_WINDOW).toBeGreaterThan(0);
851+
expect(ACTIVITY_RENDER_WINDOW).toBeLessThan(LIVE_TAIL_CAP);
852+
});
853+
854+
it("the windowed slice keeps the newest rows and is capped at the window size", async () => {
855+
invokeMock.mockResolvedValueOnce(makePage([], 0, 0));
856+
const store = useActivityStore();
857+
await store.subscribeLive();
858+
await store.loadInitial();
859+
860+
const burst = ACTIVITY_RENDER_WINDOW + 80;
861+
for (let i = 1; i <= burst; i++) {
862+
liveHandler?.(makeEntry({ id: i, ts: i, bytes: null }));
863+
}
864+
store.flushLive();
865+
866+
// The store holds more than one window of entries...
867+
expect(store.entries.length).toBe(burst);
868+
// ...but a render window slices only the newest ACTIVITY_RENDER_WINDOW rows.
869+
const windowed = store.entries.slice(0, ACTIVITY_RENDER_WINDOW);
870+
expect(windowed).toHaveLength(ACTIVITY_RENDER_WINDOW);
871+
expect(windowed[0].id).toBe(burst);
872+
expect(windowed[windowed.length - 1].id).toBe(burst - ACTIVITY_RENDER_WINDOW + 1);
873+
});
874+
});

0 commit comments

Comments
 (0)