feat(im): support Markdown image rendering in post content - #874
Closed
liujiashu-shiro wants to merge 1 commit into
Closed
feat(im): support Markdown image rendering in post content#874liujiashu-shiro wants to merge 1 commit into
liujiashu-shiro wants to merge 1 commit into
Conversation
Leverage underlying URL compatibility and remove redundant URL conversion logic. Add the ability to send Markdown images.
|
|
github-actions
Bot
added
domain/im
📝 WalkthroughWalkthroughThe PR simplifies Feishu markdown post JSON construction by removing custom marshaling and element tokenization, then documents the resulting constraints around image handling in markdown for send and reply operations. ChangesMarkdown Post Simplification and Image Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 2
🤖 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 `@skills/lark-im/references/lark-im-messages-reply.md`:
- Around line 67-69: Update the Markdown guidance lines that currently state
remote Markdown image URLs are unsupported to instead recommend pre-uploading
for reliability and retain the actual runtime behavior that attempts to
download/upload remote URLs and removes failed images with a warning;
specifically replace the sentences like "Only already-uploaded `img_xxx`
Markdown images are kept reliably." and "Local paths (e.g. ``) and
remote URLs are **not** supported..." with wording such as "Pre-uploading images
(use `image_key` like `img_xxx`) is recommended for reliability; the runtime
will still attempt to download and upload remote Markdown image URLs and will
remove any image that fails with a warning." Apply the same phrasing at the
other occurrences referenced (the blocks around the text matching those lines).
In `@skills/lark-im/references/lark-im-messages-send.md`:
- Around line 67-69: Update the contradictory Markdown-image bullets so they
consistently reflect runtime behavior: state that already-uploaded `img_xxx`
images are preserved, local file paths like `` are not supported,
remote image URLs will be attempted (downloaded and uploaded) but if remote
download/upload fails the image is removed and a warning is emitted; apply this
same corrected wording to the other occurrences referenced (lines around the
other instances).
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 277249e5-3e27-4646-adb6-e939d15ad417
📒 Files selected for processing (4)
shortcuts/im/helpers.goshortcuts/im/helpers_test.goskills/lark-im/references/lark-im-messages-reply.mdskills/lark-im/references/lark-im-messages-send.md
Comment on lines
+67
to
69
| - Only already-uploaded `img_xxx` Markdown images are kept reliably. | ||
| - Local paths (e.g. ``) and remote URLs are **not** supported as image links — all images must be downloaded and uploaded via `images.create`, provided as an `image_key`. | ||
| - If remote Markdown image handling fails, that image is removed with a warning. |
There was a problem hiding this comment.
Align Markdown image guidance with current runtime behavior.
These lines now say remote Markdown image URLs are unsupported and may become placeholder text, but current runtime still tries to download/upload remote image URLs and removes failed images with a warning. Please update wording to “pre-upload is recommended for reliability” and keep failure behavior consistent with actual execution.
Suggested wording update
-- Only already-uploaded `img_xxx` Markdown images are kept reliably.
-- Local paths (e.g. ``) and remote URLs are **not** supported as image links — all images must be downloaded and uploaded via `images.create`, provided as an `image_key`.
+- Already-uploaded `img_xxx` image keys are the most reliable Markdown image input.
+- Local paths (e.g. ``) are **not** supported directly in `--markdown`.
+- Remote URLs may be auto-downloaded and uploaded during execution; if that step fails, the image is removed with a warning.
-- When using `--markdown` and the reply content includes images, you **must** first upload the image via `images.create` to obtain an `image_key`, then reference it as ``.
+- When using `--markdown` with images, prefer pre-uploading via `images.create` and referencing `` for predictable results.
-- **Using `--markdown` with images without first uploading via `images.create`.** All images must be pre-uploaded to obtain an `image_key`. Neither local paths nor remote URLs can be used directly — otherwise the image will be replaced with placeholder text.
+- **Using local file paths inside Markdown image syntax** (for example ``) with `--markdown`. This is not auto-uploaded and will not render as an image.Also applies to: 75-76, 189-190, 244-245
🤖 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 `@skills/lark-im/references/lark-im-messages-reply.md` around lines 67 - 69,
Update the Markdown guidance lines that currently state remote Markdown image
URLs are unsupported to instead recommend pre-uploading for reliability and
retain the actual runtime behavior that attempts to download/upload remote URLs
and removes failed images with a warning; specifically replace the sentences
like "Only already-uploaded `img_xxx` Markdown images are kept reliably." and
"Local paths (e.g. ``) and remote URLs are **not** supported..."
with wording such as "Pre-uploading images (use `image_key` like `img_xxx`) is
recommended for reliability; the runtime will still attempt to download and
upload remote Markdown image URLs and will remove any image that fails with a
warning." Apply the same phrasing at the other occurrences referenced (the
blocks around the text matching those lines).
Comment on lines
+67
to
69
| - Only already-uploaded `img_xxx` Markdown images are kept reliably. | ||
| - Local paths in Markdown image syntax like `` are **not** supported. | ||
| - If remote Markdown image download/upload fails, that image is removed with a warning. |
There was a problem hiding this comment.
Fix contradictory Markdown-image behavior statements.
This file now mixes two incompatible claims: remote URLs are auto-resolved vs remote URLs are unsupported. The new “placeholder text” wording is also inconsistent with runtime, which drops failed images with a warning. Please align these lines with the implemented behavior.
Also applies to: 75-76, 195-196, 245-245
🤖 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 `@skills/lark-im/references/lark-im-messages-send.md` around lines 67 - 69,
Update the contradictory Markdown-image bullets so they consistently reflect
runtime behavior: state that already-uploaded `img_xxx` images are preserved,
local file paths like `` are not supported, remote image URLs will
be attempted (downloaded and uploaded) but if remote download/upload fails the
image is removed and a warning is emitted; apply this same corrected wording to
the other occurrences referenced (lines around the other instances).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #874 +/- ##
==========================================
- Coverage 65.90% 65.87% -0.03%
==========================================
Files 518 518
Lines 48830 48731 -99
==========================================
- Hits 32181 32102 -79
+ Misses 13882 13870 -12
+ Partials 2767 2759 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4686676405c01e7d9ad643708e9468de87eeb433🧩 Skill updatenpx skills add larksuite/cli#feat/markdown_image -y -g |
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
2 participants
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
Leverage underlying URL compatibility and remove redundant URL conversion logic to simplify the Markdown-to-Post element processing pipeline. Also add a new skill for sending Markdown images.
Changes
buildPostElements,buildPostElementNodes,scanPostToken,scanBareURLToken,scanMarkdownLinkToken,scanBalancedParenToken,trimBareURLToken,appendMDPostNode,marshalJSONNoEscape,marshalStringNoEscapeand related constantsbuildSingleMDPost/buildSegmentedPostto single{"tag":"md"}serializationmarshalJSONNoEscapewith standardjson.Marshallark-im-messages-send.mdandlark-im-messages-reply.mdTest Plan
go test ./shortcuts/im/)lark im send --markdownwith bare URLs renders correctlySummary by CodeRabbit