Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Message kind classification packages/sdk/src/messaging/normalize.ts, packages/sdk/src/__tests__/messaging.test.ts, CHANGELOG.md |
The normalizer treats a dmch_ channel ID as DM evidence. Explicit kind and thread-parent precedence remain covered by tests, along with regular channel and unknown classifications. The changelog records the fix. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
Minimal · up to 17bbf
-
The generic DM readback behavior is covered, and no issue remains that should delay merging.
Pre-merge checks: 
2 · 
3
-
Failed checks: 3 warnings Passed checks: 2 passed-
Full details: Title check
-
Explanation
The title "fix(sdk): retain DM classification on generic message readback" matches the main change. The diff makes normalizeMessage classify messages from dmch_ backing channels as dm when no explicit kind is supplied, and adds regression tests. The title is specific, uses a conventional fix(sdk) prefix, and is under 72 characters.
Full details: Linked Issues check
-
Explanation
Issue [
#1987] requires generic DM reads to keepkind: 'dm'when the raw record has admch_channel ID and noconversation_id. The normalizer now maps that ID todmwhen no explicit kind exists.context.kindremains authoritative, thread replies remain higher priority, and explicit conversation IDs remain supported. The existing channel fallback still maps genuine channel records tochannel, and the normalizer still returns the existing message fields and channel reference. The supplied test summary reports coverage for DM readback, channel records, explicit kind, thread precedence, and unknown records.
-
Full details: Description check
-
Explanation
The description states the problem, the fix, and the verification results. It explains that Relaycast returns a dmch_ channel_id without conversation_id, that the SDK now recognizes dmch_ backing channels, and that explicit kind, thread-reply precedence, and explicit conversation IDs keep their priority. It lists test and check results and links the issue (
#1987). It does not fill in the template's Test Plan checkboxes, the Change type and RelayFlow case placeholders in the RelayFlow Proof section (both still read replace-me in the template and are absent from the description), or the Screenshots section. The Summary heading is replaced by 'What changed', and the Test Plan checkboxes are not ticked.Resolution
Add the RelayFlow Proof section with a Change type (
bugfixfits, since this is user-visible behavior) and a RelayFlow case under tests/relayflows/cases/<case-id>/, replacing the placeholder values and keeping the relay-pr-proof HTML comment markers. Tick the Test Plan checkboxes that apply (Tests added/updated is clearly done; Manual testing completed should be ticked only if it was actually run, otherwise state that it was not). Add a Screenshots note (for example 'N/A') or remove the placeholder. Rename or map the 'What changed' section so it sits under the template's Summary heading, or add a Summary section.
Full details: Out of Scope Changes check
-
Explanation
The fixed changelog entry supports issue [
#1987]. However, the same change renames the globalCHANGELOG.mdheading fromUnreleasedtoUnreleased - Major. That release-category change has no stated connection to the DM classification fix or to the linked issue.Resolution
Revert the unrelated
UnreleasedtoUnreleased - Majorheading change. Keep the changelog entry that documents the DM classification fix.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit reads a message bright,
A dmch_ keeps its DM right.
The channel stays a channel too,
Context guides what kind to view.
The tests hop through each case,
And the changelog marks the place.
Comment @coderabbitai help to get the list of available commands.
[Medium impact] The PR appears safe to merge.
|
Fixes #1987.
What changed
The generic messages.get(id) path receives no direct-message context. For some real one-to-one DMs, Relaycast returns a valid message with channel_id: dmch_... (its internal DM backing channel) but omits conversation_id. Previously, the SDK inferred kind: channel from any channel_id, even for these internal DM channels.
Verification
Note for maintainers
This is a compatibility inference for Relaycast's observed dmch_ internal ID. If the engine can expose conversation_id (or an explicit wire kind) consistently on generic GET /v1/messages/:id, that would be a stronger long-term discriminator; the SDK still prioritizes that metadata when present.
Need help on this PR? Tag
@codesmith-botwith what you need.