fix(messaging): reduce composer recomposition and typing jank#6318
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughMessaging mentions now use lightweight candidates with live markdown styling and replacement remapping. Draft persistence is debounced, with pending writes canceled when drafts are cleared. Tests cover composer behavior and draft timing. ChangesMention composer pipeline
Draft persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MessageScreen
participant MessageInput
participant MentionCandidateMatcher
participant MentionOutputPlan
participant LiveMarkdownParser
MessageScreen->>MessageInput: provide mention candidates
MessageInput->>MentionCandidateMatcher: match mention query
MentionCandidateMatcher-->>MessageInput: return ordered suggestions
MessageInput->>MentionOutputPlan: replace mention tokens
MentionOutputPlan->>LiveMarkdownParser: remap markdown spans
LiveMarkdownParser-->>MessageInput: return styled composer output
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)feature/messaging/build.gradle.ktsTraceback (most recent call last): 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 |
dad6f71 to
847d4b2
Compare
|
Went through this in detail — the correctness story is solid: raw text in the buffer is what's stored and transmitted ( Four smaller items, none blocking:
|
847d4b2 to
ec68328
Compare
Project nodes into a persistent immutable map of lightweight mention candidates whose equality depends only on identifiers and display names. Keep the composer parameter stable when telemetry, position, last-heard, or other unrelated node fields change so Compose can skip the input subtree without recreating its output transformation or churning the platform input connection. Keep live editing bounded with a lightweight inline-markdown scanner, cached normalized candidate fields, one query normalization, limited lazy matching, and an empty-query fast path. Document the intentionally narrower live-preview dialect while leaving sent-message rendering unchanged. Drive debounced draft persistence from text snapshots, cancel superseded writes, and clear sent drafts synchronously. Cover candidate ordering and limits, supported inline styling, debounce replacement, and immediate draft clearing in behavior-focused tests.
Build mention replacements and inline-style spans from the immutable source buffer before applying presentation edits. Remap style boundaries through length-changing friendly-name substitutions so replacing @!<hex> tokens cannot shift or discard surrounding markdown. Keep presentation-only transformations from changing the wire text, and prevent display names or delimiters inside code spans from introducing unintended bold, italic, or strikethrough styling. Use deterministic span ordering and lint-clean boundary traversal for overlapping syntax cases. Cover mentions nested within markdown, markdown contained by mention substitutions, delimiter-bearing display names, code-span isolation, and replacement length changes. Keep candidate terminology aligned throughout the transformation pipeline.
ec68328 to
830e096
Compare
|
@jamesarich I’ve addressed the earlier review feedback and completed another on-device pass. Normal typing is noticeably improved. There is still some stutter when holding backspace, which I plan to investigate further as part of follow-up lifecycle work, but I don’t think that should block this PR in its current scope. During final testing I also encountered BLE reconnect and scan-lifecycle failures. Those issues are unrelated to the composer changes and are addressed separately in #6384, so there is no code or merge-order dependency between the two PRs. |
jamesarich
left a comment
There was a problem hiding this comment.
Appreciate it - thank you!
Overview
This reduces message-composer recomposition and per-keystroke work, particularly on active meshes where telemetry changes frequently and inline Markdown is enabled.
The composer previously received full
Nodeobjects, so updates to telemetry, position, metrics, last-heard time, or other unrelated fields could invalidate the input subtree even when mention-visible identity data had not changed. The live editor also performed more parsing, matching, and draft-persistence work than necessary during rapid typing.This keeps stored and transmitted message text unchanged while narrowing reactive inputs and bounding mention matching, live styling, and draft persistence.
Changes
Stable composer inputs
MessageInputas a persistentImmutableMap, giving Compose an explicit stability contract.Bounded typing work
SavedStateHandleby 300 ms, cancel superseded writes, and keep send and explicit clear operations immediate.Presentation-only mentions and Markdown
Review guide
The commits are intended to be reviewed independently and in order:
fix(messaging): stabilize composer lifecycle and hot pathsfix(messaging): preserve markdown styling through mentionsRegression coverage
Coverage includes:
On-device validation
On-device testing was performed while connected to an active mesh with ongoing telemetry updates.
Normal typing is noticeably more responsive, and the earlier focus, keyboard, selection, mention-suggestion, and live-formatting instability was not reproduced during ordinary input.
Holding backspace can still produce some stutter. That appears to be a narrower sustained-edit performance issue and will be investigated separately rather than expanding this PR beyond its current scope.
Scope
This is limited to the message-composer hot path. It does not change transport, BLE scanning or reconnect behavior, database coordination, node identity, or application lifecycle behavior.
Summary by CodeRabbit
New Features
Bug Fixes