PERF: mount Notes before optional syntax grammars load (#663) #747

Open
opened 2026-10-02 13:08:29 +00:00 by kayg · 4 comments
Owner

Parent: #663. This is a browser runtime finding, not a measured latency regression. Related work: #639, #641 and #701; keep the same Notes owner where possible.

Context and source

  • origin/dev c4a61e8cf0.
  • The same mount barrier exists in merge-round-7a 2f4482ded0, perf-cache-665 b88bc6ac88 and tocrail-636.
  • apps/web/src/lib/notes/NoteEditorSurface.svelte:469–475: mount awaits loadEditorLowlight() before createEditor(). The catch permits a failed import but does not remove the wait for a pending import.
  • packages/editor/src/syntaxHighlight.ts:17–18,29–43: a shared empty lowlight instance is available synchronously, but first use imports syntaxLanguages and registers all grammars.
  • packages/editor/src/syntaxLanguages.ts:7 onward: the chunk includes the supported language grammars, even for a plain-text Note.

Reasoned impact
The first editable Note waits for an optional chunk download, module evaluation and registration. A pending download also blocks a cached plain-text body. No duration is claimed. Moving the grammars to a lazy chunk reduced bundle size but left that chunk on the first usable editor path. This conflicts with #663 rules 7–8.

Concrete fix
Mount with the existing synchronous lowlight instance. Load grammars after usable content paints, or when a code block needs them. When registration completes, refresh syntax decorations without changing Markdown, Yjs content or Undo history. Deduplicate the load and keep its existing retry behavior. Apply the same contract to other editor hosts if they share the barrier; do not add another loader.

Acceptance tests
Hold the grammar loader unresolved: a plain-text Note must paint and accept typing. Resolve or reject the loader: code highlighting must update or stay plain without a save, content change or focus reset. Preserve code-block Markdown round trips. Extend #549/#641 with cold grammar cache and warm body cases; use locked production/HDD measurements before claiming a budget result.

Duplicate search
Searched all issue titles for lowlight, grammar, syntax, and cold editor. Existing #639/#701 do not specify removal of this optional import barrier. No product edits were made in the audit.

Parent: #663. This is a browser runtime finding, not a measured latency regression. Related work: #639, #641 and #701; keep the same Notes owner where possible. Context and source - origin/dev c4a61e8cf090170f35b1bed3350d9de20c83ecd5. - The same mount barrier exists in merge-round-7a 2f4482ded066d9c5d9c59130377907f7fd2916c9, perf-cache-665 b88bc6ac888fd18e7e8a256f0b5b65ecaeed92c2 and tocrail-636. - apps/web/src/lib/notes/NoteEditorSurface.svelte:469–475: mount awaits loadEditorLowlight() before createEditor(). The catch permits a failed import but does not remove the wait for a pending import. - packages/editor/src/syntaxHighlight.ts:17–18,29–43: a shared empty lowlight instance is available synchronously, but first use imports syntaxLanguages and registers all grammars. - packages/editor/src/syntaxLanguages.ts:7 onward: the chunk includes the supported language grammars, even for a plain-text Note. Reasoned impact The first editable Note waits for an optional chunk download, module evaluation and registration. A pending download also blocks a cached plain-text body. No duration is claimed. Moving the grammars to a lazy chunk reduced bundle size but left that chunk on the first usable editor path. This conflicts with #663 rules 7–8. Concrete fix Mount with the existing synchronous lowlight instance. Load grammars after usable content paints, or when a code block needs them. When registration completes, refresh syntax decorations without changing Markdown, Yjs content or Undo history. Deduplicate the load and keep its existing retry behavior. Apply the same contract to other editor hosts if they share the barrier; do not add another loader. Acceptance tests Hold the grammar loader unresolved: a plain-text Note must paint and accept typing. Resolve or reject the loader: code highlighting must update or stay plain without a save, content change or focus reset. Preserve code-block Markdown round trips. Extend #549/#641 with cold grammar cache and warm body cases; use locked production/HDD measurements before claiming a budget result. Duplicate search Searched all issue titles for lowlight, grammar, syntax, and cold editor. Existing #639/#701 do not specify removal of this optional import barrier. No product edits were made in the audit.
Author
Owner

Starting work on job/notesperf, based on job/merge-round-7a at 2f4482ded066d9c5d9c59130377907f7fd2916c9. I will address the issue with focused changes and regression coverage, then report the final head SHA and verbatim gate output here.

Starting work on `job/notesperf`, based on `job/merge-round-7a` at `2f4482ded066d9c5d9c59130377907f7fd2916c9`. I will address the issue with focused changes and regression coverage, then report the final head SHA and verbatim gate output here.
Author
Owner

Evidence from the old code: NoteEditorSurface.mount() awaited loadEditorLowlight() before calling createEditor(). With the grammar promise held unresolved, the new regression timed out after two seconds with editor === null and data-editor-ready="false". The editor now mounts first; grammar loading and syntax refresh run after two animation frames.

Evidence from the old code: `NoteEditorSurface.mount()` awaited `loadEditorLowlight()` before calling `createEditor()`. With the grammar promise held unresolved, the new regression timed out after two seconds with `editor === null` and `data-editor-ready="false"`. The editor now mounts first; grammar loading and syntax refresh run after two animation frames.
Author
Owner

Completed implementation for #747, #749, #752, #754, #761, #769, #778, #780, #808 and #809 on job/notesperf.

Head: 109b5b4cf4a463a46f49556c65d55bd6e767d90f.

Built:

  • Notes now mounts before optional syntax grammars load; reminder decorations reuse/map their cached set across selection-only work.
  • Daily GET is read-only; POST creates or reads. The public Agent Skill now shows both exact routes.
  • Read-only Notes parser POSTs derive App Password scope and adapter intent from the action registry.
  • Collaboration invalidations are bounded/coalesced, target one stable Note identity, and use one resettable flush task per room. Notes/Search inactive writer-lock entries are weak.
  • IMAP STATUS uses revision metadata; SELECT filters before the 4096-row ceiling and reads only selected Notes. Unchanged refresh checks the revision clock. Immutable source bytes use Arc<str> and a weak revision cache.
  • Added/extended the Notes editor and IMAP benchmark profiles. The adversarial IMAP fixture can select the unchanged STATUS/SELECT/NOOP profile.

Files changed: Notes web/editor and API clients; Notes, Search, Collab, IMAP, CLI and server Rust crates; OpenAPI/action registry/API client/parity outputs; bench/notes-external-write.sh, bench/notes-bridge.py, and adversarial Notes probes; Notes docs and Agent Skill text.

Commits include 07b7d3133, 25232cd10, db48e08cc, 64197bb06, eb8a5d100, 750b36129, 3537938f1, e016b6225, 2e4be1398, and 109b5b4cf (plus benchmark documentation commit 93136f2e9).

Gates and evidence:

$ cargo fmt --check
[exit 0; no output]

$ cargo test -p calternal-server notes_daily_and_preview_app_password_scopes_follow_action_intent -- --exact wire::tests::notes_daily_and_preview_app_password_scopes_follow_action_intent
running 1 test
test wire::tests::notes_daily_and_preview_app_password_scopes_follow_action_intent ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 166 filtered out; finished in 0.01s

$ python3 -m unittest test_action_registry.py
.............
----------------------------------------------------------------------
Ran 13 tests in 0.218s

OK

$ python3 scripts/parity_matrix.py --check
Parity matrix: 341 API actions, 126 shortcuts, 2 static commands, 147 menu actions, 39 settings groups, 0 actions with adapter gaps

$ bun run check
User browser caches use userStorage; only documented device/public-link exceptions remain.
Text sizes and UI shape values use shared role tokens.
UI transitions and animation options use shared motion tokens or documented exceptions.
Loading svelte-check in workspace: /home/kayg/Developer/calternal-wt/notesperf/apps/web
Getting Svelte diagnostics...

svelte-check found 0 errors and 0 warnings

$ bun run test
 Test Files  4 failed | 152 passed (156)
      Tests  5 failed | 1070 passed (1075)

The five web failures were 5-second timeouts in Agenda duplicate menu, two Connected Accounts cases, Composer Event availability, and ThemePicker saved variant. They were unrelated to Notes and occurred during heavy shared-host load. API-client tests reported 17 pass, 1 fail; the failure was the generated-tool raw-text test timing out after 5000 ms.

The server test binary ran 167 tests: 161 passed; 1 failed; 5 ignored. The failure expected the exact Daily GET URL in the generated Skill. Commit 109b5b4cf updates the source Skill text to include that route without changing the test expectation; this correction could not be rebuilt/retested. The full cargo test -p calternal-server wrapper remained blocked on shared package-cache/filesystem I/O and was interrupted. No clippy gates or full per-crate test gates completed.

bun run build passed and ended with:

✓ built in 1m 6s
Run npm run preview to preview your production build locally.

> Using @sveltejs/adapter-static
  Wrote site to "build"
  ✔ done

cargo clean passed: Removed 13467 files, 10.9GiB total.

Known gaps:

  • The perf VM lock was busy, so no perf-VM measurements were taken. The local Notes E2E server did not become ready within 30 seconds under shared-host load; neither new benchmark profile produced measurements. There is no matching before baseline for IMAP SELECT in docs/perf/baseline.json.
  • The one adversarial API round did not run: rebuilding the current server binary was blocked on filesystem I/O with host load above 120.
  • The Playwright module was available from the pinned 1.63.0 cache, but the Notes screenshot run could not start its local server. No screenshot set was produced or attached. Production web build output has been removed as required.
  • Full crate clippy/tests remain unverified. The five web timeouts and one API-client timeout are SLOW findings; assertions were not changed.

UX gaps closed: the editor accepts input before optional grammars load; reminder decorations no longer rebuild on caret moves. UX gaps left: the required 390/820/1440 light/dark screenshot review could not run because the local API server did not become ready.

Decisions where the design is silent: derive all read-only POST scopes from registry metadata; on collaboration overflow request one bounded reconciliation; use a weak per-User lock registry; initialize empty-mailbox UIDVALIDITY from the immutable User UUID; share IMAP source bytes only through weak revision-keyed entries; reuse the existing adversarial fixture for the IMAP select profile.

No push or deploy was made. Local merges of origin/dev and origin/job/merge-round-7a are included.

Completed implementation for #747, #749, #752, #754, #761, #769, #778, #780, #808 and #809 on `job/notesperf`. Head: `109b5b4cf4a463a46f49556c65d55bd6e767d90f`. Built: - Notes now mounts before optional syntax grammars load; reminder decorations reuse/map their cached set across selection-only work. - Daily GET is read-only; POST creates or reads. The public Agent Skill now shows both exact routes. - Read-only Notes parser POSTs derive App Password scope and adapter intent from the action registry. - Collaboration invalidations are bounded/coalesced, target one stable Note identity, and use one resettable flush task per room. Notes/Search inactive writer-lock entries are weak. - IMAP STATUS uses revision metadata; SELECT filters before the 4096-row ceiling and reads only selected Notes. Unchanged refresh checks the revision clock. Immutable source bytes use `Arc<str>` and a weak revision cache. - Added/extended the Notes editor and IMAP benchmark profiles. The adversarial IMAP fixture can select the unchanged STATUS/SELECT/NOOP profile. Files changed: Notes web/editor and API clients; Notes, Search, Collab, IMAP, CLI and server Rust crates; OpenAPI/action registry/API client/parity outputs; `bench/notes-external-write.sh`, `bench/notes-bridge.py`, and adversarial Notes probes; Notes docs and Agent Skill text. Commits include `07b7d3133`, `25232cd10`, `db48e08cc`, `64197bb06`, `eb8a5d100`, `750b36129`, `3537938f1`, `e016b6225`, `2e4be1398`, and `109b5b4cf` (plus benchmark documentation commit `93136f2e9`). Gates and evidence: ```text $ cargo fmt --check [exit 0; no output] $ cargo test -p calternal-server notes_daily_and_preview_app_password_scopes_follow_action_intent -- --exact wire::tests::notes_daily_and_preview_app_password_scopes_follow_action_intent running 1 test test wire::tests::notes_daily_and_preview_app_password_scopes_follow_action_intent ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 166 filtered out; finished in 0.01s $ python3 -m unittest test_action_registry.py ............. ---------------------------------------------------------------------- Ran 13 tests in 0.218s OK $ python3 scripts/parity_matrix.py --check Parity matrix: 341 API actions, 126 shortcuts, 2 static commands, 147 menu actions, 39 settings groups, 0 actions with adapter gaps $ bun run check User browser caches use userStorage; only documented device/public-link exceptions remain. Text sizes and UI shape values use shared role tokens. UI transitions and animation options use shared motion tokens or documented exceptions. Loading svelte-check in workspace: /home/kayg/Developer/calternal-wt/notesperf/apps/web Getting Svelte diagnostics... svelte-check found 0 errors and 0 warnings $ bun run test Test Files 4 failed | 152 passed (156) Tests 5 failed | 1070 passed (1075) ``` The five web failures were 5-second timeouts in Agenda duplicate menu, two Connected Accounts cases, Composer Event availability, and ThemePicker saved variant. They were unrelated to Notes and occurred during heavy shared-host load. API-client tests reported `17 pass`, `1 fail`; the failure was the generated-tool raw-text test timing out after 5000 ms. The server test binary ran 167 tests: `161 passed; 1 failed; 5 ignored`. The failure expected the exact Daily GET URL in the generated Skill. Commit `109b5b4cf` updates the source Skill text to include that route without changing the test expectation; this correction could not be rebuilt/retested. The full `cargo test -p calternal-server` wrapper remained blocked on shared package-cache/filesystem I/O and was interrupted. No clippy gates or full per-crate test gates completed. `bun run build` passed and ended with: ```text ✓ built in 1m 6s Run npm run preview to preview your production build locally. > Using @sveltejs/adapter-static Wrote site to "build" ✔ done ``` `cargo clean` passed: `Removed 13467 files, 10.9GiB total`. Known gaps: - The perf VM lock was busy, so no perf-VM measurements were taken. The local Notes E2E server did not become ready within 30 seconds under shared-host load; neither new benchmark profile produced measurements. There is no matching before baseline for IMAP SELECT in `docs/perf/baseline.json`. - The one adversarial API round did not run: rebuilding the current server binary was blocked on filesystem I/O with host load above 120. - The Playwright module was available from the pinned 1.63.0 cache, but the Notes screenshot run could not start its local server. No screenshot set was produced or attached. Production web build output has been removed as required. - Full crate clippy/tests remain unverified. The five web timeouts and one API-client timeout are SLOW findings; assertions were not changed. UX gaps closed: the editor accepts input before optional grammars load; reminder decorations no longer rebuild on caret moves. UX gaps left: the required 390/820/1440 light/dark screenshot review could not run because the local API server did not become ready. Decisions where the design is silent: derive all read-only POST scopes from registry metadata; on collaboration overflow request one bounded reconciliation; use a weak per-User lock registry; initialize empty-mailbox UIDVALIDITY from the immutable User UUID; share IMAP source bytes only through weak revision-keyed entries; reuse the existing adversarial fixture for the IMAP select profile. No push or deploy was made. Local merges of `origin/dev` and `origin/job/merge-round-7a` are included.
Author
Owner

Orchestrator review: job/notesperf (head 87f8647e9)

Reviewed the 12 job commits (07b7d3133..109b5b4cf) and re-ran the gates. I added six commits.

Fixes added

  • ff50b888f collab, data loss on crash (P1). The single room flush task (#778) read the generation again after its flush. An edit made after the flush snapshot was then not seen: its own schedule_flush returned because the task was still running, and the task exited. The edit stayed only in memory until another edit, the last client leaving or shutdown. The task now compares against the generation it read before the flush. Regression test edit_during_timer_flush_is_saved_without_another_edit (new FlushAfterSnapshot hook) fails on the old code. Also fixed a collapsible_if clippy failure in ExternalChangeInbox::push.
  • f9885e31f IMAP, duplicate Notes in Apple Notes (P1). The Index-side SELECT/STATUS for the top-level Notes mailbox (#780) returned every Note. folders_for_note puts only untagged Notes there, so tagged Notes appeared twice. Both queries now exclude Notes that have note_tags rows. Regression test top_level_folder_lists_only_untagged_notes fails on the old code. Also fixed an unnecessary_cast clippy failure.
  • 87f8647e9 IMAP. initial_uidvalidity parsed the User id as a UUID, so store::index failed for non-UUID ids. This broke daily_note_link_migration_marks_homes_independently_and_retries_failures. The epoch is now a blake3 hash of the User id, which is still deterministic.
  • 98f150cc4 + c7d834c93 web, reminder chips disappear (P2). The cached reminder DecorationSet (#749) mapped widgets through every transaction. y-tiptap applies each remote or initial Yjs update as one whole-document ReplaceStep, so all chips were dropped until the reminder list refreshed. The set is now rebuilt for a whole-document replace. The new Vitest case fails on the old code.
  • 78a859d44 test. Proves that the post-load grammar refresh (#747) causes no Yjs update for a Note with a code block. This was an acceptance criterion with no test.

Gates (after fixes)

cargo fmt --check                                   exit 0
clippy -D warnings: calternal-collab, calternal-imap, calternal-search, calternal-plugin-notes, calternal-cli, calternal-server   all exit 0
cargo test -p calternal-collab      all binaries ok (lib 38 passed)
cargo test -p calternal-imap        ok (30+8+8+5+5+3 passed)
cargo test -p calternal-search      lib 41 passed; tests/indexer 1 failure in the loaded run (incremental_update_after_rebuild_keeps_another_users_new_result), alone: 21 passed -> load
cargo test -p calternal-plugin-notes  188 passed, 0 failed
cargo test -p calternal-cli         33 + 15 passed
cargo test -p calternal-server      162 passed, 0 failed, 5 ignored
bun run check                       svelte-check 0 errors, 0 warnings
vitest editorHost / editorStartup / api/notes   3 files, 6 tests passed

Before the fixes, clippy failed in calternal-plugin-notes and calternal-collab, and the notes tests failed 2 of 188. One of those failures was caused by the UUID parse; the other passed on re-run.

Review notes (not fixed)

  • Collaboration hint coalescing has no lost-edit risk. Hints only re-read disk into rooms. Room close (leave_last) and shutdown flush on their own path. A panic in the external-change actor would stop all later reconciliation, but that was also true before this change.
  • Weak per-User lock registries (Notes, Search) cannot leak state across Users. Entries are keyed by User id, and a lock lives as long as a guard or waiter holds its Arc. P3: user_mutex is the same code in two crates.
  • The App Password read-only POST set comes from the registry (zip, composer/parse, tasks/parse). Both parsers are pure, and the exact path match fails closed. This is OK.
  • Daily: GET now returns 404 for a missing day, and the MCP calternal_today tool is read-only. P3: apps/web/e2e/route-perf.mjs still measures GET /notes/daily and will record 404s on a fresh seed.
  • Duplicate of #752 in job/surfaces-p1 (064e72699, 711064a02). Both branches add POST /api/v1/notes/daily with different request shapes: here it is a JSON body {date} with deny_unknown_fields, in surfaces-p1 it is the ?date= query with no body. They conflict in notes/lib.rs, api/notes.ts, the tests and the generated contracts. Recommendation: surfaces-p1 wins for the handler, wire shape, contract/policy and the web client (it does a GET first and a POST only on 404, so opening a Daily note no longer takes the writer lock). From this branch, port the CLI today change to POST (surfaces-p1 still uses GET, which returns 404 on a new day), the MCP description, skill_intro/README text and the adversarial probe, all adapted to the query form. #754 also overlaps: surfaces-p1 marks five POSTs read-only in action-policy.json, including the reminders preview and the tag rename preview. This branch's build.rs generator should read that policy, and the two previews must be confirmed pure first.

Perf numbers, the adversarial round and the screenshots are still missing, as the job reported.

## Orchestrator review: job/notesperf (head 87f8647e9) Reviewed the 12 job commits (07b7d3133..109b5b4cf) and re-ran the gates. I added six commits. ### Fixes added - **ff50b888f collab, data loss on crash (P1).** The single room flush task (#778) read the generation again after its flush. An edit made after the flush snapshot was then not seen: its own `schedule_flush` returned because the task was still running, and the task exited. The edit stayed only in memory until another edit, the last client leaving or shutdown. The task now compares against the generation it read before the flush. Regression test `edit_during_timer_flush_is_saved_without_another_edit` (new `FlushAfterSnapshot` hook) fails on the old code. Also fixed a `collapsible_if` clippy failure in `ExternalChangeInbox::push`. - **f9885e31f IMAP, duplicate Notes in Apple Notes (P1).** The Index-side SELECT/STATUS for the top-level `Notes` mailbox (#780) returned every Note. `folders_for_note` puts only untagged Notes there, so tagged Notes appeared twice. Both queries now exclude Notes that have `note_tags` rows. Regression test `top_level_folder_lists_only_untagged_notes` fails on the old code. Also fixed an `unnecessary_cast` clippy failure. - **87f8647e9 IMAP.** `initial_uidvalidity` parsed the User id as a UUID, so `store::index` failed for non-UUID ids. This broke `daily_note_link_migration_marks_homes_independently_and_retries_failures`. The epoch is now a blake3 hash of the User id, which is still deterministic. - **98f150cc4 + c7d834c93 web, reminder chips disappear (P2).** The cached reminder DecorationSet (#749) mapped widgets through every transaction. y-tiptap applies each remote or initial Yjs update as one whole-document ReplaceStep, so all chips were dropped until the reminder list refreshed. The set is now rebuilt for a whole-document replace. The new Vitest case fails on the old code. - **78a859d44 test.** Proves that the post-load grammar refresh (#747) causes no Yjs update for a Note with a code block. This was an acceptance criterion with no test. ### Gates (after fixes) ``` cargo fmt --check exit 0 clippy -D warnings: calternal-collab, calternal-imap, calternal-search, calternal-plugin-notes, calternal-cli, calternal-server all exit 0 cargo test -p calternal-collab all binaries ok (lib 38 passed) cargo test -p calternal-imap ok (30+8+8+5+5+3 passed) cargo test -p calternal-search lib 41 passed; tests/indexer 1 failure in the loaded run (incremental_update_after_rebuild_keeps_another_users_new_result), alone: 21 passed -> load cargo test -p calternal-plugin-notes 188 passed, 0 failed cargo test -p calternal-cli 33 + 15 passed cargo test -p calternal-server 162 passed, 0 failed, 5 ignored bun run check svelte-check 0 errors, 0 warnings vitest editorHost / editorStartup / api/notes 3 files, 6 tests passed ``` Before the fixes, clippy failed in calternal-plugin-notes and calternal-collab, and the notes tests failed 2 of 188. One of those failures was caused by the UUID parse; the other passed on re-run. ### Review notes (not fixed) - Collaboration hint coalescing has no lost-edit risk. Hints only re-read disk into rooms. Room close (`leave_last`) and shutdown flush on their own path. A panic in the external-change actor would stop all later reconciliation, but that was also true before this change. - Weak per-User lock registries (Notes, Search) cannot leak state across Users. Entries are keyed by User id, and a lock lives as long as a guard or waiter holds its Arc. P3: `user_mutex` is the same code in two crates. - The App Password read-only POST set comes from the registry (zip, composer/parse, tasks/parse). Both parsers are pure, and the exact path match fails closed. This is OK. - Daily: GET now returns 404 for a missing day, and the MCP `calternal_today` tool is read-only. P3: `apps/web/e2e/route-perf.mjs` still measures GET `/notes/daily` and will record 404s on a fresh seed. - **Duplicate of #752 in job/surfaces-p1 (064e72699, 711064a02).** Both branches add `POST /api/v1/notes/daily` with different request shapes: here it is a JSON body `{date}` with deny_unknown_fields, in surfaces-p1 it is the `?date=` query with no body. They conflict in notes/lib.rs, api/notes.ts, the tests and the generated contracts. Recommendation: surfaces-p1 wins for the handler, wire shape, contract/policy and the web client (it does a GET first and a POST only on 404, so opening a Daily note no longer takes the writer lock). From this branch, port the CLI `today` change to POST (surfaces-p1 still uses GET, which returns 404 on a new day), the MCP description, skill_intro/README text and the adversarial probe, all adapted to the query form. #754 also overlaps: surfaces-p1 marks five POSTs read-only in action-policy.json, including the reminders preview and the tag rename preview. This branch's build.rs generator should read that policy, and the two previews must be confirmed pure first. Perf numbers, the adversarial round and the screenshots are still missing, as the job reported.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
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
kayg/calternal#747
No description provided.