feat(doc): warn when replace_range targets a heading but markdown lacks matching heading#619
Open
herbertliu wants to merge 2 commits intomainfrom
Open
feat(doc): warn when replace_range targets a heading but markdown lacks matching heading#619herbertliu wants to merge 2 commits intomainfrom
herbertliu wants to merge 2 commits intomainfrom
Conversation
…ks a matching heading
Lark's block model stores {type, content} as independent attributes, so
replace_range rewrites content only and will not downgrade a heading
block to a paragraph. When --selection-by-title selects a heading but
--markdown leads with plain prose (or a different heading level), the
user's intent to change block type is silently dropped and the block
keeps its heading styling.
Extend docsUpdateWarnings with checkDocsUpdateReplaceHeadingBlockType
that fires only on the high-signal combination of replace_range +
heading selection-by-title + non-matching markdown first prose line.
Other selection modes can target arbitrary blocks and would produce
too many false positives, so they are left untouched.
The helper atxHeadingLevel is intentionally narrow (ATX only, requires
a trailing space per CommonMark, caps at level 6) and firstProseHeadingLevel
reuses the existing fence-tracking helpers so fenced code samples are
skipped consistently with the other warnings.
github-actions
Bot
added
domain/ccm
📝 WalkthroughWalkthroughAdds heading-level validation to docs update warnings: when Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #619 +/- ##
==========================================
+ Coverage 60.05% 60.10% +0.04%
==========================================
Files 406 406
Lines 43089 43147 +58
==========================================
+ Hits 25877 25933 +56
- Misses 15194 15195 +1
- Partials 2018 2019 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (1)
shortcuts/doc/docs_update_check.go (1)
23-47: Update the warning inventory comment.
docsUpdateWarningsnow emits a third warning, but the “Warnings emitted” comment still documents only the multiline and bold+italic checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_update_check.go` around lines 23 - 47, The top-of-file "Warnings emitted" comment is out of date: docsUpdateWarnings now can emit a third warning from checkDocsUpdateReplaceHeadingBlockType but the comment only lists the multiline and bold+italic warnings; update that inventory to include a third entry describing the replace-heading/block-type warning (mentioning replace_range/replace_all behavior when headings change block type or selectionByTitle interactions) so the comment matches the current behavior of docsUpdateWarnings and the three helper functions checkDocsUpdateReplaceMultilineMarkdown, checkDocsUpdateBoldItalic, and checkDocsUpdateReplaceHeadingBlockType.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/doc/docs_update_check.go`:
- Around line 86-101: The atxHeadingLevel function currently trims all leading
whitespace and misclassifies headings; update it to call fenceIndentOK on the
original line to enforce CommonMark 0–3 leading spaces rule, then scan the run
of '#' to compute level, allow either a space or a tab (or nothing for empty
heading) after the hashes (i.e., accept "##\tH2" and "##" as valid), and return
the computed level for empty headings instead of 0; reference the
atxHeadingLevel function and the fenceIndentOK helper and add tests for " ##
code": 0, "\t## code": 0, "##\tH2": 2 and "##": 2.
---
Nitpick comments:
In `@shortcuts/doc/docs_update_check.go`:
- Around line 23-47: The top-of-file "Warnings emitted" comment is out of date:
docsUpdateWarnings now can emit a third warning from
checkDocsUpdateReplaceHeadingBlockType but the comment only lists the multiline
and bold+italic warnings; update that inventory to include a third entry
describing the replace-heading/block-type warning (mentioning
replace_range/replace_all behavior when headings change block type or
selectionByTitle interactions) so the comment matches the current behavior of
docsUpdateWarnings and the three helper functions
checkDocsUpdateReplaceMultilineMarkdown, checkDocsUpdateBoldItalic, and
checkDocsUpdateReplaceHeadingBlockType.
🪄 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: 7bba7985-e5ff-45a9-97b8-bd6c00b31160
📒 Files selected for processing (3)
shortcuts/doc/docs_update.goshortcuts/doc/docs_update_check.goshortcuts/doc/docs_update_check_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@13829fa11f1c1b25ca74d413e07d1f0230e83bea🧩 Skill updatenpx skills add larksuite/cli#feat/docs-update-heading-warning -y -g |
CodeRabbit flagged that the first-cut atxHeadingLevel used strings.TrimSpace,
which silently accepted 4+ leading spaces and leading tabs (CommonMark treats
both as indented code blocks, not headings) and rejected two valid forms:
empty headings (e.g. '##') and hash-tab-text ('##\tTitle').
Rewrite using the existing fenceIndentOK helper so leading-space handling
stays consistent with the fence-open/close logic, accept space or tab as the
closing delimiter of the opening hash run, and treat a bare '#' run with no
trailing content as a valid empty heading.
Callers that accept whitespace-tolerant user input (the selection-by-title
flag) now trim explicitly before calling the helper; the markdown-payload
caller keeps raw lines so leading-space semantics stay CommonMark-accurate.
Also fix a golangci-lint copyloopvar warning in the new table-driven test
(redundant 'tc := tc' on Go 1.22+).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/doc/docs_update_check.go (1)
17-37:⚠️ Potential issue | 🟡 MinorUpdate the warning inventory comment for the new third check.
The comment still says “Both checks” and lists only two warnings, but Line 45 now adds the heading block-type warning.
Suggested doc-comment update
-// Both checks ignore fenced code blocks (```…``` and ~~~…~~~, with up -// to 3 leading spaces per CommonMark §4.5), inline code spans, and -// backslash-escaped emphasis markers so that literal Markdown content -// embedded in code samples or escaped prose does not produce false -// positives. +// The checks avoid fenced code blocks where relevant (```…``` and +// ~~~…~~~, with up to 3 leading spaces per CommonMark §4.5). The +// bold+italic check also ignores inline code spans and backslash-escaped +// emphasis markers so literal Markdown samples or escaped prose do not +// produce false positives. @@ // 2. Lark does not round-trip bold+italic. Six shapes are detected: // ***text*** ___text___ // **_text_** __*text*__ // _**text**_ *__text__* // Lark stores only one of the two emphases (usually italic), silently // dropping the other. The user wanted both; they will get one. +// +// 3. replace_range with --selection-by-title targeting a heading does not +// change block type. If --markdown does not lead with a matching ATX +// heading level, the target keeps its existing heading styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_update_check.go` around lines 17 - 37, The file comment for docsUpdateWarnings is out of date: it still says “Both checks” and lists only two warnings while a third ("replace_range with --selection-by-title targeting a heading does not change block type") was added; update the doc-comment above func docsUpdateWarnings to describe three checks, reword the fenced-code/inline-code coverage as suggested (mention fenced blocks and that the bold+italic check ignores inline code spans and backslash-escaped emphasis), and append the new third warning about replace_range/--selection-by-title leaving heading block type unchanged so the comment matches the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/doc/docs_update_check.go`:
- Around line 96-115: The atxHeadingLevel function fails on CRLF-terminated
empty ATX headings because body can end with '\r' (e.g., "##\r"); update
atxHeadingLevel (and its use of fenceIndentOK/body inspection) to either trim a
trailing '\r' from body before analysis or treat '\r' as whitespace in the final
switch (i.e., consider '\r' like ' ' and '\t'), and add a regression test that
feeds a CRLF payload whose first prose line is an empty heading like "##\r\n" to
ensure firstProseHeadingLevel/atxHeadingLevel returns 2 instead of 0.
---
Outside diff comments:
In `@shortcuts/doc/docs_update_check.go`:
- Around line 17-37: The file comment for docsUpdateWarnings is out of date: it
still says “Both checks” and lists only two warnings while a third
("replace_range with --selection-by-title targeting a heading does not change
block type") was added; update the doc-comment above func docsUpdateWarnings to
describe three checks, reword the fenced-code/inline-code coverage as suggested
(mention fenced blocks and that the bold+italic check ignores inline code spans
and backslash-escaped emphasis), and append the new third warning about
replace_range/--selection-by-title leaving heading block type unchanged so the
comment matches the implementation.
🪄 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: f63604df-7c39-42f2-953f-fa6e6814e380
📒 Files selected for processing (2)
shortcuts/doc/docs_update_check.goshortcuts/doc/docs_update_check_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/doc/docs_update_check_test.go
This was referenced Apr 23, 2026
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
Addresses Case 2 in the lark-cli pitfall list:
replace_rangecannot change a block's type, so using it to "demote" a heading block to a paragraph silently fails — the API returns success but the heading styling persists.This PR extends the existing
docsUpdateWarningsinfrastructure (from the PR #569 pre-write semantic check series) with a targeted, high-signal warning: when the invocation is--mode=replace_range --selection-by-title "## X"and--markdowndoes not lead with a matching heading level, the user almost certainly expects a block-type change that won't happen.Changes
shortcuts/doc/docs_update_check.go:checkDocsUpdateReplaceHeadingBlockType— fires only onreplace_range+ heading-style--selection-by-title+ non-matching first-prose-line heading levelatxHeadingLevel(ATX-only, CommonMark-conformant) andfirstProseHeadingLevel(reuses existing fence-tracking helpers)docsUpdateWarningsextended withselectionByTitleargumentshortcuts/doc/docs_update.go: pass--selection-by-titlethrough todocsUpdateWarningsshortcuts/doc/docs_update_check_test.go: table-driven tests (8 cases on the check + 9 on the ATX helper); 100% coverage on the new functionScope notes
The warning is deliberately narrow:
--selection-by-titleis used. Other selection modes (--selection-with-ellipsis, bare token) can target any block and would produce false positives.Test Plan
go test ./shortcuts/doc/...passesgo vet ./...cleangofmt -lcleancheckDocsUpdateReplaceHeadingBlockTypeis 100%Summary by CodeRabbit
Bug Fixes
Tests