Faster initial card load: two calls for channel labels, and a skeleton while they arrive #331

Merged

robbertbos merged 2 commits from faster-initial-card-load into main 2026-08-17 12:01:35 +00:00

Conversation 0 Commits 2 Files changed 16 +899 -75

robbertbos commented 2026-08-17 09:09:06 +00:00

Owner

Copy link

Initial load went past ten seconds on a few hundred cards after 83996c0
(cards store references only, content comes live from Mattermost). Two commits:
the round trips, then what a card shows while it waits for them.

1. Resolve card-channel labels in two calls instead of one per channel

Not the number of cards - the number of channels. POST /api/mm/cards/content
bulked posts and authors, then resolved channels one at a time:

for channel_id in sorted({p.channel_id for p in posts.values() ...}):
    channel = await client.get_channel(channel_id)
    team_names[channel.team_id] = await client.get_team_name(...)

await in a for is not concurrency, and MattermostClient._client() opens a
new httpx.AsyncClient per call (26 call sites do async with self._client()),
so each of those round trips also paid its own TCP and TLS handshake. At ~60ms
RTT that is roughly 3 RTT per channel.

Verified against the Mattermost server source: there is no cross-team
bulk-by-id route for channels (POST /teams/{id}/channels/ids is team-scoped
and public-only). So the resolver asks the two collection endpoints instead -
GET /users/me/channels and GET /users/me/teams - and filters in process.
Four round trips per linked server whatever the card count, two once the
labels are warm.

Those listings only carry current memberships, so a card from a channel the
user has left is absent there and still needs its own fetch. That fallback is
concurrent, and a channel that stays unresolvable (403/404) is left out of the
result rather than failing the page.

The label caches move to the process, keyed on (base_url, id) so two
linked servers cannot answer for each other, and holding only fields that are
equal for every member of a channel. They were unreachable before:
build_mm_client hands every request a fresh client, so the hour-long TTL the
endpoint's docstring promised never scored a hit, not even on a reload.

2. Render a loading card as a skeleton, not a half-filled card

