fix: store inline prosemirror JSON in ChatMessage.message (HULY-20) - #46
Conversation
|
Warning Review limit reached
Next review available in: 48 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI now converts HTML message bodies to inline ProseMirror markup before storing activity replies, approval comments, channel messages, thread replies, direct messages, and comments. Comment creation output returns the converted markup. ChangesMessage markup serialization
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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: 1
🤖 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 `@packages/cli/src/resources/channel.ts`:
- Around line 1054-1057: Move the htmlToMarkup conversion in the person-message
flow before the --person --dry-run early return, and update the preview output
to use messageMarkup instead of raw body. Keep the actual send path persisting
the same stored markup.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bbf9452d-34a0-4c62-af54-06a338ed7e9f
📒 Files selected for processing (4)
packages/cli/src/resources/activity.tspackages/cli/src/resources/approvals.tspackages/cli/src/resources/channel.tspackages/cli/src/resources/comment.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Kilo Code Review
🔇 Additional comments (4)
packages/cli/src/resources/activity.ts (1)
21-21: LGTM!Also applies to: 375-379, 416-421
packages/cli/src/resources/approvals.ts (1)
21-21: LGTM!Also applies to: 301-304
packages/cli/src/resources/channel.ts (1)
9-9: LGTM!Also applies to: 550-555, 618-621, 747-751, 804-806
packages/cli/src/resources/comment.ts (1)
9-9: LGTM!Also applies to: 89-97, 107-113, 143-153
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // HULY-20: DM messages are ChatMessage instances — message is TypeMarkup | ||
| // (inline prosemirror JSON). Inline storage, same as channel messages. | ||
| const messageMarkup = htmlToMarkup(body) | ||
| const data: Record<string, unknown> = { message: messageMarkup } |
There was a problem hiding this comment.
SUGGESTION: sendDmMessage has two dry-run preview paths; the first one (lines 1029-1043, triggered when --person is provided) was not updated alongside the second path.
The first dry-run preview at line 1036 still emits the raw HTML body (message: body,), even though the actual stored value (line 1057) is the converted markup. The second dry-run path (lines 1058-1073) now correctly shows the converted markup via data: { message: messageMarkup }. Before this PR both paths were consistent (both showed raw HTML); the change to the second path makes them inconsistent.
This contradicts the PR description's claim that "dry-run preview now shows the converted prosemirror JSON (which is what would be stored)". A user running huly dm message send --person <x> --dry-run --body "<p>hello</p>" will see raw HTML in the preview, but the actual stored value will be the converted markup. This is misleading for dry-run previews.
Consider either (a) moving the messageMarkup = htmlToMarkup(body) computation above the if (opts.person !== undefined && opts.person !== '') block so both dry-run paths can use it, or (b) updating the first dry-run path's JSON to show message: htmlToMarkup(body) directly.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The previous review's suggestion (inconsistent dry-run preview between the two Files Reviewed (1 file)
Previous Review Summary (commit 01172ea)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 01172ea)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Reviewed by minimax-m3 · Input: 20.2K · Output: 2.2K · Cached: 107K |
The original attempt (PR #44, reverted in #45) called uploadMarkup and stored a MarkupBlobRef string in `message`. That rendered as literal text in the web UI because the front-end's MessageViewer component does: $: node = markupToJSON(message) with no MarkupBlobRef resolution step. ChatMessage.message is typed `TypeMarkup()` — an inline prosemirror-JSON string (`Markup = string` in @hcengineering/core), not a MarkupBlobRef. Why the same pattern appeared to work for Issue.description: Issue.description is also `TypeMarkup()`, but the web UI renders it through `<CollaborativeTextEditor>`, which DOES resolve MarkupBlobRef via the collaborator service. So `huly issue update --body` accidentally worked. The chat/dm/thread/activity paths render through plain `MessageViewer` and broke. Fix: * Convert HTML → prosemirror JSON locally via the existing `htmlToMarkup()` helper. * Store the resulting prosemirror-JSON string inline in `message` — no blob upload, no MarkupBlobRef. * Same pattern applied to all 10 chat-message write paths: comment add/update, channel message send/update, thread reply add/update, dm message send, activity reply add/update, approval comment. Removed from the previous PR (no longer relevant): * `uploadMarkup` and `generateId` imports — not used anymore. * Deferred-upload pattern for dry-run on channel paths — dry-run preview now shows the converted prosemirror JSON (which is what would be stored). No external IO involved in conversion, so no point deferring. * `wouldUploadMarkup` notes in dry-run output — n/a, no upload. Verified on https://huly.aaravlabs.com (real server): * Posted via the local build: `node dist/index.js comment add --issue HULY-20 --body-file /tmp/kilo/huly20-fix2.html`. * message field stores inline prosemirror JSON starting with `{"type":"doc","content":[{"type":"heading",...}]}`. * Web UI's MessageViewer can render this directly without any collaborator lookup.
01172ea to
f8a8ab8
Compare
1 participant
Summary
Closes HULY-20. Replaces the reverted PR #44 with the correct approach.
What was wrong with #44
The previous attempt called
uploadMarkup()and stored aMarkupBlobRefstring inmessage. The web UI rendered it as literal text:because
ChatMessagePresenteruses plain<MessageViewer>, which does:with no MarkupBlobRef resolution step.
Root cause
ChatMessage.message(andIssue.description,Milestone.description,ActivityMessage.message, etc.) is typedTypeMarkup()—Markup = string(inline prosemirror-JSON string). TheMarkupBlobRefindirection only applies toTypeCollaborativeDoc(used byDocument.content, etc.).The reason
huly issue update --bodyappeared to work despite the same broken pattern:Issue.descriptionis rendered via<CollaborativeTextEditor>, which DOES resolveMarkupBlobRefthrough the collaborator service. Chat/DM/thread/activity/approval paths use plain<MessageViewer>which doesn't — so they fall back to rendering the ref string as text.Fix
Convert HTML → prosemirror JSON locally via the existing
htmlToMarkup()helper (used byhuly issue createsince v1.0.x) and store the resulting prosemirror-JSON string inline inmessage. No blob upload. NoMarkupBlobRef. Matches what the web UI's Tiptap editor does.Applied to all 10 chat-message write paths:
huly comment addchunter:class:ChatMessagehuly comment updatechunter:class:ChatMessagehuly activity reply addactivity:class:ActivityMessagehuly activity reply updateactivity:class:ActivityMessagehuly channel message sendchunter:class:ChatMessagehuly channel message updatechunter:class:ChatMessagehuly channel thread addchunter:class:ThreadMessagehuly channel thread updatechunter:class:ThreadMessagehuly dm message sendchunter:class:ChatMessagehuly approval commentchunter:class:ChatMessageVerification
Posted via the local build on
https://huly.aaravlabs.com(the real server, your workspace):The new comment's
messagefield now starts with{"type":"doc","content":[{"type":"heading","attrs":{...},"content":[{"type":"text","text":"Fixed (v2): inline prosemirror JSON, NOT MarkupBlobRef"}]}, ...]}— valid prosemirror JSON thatMessageViewerrenders directly.When you view HULY-20 in the web UI, the comment should now render as a proper heading + paragraph + list. (The old broken comment from #44 has been deleted.)
CI: typecheck ✅, tests 43/43 ✅, format ✅, lint ✅.