Skip to content

fix(docs): preserve markdown paragraph breaks in tabs#662

Merged
steipete merged 1 commit into
mainfrom
fix/docs-tab-markdown-paragraphs-644
May 30, 2026
Merged

fix(docs): preserve markdown paragraph breaks in tabs#662
steipete merged 1 commit into
mainfrom
fix/docs-tab-markdown-paragraphs-644

Conversation

@steipete
Copy link
Copy Markdown
Collaborator

Summary

  • Preserve interior blank Markdown lines in the local Docs renderer used by docs write --replace --markdown --tab.
  • Collapse repeated blank lines and trim trailing blanks so paragraph separators survive without adding tail noise.
  • Update parser/tab/append regression expectations and changelog.

Fixes #644.

Testing

  • go test ./internal/cmd -run 'TestDocsWrite_MarkdownReplaceWithTab|TestMarkdownToDocsRequests|TestParseMarkdown'
  • go test ./internal/cmd
  • make test
  • Live clawdbot Docs round-trip: wrote issue markdown to a Test tab, exported Markdown, confirmed expected blank-line paragraphs present and collapsed soft-break form absent.
  • Autoreview: clean.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

Codex review: needs changes before merge. Reviewed May 30, 2026, 2:32 PM ET / 18:32 UTC.

Summary
The PR preserves interior blank Markdown lines in the Docs Markdown parser and updates parser, tab-replace, append, and changelog expectations.

Reproducibility: yes. for the original report: the linked issue gives concrete v0.19.0 tab-write round-trip steps, and the PR body reports a live Docs round-trip that confirms the intended paragraph behavior. The table-spacing regression is source-reproducible from the new MDEmptyLine elements plus existing MDTable placeholder/newline handling, but I did not run the API path in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Diff size: 5 files, +20/-7. The patch is small, but it changes a shared Markdown parser used by multiple Docs write modes.
  • Parser consumers: 2 Docs write paths. Both replace and append routes call ParseMarkdown through docs_mutation.go, so the behavior is broader than single-tab replace.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Suppress or normalize MDEmptyLine directly adjacent to MDTable.
  • [P2] Add a regression covering prose, a blank line, a pipe table, another blank line, and prose.

Risk before merge

  • [P1] Markdown containing pipe tables with ordinary blank-line separators may gain extra empty paragraphs before and after native tables if this patch lands as-is.
  • [P1] I did not run Go tests or live Docs calls because this review is read-only; validation is based on source, diff, GitHub discussion, and the PR body's reported test/live run.

Maintainer options:

  1. Fix table-adjacent blank lines before merge (recommended)
    Adjust the parser or formatter so MDEmptyLine does not add extra paragraphs immediately before or after MDTable, then cover prose-table-prose spacing in tests.
  2. Accept changed table spacing intentionally
    If maintainers want blank Markdown lines around tables to become visible empty Docs paragraphs, keep the patch but add explicit tests documenting that new output contract.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Adjust ParseMarkdown/MarkdownToDocsRequests so paragraph-separating MDEmptyLine elements are not emitted directly before or after MDTable, then add a regression test for prose + blank line + pipe table + blank line + prose that keeps exactly the existing native-table spacing while preserving paragraph blank lines between text blocks.

Next step before merge

  • [P2] A narrow repair can keep the paragraph-break fix while correcting table-adjacent blank-line handling and adding the missing regression coverage.

Security
Cleared: The diff only changes Docs Markdown parsing expectations, tests, and a changelog line; I found no security or supply-chain concern.

Review findings

  • [P2] Avoid inserting empty paragraphs around tables — internal/cmd/docs_markdown.go:109-110
Review details

Best possible solution:

Preserve blank-line paragraph separators between text blocks while suppressing or normalizing empty-line elements directly adjacent to native tables, with focused parser and Docs request regression coverage.

Do we have a high-confidence way to reproduce the issue?

Yes for the original report: the linked issue gives concrete v0.19.0 tab-write round-trip steps, and the PR body reports a live Docs round-trip that confirms the intended paragraph behavior. The table-spacing regression is source-reproducible from the new MDEmptyLine elements plus existing MDTable placeholder/newline handling, but I did not run the API path in this read-only review.

Is this the best way to solve the issue?

No as currently written: preserving blank-line elements is the right direction for paragraph text, but the implementation should avoid adding extra empty paragraphs around native tables. A narrower fix should keep paragraph preservation while special-casing or normalizing table-adjacent blank lines.

Full review comments:

  • [P2] Avoid inserting empty paragraphs around tables — internal/cmd/docs_markdown.go:109-110
    Markdown tables are represented as MDTable, and the formatter already writes a placeholder newline before inserting the native table while the table inserter accounts for the Docs API's own table newline. With this change, normal input like prose, blank line, pipe table, blank line, prose parses with MDEmptyLine on both sides of the table, so the rendered Doc gains extra empty paragraphs around the native table instead of preserving existing table spacing. Skip or squash MDEmptyLine adjacent to MDTable and add a regression for prose/table/prose spacing.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6bd333a1e141.

