fix(reticulum): stop announce storms from dropping inbound LXMF - #739
Conversation
Coalesce announce.received to ≤1 frame per window at 100k scale, merge-safe connect DB refresh, and add watermarked periodic catch-up plus diagnostics.
Expose sidecar announce_ws coalesce counters and emit reticulum/announce-bus-pressure so large-mesh users see why Chat may lag while catch-up runs.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe sidecar coalesces announce WebSocket events, exposes pressure and LXMF ring metrics, and supports detailed inbound catch-up. The renderer tracks catch-up diagnostics, merges recovered messages, detects announce-bus pressure, and parses batched announce payloads. ChangesAnnounce WebSocket coalescing
Inbound LXMF recovery
Announce-bus diagnostics
Batched announce consumers
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReticulumSidecar
participant RendererRuntime
participant MessageStore
participant Diagnostics
ReticulumSidecar->>RendererRuntime: coalesced announce frames and LXMF ring diagnostics
RendererRuntime->>MessageStore: ingest or merge inbound messages
RendererRuntime->>Diagnostics: record lag, catch-up, watermark, and pressure metrics
Diagnostics-->>RendererRuntime: announce-bus-pressure warning when thresholds are recent
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Show four practical tips under the warning and an Open Interfaces action so large-mesh users know what they can try while catch-up runs.
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
src/renderer/lib/reticulum/fetchRecentInboundLxmf.ts-57-60 (1)
57-60: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUnrate-limited warning on every proxy failure.
This path is invoked every 15–60s by the periodic inbound LXMF catch-up (
useReticulumRuntime.ts). If the sidecar is unreachable for an extended period, this will emit aconsole.warnon every poll cycle, producing sustained log spam that can bury more actionable diagnostics.As per path instructions (AGENTS.md), "Use debug/warn/error rather than bare console.log, rate-limit noisy warnings where appropriate."
🤖 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/lib/reticulum/fetchRecentInboundLxmf.ts` around lines 57 - 60, Rate-limit the proxy-failure warning in the catch block of fetchRecentInboundLxmf so repeated polling failures do not emit a warning every cycle. Reuse the project’s established rate-limited logging mechanism, while preserving the existing errLikeToLogString(e) details and { messages: [], ringLen: null } fallback.Source: Path instructions
🤖 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 `@reticulum-sidecar/src/stack/announce_ws_coalesce.rs`:
- Around line 177-285: Serialize the four tests that read pressure
metrics—last_write_wins_per_destination,
flush_prefers_named_when_over_cap_and_records_overflow,
storm_widens_coalesce_duration_and_stamps_storm_time, and
many_distinct_dests_still_one_flush_batch—using a shared Mutex<()> acquired
before constructing each AnnounceWsCoalescer. Reuse one test-module guard and
retain the existing assertions and test behavior.
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 612-641: Extract the fetch, warning, row-ingestion, and watermark
calculation from catchUpRecentInboundLxmf into a pure helper under
src/renderer/lib/reticulum/. Keep identityId and the ingest callback as helper
inputs, return the resulting catch-up state or required watermark to the hook,
and leave useReticulumRuntime responsible only for wiring dependencies and
applying runtime-specific callbacks such as noteReticulumInboundCatchUp and
advanceReticulumInboundCatchUpWatermark.
---
Other comments:
In `@src/renderer/lib/reticulum/fetchRecentInboundLxmf.ts`:
- Around line 57-60: Rate-limit the proxy-failure warning in the catch block of
fetchRecentInboundLxmf so repeated polling failures do not emit a warning every
cycle. Reuse the project’s established rate-limited logging mechanism, while
preserving the existing errLikeToLogString(e) details and { messages: [],
ringLen: null } fallback.
🪄 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: 5b9ece5f-cc6c-4147-ac88-d3ead2ef2cc9
⛔ Files ignored due to path filters (16)
src/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/en/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (23)
docs/diagnostics.mddocs/reticulum-sidecar-ipc.mdreticulum-sidecar/src/api/lxmf.rsreticulum-sidecar/src/stack/announce_ws_coalesce.rsreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/lxmf_delivery.rsreticulum-sidecar/src/stack/lxmf_inbound_log.rsreticulum-sidecar/src/stack/mod.rssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.test.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/reticulum/catchUpInboundLxmf.test.tssrc/renderer/lib/reticulum/fetchRecentInboundLxmf.test.tssrc/renderer/lib/reticulum/fetchRecentInboundLxmf.tssrc/renderer/lib/reticulum/reticulumDiagnosticSnapshot.tssrc/renderer/lib/reticulum/reticulumInboundLxmfDiagnostics.test.tssrc/renderer/lib/reticulum/reticulumInboundLxmfDiagnostics.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/stores/messageStore.test.tssrc/renderer/stores/messageStore.tssrc/renderer/stores/reticulumIdentityActivityStore.test.tssrc/renderer/stores/reticulumIdentityActivityStore.tssrc/renderer/stores/reticulumPeerStore.test.tssrc/renderer/stores/reticulumPeerStore.ts
…MF catch-up Avoid flaky parallel reads of process-global announce_ws atomics, and move catch-up fetch/ingest/watermark into a lib helper so the runtime only wires diagnostics callbacks.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts (1)
34-51: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd runtime validation for sidecar pressure metrics.
/api/v1/diagnosticsis cast directly fromunknown; validateannounce_wsfields as finite, non-negative integers before use. The predicate rejects negative/future timestamps, but overflow lacks type/finite checks and accepts coercible values such as'1'orInfinity.🤖 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/lib/diagnostics/ReticulumDiagnosticEngine.ts` around lines 34 - 51, In the diagnostics parsing/validation flow for ReticulumAnnounceWsDiagnostics, validate every announce_ws metric before use as a finite, non-negative integer, rejecting string-coercible values and Infinity. Apply the same constraints to ingress, unique, overflow, and timestamps, additionally rejecting future timestamp values; do not rely on the direct unknown cast or allow overflow to bypass type and finite checks.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/renderer/lib/diagnostics/ReticulumDiagnosticEngine.ts`:
- Around line 34-51: In the diagnostics parsing/validation flow for
ReticulumAnnounceWsDiagnostics, validate every announce_ws metric before use as
a finite, non-negative integer, rejecting string-coercible values and Infinity.
Apply the same constraints to ingress, unique, overflow, and timestamps,
additionally rejecting future timestamp values; do not rely on the direct
unknown cast or allow overflow to bypass type and finite checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 7b0ed678-458a-4616-b829-a1710b5d06c3
⛔ Files ignored due to path filters (16)
src/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/en/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (6)
docs/diagnostics.mdsrc/renderer/components/ReticulumDiagnosticsSection.test.tsxsrc/renderer/components/ReticulumDiagnosticsSection.tsxsrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.test.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/types.ts
Summary
announce.receivedWS frames (time-based, HashMap by dest, 1024 cap) so announce/path-response storms no longer starve inbound LXMF on large meshes; merge-safe connect refresh + periodic/lxmf/recentcatch-up.announce_wspressure counters onGET /api/v1/diagnosticsand emit Diagnostics warningreticulum/announce-bus-pressurewhen recent WS lag, storm flush, or coalesce overflow indicates Chat may feel incomplete while catch-up runs.Test plan
ReticulumDiagnosticEngineannounce-bus-pressure cases (lag / storm / neither)announce_ws_coalesceoverflow + storm stamp testsreticulum/announce-bus-pressureafter lag/storm and clears after ~5 minutes idleGET /api/v1/diagnosticsincludesannounce_wsfields after an announce flushSummary by CodeRabbit
New Features
Bug Fixes
Documentation