Repository navigation
perf: one list read per refresh, parallel loads, no duplicate watcher refresh (v0.12.0) - #15
Conversation
… refresh (v0.12.0) - /tasks (unfiltered) and /headless-runs join one in-flight upstream GET /tasks (SharedRead). Only the read is shared; nothing is reused once it settles. Every mutation goes through mutate() -> invalidatingWrite, which invalidates before the route responds; the watcher invalidates on the raw fs event. - The UI's three reads, and a button's list + detail refresh, run in parallel. Results apply last-started-wins (Latest). - A watcher event already covered by a successful refresh that started after the change is skipped (coveredByRefresh), removing the second full refresh after every button press. - Per-task in-flight guard: a second click (Start included) is dropped before confirm(); mutations are never parallel or duplicated (task-queue-mcp F-01). - Includes 0.11.1's redirect fix. Measured vs live API: tab load 0.372 s / 3 upstream reads -> 0.250 s / 2; Park cycle 0.382 s / 4 -> 0.268 s / 3. Programme task-queue-read-perf-2026-09 part 3; vikunja#1003. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared in-flight reads for eligible task lists and invalidates them after writes or matching watcher events. The UI coordinates parallel reads, prevents stale refresh results from applying, skips covered watcher events, and serializes actions per task. ChangesTask queue coordination
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant TaskQueueUI
participant TaskQueueServer
participant SharedRead
participant ControlAPI
TaskQueueUI->>TaskQueueServer: Submit task mutation
TaskQueueServer->>ControlAPI: Execute mutation
TaskQueueServer->>SharedRead: Invalidate shared task read
TaskQueueUI->>TaskQueueServer: Request refreshed task data
TaskQueueServer->>SharedRead: Get shared in-flight task read
SharedRead->>ControlAPI: GET /tasks
Merge Risk: ⚪ Minimal · up to This change reduces redundant task reads and duplicate refreshes. No actionable merge-blocking issue was identified. The watcher-skip behavior has not been tested after deployment, so it is worth a quick check once released. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Existing privileges and access boundaries appear unchanged. However, partial refresh failures can hide failed work, and a stalled shared read can delay subsequent refreshes across the configured queue. Deployment-specific access controls were not verified. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
CodeRabbit round 1 complete — 0 findings passed to the audit for verification. Security audit started.
Findings will be posted here when the audit completes. |
Security audit complete (round 1) — 0 findings.
Finding detail is in the internal report referenced above. |
Summary
GET /tasksper refresh, down from two./tasks(unfiltered) and/headless-runs(viaqueueIndexByPrefix) now join one in-flight read (src/shared-read.ts). Only the read is shared: a settled promise is dropped immediately, so there is no cache and no time window.loadTasksusesPromise.allSettledfor tasks / headless-runs / dead-letters. After a button press the list and the task detail refresh in parallel. Results apply last-started-wins (Latestinsrc/refresh-rules.ts).tasksevent for the tab's own write used to trigger another full refresh about 3 s later.coveredByRefreshskips it when a refresh whose list read succeeded started after the change the event reports.confirm(). Mutations are never sent in parallel or twice. The server's accepted unpark race assumes that clients don't do this.Why: the list read grows linearly with the active queue, and each refresh read it twice, sequentially.
Deliberately not done
/tasksand the dead-letters read are not shared. They are different queries.server.tscallslisten()at import, as before. The invalidation behaviour is tested through the realinvalidatingWrite+callControlApi+queueGetagainst an injected fetch, and source-level pins assert thatserver.tscallscallControlApionly insidemutate()and that the watcher invalidates before its debounce. This follows the existing union/regex drift-gate precedent.Look hardest at
src/shared-read.tsget()/invalidate(): a read that began before a mutation must never be joined by a request made after the mutation returned, and an old read settling must not detach a newer one.src/server.tsmutate(): invalidation runs infinally, i.e. also on refused and transport-failed writes.src/refresh-rules.tscoveredByRefresh: the argument that timer lateness and WS latency can only err toward refreshing.src/index.tsmutateThenRefresh/pendingActions: the guard is released after the mutation returns, not after the refresh.Test plan
npm run build,npm test(221 pass, 17 new),gate:vocabulary,gate:corpusNo
.coderabbit.yamlin this repo (noted, not added here).Tracked internally as vikunja#1003.
🤖 Generated with Claude Code
Summary by CodeRabbit