Label changes

Label changes:

  • add P2: This is a normal-priority Docs Markdown rendering bug fix with limited blast radius and a concrete patch-level regression to address.
  • add merge-risk: 🚨 compatibility: The parser change can alter existing Markdown-to-Docs output by adding extra empty paragraphs around native pipe tables.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR is from a collaborator, so the external-contributor proof gate does not apply; the body still reports a live Docs round-trip for the main paragraph behavior.

Label justifications:

  • P2: This is a normal-priority Docs Markdown rendering bug fix with limited blast radius and a concrete patch-level regression to address.
  • merge-risk: 🚨 compatibility: The parser change can alter existing Markdown-to-Docs output by adding extra empty paragraphs around native pipe tables.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR is from a collaborator, so the external-contributor proof gate does not apply; the body still reports a live Docs round-trip for the main paragraph behavior.
Evidence reviewed

Acceptance criteria:

  • [P1] go test ./internal/cmd -run 'TestParseMarkdown|TestMarkdownToDocsRequests|TestDocsWrite_MarkdownReplaceWithTab|TestDocsWrite_MarkdownAppend'.
  • [P1] go test ./internal/cmd.

What I checked:

  • Repository policy read: AGENTS.md was read fully and its PR review-mode guidance applied: inspect PRs read-only with gh pr view/gh pr diff style review and do not switch branches or edit code. (AGENTS.md:1, 6bd333a1e141)
  • Current parser drops blank lines: On current main, ParseMarkdown ignores empty lines, which explains why paragraph-separating blank lines do not survive the local renderer path. (internal/cmd/docs_markdown.go:107, 6bd333a1e141)
  • PR emits MDEmptyLine elements: The PR patch appends MDEmptyLine for interior blank lines and trims a trailing empty element, then updates tab/append expectations to include the extra newline. (internal/cmd/docs_markdown.go:108, 58fe96e3f39e)
  • Table renderer already uses placeholder newlines: MarkdownToDocsRequests writes a placeholder newline for every MDTable, so adding explicit empty-line elements around tables changes spacing beyond normal paragraph preservation. (internal/cmd/docs_formatter.go:301, 6bd333a1e141)
  • Table insertion adds its own newline behavior: The table inserter comments document that Google Docs inserts a newline before an inserted table and that the markdown path already pre-inserts a placeholder newline. (internal/cmd/docs_table_inserter.go:159, 6bd333a1e141)
  • Related issue remains open with concrete repro: The linked issue reports v0.19.0 docs write --replace --markdown --tab collapsing paragraph blank lines into soft-break Markdown on round-trip export.

Likely related people:

  • steipete: Current main blame and history show recent work across the shared docs mutation, Markdown formatter, tab-aware editing, and release state for this path. (role: recent area contributor; confidence: high; commits: b25a3c029b37, 325fae564607, 6867fe850c7b; files: internal/cmd/docs_markdown.go, internal/cmd/docs_formatter.go, internal/cmd/docs_mutation.go)
  • Sebastian Roth: Introduced the tab Markdown replace test surface and native Markdown bullets/code rendering that this PR changes around. (role: feature introducer; confidence: medium; commits: 58b866e9ac55, b646e886f6dd; files: internal/cmd/docs_write_markdown_tab_test.go, internal/cmd/docs_formatter.go, internal/cmd/docs_write_markdown_test.go)
  • Gonçalo Alves: Git history points to the original docs update/editing command work that introduced the broader parser/editing area. (role: original docs editing contributor; confidence: medium; commits: 7945602f1526; files: internal/cmd/docs_markdown.go, internal/cmd/docs_mutation.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58fe96e3f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +109 to +110
if len(elements) > 0 && elements[len(elements)-1].Type != MDEmptyLine {
elements = append(elements, MarkdownElement{Type: MDEmptyLine})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid doubling blank lines around native tables

When Markdown contains the usual blank line before or after a pipe table, this unconditional MDEmptyLine is emitted adjacent to MDTable. The table renderer already writes a placeholder newline for every table (docs_formatter.go:301-303), and the table inserter documents that Google Docs also inserts a newline before the table (docs_table_inserter.go:159-162), so inputs like Intro\n\n| A |...\n\nOutro now gain extra empty paragraphs around the native table rather than preserving the original spacing.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 30, 2026
@steipete steipete merged commit e4941e7 into main May 30, 2026
9 checks passed
@steipete steipete deleted the fix/docs-tab-markdown-paragraphs-644 branch May 30, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs write --replace --markdown --tab: paragraph blank lines collapse into soft line breaks

1 participant