feat：resolve markdown blank-line formatting inconsistency in post messages#1214
Closed
91-enjoy wants to merge 1 commit into
Closed
feat：resolve markdown blank-line formatting inconsistency in post messages#121491-enjoy wants to merge 1 commit into
91-enjoy wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR inlines code-block placeholder handling in optimizeMarkdownStyle, removes segmented-paragraph logic from wrapMarkdownAsPost and resolveMarkdownAsPost (always emitting a single ChangesIM Markdown Post Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
github-actions
Bot
added
domain/im
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/im/helpers.go (1)
803-815:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIgnore fenced-code headings when deciding whether to downgrade.
reHasH1toH3.MatchString(text)still sees#markers inside fenced code blocks, so markdown like#### Keepplus a fenced# promptline will downgrade#### Keepto##### Keepeven though there is no real H1-H3 outside the fence. Check the placeholder-stripped string instead.Suggested fix
func optimizeMarkdownStyle(text string) string { const mark = "___CB_" var codeBlocks []string r := reCodeBlock.ReplaceAllStringFunc(text, func(m string) string { idx := len(codeBlocks) codeBlocks = append(codeBlocks, m) return fmt.Sprintf("%s%d___", mark, idx) }) // Only downgrade when original text has H1~H3; order matters (H2~H6 first). - if reHasH1toH3.MatchString(text) { + if reHasH1toH3.MatchString(r) { r = reH2toH6.ReplaceAllString(r, "##### $1") r = reH1.ReplaceAllString(r, "#### $1") }🤖 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 `@shortcuts/im/helpers.go` around lines 803 - 815, The downgrade check is inspecting the original text which still contains fenced-code `#` markers; in optimizeMarkdownStyle replace the condition reHasH1toH3.MatchString(text) with a check against the placeholder-stripped string (r) so fenced code is ignored (i.e., use reHasH1toH3.MatchString(r)), then proceed with the existing reH2toH6 and reH1 replacements on r and restore codeBlocks afterwards.
🧹 Nitpick comments (1)
shortcuts/im/helpers_test.go (1)
335-346: ⚡ Quick winDecode the payload instead of substring-matching it.
These checks won't catch invalid JSON, broken nesting, or escaping regressions in
wrapMarkdownAsPost, and this helper returns raw JSON that the send/reply paths forward directly. Please keep this as a structural assertion by unmarshaling and checking the singlezh_cn.content[0][0]node.Suggested test shape
import ( "context" + "encoding/json" "encoding/binary" "errors" "fmt" "net/http" @@ func TestWrapMarkdownAsPost(t *testing.T) { got := wrapMarkdownAsPost("hello **world**") - // Should produce valid JSON with post structure - if !strings.Contains(got, `"tag":"md"`) { - t.Fatalf("wrapMarkdownAsPost() missing md tag: %s", got) - } - if !strings.Contains(got, `"zh_cn"`) { - t.Fatalf("wrapMarkdownAsPost() missing zh_cn: %s", got) - } - if !strings.Contains(got, "hello **world**") { - t.Fatalf("wrapMarkdownAsPost() missing content: %s", got) - } + var payload struct { + ZhCN struct { + Content [][]struct { + Tag string `json:"tag"` + Text string `json:"text"` + } `json:"content"` + } `json:"zh_cn"` + } + if err := json.Unmarshal([]byte(got), &payload); err != nil { + t.Fatalf("wrapMarkdownAsPost() returned invalid JSON: %v", err) + } + if len(payload.ZhCN.Content) != 1 || len(payload.ZhCN.Content[0]) != 1 { + t.Fatalf("wrapMarkdownAsPost() content shape = %#v, want single md node", payload.ZhCN.Content) + } + if payload.ZhCN.Content[0][0].Tag != "md" || payload.ZhCN.Content[0][0].Text != "hello **world**" { + t.Fatalf("wrapMarkdownAsPost() node = %#v, want md/hello **world**", payload.ZhCN.Content[0][0]) + } }🤖 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 `@shortcuts/im/helpers_test.go` around lines 335 - 346, The test TestWrapMarkdownAsPost should unmarshal the JSON returned by wrapMarkdownAsPost instead of substring-matching; parse the returned string into a map/struct, assert valid JSON, then navigate to the single node at zh_cn.content[0][0] and verify it equals "hello **world**" and that the tag field equals "md" (use the symbols wrapMarkdownAsPost, TestWrapMarkdownAsPost and the JSON path zh_cn.content[0][0] to locate where to change the assertions).
🤖 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.
Outside diff comments:
In `@shortcuts/im/helpers.go`:
- Around line 803-815: The downgrade check is inspecting the original text which
still contains fenced-code `#` markers; in optimizeMarkdownStyle replace the
condition reHasH1toH3.MatchString(text) with a check against the
placeholder-stripped string (r) so fenced code is ignored (i.e., use
reHasH1toH3.MatchString(r)), then proceed with the existing reH2toH6 and reH1
replacements on r and restore codeBlocks afterwards.
---
Nitpick comments:
In `@shortcuts/im/helpers_test.go`:
- Around line 335-346: The test TestWrapMarkdownAsPost should unmarshal the JSON
returned by wrapMarkdownAsPost instead of substring-matching; parse the returned
string into a map/struct, assert valid JSON, then navigate to the single node at
zh_cn.content[0][0] and verify it equals "hello **world**" and that the tag
field equals "md" (use the symbols wrapMarkdownAsPost, TestWrapMarkdownAsPost
and the JSON path zh_cn.content[0][0] to locate where to change the assertions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c894bab0-df1c-48ec-befb-464008d2caf0
📒 Files selected for processing (3)
shortcuts/im/coverage_additional_test.goshortcuts/im/helpers.goshortcuts/im/helpers_test.go
💤 Files with no reviewable changes (1)
- shortcuts/im/coverage_additional_test.go
91-enjoy
changed the title
…ssages Change-Id: Ic2870ecbcb31ae7d36121f120102f2ff964f5169
91-enjoy
force-pushed
the
feat/revert-markdown-support-line-breaks
branch
from
June 2, 2026 06:51
c447d1e to
91c17e3
Compare
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 `@shortcuts/im/helpers_test.go`:
- Around line 337-345: The test currently only does substring checks and must
instead unmarshal the output of wrapMarkdownAsPost() and assert the JSON shape;
call wrapMarkdownAsPost(), json.Unmarshal the resulting bytes into a small
struct matching the expected envelope (e.g., Envelope{Tag string `json:"tag"`,
Language map[string]string `json:"zh_cn"` or Post struct with Content fields}),
then assert Tag == "md", that the zh_cn entry exists and equals the markdown
(contains "hello **world**), and fail the test on unmarshal error or
missing/incorrect fields; update the assertions around wrapMarkdownAsPost()
accordingly to use json.Unmarshal and field checks rather than substrings.
🪄 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: e64d0225-991b-443f-96ed-45e299bbf392
📒 Files selected for processing (3)
shortcuts/im/coverage_additional_test.goshortcuts/im/helpers.goshortcuts/im/helpers_test.go
💤 Files with no reviewable changes (1)
- shortcuts/im/coverage_additional_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/im/helpers.go
Comment on lines
+337
to
+345
| // Should produce valid JSON with post structure | ||
| if !strings.Contains(got, `"tag":"md"`) { | ||
| t.Fatalf("wrapMarkdownAsPost() missing md tag: %s", got) | ||
| } | ||
| node := decodePostParagraphForTest(t, got, 0) | ||
| if node["tag"] != "md" { | ||
| t.Fatalf("wrapMarkdownAsPost() tag = %#v, want md", node["tag"]) | ||
| } | ||
| if node["text"] != "hello **world**" { | ||
| t.Fatalf("wrapMarkdownAsPost() text = %#v, want %q", node["text"], "hello **world**") | ||
| } | ||
| } | ||
|
|
||
| func TestShouldUseSegmentedPost(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| markdown string | ||
| want bool | ||
| }{ | ||
| {name: "single newline", markdown: "a\nb", want: false}, | ||
| {name: "blank line", markdown: "a\n\nb", want: true}, | ||
| {name: "blank line with spaces", markdown: "a\n \nb", want: true}, | ||
| {name: "multiple blank lines", markdown: "a\n \n \n b", want: true}, | ||
| {name: "blank lines inside code block only", markdown: "```go\n\n\nfmt.Println(1)\n```\nnext", want: false}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if got := shouldUseSegmentedPost(tt.markdown); got != tt.want { | ||
| t.Fatalf("shouldUseSegmentedPost(%q) = %v, want %v", tt.markdown, got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestWrapMarkdownAsPost_SegmentedBlankLines(t *testing.T) { | ||
| got := wrapMarkdownAsPost("a\n\nb") | ||
| content := decodePostContentForTest(t, got) | ||
| if len(content) != 3 { | ||
| t.Fatalf("wrapMarkdownAsPost(a\\n\\nb) content len = %d, want 3", len(content)) | ||
| } | ||
|
|
||
| first := decodePostParagraphForTest(t, got, 0) | ||
| if first["tag"] != "md" || first["text"] != "a" { | ||
| t.Fatalf("first paragraph = %#v, want md/a", first) | ||
| } | ||
|
|
||
| second := decodePostParagraphForTest(t, got, 1) | ||
| if second["tag"] != "text" || second["text"] != postBlankLinePlaceholder { | ||
| t.Fatalf("second paragraph = %#v, want blank text placeholder", second) | ||
| } | ||
|
|
||
| third := decodePostParagraphForTest(t, got, 2) | ||
| if third["tag"] != "md" || third["text"] != "b" { | ||
| t.Fatalf("third paragraph = %#v, want md/b", third) | ||
| } | ||
| } | ||
|
|
||
| func TestWrapMarkdownAsPost_SegmentedMultipleBlankLines(t *testing.T) { | ||
| got := wrapMarkdownAsPost("a\n\n\nb") | ||
| content := decodePostContentForTest(t, got) | ||
| if len(content) != 4 { | ||
| t.Fatalf("wrapMarkdownAsPost(a\\n\\n\\nb) content len = %d, want 4", len(content)) | ||
| } | ||
|
|
||
| for i := 1; i <= 2; i++ { | ||
| node := decodePostParagraphForTest(t, got, i) | ||
| if node["tag"] != "text" || node["text"] != postBlankLinePlaceholder { | ||
| t.Fatalf("blank paragraph %d = %#v, want blank text placeholder", i, node) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestWrapMarkdownAsPost_SegmentedBlankLinesWithSpaces(t *testing.T) { | ||
| got := wrapMarkdownAsPost("a\n \nb") | ||
| content := decodePostContentForTest(t, got) | ||
| if len(content) != 3 { | ||
| t.Fatalf("wrapMarkdownAsPost(a\\n \\nb) content len = %d, want 3", len(content)) | ||
| if !strings.Contains(got, `"zh_cn"`) { | ||
| t.Fatalf("wrapMarkdownAsPost() missing zh_cn: %s", got) | ||
| } | ||
| node := decodePostParagraphForTest(t, got, 1) | ||
| if node["tag"] != "text" || node["text"] != postBlankLinePlaceholder { | ||
| t.Fatalf("middle paragraph = %#v, want blank text placeholder", node) | ||
| if !strings.Contains(got, "hello **world**") { | ||
| t.Fatalf("wrapMarkdownAsPost() missing content: %s", got) |
There was a problem hiding this comment.
Keep a real JSON-shape assertion here.
These substring checks no longer verify that wrapMarkdownAsPost() returns valid JSON with the expected nested post structure. A malformed envelope can still pass as long as it contains "zh_cn", "tag":"md", and the markdown text. Please keep a minimal json.Unmarshal-based assertion for the wrapper contract this PR is changing.
Suggested test shape
+ var payload map[string]any
+ if err := json.Unmarshal([]byte(got), &payload); err != nil {
+ t.Fatalf("wrapMarkdownAsPost() returned invalid JSON: %v\npayload=%s", err, got)
+ }
if !strings.Contains(got, `"tag":"md"`) {
t.Fatalf("wrapMarkdownAsPost() missing md tag: %s", got)
}
if !strings.Contains(got, `"zh_cn"`) {
t.Fatalf("wrapMarkdownAsPost() missing zh_cn: %s", got)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Should produce valid JSON with post structure | |
| if !strings.Contains(got, `"tag":"md"`) { | |
| t.Fatalf("wrapMarkdownAsPost() missing md tag: %s", got) | |
| } | |
| node := decodePostParagraphForTest(t, got, 0) | |
| if node["tag"] != "md" { | |
| t.Fatalf("wrapMarkdownAsPost() tag = %#v, want md", node["tag"]) | |
| } | |
| if node["text"] != "hello **world**" { | |
| t.Fatalf("wrapMarkdownAsPost() text = %#v, want %q", node["text"], "hello **world**") | |
| } | |
| } | |
| func TestShouldUseSegmentedPost(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| markdown string | |
| want bool | |
| }{ | |
| {name: "single newline", markdown: "a\nb", want: false}, | |
| {name: "blank line", markdown: "a\n\nb", want: true}, | |
| {name: "blank line with spaces", markdown: "a\n \nb", want: true}, | |
| {name: "multiple blank lines", markdown: "a\n \n \n b", want: true}, | |
| {name: "blank lines inside code block only", markdown: "```go\n\n\nfmt.Println(1)\n```\nnext", want: false}, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| if got := shouldUseSegmentedPost(tt.markdown); got != tt.want { | |
| t.Fatalf("shouldUseSegmentedPost(%q) = %v, want %v", tt.markdown, got, tt.want) | |
| } | |
| }) | |
| } | |
| } | |
| func TestWrapMarkdownAsPost_SegmentedBlankLines(t *testing.T) { | |
| got := wrapMarkdownAsPost("a\n\nb") | |
| content := decodePostContentForTest(t, got) | |
| if len(content) != 3 { | |
| t.Fatalf("wrapMarkdownAsPost(a\\n\\nb) content len = %d, want 3", len(content)) | |
| } | |
| first := decodePostParagraphForTest(t, got, 0) | |
| if first["tag"] != "md" || first["text"] != "a" { | |
| t.Fatalf("first paragraph = %#v, want md/a", first) | |
| } | |
| second := decodePostParagraphForTest(t, got, 1) | |
| if second["tag"] != "text" || second["text"] != postBlankLinePlaceholder { | |
| t.Fatalf("second paragraph = %#v, want blank text placeholder", second) | |
| } | |
| third := decodePostParagraphForTest(t, got, 2) | |
| if third["tag"] != "md" || third["text"] != "b" { | |
| t.Fatalf("third paragraph = %#v, want md/b", third) | |
| } | |
| } | |
| func TestWrapMarkdownAsPost_SegmentedMultipleBlankLines(t *testing.T) { | |
| got := wrapMarkdownAsPost("a\n\n\nb") | |
| content := decodePostContentForTest(t, got) | |
| if len(content) != 4 { | |
| t.Fatalf("wrapMarkdownAsPost(a\\n\\n\\nb) content len = %d, want 4", len(content)) | |
| } | |
| for i := 1; i <= 2; i++ { | |
| node := decodePostParagraphForTest(t, got, i) | |
| if node["tag"] != "text" || node["text"] != postBlankLinePlaceholder { | |
| t.Fatalf("blank paragraph %d = %#v, want blank text placeholder", i, node) | |
| } | |
| } | |
| } | |
| func TestWrapMarkdownAsPost_SegmentedBlankLinesWithSpaces(t *testing.T) { | |
| got := wrapMarkdownAsPost("a\n \nb") | |
| content := decodePostContentForTest(t, got) | |
| if len(content) != 3 { | |
| t.Fatalf("wrapMarkdownAsPost(a\\n \\nb) content len = %d, want 3", len(content)) | |
| if !strings.Contains(got, `"zh_cn"`) { | |
| t.Fatalf("wrapMarkdownAsPost() missing zh_cn: %s", got) | |
| } | |
| node := decodePostParagraphForTest(t, got, 1) | |
| if node["tag"] != "text" || node["text"] != postBlankLinePlaceholder { | |
| t.Fatalf("middle paragraph = %#v, want blank text placeholder", node) | |
| if !strings.Contains(got, "hello **world**") { | |
| t.Fatalf("wrapMarkdownAsPost() missing content: %s", got) | |
| // Should produce valid JSON with post structure | |
| var payload map[string]any | |
| if err := json.Unmarshal([]byte(got), &payload); err != nil { | |
| t.Fatalf("wrapMarkdownAsPost() returned invalid JSON: %v\npayload=%s", err, got) | |
| } | |
| if !strings.Contains(got, `"tag":"md"`) { | |
| t.Fatalf("wrapMarkdownAsPost() missing md tag: %s", got) | |
| } | |
| if !strings.Contains(got, `"zh_cn"`) { | |
| t.Fatalf("wrapMarkdownAsPost() missing zh_cn: %s", got) | |
| } | |
| if !strings.Contains(got, "hello **world**") { | |
| t.Fatalf("wrapMarkdownAsPost() missing content: %s", got) | |
| } |
🤖 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 `@shortcuts/im/helpers_test.go` around lines 337 - 345, The test currently only
does substring checks and must instead unmarshal the output of
wrapMarkdownAsPost() and assert the JSON shape; call wrapMarkdownAsPost(),
json.Unmarshal the resulting bytes into a small struct matching the expected
envelope (e.g., Envelope{Tag string `json:"tag"`, Language map[string]string
`json:"zh_cn"` or Post struct with Content fields}), then assert Tag == "md",
that the zh_cn entry exists and equals the markdown (contains "hello **world**),
and fail the test on unmarshal error or missing/incorrect fields; update the
assertions around wrapMarkdownAsPost() accordingly to use json.Unmarshal and
field checks rather than substrings.
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
1 participant
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
Simplifies the markdown-to-post rendering pipeline in the IM shortcut. The previous
implementation split markdown at blank-line boundaries into multiple post paragraphs,
using zero-width space (
\u200B) sentinel characters to preserve visual spacing.While well-intentioned, this approach introduced fragility around edge cases such as
blank lines inside fenced code blocks, messages with only blank lines, and interactions
with the heading-normalization pass. This change consolidates rendering back into a
single
{"tag":"md"}segment, making the output more predictable, the code significantlyeasier to follow, and the test surface easier to maintain.
Changes
wrapMarkdownAsPostandresolveMarkdownAsPostto produce a single-segmentpost payload directly, without dispatching through a multi-step segmentation pipeline
directly into
optimizeMarkdownStyle, removing the need for standaloneprotectMarkdownCodeBlocks/restoreMarkdownCodeBlockshelpersmarkdownPartstruct,shouldUseSegmentedPost,splitMarkdownByBlankLines,marshalMarkdownPostContent,buildSingleMDPost,buildSegmentedPost,buildMarkdownPostContentreBlankLineSeparatorregex and thepostBlankLinePlaceholder/markdownCodeBlockPlaceholderpackage-level constantsdecodePostContentForTest,decodePostParagraphForTest)and all test cases that covered the removed multi-segment behavior; simplify
the remaining
TestWrapMarkdownAsPostto assert on the raw JSON structureTest Plan
go test ./shortcuts/im/... -racepassesgo build ./...passes with no errorsTestOptimizeMarkdownStyle(heading normalization, table formatting, code-blockprotection, excess-newline compression) remains fully covered and unchanged
lark im message send --markdownflowworks as expected
Related Issues
Summary by CodeRabbit
Bug Fixes
Tests