Skip to content

perf(api): issue summary counts concurrently#1418

Merged
joshunrau merged 2 commits into
mainfrom
fix/summary-parallel-counts
Jul 23, 2026
Merged

perf(api): issue summary counts concurrently#1418
joshunrau merged 2 commits into
mainfrom
fix/summary-parallel-counts

Conversation

@gdevenyi

Copy link
Copy Markdown
Contributor

Fixes #1409.

SummaryService.getSummary backs the dashboard — the first page a user sees after logging in. It issued 20 count queries, each awaited before the next was sent, even though none of them depends on another. Latency was therefore the sum of 20 round trips rather than the slowest one.

This issues them concurrently with Promise.all.

Measured effect

Against a real MongoDB 8 replica set with a database provisioned through the app's own POST /v1/setup and grown to 5,000 subjects / 50,400 sessions / 204,800 instrument records (112 MiB). Median of 9 runs, warm cache, same database for both branches:

Request main this branch
GET /v1/summary 278 ms 69 ms 4.0×
GET /v1/summary?groupId=<g> 385 ms 88 ms 4.4×

The response is unchanged

Both versions were run against the same database and their payloads diffed. Once the trend timestamps are normalised (they are wall-clock values, so they differ between any two runs), the responses are byte-identical:

=== payload equivalence (timestamps normalised) ===
IDENTICAL — responses match exactly

Behavioural note

One thing did change, deliberately: Date.now() is now read before the counts run rather than after the first five complete. Previously the trend timestamps were offset by however long those queries happened to take — ~600 ms on the instance above, and growing with the size of the deployment. All three trends already shared a single reading; they now share one taken at a consistent point. The trend buckets are 30 days wide, so this is not observable in the dashboard.

Tests

The summary module had no spec; this adds one (6 tests) covering the counts, the trend shape and ordering, timestamp/value pairing, group scoping, and concurrency.

The concurrency test is the one that matters, and it is written so it cannot pass vacuously: it holds every mocked count open until all 20 have been called. If any query waits on the result of another, that number is never reached and the test times out. Verified against the previous implementation:

 ❯ src/summary/__tests__/summary.service.spec.ts (6 tests | 1 failed) 5033ms
   × should issue every query concurrently rather than awaiting them in sequence 5018ms
   Error: Test timed out in 5000ms.

and against this branch:

 Test Files  1 passed (1)
      Tests  6 passed (6)

Checks

  • tsc --noEmit on apps/api — clean
  • eslint src/summary — clean
  • prettier --check src/summary — clean
  • vitest --project api — 105 passed, 1 failed

The one failure is instrument-records.service.spec.ts > upload > should create records with pending set, which fails identically on pristine main and is unrelated to this change.

Scope

Deliberately limited to the parallelisation, which #1409 identifies as the high-value part. The other items in that issue are left for separate PRs:

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bfen9VQemZanJLXinJ6MMG

`SummaryService.getSummary` backs the dashboard, the first page a user
sees after logging in. It issued 20 count queries, each awaited before
the next was sent, even though none of them depends on another. Total
latency was therefore the sum of 20 round trips rather than the slowest
one.

Issue them concurrently with `Promise.all`. Measured against a real
MongoDB replica set with 5,000 subjects / 50,400 sessions / 204,800
instrument records, median of 9 runs:

  GET /v1/summary                278 ms -> 69 ms  (4.0x)
  GET /v1/summary?groupId=<g>    385 ms -> 88 ms  (4.4x)

The response is unchanged: both versions were diffed against the same
database and are identical once the trend timestamps are normalised.

The only behavioural difference is that `Date.now()` is now read before
the counts run rather than after, so trend timestamps no longer drift by
however long the first five queries took. All three trends already
shared one reading; they now share one taken at a consistent point.

Adds a spec for the module, which had none. The concurrency test holds
every count open until all 20 have been called, so it fails (by timing
out) against the sequential implementation rather than passing
vacuously.

Refs #1409

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfen9VQemZanJLXinJ6MMG
@gdevenyi
gdevenyi requested a review from joshunrau as a code owner July 20, 2026 17:47
Copilot AI review requested due to automatic review settings July 20, 2026 17:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@joshunrau
joshunrau merged commit 7d202b4 into main Jul 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: dashboard summary issues 20 sequential count queries and loads every instrument

3 participants