Repository navigation
perf(markdown): coalesce draft version hashing with publication - #35107
Conversation
595ebf0 to
384ade0
Compare
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 16.67% 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. |
-
Check name Status Explanation Title check The title clearly identifies the main change: coalescing Markdown draft version hashing with publication. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description follows the required structure and clearly explains the purpose, implementation, rationale, testing, visual proof, platform considerations, and checklist status. It identi… Full details: Description check
-
Explanation
The pull request description follows the required structure and clearly explains the purpose, implementation, rationale, testing, visual proof, platform considerations, and checklist status. It identifies the work as a maintainer follow-up to merged
#34775and records that manual and cross-platform testing were not performed locally.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 16.67% 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 6 functions across 5 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.
Comment @coderabbitai help to get the list of available commands.
ELI5
Markdown edits build the version stamp for the text being published. Edits superseded inside the existing 16ms publication window, and edits while mobile graph sync is disabled, no longer calculate full draft hashes.
Follow-up to merged #34775 (
31f3c524221); rebased onto main at2747079f94a3a9e91e863e2a96c69e5c48ae2830. Its Markdown/preview eligibility fix is retained.What Changed
The runtime graph scheduling key holds the admitted source IDs and exact draft strings, using map/key/text equality to detect meaningful changes. FNV version stamps are calculated by the existing mobile snapshot capture after scheduling coalesces edits. The scheduler, trailing/inflight ownership, hash function, and wire version format retain their behavior. Scheduling and snapshots share the existing Markdown/preview source policy.
Why
The production path is Monaco's accepted content callback →
setEditorDraft→ the mounteduseRuntimeGraphSyncsubscriber → scheduling key → 16ms publication coalescer. On the base, changed admitted Markdown text is hashed before that coalescer. Chunked large paste already suppresses intermediate callbacks, so this does not claim one hash per paste chunk.Controlled bursts of 100 real draft actions at 40,000/400,000 initial characters previously made 100 FNV calls, processing 4,005,050/40,005,050 characters for one publication. The follow-up hashes the final 40,100/400,100-character draft once, with the exact same final version. Disabled edits reduce 100 FNV calls to zero. A simulated 32ms cadence produced 100 publications on base; no cadence/CPU/frame or typing-latency measurement is claimed. Exact string equality can still inspect equal-length text; these are FNV operation/character counts, not a claim of zero text comparisons.
Linked Issue
Maintainer performance audit follow-up to merged #34775.
Visual Proof
N/A — internal scheduling/hash work changes; visible Markdown content and mobile version protocol retain their behavior.
Testing
Linux deterministic mounted hook tests use the real store draft action, 16ms scheduler, source policy and mobile snapshot builder. They cover N/10N bursts, unchanged recreated drafts, disabled/re-enabled sync, pending unmount, an in-flight trailing edit across disable/re-enable, preview-source admission/clear, and Markdown eligibility changes. The corrected base fails 4 tests and passes the preview correctness control; fixed focused suites pass 35 tests across 3 files.
Required
ORCA_BACKGROUND_LAUNCH=1 pnpm check:changed --maxWorkers=2passed all 21 checks in 1488.5 seconds at exact clean head595ebf00414365b1a4651774b254580705d7a01f. Log:/home/neil/.local/state/orca-electron-audit/markdown-current-main-evidence/check-changed.log.Maintainer review: rebased onto main
c58591e56d9without conflicts; follow-up commit384ade0b51freplaces two map-identity assertions with content checks and removes the test-local reference baseline that no longer exercised production code (the 4,005,050/40,005,050 figures come from the mounted hook test, which is unchanged). Gate on384ade0b51f:ORCA_BACKGROUND_LAUNCH=1 pnpm check:changed --maxWorkers=2reportedcheck:changed: 6 changed file(s) since origin/main (c58591e56d); running 21 check(s)./✓ 21 check(s) passed in 812.6s.(exit 0).All tests run with
ORCA_BACKGROUND_LAUNCH=1; no app/window was launched. macOS, Windows and live SSH/mobile transport were considered but not executed locally.Review
Notes
Platform-independent renderer logic; no process, shortcut, or path handling changes. Folder-workspace identities use the same existing source admission. Remote/mobile peers receive the existing document version format, so mixed-version compatibility is retained. No new transport fields, persistence, or security boundary.
Checklist
N/Awith reason