amadan
app,core: fix round 1 — title-echo drop is conditional, Store.Text repointed to fragment_text
Two review findings on Task 3.
(1) fixtures/generate-seed.mjs dropped every seed document's opening
block unconditionally, on the assumption it always echoed the title.
letter.txt's opening line ("Dear Members of the Harbourview Allotment
Committee,") is real content, not a title echo, and an unconditional
drop would have deleted it. blocksFor now drops the first block only
when it exactly matches the document's own Title (seed.go's seedDocs),
compared after classification so index-0's heading exclusion still
protects "Early years" and friends. letter.update.bin is unchanged;
the other five .update.bin fixtures lost exactly their echoed title
paragraph.
(2) Store.Text had zero production callers and read CRDT root
"content" (text_content), which no real document has written to since
seed data (and, later, the editor) moved to root "prosemirror" — it
returned "" for a real document with no error, a silent-empty trap.
core/src/abi.rs's text_string now reads fragment_text instead, so
docs_core_text_len/docs_core_text_read — and therefore (*Handle).Text
and Store.Text — return a document's real prose again. No ABI version
bump: same signatures, no version skew between separately-shipped
host/module. text_content itself is untouched and still backs the
Rust<->JS Y.Text wire fixtures directly.
Every test that applied a fixtures/data (bare Y.Text) update and then
read it back through Text/(*Handle).Text needed a fragment-shaped
fixture instead, since that pairing now reads back "" through the
ABI's repointed ext port. Added fixtures/vectors/fragment/{hello,
unicode,formatted,concurrent}.{update.bin,expected.json} (fixtures/
generate-fragment.mjs's new "trap-containment vectors" section) and
repointed: internal/docs/testing.go's new ReadFragmentFixture (used by
store_test.go), internal/core/core_test.go's fixture(), and
internal/core/core_internal_test.go's new loadFragmentFixtureBytes/
loadFragmentFixtureExpectedText (used only where content is read back
and checked — TestSlowUpdateIsCutOffByGuestCallTimeout's own RNG-
mutation loop and its wantHex reproducer stay on fixtures/data
unchanged, since that reproducer is pinned to those exact bytes).
core/tests/wire_compat.rs's abi_round_trips_a_fixture and core/tests/
wasm_trap_containment.rs's fixtures_dir also moved to the fragment
vectors for the same reason.
Added TestSeedTextReturnsRealProse (internal/docs/seed_test.go),
watched it fail against the old ABI before repointing it.
Rebuilt and re-embedded internal/core/testdata/docs_core.wasm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA
24 files changed,
+398
−23
core/src/abi.rs+31 −3core/tests/wasm_trap_containment.rs+17 −1core/tests/wire_compat.rs+24 −1fixtures/generate-fragment.mjs+115 −1fixtures/generate-seed.mjs+49 −8fixtures/vectors/fragment/concurrent.expected.json+3 −0fixtures/vectors/fragment/concurrent.update.binbinaryfixtures/vectors/fragment/formatted.expected.json+3 −0fixtures/vectors/fragment/formatted.update.binbinaryfixtures/vectors/fragment/hello.expected.json+3 −0fixtures/vectors/fragment/hello.update.binbinaryfixtures/vectors/fragment/unicode.expected.json+3 −0fixtures/vectors/fragment/unicode.update.binbinaryinternal/core/core_internal_test.go+56 −5internal/core/core_test.go+14 −1internal/core/testdata/docs_core.wasmbinaryinternal/docs/seed_test.go+44 −0internal/docs/seeddata/documentation.update.binbinaryinternal/docs/seeddata/history.update.binbinaryinternal/docs/seeddata/meeting-notes.update.binbinaryinternal/docs/seeddata/packing-list.update.binbinaryinternal/docs/seeddata/proposal.update.binbinaryinternal/docs/store_test.go+12 −3internal/docs/testing.go+24 −0
Intent
Human
<task-notification> <task-id>ad73c1c7c1f244926</task-id> <tool-use-id>toolu_01PbJCy4ZpDPNDWMMQM4VgWD</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-carlos-docs/70f302d1-9d5d-4eb2-8b38-e09a15fdd1c3/tasks/ad73c1c7c1f244926.output</output-file> <status>completed</status> <summary>Agent "Editing Task 3 fragments and read view" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>**Status: done.** Commit `c476a6a` on branch `editing`. **Tests:** `go test ./...` and `.amadan/ci.d/10-gate` both green. New tests `TestSeededDocumentsRenderAsHTML`/`TestWaterworksHistoryHasHeadings` pass. Three pre-existing tests asserted seeded content via `Store.Text`, which now legitimately reads empty (seed content moved to root "prosemirror"); moved them to `Store.HTML` and tightened `TestSeedContentMatchesItsSource` to also require real `<p>` markup rather than just non-empty text. **Rendered HTML** (Millbrook waterworks opening): ```html <p>A History of the Millbrook Waterworks</p><h2>Early years</h2><p>Before 1887, Millbrook drew its water the way most small towns did: from private wells, a handful of shared pumps on the main street...</p> ``` Five `<h2>`s render correctly (Early years, Construction and the first reservoir, Expansion through the early twentieth century, Modernization, The works today). **Honest read:** Signed in for real via magic link, screenshotted both the long and short documents in Chromium. It reads like a document — comfortable 6.5in measure, Georgia serif, headings clearly heavier without shouting, good rhythm between sections. Minor oddity (inherent to the brief's own classifier rule): the first paragraph repeats the title verbatim as body text right below the page's real `<h1>`, a bit redundant but working as specified. Full report written to `/home/paulca/amadan.net/carlos/docs/.superpowers/sdd/2026-08-31-editing/task-3-report.md`.</result> <usage><subagent_tokens>149019</subagent_tokens><tool_uses>95</tool_uses><duration_ms>564087</duration_ms></usage> </task-notification>
Human
<task-notification> <task-id>a8c57d87707e82e43</task-id> <tool-use-id>toolu_01K5tzoVnhWUQ5H1Mdp6ZSG8</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-carlos-docs/70f302d1-9d5d-4eb2-8b38-e09a15fdd1c3/tasks/a8c57d87707e82e43.output</output-file> <status>completed</status> <summary>Agent "Review editing task 3" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>Good — the review file is git-ignored/untracked-but-clean (empty `git status --porcelain`), so the tree remains clean as required, and I made no other changes. The dev server process was killed and no fixture/source files were modified (fixture regeneration was verified in place and confirmed no diff). ## Summary **Spec verdict: PASS.** Every brief step delivered — `blocksFor` in `fixtures/generate-seed.mjs`, `Store.HTML` mirroring `Store.Text`, `handlers.go`/`show.html` wired to it, both test suites and the gate green, fixtures byte-reproducible with `clientID = 1` pinned. Confirmed live via a real magic-link sign-in and HTML fetch. **Quality verdict: PASS, with two real gaps** worth a follow-up commit — the known title-repeat defect, and `Store.Text` left as a silent trap. **Recommended fix for title repetition:** drop the seed generator's first block only when it equals the document's `Title`, in `fixtures/generate-seed.mjs` — not unconditionally, and not in the template. Proof: `letter.txt`'s first line ("Dear Members of the Harbourview Allotment Committee,") is real content, not a title echo, and I confirmed live that the letter document does *not* show the bug today — an unconditional drop would silently delete that line. A template-side fix would also leave the duplicated title sitting in `Store.Text`/`fragment_text` output for any future consumer (list preview, search). Ranked findings (full detail in the review file): 1. `internal/docs/store.go:385-401`, `internal/docs/models.go:30` — `Store.Text` now has zero production callers (confirmed by repo-wide grep); it reads a different CRDT root (`"content"` `Y.Text`) than every real document now uses (`"prosemirror"` fragment, per Task 1), so it silently returns `""` for anything real, and neither its own doc comment nor `models.go`'s pointer to it was updated. 2. `internal/docs/seed_test.go:29-49,127-224`, `internal/docs/sample_documents_test.go:60-73` — only `TestSeedContentMatchesItsSource` was genuinely tightened (new `<p>` assertion); the other two were swapped `Text`→`HTML` at unchanged strength, despite the report's blanket "tightening" claim. None are vacuous. 3. Escaping/`template.HTML` — no bypass found; `core/src/fragment.rs` escapes all text, emits no attributes, and `Store.HTML` is the sole construction site. 4. Fixture reproducibility — confirmed clean across two regenerations. 5. Template comments/structure/`docs-*` vocabulary — all intact, no `class="rst-*"`. 6. Live render — matches the known defect on 5/6 documents exactly as predicted; the letter is the exception, and nothing else looked wrong. Full review written to `/home/paulca/amadan.net/carlos/docs/.superpowers/sdd/2026-08-31-editing/task-3-review.md`.</result> <usage><subagent_tokens>106618</subagent_tokens><tool_uses>60</tool_uses><duration_ms>374684</duration_ms></usage> </task-notification>
diff --git a/core/src/abi.rs b/core/src/abi.rs| index 10038bb..1b16cdd 100644 |
| --- a/core/src/abi.rs |
| +++ b/core/src/abi.rs |
| @@ -21,7 +21,7 @@ |
| //! it has to be: |
| //! |
| //! - No `DOCS` `RefCell` borrow is held across a call into `yrs` |
| -//! (`apply_update`, `text_content`). A wasm trap does not unwind — it is |
| +//! (`apply_update`, `fragment_text`, `fragment_html`). A wasm trap does not unwind — it is |
| //! compiled to an `unreachable` instruction with `panic = "abort"` |
| //! (`Cargo.toml`), which does not run destructors — so a borrow guard |
| //! still on the stack at the moment of a trap never gets dropped and |
| @@ -46,7 +46,7 @@ |
| //! held a stale handle is exactly the kind of silent corruption §5.2 |
| //! exists to avoid. |
| -use crate::{apply_update, fragment_html, text_content, CoreError}; |
| +use crate::{apply_update, fragment_html, fragment_text, CoreError}; |
| use std::cell::RefCell; |
| use std::collections::HashMap; |
| use yrs::{Doc, Options}; |
| @@ -109,8 +109,36 @@ pub fn apply_slice(handle: u32, update: &[u8]) -> i32 { |
| /// `None` for an unknown handle — distinct from `Some(String::new())`, a |
| /// real empty document. Callers must not collapse the two (Finding 3). |
| +/// |
| +/// Reads `fragment_text` (root "prosemirror"), not `text_content` (root |
| +/// "content"). Fix round 1 on the fragment-migration task: every real |
| +/// document has stored its content as a Y.XmlFragment on "prosemirror" |
| +/// since seed data and the editor moved there, so `text_content`'s root |
| +/// has been empty for any of them since — `docs_core_text_len`/ |
| +/// `docs_core_text_read` (and this function, the thing both of them |
| +/// call) were a silent trap, reporting an empty document's worth of |
| +/// success instead of a real one's prose. "This document's text" means |
| +/// `fragment_text`'s plain-text rendering of the fragment now. |
| +/// |
| +/// No ABI version bump for this: `docs_core_text_len`/ |
| +/// `docs_core_text_read`'s signatures and calling convention are |
| +/// unchanged — same byte-length-then-bytes shape the host already |
| +/// speaks — only what they read changed, and the host and this module |
| +/// ship embedded together with no version skew between separately |
| +/// deployed copies to guard against (`docs_core_abi_version`'s own doc |
| +/// comment covers what *does* need a bump: an incompatible export |
| +/// shape). Recorded here because "the semantics changed but the number |
| +/// did not" is exactly the kind of thing that looks like an oversight |
| +/// later. |
| +/// |
| +/// `text_content` itself is untouched and still backs the Rust↔JS |
| +/// Y.Text wire fixtures (`core/tests/wire_compat.rs`) directly — this |
| +/// repoints only what the wasm ABI exposes as "text", not the function |
| +/// the wire contract tests call by name. A future consumer that |
| +/// genuinely wants the bare-Y.Text root back (there is none today) can |
| +/// still call `text_content` directly; nothing here deletes it. |
| pub fn text_string(handle: u32) -> Option<String> { |
| - get_doc(handle).map(|doc| text_content(&doc)) |
| + get_doc(handle).map(|doc| fragment_text(&doc)) |
| } |
| /// `None` for an unknown handle — distinct from `Some(String::new())`, a |
diff --git a/core/tests/wasm_trap_containment.rs b/core/tests/wasm_trap_containment.rs| index 3d2ac57..87e70aa 100644 |
| --- a/core/tests/wasm_trap_containment.rs |
| +++ b/core/tests/wasm_trap_containment.rs |
| @@ -303,11 +303,27 @@ fn build_wasm() -> Vec<u8> { |
| std::fs::read(&artifact).unwrap_or_else(|e| panic!("reading {artifact:?}: {e}")) |
| } |
| +// Fix round 1 on the fragment-migration task: this used to point at |
| +// fixtures/data (bare Y.Text on root "content"). It now points at |
| +// fixtures/vectors/fragment instead: `read_text` below goes through |
| +// the wasm ABI's docs_core_text_len/docs_core_text_read, which now |
| +// read fragment_text (root "prosemirror"), not text_content — see |
| +// core/src/abi.rs's text_string doc comment. A fixtures/data update |
| +// still applies cleanly through this same ABI (apply_slice decodes |
| +// and merges regardless of which root it targets), but reads back as |
| +// "" now, which would make every content check below compare against |
| +// the wrong thing silently. fixtures/generate-fragment.mjs's own |
| +// 'trap-containment vectors' section builds hello/unicode/formatted/ |
| +// concurrent to match — same names, same content and merge |
| +// characteristics fixtures/data's originals have, wrapped in a single |
| +// <paragraph> each instead of a bare Y.Text, with a matching |
| +// expected.json alongside (that generator's plainTextOf mirrors |
| +// fragment_text, so the two agree on what "text" means). |
| fn fixtures_dir() -> PathBuf { |
| PathBuf::from(env!("CARGO_MANIFEST_DIR")) |
| .parent() |
| .unwrap() |
| - .join("fixtures/data") |
| + .join("fixtures/vectors/fragment") |
| } |
| fn fixtures() -> Vec<(&'static str, Vec<u8>)> { |
diff --git a/core/tests/wire_compat.rs b/core/tests/wire_compat.rs| index 3e8e1bd..9c30ad9 100644 |
| --- a/core/tests/wire_compat.rs |
| +++ b/core/tests/wire_compat.rs |
| @@ -139,9 +139,32 @@ fn applies_concurrent() { |
| // `fixture()` above returns a 3-tuple (update, text, delta). Adjusted here |
| // to match; the round-trip and error-code behaviour the brief cares about |
| // are unchanged. |
| +// |
| +// Fix round 1 on the fragment-migration task: this used to apply the |
| +// "unicode" fixture (a bare Y.Text on root "content", fixtures/data) and |
| +// compare against its plain text. That stopped being what this ABI call |
| +// exercises the moment `abi::text_string` was repointed to read |
| +// `fragment_text` (root "prosemirror") instead of `text_content` — see |
| +// that function's own doc comment. A Y.Text-only fixture round-trips |
| +// through `apply_slice` fine (the update decodes and applies regardless |
| +// of which root it targets) but reads back as "" through `text_string` |
| +// now, which would make this test assert the wrong thing quietly. The |
| +// fixture is a fragment one instead (`fixtures/vectors/fragment/simple.bin` |
| +// — the same vectors `core/tests/fragment.rs` tests `fragment_text`/ |
| +// `fragment_html` against directly), so this still proves what it always |
| +// proved: a real Yjs update, applied through the wasm ABI exactly as the |
| +// Go host calls it, reads back through that same ABI as the text a |
| +// document actually holds. |
| #[test] |
| fn abi_round_trips_a_fixture() { |
| - let (update, expected, _delta) = fixture("unicode"); |
| + let update = fs::read( |
| + PathBuf::from(env!("CARGO_MANIFEST_DIR")) |
| + .parent() |
| + .unwrap() |
| + .join("fixtures/vectors/fragment/simple.bin"), |
| + ) |
| + .expect("missing fixtures/vectors/fragment/simple.bin"); |
| + let expected = "First paragraph.\n\nSecond paragraph."; |
| let handle = docs_core::abi::doc_new("abi-round-trips-a-fixture-guid", 42); |
| assert_ne!(handle, 0, "doc_new returned a null handle"); |
diff --git a/fixtures/generate-fragment.mjs b/fixtures/generate-fragment.mjs| index 3ddd59b..1f1b332 100644 |
| --- a/fixtures/generate-fragment.mjs |
| +++ b/fixtures/generate-fragment.mjs |
| @@ -54,4 +54,118 @@ writeFileSync('vectors/fragment/nested.bin', docWith(f => { |
| ]) |
| })) |
| -console.log('wrote 3 fragment vectors') |
| +// --- trap-containment vectors --- |
| +// |
| +// core/tests/wasm_trap_containment.rs mutation-fuzzes real fixtures and |
| +// checks a control document's content survives untouched, reading it |
| +// back through the wasm ABI's docs_core_text_len/docs_core_text_read. |
| +// Those now read fragment_text (root "prosemirror"), not text_content |
| +// (root "content") — see core/src/abi.rs's text_string doc comment — |
| +// so that test needs fragment-shaped fixtures of its own rather than |
| +// fixtures/data's Y.Text ones (which read back as "" through that ABI |
| +// now). These four mirror fixtures/data's hello/unicode/formatted/ |
| +// concurrent in content and purpose (plain ASCII, surrogate pairs and |
| +// combining marks, inline formatting, a concurrent merge) so the same |
| +// coverage that fuzzing wants — plain text, non-ASCII byte lengths, |
| +// format-op decoding, CRDT merge structure — survives the move, just |
| +// wrapped in a single top-level <paragraph> each instead of a bare |
| +// Y.Text. |
| +// |
| +// plainTextOf mirrors core/src/fragment.rs's fragment_text: walk the |
| +// tree, concatenate each Y.XmlText's own rendering, and join top-level |
| +// block texts with a blank line. |
| +// |
| +// Uses each Y.XmlText's own toString(), not toDelta() — verified |
| +// against the real Rust output, not assumed: fragment_text calls |
| +// yrs's XmlTextRef::get_string(txn), and yrs's own get_string embeds |
| +// formatting as inline pseudo-tags exactly the way Y.XmlText.toString() |
| +// does in JS ("plain <strong>bold</strong> plain", not "plain bold |
| +// plain") — this is XmlText's own "plain text" convention in both |
| +// implementations, not the HTML fragment_html builds via its own MARKS |
| +// table. A first version of this generator used toDelta() (mark-free) |
| +// and it disagreed with the real Rust output on the very first run — |
| +// core/tests/wasm_trap_containment.rs caught it immediately, which is |
| +// the whole reason this fixture set exists. |
| +function plainTextOf(doc) { |
| + const f = doc.getXmlFragment('prosemirror') |
| + const isBlock = (name) => |
| + ['paragraph', 'heading', 'list_item', 'blockquote'].includes(name) |
| + const blocks = [] |
| + let cur = '' |
| + function walk(node) { |
| + if (node instanceof Y.XmlText) { |
| + cur += node.toString() |
| + } else if (node instanceof Y.XmlElement) { |
| + for (const child of node.toArray()) walk(child) |
| + if (isBlock(node.nodeName) && cur.length > 0) { |
| + blocks.push(cur) |
| + cur = '' |
| + } |
| + } |
| + } |
| + for (const child of f.toArray()) walk(child) |
| + if (cur.length > 0) blocks.push(cur) |
| + return blocks.join('\n\n') |
| +} |
| + |
| +// emitTrap writes both halves a fixture needs: the update bytes, and |
| +// (unlike docWith's three vectors above, which core/tests/fragment.rs |
| +// asserts against with hand-written literals) an expected.json next to |
| +// it, the same shape fixtures/generate.mjs writes for fixtures/data — |
| +// wasm_trap_containment.rs reads that "text" field the same way |
| +// wire_compat.rs's fixture() does. |
| +function emitTrap(name, build) { |
| + const doc = new Y.Doc() |
| + doc.clientID = 1 |
| + build(doc) |
| + const update = Y.encodeStateAsUpdate(doc) |
| + writeFileSync(`vectors/fragment/${name}.update.bin`, Buffer.from(update)) |
| + writeFileSync( |
| + `vectors/fragment/${name}.expected.json`, |
| + JSON.stringify({ text: plainTextOf(doc) }, null, 2) + '\n', |
| + ) |
| + console.log(`${name}: ${update.length} bytes`) |
| +} |
| + |
| +emitTrap('hello', (doc) => { |
| + doc.getXmlFragment('prosemirror').insert(0, [el('paragraph', null, text('hello world'))]) |
| +}) |
| + |
| +emitTrap('unicode', (doc) => { |
| + // Surrogate pairs and combining marks: Yjs indexes UTF-16 code units and |
| + // Rust indexes UTF-8 bytes, same reasoning as fixtures/generate.mjs's |
| + // own 'unicode' fixture. |
| + doc.getXmlFragment('prosemirror').insert(0, [el('paragraph', null, text('héllo 🌍 café'))]) |
| +}) |
| + |
| +emitTrap('formatted', (doc) => { |
| + // Inline formatting: exercises format-op decoding under mutation |
| + // fuzzing, same reasoning as fixtures/generate.mjs's own 'formatted' |
| + // fixture. plainTextOf strips marks via toDelta, so the expected text |
| + // is just the concatenated runs regardless of the strong attribute. |
| + doc.getXmlFragment('prosemirror').insert(0, [ |
| + el('paragraph', null, text('plain '), text('bold', { strong: {} }), text(' plain')), |
| + ]) |
| +}) |
| + |
| +emitTrap('concurrent', (doc) => { |
| + // Two docs each insert their own top-level paragraph before syncing, |
| + // then merge — order-independent CRDT merge structure, same |
| + // reasoning as fixtures/generate.mjs's own 'concurrent' fixture, at |
| + // the block level rather than mid-run since Y.XmlFragment's children |
| + // are blocks, not characters. |
| + const a = new Y.Doc() |
| + a.clientID = 1 |
| + const b = new Y.Doc() |
| + b.clientID = 2 |
| + // doc.clientID (set to 3 here) overrides the emitTrap assignment |
| + // above, same as generate.mjs's own concurrent fixture. |
| + doc.clientID = 3 |
| + a.getXmlFragment('prosemirror').insert(0, [el('paragraph', null, text('AAA'))]) |
| + b.getXmlFragment('prosemirror').insert(0, [el('paragraph', null, text('BBB'))]) |
| + Y.applyUpdate(a, Y.encodeStateAsUpdate(b)) |
| + Y.applyUpdate(b, Y.encodeStateAsUpdate(a)) |
| + Y.applyUpdate(doc, Y.encodeStateAsUpdate(a)) |
| +}) |
| + |
| +console.log('wrote 3 fragment vectors + 4 trap-containment vectors') |
diff --git a/fixtures/generate-seed.mjs b/fixtures/generate-seed.mjs| index 456396f..4c40a77 100644 |
| --- a/fixtures/generate-seed.mjs |
| +++ b/fixtures/generate-seed.mjs |
| @@ -35,21 +35,58 @@ const names = readdirSync(seedDir) |
| .map((f) => basename(f, '.txt')) |
| .sort() |
| +// titles mirrors internal/docs/seed.go's seedDocs Title field, by Base |
| +// — kept in sync by hand, the same way this script's own Base names |
| +// already mirror seed.go's. blocksFor (below) uses it to decide |
| +// whether a document's opening block is a literal echo of its own |
| +// title (drop it) or real content that merely looks similar (keep |
| +// it). If the two ever drift, the failure is visible and harmless |
| +// either way: a document whose title changed here keeps a stray |
| +// opening paragraph until this map is updated, never the reverse |
| +// (silently eating real content it doesn't recognise as a title). |
| +const titles = { |
| + 'packing-list': 'Packing List — Thursday Site Visit', |
| + 'proposal': 'Proposal: Refitting the Elm Street Reading Room', |
| + 'meeting-notes': 'Notes — Tuesday Planning Sync', |
| + 'documentation': 'Quick Reference: Resetting the Site Printer', |
| + 'letter': 'Letter to the Harbourview Allotment Committee', |
| + 'history': 'A History of the Millbrook Waterworks', |
| +} |
| + |
| // blocksFor turns a .txt source into the paragraph/heading blocks its |
| // matching Y.XmlFragment gets built from: split on blank lines, then |
| // classify each block. A block reads as a heading only when it looks |
| // like one — short, a single line, no trailing sentence-ending |
| -// punctuation — and never for the very first block, which is always |
| -// the document's own opening line (usually a restatement of its |
| -// title) rather than a section break. This is what gives "Early |
| -// years" and its neighbours in the Millbrook waterworks history their |
| -// real shape, rather than leaving every block a paragraph. |
| -function blocksFor(text) { |
| +// punctuation — and never for the very first block (i === 0): that |
| +// guard exists independently of the title check below, because a |
| +// short, unpunctuated opening line is not always a title echo — |
| +// letter.txt's own opening block, "Dear Members of the Harbourview |
| +// Allotment Committee,", fits the heading shape (52 chars, one line, |
| +// trailing comma) but is a real salutation, not a section break. This |
| +// is what gives "Early years" and its neighbours in the Millbrook |
| +// waterworks history their real shape, rather than leaving every |
| +// block a paragraph. |
| +// |
| +// title, when given, is compared against the *first* paragraph only |
| +// (trimmed, exact match) — fix round 1: an earlier version of this |
| +// function dropped every document's opening block unconditionally, on |
| +// the assumption that a seed .txt always opens by restating its own |
| +// title. That is true for five of the six seed documents but not |
| +// letter.txt, whose opening line is real content the unconditional |
| +// version would have silently deleted. Comparing against the actual |
| +// title, rather than guessing from shape or position, is what lets |
| +// this stay correct for both cases at once. The comparison runs after |
| +// classification (on the finished `blocks` array), not before it on |
| +// `paras`: dropping the first *paragraph* before classifying would |
| +// shift every remaining block's index down by one, and the i !== 0 |
| +// guard above would then wrongly protect the new index-0 block (e.g. |
| +// history.txt's "Early years") from ever becoming a heading. |
| +function blocksFor(text, title) { |
| const paras = text |
| .split(/\n\s*\n/) |
| .map((s) => s.trim()) |
| .filter(Boolean) |
| - return paras.map((p, i) => { |
| + const blocks = paras.map((p, i) => { |
| const looksLikeHeading = |
| p.length < 60 && !p.includes('\n') && !/[.!?:]$/.test(p) && i !== 0 |
| if (looksLikeHeading) { |
| @@ -66,6 +103,10 @@ function blocksFor(text) { |
| el.insert(0, [t]) |
| return el |
| }) |
| + if (blocks.length > 0 && title !== undefined && paras[0] === title.trim()) { |
| + return blocks.slice(1) |
| + } |
| + return blocks |
| } |
| for (const name of names) { |
| @@ -79,7 +120,7 @@ for (const name of names) { |
| // per-document ClientID (internal/docs/store.go's Store.Create) is |
| // what a real edit would be attributed to. |
| doc.clientID = 1 |
| - const blocks = blocksFor(text) |
| + const blocks = blocksFor(text, titles[name]) |
| doc.getXmlFragment('prosemirror').insert(0, blocks) |
| const update = Y.encodeStateAsUpdate(doc) |
diff --git a/fixtures/vectors/fragment/concurrent.expected.json b/fixtures/vectors/fragment/concurrent.expected.json| new file mode 100644 |
| index 0000000..4f2e26f |
| --- /dev/null |
| +++ b/fixtures/vectors/fragment/concurrent.expected.json |
| @@ -0,0 +1,3 @@ |
| +{ |
| + "text": "AAA\n\nBBB" |
| +} |
diff --git a/fixtures/vectors/fragment/concurrent.update.bin b/fixtures/vectors/fragment/concurrent.update.bin| new file mode 100644 |
| index 0000000..1776e7b |
| Binary files /dev/null and b/fixtures/vectors/fragment/concurrent.update.bin differ |
diff --git a/fixtures/vectors/fragment/formatted.expected.json b/fixtures/vectors/fragment/formatted.expected.json| new file mode 100644 |
| index 0000000..00adb0d |
| --- /dev/null |
| +++ b/fixtures/vectors/fragment/formatted.expected.json |
| @@ -0,0 +1,3 @@ |
| +{ |
| + "text": "plain <strong>bold</strong> plain" |
| +} |
diff --git a/fixtures/vectors/fragment/formatted.update.bin b/fixtures/vectors/fragment/formatted.update.bin| new file mode 100644 |
| index 0000000..97de9f6 |
| Binary files /dev/null and b/fixtures/vectors/fragment/formatted.update.bin differ |
diff --git a/fixtures/vectors/fragment/hello.expected.json b/fixtures/vectors/fragment/hello.expected.json| new file mode 100644 |
| index 0000000..da7d1ad |
| --- /dev/null |
| +++ b/fixtures/vectors/fragment/hello.expected.json |
| @@ -0,0 +1,3 @@ |
| +{ |
| + "text": "hello world" |
| +} |
diff --git a/fixtures/vectors/fragment/hello.update.bin b/fixtures/vectors/fragment/hello.update.bin| new file mode 100644 |
| index 0000000..6161909 |
| Binary files /dev/null and b/fixtures/vectors/fragment/hello.update.bin differ |
diff --git a/fixtures/vectors/fragment/unicode.expected.json b/fixtures/vectors/fragment/unicode.expected.json| new file mode 100644 |
| index 0000000..4320fb0 |
| --- /dev/null |
| +++ b/fixtures/vectors/fragment/unicode.expected.json |
| @@ -0,0 +1,3 @@ |
| +{ |
| + "text": "héllo 🌍 café" |
| +} |
diff --git a/fixtures/vectors/fragment/unicode.update.bin b/fixtures/vectors/fragment/unicode.update.bin| new file mode 100644 |
| index 0000000..c4da6ce |
| Binary files /dev/null and b/fixtures/vectors/fragment/unicode.update.bin differ |
diff --git a/internal/core/core_internal_test.go b/internal/core/core_internal_test.go| index 300cffc..dfe7251 100644 |
| --- a/internal/core/core_internal_test.go |
| +++ b/internal/core/core_internal_test.go |
| @@ -113,6 +113,54 @@ func loadFixtureExpectedText(t *testing.T, name string) string { |
| return want.Text |
| } |
| +// loadFragmentFixtureBytes/loadFragmentFixtureExpectedText are |
| +// loadFixtureBytes/loadFixtureExpectedText's counterparts over |
| +// fixtures/vectors/fragment instead of fixtures/data — a real Yjs |
| +// update building a Y.XmlFragment on root "prosemirror" |
| +// (fixtures/generate-fragment.mjs's "trap-containment vectors") |
| +// rather than a bare Y.Text on root "content". |
| +// |
| +// Used only where a test in this file reads content back through |
| +// Handle.Text and checks it against a real string: Text now reads |
| +// fragment_text off root "prosemirror" (core/src/abi.rs's |
| +// text_string doc comment), so fixtures/data's updates — which apply |
| +// cleanly but touch a root Text no longer reads — would read back "" |
| +// there, same silent-empty trap this whole fix round exists to close. |
| +// |
| +// Deliberately NOT used for TestSlowUpdateIsCutOffByGuestCallTimeout's |
| +// own fixtureNames/fixtures RNG-mutation loop below: that loop's |
| +// wantHex assertion pins an exact mutated byte sequence discovered by |
| +// walking a fixed-seed PRNG over fixtures/data's specific byte |
| +// content at a specific trial index. Swapping in different-content, |
| +// different-length fragment fixtures there would silently walk the |
| +// same RNG sequence to different bytes and break that reproducer, |
| +// not fix anything — this pair exists only for the places that read |
| +// content back and check it, not for that fixed-sequence mutation |
| +// input. |
| +func loadFragmentFixtureBytes(t *testing.T, name string) []byte { |
| + t.Helper() |
| + b, err := os.ReadFile(filepath.Join("..", "..", "fixtures", "vectors", "fragment", name+".update.bin")) |
| + if err != nil { |
| + t.Fatalf("reading fragment fixture %s: %v", name, err) |
| + } |
| + return b |
| +} |
| + |
| +func loadFragmentFixtureExpectedText(t *testing.T, name string) string { |
| + t.Helper() |
| + raw, err := os.ReadFile(filepath.Join("..", "..", "fixtures", "vectors", "fragment", name+".expected.json")) |
| + if err != nil { |
| + t.Fatalf("reading expectation for fragment fixture %s: %v", name, err) |
| + } |
| + var want struct { |
| + Text string `json:"text"` |
| + } |
| + if err := json.Unmarshal(raw, &want); err != nil { |
| + t.Fatalf("parsing expectation for fragment fixture %s: %v", name, err) |
| + } |
| + return want.Text |
| +} |
| + |
| // TestTrappedInstanceIsDiscardedAndRebuilds is the test the plan's |
| // containment argument depends on. §5.2's design is: a trap poisons the |
| // module instance's linear memory, so the host must discard that instance |
| @@ -147,8 +195,8 @@ func TestTrappedInstanceIsDiscardedAndRebuilds(t *testing.T) { |
| // against more than an empty document: a rebuild that silently lost |
| // this would be a much worse failure than one that merely can't apply |
| // anything further. |
| - helloUpdate := loadFixtureBytes(t, "hello") |
| - helloWant := loadFixtureExpectedText(t, "hello") |
| + helloUpdate := loadFragmentFixtureBytes(t, "hello") |
| + helloWant := loadFragmentFixtureExpectedText(t, "hello") |
| if err := doc.Apply(ctx, helloUpdate); err != nil { |
| t.Fatalf("seeding the document before the trap: %v", err) |
| } |
| @@ -199,8 +247,8 @@ func TestTrappedInstanceIsDiscardedAndRebuilds(t *testing.T) { |
| // And the rebuilt instance must be fully usable going forward, not |
| // just readable: apply a further update and confirm it merges in. |
| - unicodeUpdate := loadFixtureBytes(t, "unicode") |
| - unicodeWant := loadFixtureExpectedText(t, "unicode") |
| + unicodeUpdate := loadFragmentFixtureBytes(t, "unicode") |
| + unicodeWant := loadFragmentFixtureExpectedText(t, "unicode") |
| doc2, err := rt.NewDoc(ctx, "go-host-test-guid-2", 9002) |
| if err != nil { |
| t.Fatalf("NewDoc for a second, unrelated document: %v", err) |
| @@ -379,7 +427,10 @@ func TestSlowUpdateIsCutOffByGuestCallTimeout(t *testing.T) { |
| // And the rebuilt instance must actually work: apply a real update |
| // and read it back, proving "a fresh instance for the same document |
| // rebuilds correctly" isn't just an unused module sitting there. |
| - hello := loadFixtureBytes(t, "hello") |
| + // Fragment fixture, not the RNG loop's fixtures/data one: this |
| + // checks Handle.Text's real content, which now reads root |
| + // "prosemirror" (see loadFragmentFixtureBytes's own doc comment). |
| + hello := loadFragmentFixtureBytes(t, "hello") |
| if err := handle.Apply(ctx, hello); err != nil { |
| t.Fatalf("instance unusable after timeout-triggered rebuild: %v", err) |
| } |
diff --git a/internal/core/core_test.go b/internal/core/core_test.go| index e0d40b4..0702440 100644 |
| --- a/internal/core/core_test.go |
| +++ b/internal/core/core_test.go |
| @@ -10,9 +10,22 @@ import ( |
| "github.com/carlosframework/docs/internal/core" |
| ) |
| +// fixture loads fixtures/vectors/fragment/<name>.update.bin, a real |
| +// Yjs update building a Y.XmlFragment on root "prosemirror" |
| +// (fixtures/generate-fragment.mjs's own "trap-containment vectors" |
| +// section), plus its expected plain text. |
| +// |
| +// Not fixtures/data (a bare Y.Text on root "content"): every test in |
| +// this file reads content back through (*Handle).Text, which now |
| +// reads fragment_text off root "prosemirror" — see |
| +// core/src/abi.rs's text_string doc comment. fixtures/data's updates |
| +// still apply cleanly (Apply/apply_slice decode and merge regardless |
| +// of which root they target) but read back as "" through Text now, |
| +// which is exactly the silent-empty failure this whole fix round |
| +// exists to stop reintroducing. |
| func fixture(t *testing.T, name string) ([]byte, string) { |
| t.Helper() |
| - dir := filepath.Join("..", "..", "fixtures", "data") |
| + dir := filepath.Join("..", "..", "fixtures", "vectors", "fragment") |
| update, err := os.ReadFile(filepath.Join(dir, name+".update.bin")) |
| if err != nil { |
| t.Fatalf("reading fixture: %v", err) |
diff --git a/internal/core/testdata/docs_core.wasm b/internal/core/testdata/docs_core.wasm| index ee39021..e58b607 100755 |
| Binary files a/internal/core/testdata/docs_core.wasm and b/internal/core/testdata/docs_core.wasm differ |
diff --git a/internal/docs/seed_test.go b/internal/docs/seed_test.go| index 9ee561e..c100db2 100644 |
| --- a/internal/docs/seed_test.go |
| +++ b/internal/docs/seed_test.go |
| @@ -222,3 +222,47 @@ func TestWaterworksHistoryHasHeadings(t *testing.T) { |
| } |
| t.Fatal("the Millbrook waterworks document is missing from the seed set") |
| } |
| + |
| +// TestSeedTextReturnsRealProse pins the plain-text accessor's own |
| +// contract for a real document: Store.Text must return the seeded |
| +// document's actual prose, not silently read back "". |
| +// |
| +// Fix round 1 on this task: Store.Text existed with zero production |
| +// callers and read CRDT root "content" (a bare Y.Text) — the shape |
| +// every real document left behind the moment seed data (and, later, |
| +// the editor) moved to root "prosemirror" (a Y.XmlFragment, Task 1). |
| +// Nothing caught that Text had quietly stopped meaning anything for a |
| +// real document: it returned "" with no error, the exact silent-empty |
| +// failure shape this codebase keeps producing. This test is the |
| +// assertion that would have caught it — it must pass against |
| +// core/src/abi.rs's text_string once that reads fragment_text instead |
| +// of text_content, and it must fail before that change lands. |
| +func TestSeedTextReturnsRealProse(t *testing.T) { |
| + app, err := docs.NewForTest(t) |
| + if err != nil { |
| + t.Fatalf("NewForTest: %v", err) |
| + } |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "text-prose@example.com") |
| + if err := docs.Seed(ctx, app.Store(), owner); err != nil { |
| + t.Fatalf("Seed: %v", err) |
| + } |
| + docList, err := app.Store().List(ctx, owner) |
| + if err != nil { |
| + t.Fatalf("List: %v", err) |
| + } |
| + for _, d := range docList { |
| + if !strings.Contains(d.Title, "Packing List") { |
| + continue |
| + } |
| + text, err := app.Store().Text(ctx, d) |
| + if err != nil { |
| + t.Fatalf("Text: %v", err) |
| + } |
| + if !strings.Contains(text, "Leaving from the depot at 7:30") { |
| + t.Errorf("Text = %.200q, want it to contain the packing list's real opening line", text) |
| + } |
| + return |
| + } |
| + t.Fatal("the packing list document is missing from the seed set") |
| +} |
diff --git a/internal/docs/seeddata/documentation.update.bin b/internal/docs/seeddata/documentation.update.bin| index 7800d06..e21305e 100644 |
| Binary files a/internal/docs/seeddata/documentation.update.bin and b/internal/docs/seeddata/documentation.update.bin differ |
diff --git a/internal/docs/seeddata/history.update.bin b/internal/docs/seeddata/history.update.bin| index 34ad719..1df894a 100644 |
| Binary files a/internal/docs/seeddata/history.update.bin and b/internal/docs/seeddata/history.update.bin differ |
diff --git a/internal/docs/seeddata/meeting-notes.update.bin b/internal/docs/seeddata/meeting-notes.update.bin| index e4c59a7..1d1d0ec 100644 |
| Binary files a/internal/docs/seeddata/meeting-notes.update.bin and b/internal/docs/seeddata/meeting-notes.update.bin differ |
diff --git a/internal/docs/seeddata/packing-list.update.bin b/internal/docs/seeddata/packing-list.update.bin| index fe07e31..ec7e59d 100644 |
| Binary files a/internal/docs/seeddata/packing-list.update.bin and b/internal/docs/seeddata/packing-list.update.bin differ |
diff --git a/internal/docs/seeddata/proposal.update.bin b/internal/docs/seeddata/proposal.update.bin| index 02ddc05..a202b91 100644 |
| Binary files a/internal/docs/seeddata/proposal.update.bin and b/internal/docs/seeddata/proposal.update.bin differ |
diff --git a/internal/docs/store_test.go b/internal/docs/store_test.go| index b4bbf7a..1a4cc97 100644 |
| --- a/internal/docs/store_test.go |
| +++ b/internal/docs/store_test.go |
| @@ -59,8 +59,14 @@ func TestTextComesFromTheCore(t *testing.T) { |
| t.Fatalf("Create: %v", err) |
| } |
| - // The "hello" fixture is a real Yjs update produced by browser yjs. |
| - update := docs.ReadFixture(t, "hello") |
| + // The "hello" fragment fixture is a real Yjs update produced by |
| + // real yjs (fixtures/generate-fragment.mjs), building a |
| + // Y.XmlFragment on root "prosemirror" — the root Store.Text |
| + // actually reads (core/src/abi.rs's text_string doc comment). |
| + // ReadFixture's own fixtures/data "hello" applies just as cleanly |
| + // but targets root "content", which Store.Text no longer reads — |
| + // it would read back "", proving nothing about the CRDT. |
| + update := docs.ReadFragmentFixture(t, "hello") |
| if err := app.Store().ApplyUpdate(ctx, doc, update); err != nil { |
| t.Fatalf("ApplyUpdate: %v", err) |
| } |
| @@ -243,7 +249,10 @@ func TestApplyUpdateRefusesAnotherOwnersDocument(t *testing.T) { |
| tampered := *doc |
| tampered.OwnerID = owner + 1_000_000 // guaranteed to belong to nobody |
| - update := docs.ReadFixture(t, "hello") |
| + // Fragment fixture, not ReadFixture's plain-Y.Text one: this test's |
| + // content assertions below go through Store.Text, which reads root |
| + // "prosemirror" now — see TestTextComesFromTheCore's own comment. |
| + update := docs.ReadFragmentFixture(t, "hello") |
| if err := app.Store().ApplyUpdate(ctx, &tampered, update); err == nil { |
| t.Fatal("ApplyUpdate returned nil for a mismatched owner — want a real error, before anything is written") |
| } |
diff --git a/internal/docs/testing.go b/internal/docs/testing.go| index 1a1136f..5d81d5c 100644 |
| --- a/internal/docs/testing.go |
| +++ b/internal/docs/testing.go |
| @@ -178,3 +178,27 @@ func ReadFixture(t *testing.T, name string) []byte { |
| } |
| return b |
| } |
| + |
| +// ReadFragmentFixture loads fixtures/vectors/fragment/<name>.update.bin |
| +// — a real Yjs update, produced by real yjs |
| +// (fixtures/generate-fragment.mjs), that builds a Y.XmlFragment on |
| +// root "prosemirror" rather than a bare Y.Text on root "content". |
| +// |
| +// Distinct from ReadFixture, not a parameter on it: since Store.Text |
| +// and Store.HTML were repointed to read the "prosemirror" root |
| +// (core/src/abi.rs's text_string doc comment — fix round 1 on the |
| +// fragment-migration task), a test that wants to see real content |
| +// come back through either of them needs an update built on that |
| +// root. ReadFixture's own fixtures/data updates still apply cleanly |
| +// (ApplyUpdate/apply_slice decode and merge regardless of which root |
| +// an update targets) but read back as "" through Text/HTML now — a |
| +// test reaching for ReadFixture("hello") expecting "hello world" back |
| +// through Store.Text would be reaching for the wrong fixture set. |
| +func ReadFragmentFixture(t *testing.T, name string) []byte { |
| + t.Helper() |
| + b, err := os.ReadFile(filepath.Join("..", "..", "fixtures", "vectors", "fragment", name+".update.bin")) |
| + if err != nil { |
| + t.Fatalf("docs.ReadFragmentFixture(%q): %v", name, err) |
| + } |
| + return b |
| +} |