Skip to content

fix(editor): keep a blockquote's blank line and its nesting on the way back - #1886

Merged
h4yfans merged 3 commits into
mainfrom
quote-region-byte-roundtrip
Aug 28, 2026
Merged

fix(editor): keep a blockquote's blank line and its nesting on the way back#1886
h4yfans merged 3 commits into
mainfrom
quote-region-byte-roundtrip

Conversation

@h4yfans

@h4yfans h4yfans commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

A blockquote's own structure never survived a save. > A\n>\n> B came back as > A\n> B, and a nested > > [!warning] callout came back unnested. On an Obsidian vault that is a rewrite of a file Memry never wrote (#1881, and the byte-compat guarantee #1846 exists to protect). #1875 deliberately declined these shapes from the callout claim, which left them on the generic quote path where the damage happens.

Serialization was never the culprit. BlockNote parses a whole blockquote into ONE block of inline content, so the blank > separator and the > > level are gone before serialization runs. Measured: > [!info]\n> One\n>\n> Two parses to text: "[!info]\nOne\nTwo\n". BlockNote's quote block cannot hold the structure either, since its children serialize outside the quote (> A\n\nB) and a \n\n in content becomes backslash hard breaks. So the fix is at the parse seam, not the serializer.

A blockquote run carrying a blank > separator or a > > level is now claimed whole. One > level comes off, the inside parses as ordinary markdown, and the first block becomes the quote's content with the rest as its children. serializeQuote writes that list back inside the blockquote, one > per line and a bare > per gap.

The claim is proven, not pattern-matched, the same rule resolveCalloutRun already applies: re-serializing the parse must reproduce the run byte-for-byte, or the run is left exactly where it was. A run with neither shape is already flat and never enters this path, so no quote that round-trips today changes.

Reviewer notes:

  • No new node type. quote and paragraph only, so a doc written by this build still opens on an older install rather than hitting y-prosemirror's delete-unknown-node repair.
  • Both pipelines share the seam. splitMarkdownByCallouts is now splitMarkdownByBlockquoteRuns and emits quote runs alongside callout runs; the main process claims in its existing line loop.
  • Blockquote blank lines collapse on write-back, merging quoted paragraphs #1881 stays open. A lazily continued > Outer\n> > Inner (no blank line between the levels) only comes back through a separator the author never wrote, so it declines on the byte proof and keeps today's flat behavior. It is pinned as a new pending corpus case rather than left undocumented.
  • First commit adds the failing shapes to the corpus as pending cases; the second lands the fix and removes the flags. Each commit is green on its own.

Release note

Quotes with more than one paragraph, and quotes nested inside other quotes, now survive a save instead of being flattened. Notes written in Obsidian with these shapes are written back byte for byte.

