Skip to content

fix: preserve heading auto-numbering in im-markdown and warn for plain markdown#2038

Open
godququ5-code wants to merge 2 commits into
larksuite:mainfrom
godququ5-code:fix/docs-markdown-heading-seq
Open

fix: preserve heading auto-numbering in im-markdown and warn for plain markdown#2038
godququ5-code wants to merge 2 commits into
larksuite:mainfrom
godququ5-code:fix/docs-markdown-heading-seq

Conversation

@godququ5-code

@godququ5-code godququ5-code commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Why

Heading auto-numbering was silently lost in im-markdown output, and plain markdown has no way to carry it. Callers (including AI agents) need either correct numbering or an explicit, machine-readable signal to re-fetch.

Root cause

  • --doc-format markdown: the server returns literal markdown and drops seq/seq-level server-side; the CLI never receives the data, so it cannot be recovered locally. → handled via a warning (Option B).
  • --doc-format im-markdown: the server returns HTML5/DocxXML that does contain seq="auto" seq-level="auto". The renderer already preserved seq for list items, but handleIMMarkdownHeading ignored its attrs argument and discarded the numbering. → fixed in-CLI (Option A).

Test plan

  • Added TestConvertToIMMarkdownHeadingSeq: auto-numbered h1/h2 with seq="auto" seq-level="auto" render as # 1. Section A / ## 1.1. Section A.1 (and nesting resets deeper counters: 1., 1.1., 1.2., 2., 2.1.). Explicit seq and empty-heading regressions covered.
  • Updated TestDocsFetchMarkdownDetailDowngradeWarnsInOutput to expect the new heading-seq warning alongside the existing detail-downgrade warning.
  • go test ./shortcuts/doc/... passes; go vet ./shortcuts/..., gofmt -l ., and go mod tidy are clean.

Changed files

  • shortcuts/doc/docs_fetch_im_markdown.go (heading seq state + handleIMMarkdownHeading/resolveIMMarkdownHeadingSeq)
  • shortcuts/doc/docs_fetch_im_markdown_test.go (TestConvertToIMMarkdownHeadingSeq)
  • shortcuts/doc/docs_fetch_v2.go (addFetchMarkdownHeadingSeqWarning + call site)
  • shortcuts/doc/docs_fetch_v2_test.go (expect new warning)

Related Issues

Summary by CodeRabbit

  • New Features

    • Added automatic hierarchical numbering for headings in converted IM Markdown.
    • Added support for explicit heading sequence values, including proper normalization and per-heading sequencing overrides (via seq-level).
  • Bug Fixes

    • Heading counters now maintain and reset correctly across heading levels during a single conversion.
  • Documentation

    • Added a CLI warning for plain Markdown output noting that automatic heading numbering may be lost.
  • Tests

    • Added coverage for heading sequencing behavior and seq-level precedence.

…n markdown

Option A: handleIMMarkdownHeading now reads the seq/seq-level attrs emitted by the server for --doc-format im-markdown. seq="auto" resolves against a per-level running counter that resets deeper levels (1., 1.1., 1.2., 2., ...); an explicit seq value is used verbatim, mirroring the existing ordered-list-item logic. Capped at 6 levels like before.

Option B: for --doc-format markdown the server returns literal markdown and drops seq/seq-level server-side, so the data is unrecoverable locally. addFetchMarkdownHeadingSeqWarning appends a warnings entry (reusing appendDocWarning, same precedent as addFetchDetailDowngradeWarning) telling callers to re-fetch with --doc-format xml or im-markdown.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42441951-2230-4f73-bbd8-ae5a153e1d1f

📥 Commits

Reviewing files that changed from the base of the PR and between 98db11c and 043cc2f.

📒 Files selected for processing (2)
  • shortcuts/doc/docs_fetch_im_markdown.go
  • shortcuts/doc/docs_fetch_im_markdown_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • shortcuts/doc/docs_fetch_im_markdown_test.go
  • shortcuts/doc/docs_fetch_im_markdown.go

📝 Walkthrough

Walkthrough

Adds hierarchical heading numbering to IM Markdown conversion and warns when plain Markdown fetches lose seq/seq-level metadata. Tests cover automatic, explicit, absent, and empty heading sequences plus combined warnings.

Changes

Markdown heading sequence support

Layer / File(s) Summary
Heading sequence conversion
shortcuts/doc/docs_fetch_im_markdown.go, shortcuts/doc/docs_fetch_im_markdown_test.go
Tracks heading-depth counters, resolves heading levels, renders automatic or explicit prefixes, resets deeper levels, and validates conversion behavior.
Fetch warning integration
shortcuts/doc/docs_fetch_v2.go, shortcuts/doc/docs_fetch_v2_test.go
Adds and prints a warning for heading-numbering loss in plain Markdown output, with assertions for both fetch warnings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant FetchCommand
  participant executeFetchV2
  participant convertToIMMarkdown
  participant MarkdownOutput
  FetchCommand->>executeFetchV2: request Markdown document
  executeFetchV2->>convertToIMMarkdown: convert document headings
  convertToIMMarkdown-->>MarkdownOutput: emit numbered headings
  executeFetchV2-->>FetchCommand: append and print heading-sequence warning
Loading

Possibly related PRs

  • larksuite/cli#1466 — Updates related warning emission and assertions in the document fetch flow.
  • larksuite/cli#1550 — Modifies the IM Markdown heading conversion path used by this change.

Suggested reviewers: fangshuyu-768

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: preserving heading auto-numbering in im-markdown and warning for plain markdown.
Description check ✅ Passed The description covers summary, motivation, test plan, and related issue; the template's Changes section is implied but not explicitly named.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact labels Jul 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@shortcuts/doc/docs_fetch_im_markdown_test.go`:
- Around line 247-260: Add a focused test case in the existing document-fetch
Markdown tests that uses server-provided seq-level values to determine numbering
prefixes and reset behavior, rather than relying only on h1/h2 nesting. Ensure
the expected Markdown verifies seq-level is honored, including a level
transition or reset that would fail if seq-level were ignored.

In `@shortcuts/doc/docs_fetch_im_markdown.go`:
- Around line 293-318: The automatic numbering in resolveIMMarkdownHeadingSeq
currently uses only the heading level; update it to parse and honor the
server-provided seq-level attribute for sequence depth, including counter
incrementing and deeper-level resets, while preserving the existing
heading-level fallback when seq-level is absent or invalid. In
shortcuts/doc/docs_fetch_im_markdown_test.go lines 247-260, add a fixture where
seq-level differs from the heading tag and assert the resulting numbering
reflects seq-level.
🪄 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 Plus

Run ID: 166a459c-77ee-407a-b04e-ad080bb806ff

📥 Commits

Reviewing files that changed from the base of the PR and between ebc0c53 and 98db11c.

📒 Files selected for processing (4)
  • shortcuts/doc/docs_fetch_im_markdown.go
  • shortcuts/doc/docs_fetch_im_markdown_test.go
  • shortcuts/doc/docs_fetch_v2.go
  • shortcuts/doc/docs_fetch_v2_test.go

Comment thread shortcuts/doc/docs_fetch_im_markdown_test.go
Comment thread shortcuts/doc/docs_fetch_im_markdown.go
Read the seq-level attribute to determine a heading's depth (markdown
'#' count and auto-numbering counter), falling back to the <hN> tag
level when seq-level is absent or 'auto'. This preserves the
seq/seq-level metadata the server attaches to headings (see larksuite#1781)
and addresses a CodeRabbit review suggestion on the PR.

Co-Authored-By: WorkBuddy <noreply@workbuddy.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant