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 added 3 commits 2026-08-17 09:09:06 +00:00

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

Card content resolved channels sequentially - `await client.get_channel()`
inside a for loop - and MattermostClient._client() opens a new
httpx.AsyncClient per call, so every one of those round trips paid its own
TCP and TLS handshake. A few hundred cards over ~40 channels took upwards of
ten seconds to paint.

Mattermost has no cross-team bulk-by-id route for channels
(POST /teams/{id}/channels/ids is team-scoped and public-only, verified
against the server source), so ask the collection endpoints instead:
/users/me/channels and /users/me/teams, filtered in process. That is four
round trips per linked server regardless of card count. Those listings only
carry current memberships, so a card from a channel the user has left still
falls back to a per-channel fetch - concurrently, and a channel that stays
unresolvable is omitted rather than failing the request.

The label caches move from the client instance to the process. 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. Keyed on (base_url, id) so two linked servers cannot answer
for each other, and holding only fields that are equal for every member.

That shared cache needs the reset fixture in conftest.py: three existing
tests already shared channel id "c1" and broke immediately, which was luck -
with different ids the cross-test contamination would have travelled silently.

Also translate 401 in get_team_name, which was the one method in the client
that did not, so an expired token surfaced as a 502 instead of prompting
re-auth.

Hold the source breadcrumb back until its labels arrive ... 59dd3ecd80

The crumb fell back to source_data.channel_name while the content query was
in flight. That slug is Mattermost's URL name, not a name it displays
anywhere, and as a placeholder it was indistinguishable from a resolved crumb
- so a loading card read as a loaded one with the wrong channel name.

The slug stays as a label of last resort for a channel that resolved without
a display name. What it stops being is a stand-in for one still loading. A
card whose post is gone never gets content, so "no content" alone cannot mean
"still loading"; the tombstone keeps its crumb through an explicit `missing`
prop rather than losing its only piece of context.

Note the faster initial load in the CHANGELOG 358e4911e5

robbertbos added 1 commit 2026-08-17 09:43:09 +00:00

Give the dev Mattermost mock a per-call latency knob ... 54da82d9fa

The mock answers in microseconds, which hides the one thing worth measuring
about a Mattermost call: that there was one. N calls on a row read exactly like
one, which is how a sequential per-channel loop stayed invisible locally until
it cost ten seconds against a real server.

WAGGLE_DEV_MM_MOCK_LATENCY_MS puts a delay on every coroutine the mock defines.
Wrapped on the class rather than per method on purpose - a benchmark whose
numbers depend on someone remembering to add a sleep to the next mock method is
a benchmark that lies. Read from the environment rather than Settings: this is
devtools, and the production config surface should not grow a field for it.

The skill notes the two ways this misleads: timing a fetch from the page
measures the browser's connection queue rather than the endpoint (8.4s in the
browser for a 260ms request), and dev_seed puts every card in one channel, so
the preview cannot reproduce a per-channel N+1 at all.

robbertbos added 1 commit 2026-08-17 10:11:00 +00:00

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

The timestamp fell back to card.updated_at while the content query was in
flight - the moment Waggle last touched the row, which is not when the message
was posted, rendered in the format a real timestamp uses. Same defect class as
the channel slug in the breadcrumb, but worse: a date never looks like a
placeholder.

So the row now shows static placeholder bars for what it does not have yet, and
holds the quick actions until it does. The actions do not need the content -
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, which keeps their arrival from being a second reflow.

Following NLDD's own loading guidelines, which turn out to be specific here:

- 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, with the loaded
  content dimmed behind it. That is the component in overlay mode: the hold, the
  frosted backdrop, role="status" announcing "Berichten laden" and inert content
  are all its defaults, so this adds no timer of its own.

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

Measured on the preview with a delayed response: 21 of 42 cards aria-busy while
loading, no animation on the bars, the action bar holding its 102px, and the row
settling 4px shorter when a one-line message lands (the 3em the old placeholder
reserved made that 20px).

robbertbos added 1 commit 2026-08-17 10:23:30 +00:00

Give the source breadcrumb its own skeleton bar ... b79153c791

The crumb rendered nothing while its labels were in flight, which left a gap
above a card that was otherwise a full skeleton. It now holds its line with a
placeholder bar instead - in MmSourceBreadcrumb rather than in the card, so the
inbox row, the reading-list row, the card modal and Concept-berichten all get it
from one place.

The bar has to shrink like the channel label beside it: the crumb's flex parent
sizes to its content, so a fixed 11rem pushed the card into horizontal overflow
at 500px wide. attachments-multi.spec.ts caught it.

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.