Conversation
Prefer bulk getWaitingMessages for silent event-131 drains (BLE/serial/TCP) with header X/Y progress, falling back to syncNextMessage on timeout without disconnecting. Erase incorrect SoftAP jargon in favor of OpenHop naming.
Align telemetry skip wording, use silent-timeout constants in tests, drop contradictory catch-no-log-ok, cover stale bulk-attempt abandon, and remove redundant OpenHop dead-bridge check in stats fetch.
) Direct LXMF timeouts to third-party clients often failed without propagation fallback, and inbound catch-up was starved by the shared 300/min proxy ceiling. Cascade preferred → other remotes → local-prop (PN 🏠 badge), raise/split proxy budgets with backoff, and improve outbound logging in developer bundles.
Address review findings: per-message cascade PN targets, advance on pack/max-attempt failure, safer link-timeout bridge hydration, catch-up single-flight, per-bucket rate-limit backoff, locale/doc accuracy for multi-PN + stored_locally.
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (14)
📝 WalkthroughWalkthroughReticulum LXMF delivery now uses a multi-node propagation cascade with local storage fallback. The change adds distinct delivery statuses, attempt tracking, guarded timeout handling, proxy rate-limit backoff, catch-up coalescing, diagnostics, and updated documentation. ChangesLXMF propagation cascade
Estimated code review effort: 4 (Complex) | ~60 minutes 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)
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: 6
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (3)
src/renderer/lib/reticulum/reticulumProxyRateLimitBackoff.ts-72-74 (1)
72-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClamp the final jittered delay to the configured range.
Line 73 applies jitter after
baseis capped. A low jitter value produces 4,500 ms on the first hit. A high jitter value produces 66,000 ms at the capped tier. This violates the stated 5,000–60,000 ms backoff range. Clamp the jittered value and add lower- and upper-bound tests.Proposed fix
- const delay = applyJitter(base); + const delay = Math.min(MAX_BACKOFF_MS, Math.max(DEFAULT_BACKOFF_MS, applyJitter(base)));🤖 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/reticulumProxyRateLimitBackoff.ts` around lines 72 - 74, Update the backoff calculation around applyJitter so the final jittered delay is clamped between DEFAULT_BACKOFF_MS and MAX_BACKOFF_MS before assigning state.backoffUntilMs. Add tests covering jitter below the minimum and above the maximum, while preserving the existing exponential backoff behavior.src/main/support-bundle.ts-188-195 (1)
188-195: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
propagation-retrievediagnostics.The diagnostics contract in
docs/reticulum.mdLine [275] names bothpropagation-depositandpropagation-retrievetargets. This filter keeps onlypropagation-deposit. Retrieval and catch-up failures will be absent fromreticulum/lxmf-outbound.log. Add the missing pattern and a behavioral assertion insrc/main/support-bundle.test.ts.Proposed fix
const patterns = [ /lxmf-outbound/i, /propagation-deposit/i, + /propagation-retrieve/i, /LXMF advancing PN cascade/i,+ 'info target=propagation-retrieve PN retrieve', ... + expect(slice).toContain('propagation-retrieve');As per path instructions, support bundles may include only filtered, redacted, truncated LXMF/PN cascade logs.
🤖 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/main/support-bundle.ts` around lines 188 - 195, Update the diagnostics filter pattern list in the support-bundle construction flow to include case-insensitive matching for propagation-retrieve alongside propagation-deposit, and add a behavioral assertion in the support-bundle tests confirming retrieval diagnostics are retained. Preserve the existing filtering, redaction, and truncation behavior for LXMF and PN cascade logs.Source: Path instructions
src/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.ts-161-162 (1)
161-162: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve
deliveryAttemptson every status path.The failed-to-sending revival at Lines 169-193 returns before Lines 235-242 patch
reticulumDeliveryAttempts. Asendingcascade event can therefore clear the failure state but lose its attempt count in both the store and SQLite record.The pending-before-rekey path also drops this field because
bufferPendingDeliveryStatus()andflushPendingReticulumOutboundDeliveryStatus()do not carrydeliveryAttempts.Add the clamped value to the revival record and to the pending-status payload. Add regression tests for both paths.
As per coding guidelines,
**/*.{ts,tsx}requires a passing test for behavioral changes before considering the task complete.Also applies to: 304-315
🤖 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/applyReticulumOutboundDeliveryStatus.ts` around lines 161 - 162, Preserve the clamped deliveryAttempts value throughout applyReticulumOutboundDeliveryStatus: include it when creating the failed-to-sending revival record and pass it through bufferPendingDeliveryStatus and flushPendingReticulumOutboundDeliveryStatus for pending-before-rekey updates, so both store and SQLite persistence retain it. Add regression tests covering the revival and pending-status paths.Source: Coding guidelines
🧹 Nitpick comments (1)
src/renderer/lib/reticulum/reticulumPropagationEffective.test.ts (1)
71-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a negative case for a disabled
local-proprow.The suite covers "local-prop enabled → capacity true" and "empty list → capacity false". It does not cover a present but disabled
local-proprow with no remote target. That case controlsshouldApplyLinkDeliveryTimeoutFailureBridge: if the predicate wrongly returns true there, the timeout bridge stays suppressed and outbound messages never reach a failed state in the UI.💚 Proposed test
it('is false when nothing is available', () => { expect(hasReticulumPnCascadeCapacity([], null, 'off')).toBe(false); }); + + it('is false when local-prop is present but disabled and no remote exists', () => { + const localDisabled: PropagationNodeRow = { + id: 'local-prop', + name: 'Local', + enabled: false, + status: 'idle', + }; + expect(hasEnabledLocalPropagation([localDisabled])).toBe(false); + expect(hasReticulumPnCascadeCapacity([localDisabled], null, 'auto')).toBe(false); + });🤖 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/reticulumPropagationEffective.test.ts` around lines 71 - 88, Add a negative test in the hasReticulumPnCascadeCapacity suite using a present but disabled local-prop row, with no remote target and propagation mode off, and assert the predicate returns false. Keep the existing enabled local-prop and empty-list cases unchanged.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/lxmf_outbound.rs`:
- Around line 2086-2122: Add a behavioral test for the driver method
try_advance_pn_cascade, following
requeue_direct_after_path_failover_exhausts_then_clears_state as the setup and
invocation template. Configure preferred remote, next remote, and local
candidates, then verify successive Direct-failure advances update
pending_pn_targets in that order, set pn_cascade_local for the local step, and
return Err after exhaustion. Retain the existing source-contract test as an
additional guard if desired, without changing its asserted log strings.
- Around line 845-868: Update the victim-eviction branch in mark_pn_tried to
also remove the evicted message hash from pn_deposit_defer_counts, alongside
pn_cascade_tried, pn_cascade_local, and pending_pn_targets.
In `@reticulum-sidecar/src/stack/pn_cascade.rs`:
- Around line 116-123: In the local-prop branch of the cascade candidate
filtering, remove the redundant enabled_local computation and second guard. Use
the row’s enabled flag as the single source of truth for eligibility, so a
local-prop row is skipped whenever *enabled is false regardless of
local_prop_enabled; preserve candidate inclusion only when the row is enabled.
- Around line 124-128: The local cascade path must use the lxmf.propagation
destination rather than the local self-hash identity. In
reticulum-sidecar/src/stack/pn_cascade.rs:124-128, remove the self_norm fallback
and propagate the actual destination, or bypass propagation-link handling for
PnCascadePick::Local and store directly in the local inbox. Update the
corresponding local delivery handling in
reticulum-sidecar/src/stack/lxmf_outbound.rs:946-966 so deliver_propagated does
not perform identity/path checks against an incorrect Nomad target.
In `@src/renderer/lib/reticulum/catchUpRecentInboundLxmf.ts`:
- Around line 22-81: Scope the single-flight state by identityId instead of
using the global catchUpInFlight, catchUpInFlightOpts, and catchUpPending
variables. Update the catch-up orchestration to maintain independent in-flight,
options, and pending entries per identity so concurrent calls for different
identities never share promises or cursor/watermark outcomes, while preserving
coalescing for calls with the same identityId. Add coverage for overlapping
calls across two identities.
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 1501-1562: Introduce a bridge-generation ref for the link-timeout
async flow around the bridge IIFE, capture its value before starting, and abort
the continuation after refreshFromSidecar() when the captured generation is
stale. Increment the generation on identity changes, tearDownFromSidecarStop(),
and disconnect(), alongside the existing propagationHydratedForBridgeRef resets,
and ensure the loop cannot call failReticulumSendingOutboundToDestHash after
invalidation. Add a delayed-hydration test covering sidecar stop or disconnect
before hydration completes.
---
Other comments:
In `@src/main/support-bundle.ts`:
- Around line 188-195: Update the diagnostics filter pattern list in the
support-bundle construction flow to include case-insensitive matching for
propagation-retrieve alongside propagation-deposit, and add a behavioral
assertion in the support-bundle tests confirming retrieval diagnostics are
retained. Preserve the existing filtering, redaction, and truncation behavior
for LXMF and PN cascade logs.
In `@src/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.ts`:
- Around line 161-162: Preserve the clamped deliveryAttempts value throughout
applyReticulumOutboundDeliveryStatus: include it when creating the
failed-to-sending revival record and pass it through bufferPendingDeliveryStatus
and flushPendingReticulumOutboundDeliveryStatus for pending-before-rekey
updates, so both store and SQLite persistence retain it. Add regression tests
covering the revival and pending-status paths.
In `@src/renderer/lib/reticulum/reticulumProxyRateLimitBackoff.ts`:
- Around line 72-74: Update the backoff calculation around applyJitter so the
final jittered delay is clamped between DEFAULT_BACKOFF_MS and MAX_BACKOFF_MS
before assigning state.backoffUntilMs. Add tests covering jitter below the
minimum and above the maximum, while preserving the existing exponential backoff
behavior.
---
Nitpick comments:
In `@src/renderer/lib/reticulum/reticulumPropagationEffective.test.ts`:
- Around line 71-88: Add a negative test in the hasReticulumPnCascadeCapacity
suite using a present but disabled local-prop row, with no remote target and
propagation mode off, and assert the predicate returns false. Keep the existing
enabled local-prop and empty-list cases unchanged.
🪄 Autofix
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: 2cfae4d6-04da-4a08-b881-4db0dcaf1086
⛔ 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 (42)
AGENTS.mdREADME.mddocs/reticulum-sidecar-ipc.mddocs/reticulum.mddocs/troubleshooting.mdreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/lxmf_outbound.rsreticulum-sidecar/src/stack/mod.rsreticulum-sidecar/src/stack/pn_cascade.rssrc/main/ipc/reticulum-handlers.tssrc/main/ipc/reticulum-proxy-rate-limit.contract.test.tssrc/main/ipc/reticulumLxmfRecentPath.tssrc/main/reticulum-proxy-path.tssrc/main/support-bundle.test.tssrc/main/support-bundle.tssrc/renderer/components/ReticulumMessageStatusBadge.test.tsxsrc/renderer/components/ReticulumMessageStatusBadge.tsxsrc/renderer/lib/ingest/reticulumIngest.tssrc/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.test.tssrc/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.tssrc/renderer/lib/reticulum/catchUpInboundLxmf.test.tssrc/renderer/lib/reticulum/catchUpRecentInboundLxmf.test.tssrc/renderer/lib/reticulum/catchUpRecentInboundLxmf.tssrc/renderer/lib/reticulum/fetchRecentInboundLxmf.test.tssrc/renderer/lib/reticulum/fetchRecentInboundLxmf.tssrc/renderer/lib/reticulum/reticulumOutboundFailureBridge.test.tssrc/renderer/lib/reticulum/reticulumOutboundFailureBridge.tssrc/renderer/lib/reticulum/reticulumPropagationEffective.test.tssrc/renderer/lib/reticulum/reticulumPropagationEffective.tssrc/renderer/lib/reticulum/reticulumPropagationSync.test.tssrc/renderer/lib/reticulum/reticulumPropagationSync.tssrc/renderer/lib/reticulum/reticulumProxyRateLimitBackoff.test.tssrc/renderer/lib/reticulum/reticulumProxyRateLimitBackoff.tssrc/renderer/runtime/useReticulumRuntime.reconnect-hardening.test.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/stores/messageStore.tssrc/renderer/stores/reticulumPeerStore.test.tssrc/renderer/stores/reticulumPeerStore.tssrc/shared/electron-api.types.tssrc/shared/reticulumApiPaths.tssrc/shared/reticulumDeliveryMethod.test.tssrc/shared/reticulumDeliveryMethod.ts
Verify-and-fix: cascade eviction/local-prop dest, try_advance behavioral coverage, per-identity catch-up single-flight, bridge generation abort, deliveryAttempts persistence, jitter clamp, retrieve log slice, and cascade-capacity negative test.
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
stored_locallyUI (PN 🏠) when the message lands only in the local inbox (#817).Test plan
propagatedwhen a remote PN acceptsstored_locally(house icon / amber PN badge), not a green delivered checkreticulum/lxmf-outbound.logpresent with truncated hex idspnpm run check:pr/ CI greenSummary by CodeRabbit
New Features
delivered) from local storage (stored_locally), with attempt counts shown and persisted.Bug Fixes