-
Notifications
You must be signed in to change notification settings - Fork 0
[webhooks] add multi-part messages behavior #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying docs with
|
Latest commit: |
3683c0c
|
Status: | ✅ Deploy successful! |
Preview URL: | https://da8a297e.docs-7wm.pages.dev |
Branch Preview URL: | https://webhooks-multipart-behavior.docs-7wm.pages.dev |
WalkthroughAdds documentation about multipart SMS webhook behavior and webhook payload signing: a new FAQ entry and a “Multipart Message Behavior” section (both duplicated), expanded Security Considerations, and a new Payload Signing section with HMAC-SHA256 verification examples in multiple languages. No code or API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Sender as Sender Device
participant Carrier as Carrier Network
participant Gateway as SMS Gateway
participant Webhook as Webhook Endpoint
rect rgb(245,245,255)
note over Sender,Carrier: Outbound long SMS (multipart)
Sender->>Carrier: Submit long SMS
Carrier->>Gateway: Deliver parts P1..Pn
Gateway-->>Webhook: sms:sent (once, after all parts queued)
par For each part P1..Pn
Carrier-->>Gateway: Delivery report for Pi
Gateway-->>Webhook: sms:delivered (per part Pi)
end
opt Any part fails
Gateway-->>Webhook: sms:failed (once)
end
end
sequenceDiagram
autonumber
participant Sender as Sender Device
participant Carrier as Carrier Network
participant Gateway as SMS Gateway
participant Webhook as Webhook Endpoint
rect rgb(245,255,245)
note over Sender,Carrier: Inbound long SMS (multipart)
Sender->>Carrier: Send long SMS (multipart)
Carrier->>Gateway: Deliver parts P1..Pn
Gateway-->>Webhook: sms:received (once, after all parts assembled)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/faq/webhooks.md (2)
138-144
: Soften broad claim and add “See also” cross-link to the canonical behavior section.“This behavior is standard across all SMS providers” is too absolute; better to say “most” and link to the detailed behavior section for one source of truth.
Apply this diff:
-You'll receive separate `sms:delivered` events for each part, but they share the same: +You'll receive separate `sms:delivered` events for each part, but they share the same: - `messageId` (links all parts to the original message) - `phoneNumber` (recipient) -This behavior is standard across all SMS providers and ensures reliable delivery tracking for long messages that exceed carrier limits. +This behavior is common across most SMS providers and ensures reliable delivery tracking for long messages that exceed carrier limits. + +See also: [Multipart Message Behavior](../features/webhooks.md#multiparty-message-behavior-)Note: the anchor will work once we add an explicit ID to the section header in the features doc (see my related comment there).
128-128
: Optional: clarify the heading for searchability.Consider “Why do I receive multiple delivery reports for a single message?” to help readers who search “multiple receipts for one message.”
Apply this diff:
-## Why do I receive multiple delivery reports? 📨 +## Why do I receive multiple delivery reports for a single message? 📨docs/features/webhooks.md (1)
225-231
: Clarify event semantics, especially for “failed.”Current text can be interpreted as an overall failure. Recommend clarifying that “failed” fires once when any part fails (even if others deliver), and “received” is emitted after all parts assemble.
Apply this diff:
-| `sms:received` | Triggered once when all parts of the incoming message are received | +| `sms:received` | Triggered once after all parts of an incoming multipart message are received and assembled | | `sms:sent` | Triggered once when all parts of the outgoing message are successfully sent | | `sms:delivered` | Triggered once **for each individual part** of the message :material-information-outline: | -| `sms:failed` | Triggered once if any part of the message fails to send | +| `sms:failed` | Triggered once if any part of the message fails to send or deliver; other parts may still succeed |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/faq/webhooks.md
[grammar] ~128-~128: There might be a mistake here.
Context: ... I receive multiple delivery reports? 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When an SMS message exceeds the standard ...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ... | | --------------- | --------------------...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...-------------------------------------- | | sms:received
| Triggered once when ...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...ge are received | | sms:sent
| Triggered once when ...
(QB_NEW_EN)
[grammar] ~228-~228: There might be a mistake here.
Context: ...ge are successfully sent | | sms:delivered
| Triggered once **for...
(QB_NEW_EN)
[grammar] ~229-~229: There might be a mistake here.
Context: ...message :material-information-outline: | | sms:failed
| Triggered once if an...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ... | !!! tip "Handling Multipart Delivery Reports" To avoi...
(QB_NEW_EN)
[grammar] ~351-~351: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
🔇 Additional comments (2)
docs/features/webhooks.md (2)
221-237
: No duplicate “Multipart Message Behavior” section detected
A search ofdocs/features/webhooks.md
shows only one occurrence at line 221. No duplicate blocks were found, so no removal is needed.
351-351
: Link target file verifiedThe file
docs/features/status-tracking.md
exists at the expected location, and the relative path (../features/status-tracking.md
) fromdocs/features/webhooks.md
correctly resolves to it. No changes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docs/features/webhooks.md (2)
221-224
: Add a stable anchor and standardize segmentation wording.
- Add an explicit anchor so other pages can deep-link reliably.
- Align the segmentation phrasing with the FAQ (Android/SMS stack performs splitting).
-## Multipart Message Behavior 📨 +## Multipart Message Behavior 📨 {#multipart-message-behavior} -When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the app automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways: +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways:
232-237
: Fix deduplication guidance: use the event envelope id, not messageId; add expected-part-count note.
- “eventId” isn’t defined in the payloads; the top-level identifier is
id
. Recommend deduplicating by that.- Clarify that
messageId
is for grouping parts, not deduplication.- Call out that part index/count isn’t present; the expected count must come from the sending context.
-!!! tip "Handling Multipart Delivery Reports" - To avoid processing duplicate deliveries in your webhook handler: - - Use `messageId` to group related delivery reports - - Deduplicate based on `eventId` - - Consider the message fully delivered when you receive delivery confirmations for all expected parts +!!! tip "Handling Multipart Delivery Reports" + To correctly process multipart deliveries: + - Use `messageId` to group related delivery reports (all parts share it). + - Deduplicate exact duplicates by the webhook event’s top-level `id` (not by `messageId`). + - Consider the message fully delivered when you have confirmations for all expected parts for that `messageId`. + - Determine the expected part count from your sending context (part index/count isn’t included in the delivery payload).docs/faq/webhooks.md (1)
130-137
: Correct who performs splitting and tighten encoding terminology.Splitting is done on the sending stack (Android/SMS), not “the carrier.” Make this consistent with the feature doc and use GSM‑7 / UCS‑2 wording.
-When sending SMS messages longer than the standard character limits (160 characters for GSM/7-bit encoding or 70 characters for Unicode), the message is automatically split into multiple parts by the carrier. +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission. -Each message part is: +Each message part is: - Sent independently by the carrier - Processed separately at the network level - Delivered with its own confirmation receipt
🧹 Nitpick comments (1)
docs/faq/webhooks.md (1)
138-144
: Optional: mirror dedup guidance inline for quick reference.Consider adding a brief note so readers don’t have to jump pages for correct aggregation/dedup semantics.
You'll receive separate `sms:delivered` events for each part, but they share the same: - `messageId` (links all parts to the original message) - `phoneNumber` (recipient) +!!! tip "Processing multipart receipts" + Group by `messageId`, deduplicate by the webhook event’s top-level `id` (not `messageId`), and consider the message fully delivered once all expected parts are confirmed by your sending context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ...ther parts may still succeed | !!! tip "Handling Multipart Delivery Reports" To avoi...
(QB_NEW_EN)
[grammar] ~351-~351: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
docs/faq/webhooks.md
[grammar] ~128-~128: There might be a mistake here.
Context: ...elivery reports for a single message? 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
🔇 Additional comments (2)
docs/features/webhooks.md (2)
225-231
: Confirm implementation matches the table semantics for multipart flows.Please verify the app actually:
- emits exactly one
sms:received
after all inbound parts are assembled,- emits one
sms:sent
after all outbound parts are sent,- emits
sms:delivered
per part, and- emits a single
sms:failed
if any part fails.If any divergence exists, adjust the table wording accordingly.
351-351
: LGTM: useful cross-link to Status Tracking.The added See Also reference improves navigability.
2c98fb3
to
0b2150d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
docs/features/webhooks.md (2)
221-224
: Add a stable anchor and unify segmentation terminology with FAQ
- Add an explicit anchor so other pages can deep-link reliably.
- Prefer “Android SMS stack automatically splits” (device/framework-side) for consistency with the FAQ update.
Apply this diff:
-## Multipart Message Behavior 📨 +## Multipart Message Behavior 📨 {#multipart-message-behavior} -When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the app automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways: +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways:
232-237
: Tighten dedup guidance and add expected-part-count noteGood direction on grouping by messageId and deduping by eventId. Recommend small edits and one extra bullet so implementers can determine completeness without relying on part indices.
Apply this diff:
-!!! tip "Handling Multipart Delivery Reports" - To avoid processing duplicate deliveries in your webhook handler: - - Use `messageId` to group related delivery reports - - Deduplicate based on `eventId` - - Consider the message fully delivered when you receive delivery confirmations for all expected parts +!!! tip "Handling Multipart Delivery Reports" + To correctly process multipart deliveries: + - Use `messageId` to group related delivery reports (all parts share it). + - Deduplicate exact duplicates by a stable per-event identifier (e.g., `eventId`), not by `messageId`. + - Consider the message fully delivered when you have confirmations for all expected parts for that `messageId`. + - Determine the expected part count from your sending context (e.g., the API/client that initiated the send), since part index/count is not included in the delivery payload.Follow-up: If
eventId
is stable across retry attempts, document that explicitly so consumers can safely dedupe on it.docs/faq/webhooks.md (2)
130-131
: Fix technical inaccuracy: segmentation happens on the sending stack, not the carrierUse precise encoding terms and make this consistent with the features doc.
Apply this diff:
-When sending SMS messages longer than the standard character limits (160 characters for GSM/7-bit encoding or 70 characters for Unicode), the message is automatically split into multiple parts by the carrier. +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission.
138-144
: Correct the broken anchor and add actionable dedup guidance
- The trailing hyphen in the link likely breaks the anchor.
- Add concise implementation tips to align with the feature-page guidance (group by messageId, dedupe by eventId).
Apply this diff:
You'll receive separate `sms:delivered` events for each part, but they share the same: - `messageId` (links all parts to the original message) - `phoneNumber` (recipient) -See also: [Multipart Message Behavior](../features/webhooks.md#multipart-message-behavior-) +Implementation tips: + +- Use `messageId` to group related delivery reports (all parts share it). +- Deduplicate exact duplicates by `eventId`, not by `messageId`. + +See also: [Multipart Message Behavior](../features/webhooks.md#multipart-message-behavior)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/faq/webhooks.md
[grammar] ~128-~128: There might be a mistake here.
Context: ...elivery reports for a single message? 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ...ther parts may still succeed | !!! tip "Handling Multipart Delivery Reports" To avoi...
(QB_NEW_EN)
[grammar] ~351-~351: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
docs/features/webhooks.md (2)
225-231
: Clarify event cardinality for multipart “sent/failed” semanticsThe table states “sms:sent — once when all parts are sent” and “sms:failed — once if any part fails.” Please confirm whether your implementation emits:
- exactly one summary event per logical message, or
- per-part events (like delivery), or
- both (summary + per-part), and how retries affect duplicates.
If it is “summary per message,” consider adding one sentence under the table to make it explicit and to contrast with the per-part behavior of
sms:delivered
. This avoids integrators assuming per-part failures.
351-351
: LGTM: “Status Tracking” cross-link helps discoverabilityThe See Also addition is useful and consistent with the new multipart content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docs/faq/webhooks.md (2)
138-144
: Cross-reference anchor looks correct now.Good fix on the “See also” link; it targets the stable anchor. No further action needed here.
130-137
: Fix segmentation attribution and standardize encoding terminology.Segmentation is performed by the sending stack (Android SMS stack), not “the carrier.” Also prefer GSM‑7 and UCS‑2 terminology for accuracy and consistency with the feature docs.
Apply this diff:
-When sending SMS messages longer than the standard character limits (160 characters for GSM/7-bit encoding or 70 characters for Unicode), the message is automatically split into multiple parts by the carrier. +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission.docs/features/webhooks.md (1)
221-224
: Add a stable anchor and refine segmentation wording.
- Add an explicit anchor so other pages can deep-link reliably.
- Prefer “Android SMS stack automatically splits” for precision and to be consistent with FAQ guidance (after its fix).
-## Multipart Message Behavior 📨 - -When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the app automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways: +## Multipart Message Behavior 📨 {#multipart-message-behavior} + +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways:
🧹 Nitpick comments (1)
docs/faq/webhooks.md (1)
128-128
: Minor style nit: tighten the FAQ question line.Optional: drop “a single” to reduce redundancy and align with title case elsewhere.
-## Why do I receive multiple delivery reports for a single message? 📨 +## Why do I receive multiple delivery reports for one message? 📨
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/faq/webhooks.md
[grammar] ~128-~128: There might be a mistake here.
Context: ...elivery reports for a single message? 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
[grammar] ~352-~352: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/features/webhooks.md
235-235: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (3)
docs/faq/webhooks.md (1)
130-144
: No duplicate FAQ block foundVerified that the heading
## Why do I receive multiple delivery reports for a single message? 📨
appears only once in
docs/faq/webhooks.md
(at line 128) and is not duplicated elsewhere in the file.docs/features/webhooks.md (2)
225-231
: Verify “sms:failed” emission semantics for multipart.Table states “sms:failed” is triggered once if any part fails. Some stacks emit per-part failures similar to per-part deliveries. Please confirm actual behavior in the app. If failures are per-part, align with “delivered” semantics.
Proposed alternative if failures are per-part:
-| `sms:failed` | Triggered once if any part of the message fails to send or deliver; other parts may still succeed | +| `sms:failed` | Triggered **for each failed part**; other parts may still succeed |
352-352
: “Status Tracking” cross-link is a good addition.This improves discoverability for delivery-state nuances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/features/webhooks.md (1)
221-223
: Add stable anchor and unify splitting attribution
- Add an explicit anchor to enable deep links from the FAQ and other pages.
- Prefer “Android SMS stack automatically splits” to avoid implying the app itself performs segmentation and to stay consistent with the FAQ phrasing.
Apply:
-## Multipart Message Behavior 📨 +## Multipart Message Behavior 📨 {#multipart-message-behavior} -When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the app automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways: +When an SMS exceeds the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits it into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways:
🧹 Nitpick comments (1)
docs/features/webhooks.md (1)
232-237
: Tighten dedup guidance; include expected-part count note and fix markdownlint MD046
- Make the tip about “correctly processing multipart deliveries,” not just dedupe.
- Add guidance on determining the expected part count (since part index/count isn’t in payloads).
- Silence markdownlint MD046 (indented code block false-positive inside admonitions).
-!!! tip "Handling Multipart Delivery Reports" - To avoid processing duplicate deliveries in your webhook handler: - - - Use `messageId` to group related delivery reports - - Deduplicate based on `id` - - Consider the message fully delivered when you receive delivery confirmations for all expected parts +<!-- markdownlint-disable MD046 --> +!!! tip "Handling Multipart Delivery Reports" + To correctly process multipart deliveries: + - Use `messageId` to group related delivery reports. + - Deduplicate exact duplicates by the webhook event’s unique identifier (`id`). + - Consider the message fully delivered when you have confirmations for all expected parts for that `messageId`. + - Determine the expected part count from your sending context (e.g., the client/API that initiated the send), since part index/count is not included in the delivery payload. +<!-- markdownlint-enable MD046 -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
[grammar] ~352-~352: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/features/webhooks.md
235-235: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
docs/features/webhooks.md (3)
225-231
: Verify ‘sms:failed’ aggregation semantics; adjust if per-part failures are emittedThe table states “Triggered once” for
sms:failed
. If failures are reported per part (common on some stacks), this should say “Triggered per failed part” or be clarified as an aggregate event.Proposed wording if per-part failures are emitted:
-| `sms:failed` | Triggered once if any part of the message fails to send or deliver; other parts may still succeed | +| `sms:failed` | Triggered for each part that fails to send or deliver; other parts may still succeed |If it’s truly a single aggregate event, consider adding a footnote stating that no per-part
sms:failed
events will be sent.
352-352
: Link target and list formatting are correctI’ve confirmed that:
- The file
docs/features/status-tracking.md
exists and publishes the “Status Tracking” page (with an H1 heading)- All items in the “See Also” list consistently omit trailing punctuation
No changes are needed here.
221-238
: No duplicate “Multipart Message Behavior” section found; FAQ cross-link verifiedI searched all markdown files under
docs/
and confirmed that the “## Multipart Message Behavior 📨” heading appears only once indocs/features/webhooks.md
(lines 221–238). Indocs/faq/webhooks.md
, there is a single “See also: Multipart Message Behavior” link on line 143, correctly targeting the{#multipart-message-behavior}
anchor.No further action needed.
df2cf57
to
3683c0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
docs/faq/webhooks.md (2)
143-144
: Anchor link looks correct now.The “See also” link targets
#multipart-message-behavior
without the stray trailing hyphen. This resolves the previously reported broken link.
130-136
: Fix technical inaccuracy: splitting is performed by the Android SMS stack, not the carrier; tighten encoding terms.Current text attributes segmentation to the carrier and uses non-standard encoding names. Align with the feature doc and industry terms to avoid misleading implementers.
Apply this diff:
-When sending SMS messages longer than the standard character limits (160 characters for GSM/7-bit encoding or 70 characters for Unicode), the message is automatically split into multiple parts by the carrier. +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission.
🧹 Nitpick comments (2)
docs/features/webhooks.md (2)
221-224
: Add a stable anchor and unify terminology (Android SMS stack performs splitting).Helps deep-linking from the FAQ and keeps phrasing precise and consistent.
Apply this diff:
-## Multipart Message Behavior 📨 - -When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the app automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways: +## Multipart Message Behavior 📨 {#multipart-message-behavior} + +When sending SMS messages longer than the standard limits (160 characters for GSM‑7 or 70 characters for UCS‑2/Unicode), the Android SMS stack automatically splits the message into multiple parts before transmission. This multipart behavior affects webhook delivery in specific ways:
232-238
: Clarify multipart dedup strategy: group by messageId; dedupe by per-event id; note expected-part counting.Prevents dropping valid per-part receipts and tells readers how to decide “fully delivered.”
Apply this diff:
-!!! tip "Handling Multipart Delivery Reports" - To avoid processing duplicate deliveries in your webhook handler: - - - Use `messageId` to group related delivery reports - - Deduplicate based on `id` - - Consider the message fully delivered when you receive delivery confirmations for all expected parts +!!! tip "Handling Multipart Delivery Reports" + To correctly process multipart deliveries: + + - Use `messageId` to group related delivery reports (all parts share it). + - Deduplicate exact duplicates by the webhook event’s unique identifier (the top‑level `id` in the payload), not by `messageId`. + - Consider the message fully delivered when you have confirmations for all expected parts for that `messageId`. + - Determine the expected part count from your sending context (e.g., the client/API that initiated the send), since part index/count is not included in the delivery payload.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/features/webhooks.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/features/webhooks.md
[grammar] ~221-~221: There might be a mistake here.
Context: ...t SSL. ## Multipart Message Behavior 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
[grammar] ~352-~352: There might be a mistake here.
Context: ...](../faq/webhooks.md) - Status Tracking - [Private Webhook Certificate Setup](../se...
(QB_NEW_EN)
docs/faq/webhooks.md
[grammar] ~128-~128: There might be a mistake here.
Context: ...elivery reports for a single message? 📨 When sending SMS messages longer than the...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/features/webhooks.md
235-235: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (3)
docs/faq/webhooks.md (1)
128-142
: No duplicate FAQ entry detectedI ran the provided search and found only one occurrence of the “Why do I receive multiple delivery reports for a single message?” section in docs/faq/webhooks.md (line 128), confirming there’s no accidental duplication.
docs/features/webhooks.md (2)
352-352
: Link Verified: “Status Tracking”
The link../features/status-tracking.md
correctly resolves todocs/features/status-tracking.md
, which exists and includes the expected section headings (e.g., “Tracking Methods,” “Message Lifecycle”)—no broken cross-links detected.
221-231
: No duplicate “Multipart Message Behavior” section detectedVerified using ripgrep that there is only a single occurrence of the
## Multipart Message Behavior 📨
heading indocs/features/webhooks.md
(line 221). The duplicate-section concern is resolved.
Summary by CodeRabbit