refactor(delivery): sealed DeliveryState + shared DeliveryRetryPolicy replace the stringly delivery contract - #1022
Open
torlando-tech wants to merge 3 commits into
Open
Conversation
… contract
The delivery status of an outbound LXMF message was an open set of magic
strings ("delivered", "failed", "retrying_propagated", ...) produced
independently by both backend flavors and hand-matched at 20+ consumer
sites across the app. A typo or a new state added to one backend silently
no-oped in the UI, and DeliveryStatusUpdate's own KDoc listed only 3 of
the 5 live values.
This is slice 1 of the delivery-state deepening (representation only, no
behavior change):
- New sealed interface DeliveryState in :rns-api (Pending, Sent,
Delivered, Propagated, RetryingViaPropagation, Failed) with the
Issue #257 terminal-success policy as a property and an
encode()/decode() codec pinning the legacy persistence strings.
- DeliveryStatusUpdate.status: String -> state: DeliveryState. The
parcelable crosses AIDL whole, so no marshalling changes.
- Both backend emit sites (PythonEventBridge, NativeMessageSender,
NativeRnsBackendImpl) emit typed states.
- All app consumers (MessagingViewModel guards, MessageUi, MessageMapper,
MessagingScreen, MessageDetailScreen, MessageCollector,
ServicePersistenceManager, TestController) switch to exhaustive whens;
isTerminalSuccessStatus in the ViewModel is replaced by
DeliveryState.isTerminalSuccess.
- Room keeps storing the legacy strings (no migration); decode() is
nullable for unrecognized legacy values, rendered as the historical
else-branch presentation.
- New DeliveryStateTest pins the codec; ParcelRoundTripTest covers every
state across the AIDL boundary.
- CONTEXT.md seeds the domain glossary with DeliveryState /
DeliveryMethod and the "propagated" method-vs-state trap.
Slice 2 (lifting the duplicated retry state machine into :rns-host behind
this type) follows separately.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZ3PvF8KzApTXFYTfabv1b
createDeliveredMessage passes through createSentMessage's String param, so it keeps the encoded string; MessageBubbleTest's helper constructed MessageUi directly with "received" — never a valid state, decodes to null — so it now passes null explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZ3PvF8KzApTXFYTfabv1b
…liveryRetryPolicy The Sideband retry decision (direct send failed -> rebuild as PROPAGATED and re-route via the propagation node) was implemented independently in event_bridge.py and NativeMessageSender, kept in sync only by comments. - Add DeliveryRetryPolicy to :rns-api — the one shared predicate, with a truth-table test pinning the semantics for both flavors at once - NativeMessageSender consults the policy; its lxmf-kt rebuild mechanism is unchanged - event_bridge.py loses the decision: _failed now only flattens the failure (opt-in flag, desired-method, node-configured) and parks the live LXMessage in an _outbound_messages registry; the new resubmit_as_propagated/discard_outbound functions are mechanism-only - PythonEventBridge.handleLxmfFailure applies the policy and drives the mechanism through an injected OutboundRetryMechanism hook wired in ChaquopyRnsBackend; the separate retrying-propagated event sink is gone - reset_reticulum_for_restart clears the parked-message registry - CONTEXT.md gains the DeliveryRetryPolicy term Verified: :rns-api + :rns-backend-kt + :rns-backend-py unit tests green, event_bridge.py py_compile clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZ3PvF8KzApTXFYTfabv1b
Contributor
Greptile SummaryThis PR replaces delivery-status strings with a typed delivery-state contract. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Backend delivery callback] --> B[DeliveryStatusUpdate with DeliveryState]
B --> C[MessagingViewModel]
C --> D[Persist state.encode legacy string]
D --> E[Room status column]
E --> F[MessageMapper decodes status]
F --> G[MessageUi DeliveryState?]
G --> H[Compose status rendering]
F --> I[Unknown legacy value]
I --> J[Neutral fallback UI]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Backend delivery callback] --> B[DeliveryStatusUpdate with DeliveryState]
B --> C[MessagingViewModel]
C --> D[Persist state.encode legacy string]
D --> E[Room status column]
E --> F[MessageMapper decodes status]
F --> G[MessageUi DeliveryState?]
G --> H[Compose status rendering]
F --> I[Unknown legacy value]
I --> J[Neutral fallback UI]
Reviews (1): Last reviewed commit: "refactor(delivery): share the try-propag..." | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
The delivery status of an outbound LXMF message was an open set of magic
strings ("delivered", "failed", "retrying_propagated", ...) produced
independently by both backend flavors and hand-matched at 20+ consumer
sites across the app — and the try-propagation-on-fail retry logic behind
those strings was implemented twice (event_bridge.py and
NativeMessageSender), kept in sync only by "kept in lockstep" comments.
A typo or a new state added to one backend silently no-oped in the UI.
This PR lands both slices of the delivery-state deepening.
Slice 1 — typed seam (representation only, no behavior change)
DeliveryStatein:rns-api(Pending, Sent,Delivered, Propagated, RetryingViaPropagation, Failed) with the
Issue Message delivery status logic issue. First it have a checkmark, but this checkmark changes to ! after some time passed #257 terminal-success policy as a property and an
encode()/decode()codec pinning the legacy persistence strings.DeliveryStatusUpdate.status: String→state: DeliveryState. Theparcelable crosses AIDL whole, so no marshalling changes.
NativeRnsBackendImpl) emit typed states.
MessagingScreen, MessageDetailScreen, MessageCollector,
ServicePersistenceManager, TestController) switch to exhaustive whens;
isTerminalSuccessStatusin the ViewModel is replaced byDeliveryState.isTerminalSuccess.decode()isnullable for unrecognized legacy values, rendered as the historical
else-branch presentation.
Slice 2 — shared retry decision (de-duplicates the Sideband pattern)
Planned as "a state machine in :rns-host", it landed as a decision/
mechanism split instead — the backends can't depend on
:rns-host, andthe rebuild-and-resubmit mechanism must stay flavor-local because it
manipulates flavor-owned router/message objects:
DeliveryRetryPolicyin:rns-api— the one shared predicate("retry via propagation iff opted in, not already PROPAGATED, node
configured"), consulted by both flavors. A truth-table test pins the
semantics, including the give-up-after-one-retry rule encoded by the
desired-method flip.
event_bridge.pyloses the decision:_failednow only flattens thefailure and parks the live LXMessage in an
_outbound_messagesregistry; new mechanism-only
resubmit_as_propagated/discard_outboundfunctions do the flavor-local surgery.reset_reticulum_for_restartclears the registry.PythonEventBridge.handleLxmfFailureapplies the policy and drives themechanism through an injected
OutboundRetryMechanismhook wired inChaquopyRnsBackend; the separate retrying-propagated event sink isgone.
NativeMessageSenderconsults the same policy; its lxmf-kt rebuildmechanism is unchanged.
Docs & tests
CONTEXT.mdseeds the domain glossary: DeliveryState, DeliveryMethod,DeliveryRetryPolicy, propagation node, and the "propagated"
method-vs-state trap.
DeliveryStateTest(codec + legacy decode) andDeliveryRetryPolicyTest(truth table);ParcelRoundTripTestcoversevery state across the AIDL boundary.
Verification
:rns-api,:rns-ipc,:rns-backend-kt,:rns-backend-py, and:rns-host(kotlinBackend) unit tests green; the six affected:apptest classes pass (490 tests, 0 failures) with full
:appmain+testcompilation proving the exhaustiveness migration;
event_bridge.pypy_compile clean. Not covered here: an on-device send-with-propagation-
node smoke test of the Python retry mechanism.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BZ3PvF8KzApTXFYTfabv1b