Blank lines between blocks vanish when CLI-posted content is edited in Basecamp

[jorgemanrubia]

Content posted through the CLI displays correctly, then loses every blank line between blocks the first time someone opens it in Basecamp, makes any edit and saves. Paragraphs, headings and lists all end up flush against each other, and the spacing is gone for good.

The CLI spelled that blank line as a bare top-level <br>: the Markdown pipeline emitted one for each blank line between blocks. A bare top-level <br> is not a block in the editor's document model, so it can never be a child of the root — the editor drops it on import, deliberately, in lexxy src/elements/editor.js:

  // Whitespace-only text nodes (e.g. "\n" between block elements like <div>) and stray line break
  // nodes are formatting artifacts from the HTML source. They can't be appended to the root node
  // and have no semantic meaning, so we strip them during import.
  #isNotWhitespaceOnlyNode(node) {
    if ($isLineBreakNode(node)) return false

The editor's own blank line is an empty paragraph, <p><br></p>. This emits that instead, so the separator round-trips. Inside a blockquote the break stays a <br> — there it is inline content, which the editor preserves. The raw-HTML passthrough from #527 now inserts the same durable separator between adjacent paragraphs.

Evidence

Driven through lexxy v0.9.29 (the version bc3 pins), using its real Lexical import/export, on the same document each time — separator count before vs. after one edit:

• <p>Para one.</p><br><p>Para two.</p><br><h2>Heading</h2>… (what the CLI stored before this change) — 4 separators, 0 after the edit.
• <p>Para one.</p><p><br></p><p>Para two.</p><p><br></p><h2>Heading</h2>… (what it stores now) — 4 separators, 4 after the edit, and byte-identical on a second round trip.

Same harness for the blockquote cases: <blockquote>A<br>B</blockquote> and <blockquote><p>A</p><br><p>B</p></blockquote> both survive untouched, which is why nested breaks are left alone.

Scope

Separators the caller supplied in raw HTML are left exactly as they came. Only genuinely adjacent paragraphs get a separator inserted. The paragraph matching here is regex-based and not nesting-aware, so rewriting existing <br>s would reach into blockquotes and replace inline content that the editor already preserves. Normalizing caller-supplied HTML would need a real parser and is deliberately not part of this change: what this fixes is the CLI emitting a separator the editor cannot keep.

bc3 #11986 does not cover this

Markdown-in/markdown-out for the v1 API (basecamp/bc3#11986) looks adjacent but leaves this bug where it is:

• It touches no client-side code — no JS, no assets, no lexxy pin — so the import filter that drops the separators is byte-identical on that branch.
• Its conversion only fires when a request submits the new rich_text_format: markdown hint. The CLI submits HTML, so what gets stored is unchanged.
• If the CLI were rewired to markdown-in, its Commonmarker.to_html invocation emits adjacent <p> with no separator at all, and bc3's .formatted_content sets p, div { margin: 0 } — so the blank lines would be lost at write time instead of at edit time. Different mechanism, same missing spacing.

[Copilot]

Pull request overview

Updates rich-text separators so blank lines survive Basecamp editor round-trips.

Changes:

• Emits durable empty paragraphs for top-level Markdown separators.
• Rewrites raw-HTML paragraph separators and expands regression tests.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/richtext/richtext.go	Implements durable separator rendering and rewriting.
internal/richtext/richtext_test.go	Updates expectations and adds separator regressions.

Suppressed comments (1)

internal/richtext/richtext.go:436

• The paragraph regex is not aware of HTML nesting, so this also rewrites valid nested breaks that the new renderer deliberately preserves. For example, <blockquote><p>A</p><br><p>B</p></blockquote> becomes <blockquote><p>A</p><p><br></p><p>B</p></blockquote>, although a <br> inside a blockquote is legal inline content and should survive editing. Restrict separator rewriting to direct children of the document root (preferably via an HTML parser) and leave nested boundaries unchanged.

			b.WriteString(reBR.ReplaceAllString(gap, ""))
			b.WriteString(paragraphSeparator)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[robzolkos]

Validated this against the Basecamp card and Lexxy v0.9.29. The existing top-level <br> separators collapse from 4 to 0 on an editor round trip; the <p><br></p> separators emitted here survive 4 to 4, while nested blockquote <br> remains intact. The focused rich-text tests and full bin/ci pass. The documented caller-supplied raw HTML limitation does not block this fix. Approved.

[jeremy]

Pushed two commits on top of Jorge's branch — not merging, leaving that to a human:

93ea1e8 — merge origin/main. The Security / Trivy failures were unrelated to this change: the branch was ~96 commits behind with go 1.26.4 in go.mod, so govulncheck ran on go1.26.5 and flagged 7 stdlib vulns all "fixed in 1.26.6". main is on go 1.26.5 / toolchain go1.26.7 and its Security job is green; the merge picks that up. Merged (not rebased) so the existing review history stays anchored; the repo squash-merges anyway. richtext.go/richtext_test.go auto-merged cleanly against #498/#462/#479/#532.

5b99795 — same bug class, different producer: PlainToHTML. #462 added chat update --content-type text/plain, which serialized a\nb\n\nc as a<br>b<br><br>c — bare root-level <br>s with no block wrapper. Under the same lexxy import filter that motivated this PR, every root-level <br> is dropped on first edit: single breaks became separate squished paragraphs and blank lines vanished. Lexical keeps a <br> only when it sits between two inline nodes inside a block, so the one durable shape is <p>a<br>b</p><p><br></p><p>c</p> — runs of lines as a <p> with <br> inside, each blank line as the same <p><br></p> separator this PR introduces. Leading/trailing blank lines are dropped (matching the Markdown path); whitespace-only lines count as blank. It also round-trips through HTMLToMarkdown unchanged (test added), which the one-paragraph-per-line alternative would not. The chat update test now asserts the break is inline within the paragraph.

Also resolved the outdated Copilot thread with the "caller-supplied separators are left alone" rationale written into it.

One adjacent observation, not addressed here: a Markdown table never gets a separator before it (Intro.\n\n| a |… → <p>Intro.</p>\n<table>…</table>\n<p><br></p>\n<p>After.</p>) because goldmark's table transformer doesn't carry HasBlankPreviousLines over from the paragraph it replaces. Pre-existing from #498, independent of the separator spelling — worth its own small follow-up.

bin/ci green locally on both commits.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/richtext/richtext.go:500

• trimBlankLines now discards user-supplied boundary newlines. For example, plain-text input "a\n" becomes <p>a</p>, even though chat update --content-type text/plain promises verbatim text with line breaks preserved (internal/commands/chat.go:794-796,1032). Preserve leading/trailing blank lines as durable empty paragraphs too, while special-casing only the truly empty input.

	lines := trimBlankLines(strings.Split(escapeHTML(s), "\n"))