Test plan

  • vitest --project main src/main/sync/blocknote-converter.roundtrip.test.ts — 59 passed, 3 expected fail (the remaining pending cases: Blockquote blank lines collapse on write-back, merging quoted paragraphs #1881 lazy continuation, Unterminated toggle drops its open and summary lines on write-back #1883, Blank-line gaps adjacent to a toggle collapse on write-back #1877)
  • vitest --project renderer src/renderer/src/components/note/content-area/roundtrip-conformance.test.ts — 51 passed, 5 expected fail
  • New golden fixture roundtrip-quotes.md round-trips byte-identical through the converter
  • New unit tests for readStructuredQuoteRun / serializeQuoteBlock covering the decline rules (flat run, >text line, non-quote line)
  • Full desktop suites: 10206 passed | 4 expected fail (main + shared), 8524 passed | 5 expected fail (renderer)
  • tsc --noEmit on tsconfig.node.json and tsconfig.web.json, eslint and prettier clean on touched files
  • pnpm docs:impact --base <base> --strict covered, pnpm docs:build passes
  • Rebased onto current main and re-run against its new raw-html-roundtrip.probe.test.ts suite

…loses

A blockquote is parsed into ONE block of inline content, so its blank `>`
separator and its `> >` nesting are gone before serialization runs. Every
shape that costs is a corpus case now, marked pending against #1881: a
callout with a multi-paragraph body, nested foreign callouts, a plain quote
with a blank separator, and a quote holding a list or a code fence.

The two already-pending cases keep their flag; the three new ones join them,
so both pipelines assert the same bytes the moment the fix lands.
…y back

BlockNote's quote block is one flat run of inline content, so a blockquote's
own structure never reaches serialization: `> A\n>\n> B` came back `> A\n> B`
and a nested `> > [!warning]` callout came back unnested. On an Obsidian vault
that is a rewrite of a file Memry never wrote (#1881, #1846).

Serialization was never the culprit, so the fix is at the parse seam. A
blockquote run carrying a blank `>` separator or a `> >` level is now claimed
whole: one `>` level comes off, the inside parses as ordinary markdown, and the
first block becomes the quote's content with the rest as its children.
`serializeQuote` writes that list back inside the blockquote, one `> ` per line
and a bare `>` per gap, instead of BlockNote's `> A\n\nB`.

The claim is proven, not pattern-matched, the same rule `resolveCalloutRun`
already applies: re-serializing the parse must reproduce the run byte-for-byte
or the run is left exactly where it was. A lazily continued `> A\n> > B`
declines on that test and stays flat, so it keeps a pending case.

Both pipelines get the same seam. `splitMarkdownByCallouts` is now
`splitMarkdownByBlockquoteRuns` and emits quote runs alongside callout runs.

No new node type, so a doc written here still opens on an older install.

Verified: main and renderer conformance corpora green with the pending flags
off, new `roundtrip-quotes.md` golden fixture byte-identical, full desktop
suites green (10206 main/shared, 8524 renderer).
@github-actions github-actions Bot added bug Something isn't working documentation Improvements or additions to documentation test labels Aug 28, 2026
@github-actions

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 49c0ae8.

@h4yfans
h4yfans merged commit 23d2d0f into main Aug 28, 2026
15 checks passed
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

h4yfans added a commit that referenced this pull request Aug 30, 2026
`packages/editor-schema/src` is hashed into the asset's freshness hash, and for
a reason sharper than cache invalidation: a block or inline spec the mobile
bundle cannot build is deleted from the shared Y.Doc by y-prosemirror, so
shipping a schema change against a stale editor is data loss rather than a
rendering gap. This change alters when a nested quote run is claimed and what
bytes it normalizes to, so mobile has to carry the same rule or the two
disagree about the same file.

The rebuild also absorbs #1886's, which changed the same directory on 28 Aug
without regenerating. `origin/main` fails `editor:check` today because of it:
main's sources hash to 7dca7f129cf02dbe while the checked-in asset stores
083bd3ffeb1b2a64. This branch is now current at 4daa842cfef34145.
h4yfans added a commit that referenced this pull request Aug 30, 2026
#1886 changed `packages/editor-schema/src` on 28 August without rebuilding the
generated mobile editor asset, so `pnpm --filter @memry/mobile editor:check`
has failed on main ever since and every PR opened since inherited the red.

This is not a cosmetic gate. The build script hashes the editor sources, the
shared schema and the bridge contract precisely because y-prosemirror deletes
any block or inline spec the mobile bundle cannot build from the shared Y.Doc.
A schema change shipping with a stale asset is data loss on mobile, not a
rendering gap, and the blockquote change in #1886 is exactly that class.

Pre-push now runs the same check whenever the schema, the bridge contract or
the editor-web sources change, so the rebuild cannot be forgotten again. The
check only hashes files, it does not run a build.
h4yfans added a commit that referenced this pull request Aug 30, 2026
#1881)

#1886 left `> Outer\n> > Inner` pending, on the reading that its inner blocks
only come back through a blank separator the author never wrote. Measured, the
decline costs more than the separator: BlockNote's flat quote path deletes the
`>` level outright, so both pipelines returned `> Outer\n> Inner`, and a foreign
`> Outer\n> > [!warning] Inner` came back with the callout demoted to literal
text in a flat quote. That is the nesting loss #1881 was filed over, in the one
spelling the fix missed.

Byte identity is unreachable here and no code change can buy it. Lazy
continuation parses to the same block tree as the separator form
`> Outer\n>\n> > Inner`, which the corpus already pins as identity, and the tree
has nowhere to record which of the two spellings it was read from. Exactly one
of them can round-trip.

So `resolveQuoteRun` keeps the byte-identity proof as the only way a
blank-separator run is claimed, and gives a run carrying a `> >` level a second
chance: claim it when its canonical form settles, meaning re-reading and
re-parsing those bytes reproduces them. The write normalizes onto the separator
form once and the file stops moving, with the nesting intact. `QuoteRun.nested`
is what distinguishes the two, since only a nested run pays a structural price
for declining.

The corpus gains `canonical` for a case whose input is a spelling the block tree
cannot tell apart from another case's, and both halves of the suite assert those
bytes and then assert the second round-trip changes nothing. The pending flags
come off `lazily continued nested quote`, and the callout spelling from #1881's
own report joins it as a case.

No schema change, no new block type or prop: a doc written by this build opens
unchanged on an older install.
h4yfans added a commit that referenced this pull request Aug 30, 2026
#1894)

* fix(editor): keep a lazily nested quote's level instead of deleting it (#1881)

#1886 left `> Outer\n> > Inner` pending, on the reading that its inner blocks
only come back through a blank separator the author never wrote. Measured, the
decline costs more than the separator: BlockNote's flat quote path deletes the
`>` level outright, so both pipelines returned `> Outer\n> Inner`, and a foreign
`> Outer\n> > [!warning] Inner` came back with the callout demoted to literal
text in a flat quote. That is the nesting loss #1881 was filed over, in the one
spelling the fix missed.

Byte identity is unreachable here and no code change can buy it. Lazy
continuation parses to the same block tree as the separator form
`> Outer\n>\n> > Inner`, which the corpus already pins as identity, and the tree
has nowhere to record which of the two spellings it was read from. Exactly one
of them can round-trip.

So `resolveQuoteRun` keeps the byte-identity proof as the only way a
blank-separator run is claimed, and gives a run carrying a `> >` level a second
chance: claim it when its canonical form settles, meaning re-reading and
re-parsing those bytes reproduces them. The write normalizes onto the separator
form once and the file stops moving, with the nesting intact. `QuoteRun.nested`
is what distinguishes the two, since only a nested run pays a structural price
for declining.

The corpus gains `canonical` for a case whose input is a spelling the block tree
cannot tell apart from another case's, and both halves of the suite assert those
bytes and then assert the second round-trip changes nothing. The pending flags
come off `lazily continued nested quote`, and the callout spelling from #1881's
own report joins it as a case.

No schema change, no new block type or prop: a doc written by this build opens
unchanged on an older install.

* chore(mobile): rebuild the editor-web asset for the quote parse change

`packages/editor-schema/src` is hashed into the asset's freshness hash, and for
a reason sharper than cache invalidation: a block or inline spec the mobile
bundle cannot build is deleted from the shared Y.Doc by y-prosemirror, so
shipping a schema change against a stale editor is data loss rather than a
rendering gap. This change alters when a nested quote run is claimed and what
bytes it normalizes to, so mobile has to carry the same rule or the two
disagree about the same file.

Rebuilt on top of a4756d5, which is where main's own asset staleness was
fixed. This commit carries only this branch's schema change: 7dca7f129cf02dbe
to 4daa842cfef34145.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant