Skip to content

perf: cache markdown preparation by content - #172

Merged
kalvinnchau merged 3 commits into
mainfrom
perf/markdown-preparation
Sep 23, 2026
Merged

kalvinnchau merged 3 commits into
mainfrom
perf/markdown-preparation

Conversation

@kalvinnchau

@kalvinnchau kalvinnchau commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • cache pure Markdown preparation by exact content for the mounted component lifetime
  • isolate React Markdown parsing behind a memoized boundary while keeping identity, authorization, navigation, plugin, media, directory, and session inputs live
  • preserve signed source in plain fallbacks and share one literal-context classifier across preparation and restoration
  • add mounted StrictMode regressions for equivalent rows, edits, dynamic renderer inputs, and fallback transitions

Validation

  • bin/pnpm exec vitest run --maxWorkers=2 — 222 files, 2335 tests passed
  • bin/pnpm build
  • bin/pnpm test:browser tests/browser/profiles.spec.mjs tests/browser/conversation.spec.mjs — 16 passed in Chromium/WebKit, including configured measurement dependencies
  • focused Markdown/preparation/MessageRow — 101 tests passed
  • Biome checks for all changed message files

Evidence and limits

Mounted regression counters show that a fresh semantically equivalent row adds no preparation scans and no React Markdown executions; a same-ID content edit invalidates both and updates the body immediately.

This proves avoided work, not a measured user-visible latency or percentage improvement. No matched baseline/candidate production capture was performed.

Originating Buzz channel: 019392b5-3f47-4b94-80be-3f24d7fa5efb

@kalvinnchau
kalvinnchau requested review from a team, comp615 and wesbillman as code owners September 23, 2026 17:15
am added 2 commits September 23, 2026 10:36
Co-authored-by: Kalvin Chau <kalvin@block.xyz>

Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Co-authored-by: Kalvin Chau <kalvin@block.xyz>

Signed-off-by: Kalvin Chau <kalvin@block.xyz>
@kalvinnchau
kalvinnchau force-pushed the perf/markdown-preparation branch from 4b84672 to 2a45851 Compare September 23, 2026 17:38

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking correctness or security findings at head 2a45851382c21ab8a4fa7bc387385686b990a1fc against base 6e6417d713235dcfc29969f38f278ef51c8e7d82. One non-blocking test-coverage comment is inline.

Reviewed exact-content preparation, semantic invalidation, literal/source preservation, and live renderer inputs across message rows and composer decorations. Existing CI is green. Two additional mounted jsdom probes passed against this head with only a temporary review test added: authorization revoke/restore, replacement navigation callbacks, session-label changes with DOM/focus continuity, and shared-link scope changes all remained live without additional Markdown executions.

The evidence supports avoided parsing, not a measured user-visible latency improvement. No matched production benchmark or native UI acceptance was performed in this review. Required human approval remains outstanding; this is a COMMENT, not an approval.

for (const button of buttons) fireEvent.click(button);
expect(calls).toEqual([profileTarget(smith), profileTarget(mic)]);
for (const button of buttons) {
button.focus = () => calls.push("focus");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P3, non-blocking] Preserve the real focus regression assertion

Replacing button.focus with a recorder means this test now proves call order but no longer proves the clicked mention owns DOM focus when onOpenLink runs. The base version checked document.activeElement inside that callback. That matters for returning keyboard focus after profile navigation, and this cache change does not require dropping it.

Please keep the real focus method and restore the active-element assertion inside onOpenLink (a call-through spy is fine if ordering evidence is also useful). A separate mounted probe at this head confirms the actual behavior still works, so this is lost regression coverage rather than a demonstrated product defect.

Co-authored-by: Kalvin Chau <kalvin@block.xyz>

Signed-off-by: Kalvin Chau <kalvin@block.xyz>
@kalvinnchau
kalvinnchau merged commit ee2051d into main Sep 23, 2026
12 checks passed
@kalvinnchau
kalvinnchau deleted the perf/markdown-preparation branch September 23, 2026 18:28
zrmarley added a commit that referenced this pull request Sep 23, 2026
…search-send

* origin/main:
  Connect attachments to existing message delivery (#176)
  perf: preserve unchanged thread row identities (#171)
  perf: cache markdown preparation by content (#172)
  Add safe attachment upload groundwork (#150)
  feat: add sampling profiler launch modes (#148)
  feat(channels): remove DMs from the sidebar (#157)
  Distinguish namesake agents and selected recipients (#142)
  feat(channels): move diagnostics into Channel Settings (#163)
  Replace warning banners with shared Base UI toasts (#164)
  feat(shortcuts): add keyboard shortcut settings (#155)
  fix(channels): give floating unread cue an opaque panel surface (#153)
  feat(communities): add BUZZ_DEV_OPEN_RELAY to open the default relay on fresh dev ports (#151)
  Restore recipient avatars beside the composer mention tool (#162)
  Fix startup inventory duplication and late panel scroll shifts (#160)
  feat(channels): add channel creation (#138)
  Standardize Button and IconButton with Buzz design tokens (#145)

Signed-off-by: Zach Marley <zmarley@squareup.com>
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.

2 participants