fix(chat): budget real wire bytes and pace meshtastic multi-part sends - #770
Conversation
Reserve the reply_id field (5 fixed32 bytes) and a byte-accurate reply prefix estimate so near-limit Meshtastic and MeshCore replies split into multiple parts instead of overflowing into a TOO_LARGE NAK from the radio. Count real UTF-8 wire bytes instead of Unicode codepoints when chunking, since non-ASCII text (Cyrillic, CJK, emoji) could previously pass the composer's codepoint-based check while exceeding the true wire byte limit. Pace successive Meshtastic text sends — both the live composer loop and the outbox drain loop — by 2.5s to stay clear of firmware's RATE_LIMIT_EXCEEDED threshold on TEXT_MESSAGE_APP, which rejects a second locally-originated text packet sent within 2s of the last.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughChangesMeshtastic messaging behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ChatComposer
participant useChatOutbox
participant MeshtasticSend
ChatComposer->>MeshtasticSend: send first chunk immediately
ChatComposer->>ChatComposer: wait 2500 ms before next chunk
ChatComposer->>MeshtasticSend: send next chunk
useChatOutbox->>useChatOutbox: wait until interval since previous send
useChatOutbox->>MeshtasticSend: dispatch queued row
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/ChatComposer.tsx (1)
1-1: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftShare the Meshtastic send throttle between the composer and the outbox.
ChatComposerpaces multi-chunk sends with a local timeout but does not read or update the timestamp thatuseChatOutboxuses for queued drains. A live Meshtastic send can emit its first chunk, then the outbox can drain another queue entry within the same 2.5-second window before that timestamp is recorded. Move the last send timestamp into shared state so every Meshtastic send path consults and updates it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/ChatComposer.tsx` at line 1, Move the Meshtastic last-send timestamp out of ChatComposer’s local timeout/ref state into shared state used by both ChatComposer and useChatOutbox. Update the send logic in ChatComposer and the queued-drain logic in useChatOutbox to consult the shared timestamp before sending and record the timestamp when a send begins, preserving the 2.5-second throttle across all Meshtastic send paths.
🧹 Nitpick comments (1)
src/renderer/hooks/useChatOutbox.test.ts (1)
192-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider fake timers for this pacing test.
This test waits close to
MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MSin real wall-clock time (viasetTimeoutand awaitFortimeout of the interval plus buffer). This adds roughly 2 seconds of real time to the suite and is more sensitive to a slow CI runner than a deterministic fake-timer approach.ChatComposer.test.tsxalready covers the equivalent scenario withvi.useFakeTimers()andvi.advanceTimersByTimeAsync(). Apply the same pattern here for a faster, more deterministic test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/useChatOutbox.test.ts` around lines 192 - 218, Update the pacing test around useChatOutbox to use Vitest fake timers, following the established pattern in ChatComposer.test.tsx. Advance timers by less than MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS to verify only the first send occurred, then advance past the interval and await timer-driven processing to verify the second send, removing the real-time setTimeout and extended waitFor timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/ChatComposer.tsx`:
- Around line 464-470: Consolidate Meshtastic send throttling across
ChatComposer’s handleSend flow and useChatOutbox’s queued-drain flow by removing
their separate delay/state mechanisms and routing both through one shared
last-sent timestamp and synchronization boundary. Ensure every Meshtastic send,
including consecutive chunks and independently queued sends, waits until
MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS has elapsed since the previous send.
In `@src/renderer/hooks/useChatOutbox.ts`:
- Around line 198-208: The Meshtastic pacing state in useChatOutbox is local to
one hook and does not coordinate with ChatComposer sends. Move
lastMeshtasticSendAtRef and the associated interval enforcement into a shared
Meshtastic send path used by both ChatComposer.handleSend and the useChatOutbox
drain flow, ensuring every device.sendText invocation is spaced by
MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS across both paths.
In `@src/renderer/lib/timeConstants.ts`:
- Around line 287-296: Update MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS to derive
its 2.5-second duration from MS_PER_SECOND instead of using the raw 2_500
millisecond literal, preserving the existing 2.5-second interval.
---
Outside diff comments:
In `@src/renderer/components/ChatComposer.tsx`:
- Line 1: Move the Meshtastic last-send timestamp out of ChatComposer’s local
timeout/ref state into shared state used by both ChatComposer and useChatOutbox.
Update the send logic in ChatComposer and the queued-drain logic in
useChatOutbox to consult the shared timestamp before sending and record the
timestamp when a send begins, preserving the 2.5-second throttle across all
Meshtastic send paths.
---
Nitpick comments:
In `@src/renderer/hooks/useChatOutbox.test.ts`:
- Around line 192-218: Update the pacing test around useChatOutbox to use Vitest
fake timers, following the established pattern in ChatComposer.test.tsx. Advance
timers by less than MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS to verify only the
first send occurred, then advance past the interval and await timer-driven
processing to verify the second send, removing the real-time setTimeout and
extended waitFor timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: a2dd7731-8361-47b9-a828-9523c6900af3
📒 Files selected for processing (7)
src/renderer/components/ChatComposer.test.tsxsrc/renderer/components/ChatComposer.tsxsrc/renderer/hooks/useChatOutbox.test.tssrc/renderer/hooks/useChatOutbox.tssrc/renderer/lib/chatComposerLimits.test.tssrc/renderer/lib/chatComposerLimits.tssrc/renderer/lib/timeConstants.ts
| if (i > 0 && protocol === 'meshtastic') { | ||
| // Firmware rate-limits locally-originated TEXT_MESSAGE_APP packets to one per 2s | ||
| // (RATE_LIMIT_EXCEEDED) — chunks fired back-to-back would trip this on chunk 2+. | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate where onSendChunk / sendFn for meshtastic are wired up, and check for any
# shared rate-limiting or serialization at the IPC/main-process boundary.
rg -n -C5 'onSendChunk=|sendFn=' -g '*.tsx' -g '*.ts' src/renderer | head -100
rg -n -i 'rate.?limit|throttle|lastSend|RATE_LIMIT_EXCEEDED' -C3 src/mainRepository: Colorado-Mesh/mesh-client
Length of output: 38250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChatComposer outline =="
ast-grep outline src/renderer/components/ChatComposer.tsx --view compact || true
echo "== Chat Composer relevant handlers =="
sed -n '420,490p' src/renderer/components/ChatComposer.tsx
echo "== sendFn usages =="
rg -n -C4 'sendFn|onSendChunk=|handleSendChunk|sendChunk|useChatOutbox' src/renderer -g '*.tsx' -g '*.ts' | head -220
echo "== ChatOutbox relevant sections =="
fd -a 'useChatOutbox' src/renderer | while read -r f; do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact || true
done
for f in $(fd -a 'useChatOutbox' src/renderer); do
echo "--- snippets $f"
rg -n -C6 'lastMeshtasticSendAtRef|sendFn|RATE_LIMIT|meshtastic|TEXT_MESSAGE|send' "$f" || true
doneRepository: Colorado-Mesh/mesh-client
Length of output: 38295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useChatOutbox full relevant implementation =="
sed -n '120,220p' src/renderer/hooks/useChatOutbox.ts
echo "== ChatPanel composition of onSendChunk and useChatOutbox =="
rg -n -C8 'useChatOutbox|handleSendChunk|handleSend|onSendChunk=' src/renderer/components/ChatPanel.tsx src/renderer/lib src/renderer/hooks -g '*.tsx' -g '*.ts' | head -260
echo "== ChatPanel handleSendChunk definitions =="
sed -n '2950,3075p' src/renderer/components/ChatPanel.tsx 2>/dev/null || true
echo "== constants =="
rg -n -C3 'MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS|RATE_LIMIT_EXCEEDED|TEXT_MESSAGE_APP' src -g '*.ts' -g '*.tsx'Repository: Colorado-Mesh/mesh-client
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== handleSendChunk exact implementation =="
sed -n '1431,1482p' src/renderer/components/ChatPanel.tsx
echo "== outboxSendFn exact implementation =="
sed -n '1040,1072p' src/renderer/components/ChatPanel.tsx
echo "== MeshtasticProtocol send implementation =="
sed -n '230,270p' src/renderer/lib/protocols/MeshtasticProtocol.ts
rg -n -C5 'sendText|sendMessage|TEXT_MESSAGE_APP|request.*send|send.*text' src/renderer/lib/protocols/MeshtasticProtocol.ts
echo "== cross-file timing/state probes =="
python3 - <<'PY'
from pathlib import Path
composer = Path('src/renderer/components/ChatComposer.tsx').read_text()
panel = Path('src/renderer/components/ChatPanel.tsx').read_text()
outbox = Path('src/renderer/hooks/useChatOutbox.ts').read_text()
checks = {
'ChatComposer paces i>0 meshtastic chunks': "if (i > 0 && protocol === 'meshtastic') {" in composer and "MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS" in composer,
'ChatComposer calls onSendChunk immediately after local waiting': "await onSendChunk(textsToSend[i]" in composer,
'handleSendChunk calls meshtastic send without gap state': "if (protocol === 'meshtastic') return sendMeshtasticTextMessage" in panel,
'useChatOutbox has per-hook meshtastic timestamp': 'lastMeshtasticSendAtRef' in outbox,
'useChatOutbox waits on timestamp before sending each row': "MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - (Date.now() - lastMeshtasticSendAtRef.current)" in outbox,
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: Colorado-Mesh/mesh-client
Length of output: 7442
Eliminate the duplicate Meshtastic send timer across ChatComposer and useChatOutbox.
ChatComposer.tsx pauses only between chunks in the same handleSend, while useChatOutbox.ts keeps a separate lastMeshtasticSendAtRef for queued drains. Two live Meshtastic sends can still occur less than MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS apart. Put the 2.5s last-sent state/timer behind one shared synchronization boundary used by both paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/ChatComposer.tsx` around lines 464 - 470, Consolidate
Meshtastic send throttling across ChatComposer’s handleSend flow and
useChatOutbox’s queued-drain flow by removing their separate delay/state
mechanisms and routing both through one shared last-sent timestamp and
synchronization boundary. Ensure every Meshtastic send, including consecutive
chunks and independently queued sends, waits until
MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS has elapsed since the previous send.
| if (protocol === 'meshtastic') { | ||
| // Firmware rejects a second TEXT_MESSAGE_APP within 2s of the last one | ||
| // (Routing_Error.RATE_LIMIT_EXCEEDED) — pace queued Meshtastic rows the same | ||
| // way ChatComposer paces multi-chunk sends, across drainOnce() calls too. | ||
| const wait = | ||
| MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS - (Date.now() - lastMeshtasticSendAtRef.current); | ||
| if (wait > 0) { | ||
| await new Promise((resolve) => setTimeout(resolve, wait)); | ||
| } | ||
| lastMeshtasticSendAtRef.current = Date.now(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'useChatOutbox\.ts|ChatComposer\.tsx' . | sed 's#^\./##'
echo
echo "== useChatOutbox outline =="
ast-grep outline src/renderer/hooks/useChatOutbox.ts 2>/dev/null || true
echo
echo "== useChatOutbox relevant lines =="
cat -n src/renderer/hooks/useChatOutbox.ts | sed -n '1,280p'
echo
echo "== ChatComposer relevant lines around 430-490 =="
cat -n src/renderer/components/ChatComposer.tsx | sed -n '430,495p'
echo
echo "== search MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS and lastMeshtasticSendAtRef =="
rg -n "MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS|lastMeshtasticSendAtRef|handleSend|drainOnce" srcRepository: Colorado-Mesh/mesh-client
Length of output: 20949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChatPanel handleSendChunk =="
cat -n src/renderer/components/ChatPanel.tsx | sed -n '1400,1510p'
echo
echo "== RrcPanel handleSend and send chunk implementation =="
cat -n src/renderer/components/RrcPanel.tsx | sed -n '530,610p'
rg -n "sendOneMessage|sendTextMessage|TEXT_MESSAGE|meshtastic|message send|electronAPI" src/renderer/components/RrcPanel.tsx src/renderer -g '*.ts' -g '*.tsx' | head -n 200
echo
echo "== electron API chat implementations =="
rg -n "chat\.|ChatOutbox|outbox\.add|sendTextMessage|TEXT_MESSAGE_APP|schedule|setTimeout|interval|rate" src/main src/shared -g '*.ts' -g '*.tsx' | head -n 240Repository: Colorado-Mesh/mesh-client
Length of output: 49022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate App meshtastic send handlers =="
cat -n src/renderer/App.tsx | sed -n '1018,1095p'
cat -n src/renderer/App.tsx | sed -n '1316,1352p'
echo
echo "== locate meshtastic runtime/actions files =="
fd -i 'meshtastic|protocol' src/renderer src/main src/shared | sed -n '1,160p'
echo
echo "== search for Meshtastic send implementation identifiers =="
rg -n "class|function|const .*meshtastic|sendTextMessage|send\\(|TEXT_MESSAGE|PortNum.TEXT_MESSAGE|ToRadio|meshtasticPanelActions\\.send|sendChunk|sendText" src/renderer src/main src/shared -g '*.ts' -g '*.tsx' | head -n 260Repository: Colorado-Mesh/mesh-client
Length of output: 40335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useMeshtasticPanelActions outline and relevant lines =="
ast-grep outline src/renderer/hooks/useMeshtasticPanelActions.ts 2>/dev/null || true
cat -n src/renderer/hooks/useMeshtasticPanelActions.ts | sed -n '1,260p'
echo
echo "== protocol registry meshtastic send references =="
rg -n "send\\(|sendTextMessage|TEXT_MESSAGE|MeshtasticProtocol|meshtasticPanelActions\\.send" src/renderer lib -g '*.ts' -g '*.tsx' | head -n 240
echo
echo "== App handleSend callers =="
rg -n "onSendChunk=|onSend=|handleSendChunk|handleSend\\(" src/renderer/components src/renderer/App.tsx | sed -n '1,260p'Repository: Colorado-Mesh/mesh-client
Length of output: 18070
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== MeshtasticProtocol sendMessage sections =="
cat -n src/renderer/lib/protocols/MeshtasticProtocol.ts | sed -n '260,320p'
cat -n src/renderer/lib/protocols/MeshtasticProtocol.ts | sed -n '488,518p'
cat -n src/renderer/lib/protocols/MeshtasticProtocol.ts | sed -n '756,810p'
echo
echo "== App protocol message facade around handleSend =="
rg -n "const sendMessage|function sendMessage|sendMessage\\(" src/renderer/App.tsx src/renderer -g '*.ts' -g '*.tsx' | head -n 80
cat -n src/renderer/App.tsx | sed -n '1840,1905p'
echo
echo "== tests about outbox + composer MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS =="
cat -n src/renderer/hooks/useChatOutbox.test.ts | sed -n '160,220p'
cat -n src/renderer/components/ChatComposer.test.tsx | sed -n '320,380p'Repository: Colorado-Mesh/mesh-client
Length of output: 20942
Use shared Meshtastic pacing for composer sends and outbox rows.
lastMeshtasticSendAtRef in useChatOutbox only covers queued rows drained by that hook instance. ChatComposer.handleSend still checks chunks only inside its own loop, and Meshtastic sends are invoked through MeshtasticProtocol.sendMessage → device.sendText. If a live composer send and an outbox drain run close together on the same Meshtastic radio, the 2-second TEXT_MESSAGE_APP rate limit can still be violated. Move the last-send timestamp/state to a shared path used by both the Composer send path and useChatOutbox.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/hooks/useChatOutbox.ts` around lines 198 - 208, The Meshtastic
pacing state in useChatOutbox is local to one hook and does not coordinate with
ChatComposer sends. Move lastMeshtasticSendAtRef and the associated interval
enforcement into a shared Meshtastic send path used by both
ChatComposer.handleSend and the useChatOutbox drain flow, ensuring every
device.sendText invocation is spaced by MESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS
across both paths.
Stamp the TEXT_MESSAGE_APP send slot after each attempt settles and share one clock between the composer and outbox so slow writes or overlapping drains cannot shrink under firmware's RATE_LIMIT_EXCEEDED window. Charge UTF-8 wire bytes for MeshCore channel display names against the 160-byte payload max.
Also switch outbox pacing tests to Vitest fake timers so they no longer wait on real wall-clock delays.
2 participants
Summary
Data.reply_idfield (fixed32, always 5 wire bytes) and a byte-accurate MeshCore reply-prefix estimate in the chat composer's payload budget, so a near-limit reply splits into multiple parts instead of overflowing into aTOO_LARGENAK from the radio.countMessageWireBytes), since both Meshtastic and MeshCore encode text withTextEncoderbefore transmission — non-ASCII text (Cyrillic, CJK, emoji) near the limit could previously pass the composer's codepoint-based check while exceeding the true wire byte limit.ChatComposersend loop and theuseChatOutboxdrain loop — byMESHTASTIC_TEXT_CHUNK_SEND_INTERVAL_MS(2.5s) to stay clear of firmware'sRATE_LIMIT_EXCEEDEDrejection, confirmed againstPhoneAPI.cppinmeshtastic/firmware: locally-originatedTEXT_MESSAGE_APPpackets are rejected if sent less than 2s apart.Test plan
chatComposerLimits.test.ts— new/updated coverage for reply overhead accounting, MeshCore "Unknown" sanitize fallback, and UTF-8 byte-budget chunking (Cyrillic regression case)ChatComposer.test.tsx— new tests confirming multi-chunk Meshtastic sends are paced and single-chunk sends are not delayeduseChatOutbox.test.ts— new test confirming queued rows drain with pacing; existingretrytest updated to avoid a dangling real timer once pacing was addedRATE_LIMIT_EXCEEDEDpnpm run typecheck,pnpm exec eslinton all changed files — cleanvitest run --changed(225 files / 2458 tests) — all passedSummary by CodeRabbit
Bug Fixes
Tests