Two placeholders lied about what they stood for: the breadcrumb fell back to
source_data.channel_name (Mattermost's URL name, not a name it displays), and
the timestamp fell back to card.updated_at - the moment Waggle last touched
the row, in the format a real timestamp uses. A date never looks like a
placeholder, so a loading card read as a loaded one with the wrong date.

Both stay as a label of last resort for a card whose post is gone; what they
stop being is a stand-in for something still on its way. In their place: static
placeholder bars, and the quick actions held back until the content is in. The
actions do not need it - their show/disabled predicates read only card state

• so they are hidden rather than removed: visibility reserves their exact box
  and drops them from the tab order, so their arrival is not a second reflow.

This follows NLDD's design guidelines, which are specific about loading and
corrected the implementation twice while it was being built:

• Static, no shimmer. Animation moves attention to the waiting instead of
  to the interface being built.
• The skeleton shows immediately. It is the activity indicator that waits,
  not the placeholder - which is what nldd-activity-indicator's 1000ms hold is
  for.
• Past one second, an activity indicator over the whole list, content dimmed
  behind it. That is the component in overlay mode, over the list rather than
  per card: the hold, the frosted backdrop, role="status" announcing
  "Berichten laden" and inert content are all its own defaults, so this adds
  no timer of its own.

The guidelines also state the principle both fixes rest on: elements must not
appear in a provisional state and then change or disappear.

Bugs found on the way, each by a test rather than by reading

• get_team_name was the one method in the client that did not translate 401,
  so an expired token during team resolution surfaced as a 502 "mattermost
  error" instead of prompting re-auth.
• Moving the label cache to the process broke three existing tests immediately;
  they shared channel id c1. That was luck - with different ids the cross-test
  contamination would have travelled silently. Hence the reset fixture in
  conftest.py.
• A fixed-width crumb placeholder pushed the card into horizontal overflow at
  500px wide (attachments-multi.spec.ts).

Measuring

The dev Mattermost mock answered in microseconds, which hid the whole class of
bug: N calls in a row read exactly like one.
WAGGLE_DEV_MM_MOCK_LATENCY_MS now gives every mocked call a round-trip cost -
wrapped on the class rather than per method, because a benchmark that depends on
someone remembering to add a sleep to the next mock method is a benchmark that
lies.

At 60ms per call, 40 channels over 3 teams:

old: one call per channel	2566 ms (42 calls)
new: two collection calls	123 ms (2 calls)
new, warm cache	no Mattermost calls

The regression guard is a request counter, not a timing assertion: an
httpx.MockTransport records paths, and the test requires 40 channels to cost
two round trips. A wall-clock test would be flaky and would pin the wrong
property.

Two ways this measurement misleads, both documented in the run-waggle skill
after walking into them:

• Timing a fetch from the page measures the browser's connection queue, not
  the endpoint. With latency on, every request is slow and the
  six-connections-per-host limit queues yours behind them: 8.4s in the browser
  for a request that took 260ms server-side.
• dev_seed puts every card in one channel, so the preview cannot reproduce
  a per-channel N+1 at all, latency or not.

Also measured, on the preview with a delayed response: 21 of 42 cards
aria-busy while loading, six bars per card, no animation on any of them, the
action bar holding its 102px, and the row settling 4px shorter when a one-line
message lands (the 3em the old text placeholder reserved made that 20px). Two
body bars is a deliberate guess - the inbox body has no clamp, so no fixed count
is right for every message.

Verification

• Backend: 2056 tests, 100% coverage (the gate surfaced five uncovered paths,
  including both 401 re-raises - they have tests now).
• Frontend: 1369 tests, vue-tsc clean, build clean.
• E2E: 122 passed, 3 skipped.
• uvx pre-commit run --all-files green on the pinned ruff 0.8.6.

Not measured: wall-clock against a real Mattermost. The preview runs the MM mock,
so the ten seconds are not reproducible locally. What is proven is the round-trip
reduction; what the clock does depends on the RTT to the server.

Deliberately not in here

Viewport-first batching and a browser-local content cache. The decision was to
land this and measure first: with the label cache warm, what remains per load is
posts + users per server, so localStorage would save roughly 240ms at 60ms RTT
and only on a hard reload - against stale content for its TTL, quota management,
and partly reversing #189 phase 2. That one gets its own WDR if it happens.

Initial load went past ten seconds on a few hundred cards after `83996c0` (cards store references only, content comes live from Mattermost). Two commits: the round trips, then what a card shows while it waits for them. ## 1. Resolve card-channel labels in two calls instead of one per channel Not the number of cards - the number of channels. `POST /api/mm/cards/content` bulked posts and authors, then resolved channels one at a time: ```python for channel_id in sorted({p.channel_id for p in posts.values() ...}): channel = await client.get_channel(channel_id) team_names[channel.team_id] = await client.get_team_name(...) ``` `await` in a `for` is not concurrency, and `MattermostClient._client()` opens a new `httpx.AsyncClient` per call (26 call sites do `async with self._client()`), so each of those round trips also paid its own TCP and TLS handshake. At ~60ms RTT that is roughly 3 RTT per channel. Verified against the Mattermost server source: there is no cross-team bulk-by-id route for channels (`POST /teams/{id}/channels/ids` is team-scoped *and* public-only). So the resolver asks the two collection endpoints instead - `GET /users/me/channels` and `GET /users/me/teams` - and filters in process. **Four round trips per linked server whatever the card count**, two once the labels are warm. Those listings only carry *current memberships*, so a card from a channel the user has left is absent there and still needs its own fetch. That fallback is concurrent, and a channel that stays unresolvable (403/404) is left out of the result rather than failing the page. **The label caches move to the process**, keyed on `(base_url, id)` so two linked servers cannot answer for each other, and holding only fields that are equal for every member of a channel. They were unreachable before: `build_mm_client` hands every request a fresh client, so the hour-long TTL the endpoint's docstring promised never scored a hit, not even on a reload. ## 2. Render a loading card as a skeleton, not a half-filled card Two placeholders lied about what they stood for: the breadcrumb fell back to `source_data.channel_name` (Mattermost's URL name, not a name it displays), and the timestamp fell back to `card.updated_at` - the moment Waggle last touched the row, in the format a real timestamp uses. A date never looks like a placeholder, so a loading card read as a loaded one with the wrong date. Both stay as a label of last resort for a card whose post is gone; what they stop being is a stand-in for something still on its way. In their place: static placeholder bars, and the quick actions held back until the content is in. The actions do not need it - their `show`/`disabled` predicates read only card state - so they are hidden rather than removed: `visibility` reserves their exact box and drops them from the tab order, so their arrival is not a second reflow. This follows NLDD's design guidelines, which are specific about loading and corrected the implementation twice while it was being built: - **Static, no shimmer.** Animation moves attention to the waiting instead of to the interface being built. - **The skeleton shows immediately.** It is the *activity indicator* that waits, not the placeholder - which is what `nldd-activity-indicator`'s 1000ms hold is for. - **Past one second, an activity indicator over the whole list**, content dimmed behind it. That is the component in overlay mode, over the list rather than per card: the hold, the frosted backdrop, `role="status"` announcing "Berichten laden" and `inert` content are all its own defaults, so this adds no timer of its own. The guidelines also state the principle both fixes rest on: elements must not appear in a provisional state and then change or disappear. ## Bugs found on the way, each by a test rather than by reading - `get_team_name` was the one method in the client that did not translate 401, so an expired token during team resolution surfaced as a 502 "mattermost error" instead of prompting re-auth. - Moving the label cache to the process broke three existing tests immediately; they shared channel id `c1`. That was luck - with different ids the cross-test contamination would have travelled silently. Hence the reset fixture in `conftest.py`. - A fixed-width crumb placeholder pushed the card into horizontal overflow at 500px wide (`attachments-multi.spec.ts`). ## Measuring The dev Mattermost mock answered in microseconds, which hid the whole class of bug: N calls in a row read exactly like one. `WAGGLE_DEV_MM_MOCK_LATENCY_MS` now gives every mocked call a round-trip cost - wrapped on the class rather than per method, because a benchmark that depends on someone remembering to add a `sleep` to the next mock method is a benchmark that lies. At 60ms per call, 40 channels over 3 teams: | | | |---|---| | old: one call per channel | **2566 ms** (42 calls) | | new: two collection calls | **123 ms** (2 calls) | | new, warm cache | no Mattermost calls | The regression guard is a **request counter**, not a timing assertion: an `httpx.MockTransport` records paths, and the test requires 40 channels to cost two round trips. A wall-clock test would be flaky and would pin the wrong property. Two ways this measurement misleads, both documented in the `run-waggle` skill after walking into them: - **Timing a `fetch` from the page measures the browser's connection queue, not the endpoint.** With latency on, every request is slow and the six-connections-per-host limit queues yours behind them: 8.4s in the browser for a request that took 260ms server-side. - **`dev_seed` puts every card in one channel**, so the preview cannot reproduce a per-channel N+1 at all, latency or not. Also measured, on the preview with a delayed response: 21 of 42 cards `aria-busy` while loading, six bars per card, no animation on any of them, the action bar holding its 102px, and the row settling 4px shorter when a one-line message lands (the 3em the old text placeholder reserved made that 20px). Two body bars is a deliberate guess - the inbox body has no clamp, so no fixed count is right for every message. ## Verification - Backend: 2056 tests, 100% coverage (the gate surfaced five uncovered paths, including both 401 re-raises - they have tests now). - Frontend: 1369 tests, `vue-tsc` clean, build clean. - E2E: 122 passed, 3 skipped. - `uvx pre-commit run --all-files` green on the pinned ruff 0.8.6. Not measured: wall-clock against a real Mattermost. The preview runs the MM mock, so the ten seconds are not reproducible locally. What is proven is the round-trip reduction; what the clock does depends on the RTT to the server. ## Deliberately not in here Viewport-first batching and a browser-local content cache. The decision was to land this and measure first: with the label cache warm, what remains per load is posts + users per server, so localStorage would save roughly 240ms at 60ms RTT and only on a hard reload - against stale content for its TTL, quota management, and partly reversing #189 phase 2. That one gets its own WDR if it happens.

robbertbos force-pushed faster-initial-card-load from b79153c791 to e92dfcc32b 2026-08-17 10:32:31 +00:00 Compare

robbertbos changed title from Resolve card-channel labels in two calls instead of one per channel to Faster initial card load: two calls for channel labels, and a skeleton while they arrive 2026-08-17 10:33:27 +00:00

robbertbos force-pushed faster-initial-card-load from e92dfcc32b to 33650d533f 2026-08-17 11:10:31 +00:00 Compare

robbertbos force-pushed faster-initial-card-load from 33650d533f to e3c63ab5c1 2026-08-17 11:44:22 +00:00 Compare

robbertbos force-pushed faster-initial-card-load from e3c63ab5c1 to bde7846665 2026-08-17 11:54:12 +00:00 Compare

robbertbos merged commit 90b5a81285 into main 2026-08-17 12:01:35 +00:00

robbertbos deleted branch faster-initial-card-load 2026-08-17 12:01:35 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

nldd ds

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

duplicate

enhancement

help wanted

invalid

nldd ds

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

robbertbos/waggle!331

No description provided.