Skip to content

perf(renderer): hash drafts only for published Markdown sources - #34775

Merged
OrcaWin merged 1 commit into
mainfrom
OrcaWin/perf-markdown-draft-hashes
Oct 28, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
OrcaWin/perf-markdown-draft-hashes

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​103 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​101
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​62 $\color{#cf222e}{\Huge{\mathbf{−}}}$​19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43

ELI5

Typing in ordinary code files repeatedly recalculates a document fingerprint that no mobile view uses. Calculate it only when a published Markdown view or preview needs it.

What Changed

Editing an already-dirty ordinary code file currently hashes the entire document synchronously and changes the mobile runtime graph key on every content event, even though ordinary file tabs publish no documentVersion. The graph's 16 ms coalescer runs after this hash and equality work. The production path is Monaco's content callback → useEditorContentChangeHandler → setEditorDraft → the useRuntimeGraphSync store subscription → sync-key construction.

Restrict draft hashing to sources of mobile-publishable Markdown/preview files. Share the existing Markdown eligibility and workspace-scoped preview-source resolution between tab construction and hash admission. Reuse the existing open-file index and cache the admitted source IDs by openFiles identity. The hash cache and sync-key reuse now include openFiles, so opening, closing, language changes, and preview aliases invalidate admission even when the draft map is unchanged. The per-file content cache still hashes only an edited Markdown source.

Why

Hashing after admission avoids work before the existing graph equality and coalescing gates. Reusing the published Markdown eligibility and preview-source rules keeps hash admission aligned with the fields actually sent to mobile clients.

Linked Issue

N/A — internal performance audit; no pre-existing issue.

Visual Proof

N/A — published file metadata, Markdown versions, and interactions are unchanged; this removes unnecessary synchronous hashing.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

A deterministic regression test drives the production sync-key builder with 100 successive immutable draft updates for an already-dirty TypeScript file. Every other state identity is stable, matching the subscription's content-edit input. It counts actual production stableHashString calls and characters, plus key invalidations:

Initial document length Base: hashes / characters / key invalidations Fixed
40,000 characters 100 / 4,005,050 / 100 0 / 0 / 0
400,000 characters 100 / 40,005,050 / 100 0 / 0 / 0

The two assertions fail with those exact counts on base 32a50856ac388a419ffa18b2295a76d1744f7ec5 and pass with the fix. This counts renderer work removed before the graph equality/coalescing gates; it is not a claim that 100 IPCs were previously sent. The first dirty-state change still republishes ordinary file metadata through the open-files projection.

Correctness controls:

  • Markdown source edits still change their draft version; the existing 100-keystroke test still hashes exactly the edited Markdown file and reuses four unchanged drafts.
  • A preview that references an ordinary source file hashes that source. Missing preview targets retain the existing fallback to the preview file itself.
  • Diff and mirrored Markdown drafts are excluded because they do not publish Markdown versions.
  • Language changes with an unchanged draft-map identity admit the new source. Draft removal restores file:<id>, and closed sources release their cached entries.
  • Snapshot construction reuses the same hash and preserves preview source IDs and versions. A folder workspace key is included in admission coverage.

Validation: 4 focused files / 48 tests passed (sync-runtime-graph-projection-hot-path, sync key, snapshot reuse, editor/diff snapshot tests). ORCA_BACKGROUND_LAUNCH=1 pnpm --config.verifyDepsBeforeRun=false check:changed --maxWorkers=2: passed all 21 checks in 2620.7s on the published commit. All 48 focused tests and baseline-failing operation counts also passed.

Rerun the production-key regression with ORCA_BACKGROUND_LAUNCH=1 pnpm test src/renderer/src/runtime/sync-runtime-graph-projection-hot-path.test.ts --maxWorkers=2.

Tests ran in the background with ORCA_BACKGROUND_LAUNCH=1. No native-focus or visible-window test was run.

AI Disclosure

Review

Reviewed shared eligibility, workspace-scoped preview aliases, immutable draft/open-file invalidation, snapshot version reuse, and closure pruning. Existing wire fields and first-dirty metadata publication are preserved.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Measurements use Linux, Node 26.8.1, Vitest development tests. No packaged Electron frame/CPU/heap claim or native macOS/Windows/live-remote transport result is made. The change is pure renderer logic with no wire format or host-specific changes. Published Markdown edits still pay a full hash of the changed source before graph coalescing; this PR removes hashes with no published consumer. Only the latest draft/open-file snapshots and their admitted entries are cached, with no new timer or subscription.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

The required local changed-file gate is recorded above; the full build is left to CI.

@OrcaWin
OrcaWin force-pushed the OrcaWin/perf-markdown-draft-hashes branch from 5e07c34 to 32f3634 Compare October 28, 2026 06:50
@OrcaWin
OrcaWin marked this pull request as ready for review October 28, 2026 06:50
@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cf118bfc-08a8-48b6-80c5-cc4d1b433c54
Commits

Reviewing files that changed from the base of the PR and between 09dd95d and 32f3634.

Files selected for processing: 7
  • src/renderer/src/runtime/sync-runtime-graph-projection-hot-path.test.ts
  • src/renderer/src/runtime/sync-runtime-graph/mobile-session-editor-tabs.ts
  • src/renderer/src/runtime/sync-runtime-graph/mobile-session-snapshots.ts
  • src/renderer/src/runtime/sync-runtime-graph/mobile-session-surfaces.ts
  • src/renderer/src/runtime/sync-runtime-graph/sync-key.ts
  • src/renderer/src/runtime/sync-runtime-graph/sync-projections.ts
  • src/renderer/src/runtime/sync-runtime-graph/types.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

Mobile editor draft projections now use open-file state to identify eligible Markdown source files. The cache hashes draft content only for those sources and rebuilds when editor drafts or open files change. Mobile tab and snapshot builders use the updated source-selection and draft-version logic. Tests cover large non-Markdown drafts, Markdown preview aliases, version updates and clearing, and sync-key invalidation.

Priority: Low

Merge Risk: Minimal · up to 32f36

No specific issue requiring a fix before merge is established. Normal CI and release validation remain appropriate.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: limiting renderer draft hashing to published Markdown sources.
Description check Passed The description is complete and focused. It explains the change, rationale, implementation, testing, performance results, scope, and limitations. It also addresses the template exceptions for an inter…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is complete and focused. It explains the change, rationale, implementation, testing, performance results, scope, and limitations. It also addresses the template exceptions for an internal contributor, non-UI changes, and CI-owned full-build validation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 7 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OrcaWin
OrcaWin merged commit 31f3c52 into main Oct 28, 2026
117 of 119 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.

1 participant