amadan
Plan 1: the import mechanism, task by task
Twelve tasks from the ABI export through to a working drag-and-drop, with task 0 recorded as already done so the plan reads as a whole. Two of them are worth flagging to a reader skimming: task 11 is a gate on the one number in the spec that is still a guess (that a 2 MB tree builds inside the 1-second guest-call cap -- find_position is quadratic in a block's run count), and it says that if the gate fails the fix is a smaller update cap rather than a longer timeout. Task 12 removes the duplicated editor schema in verify-import.mjs, because a gate testing a schema the editor does not use would pass while the editor broke, which is the exact failure it exists to catch. Two rulings are Paul's and are named at the top rather than buried: whether to fix fragment_text's markup leak (it reverses a documented decision and means regenerating fixtures), and whether snapshot encoding version may be split off the ABI number (it overrides the snapshots design's D1, and task 1 depends on it).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMNxUbTqqxehd9JdtkYfbe
1 file changed,
+307
−0
docs/superpowers/plans/2026-09-02-import-mechanism.md+307 −0
CI passed — run details
Intent
Attributed to this commit: 203 g CO₂e (21222966 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-import-documents, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/docs/superpowers/plans/2026-09-02-import-mechanism.md b/docs/superpowers/plans/2026-09-02-import-mechanism.md| new file mode 100644 |
| index 0000000..f37d6c6 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-09-02-import-mechanism.md |
| @@ -0,0 +1,307 @@ |
| +# Import, plan 1: the mechanism |
| + |
| +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| + |
| +**Goal:** Drag a `.md` or `.txt` file onto the document list and get a real, editable document — with the whole mechanism (core builder, Go plumbing, HTTP surface, import report) in place for DOCX and PDF to plug into. |
| + |
| +**Architecture:** Import is the mirror of export. Go parses a file into canonical ProseMirror JSON — the shape `core/src/fragment.rs`'s `fragment_json` already emits — and one Rust function turns that into a `Y.XmlFragment`. The resulting whole-state snapshot is persisted through `Store.ApplyUpdate`, the same path a person's typing takes, so an imported document is indistinguishable from a typed one. |
| + |
| +**Tech Stack:** Rust (`yrs` 0.23.5, wasm32 under wazero), Go 1.26 (chi, gorm, SQLite), ProseMirror + Yjs in the browser. |
| + |
| +**Spec:** [`docs/superpowers/specs/2026-09-01-import-design.md`](../specs/2026-09-01-import-design.md) — revision 3. Read it; this plan argues from it and does not restate its reasoning. |
| + |
| +**Plans 2 (DOCX) and 3 (PDF) follow this one.** Nothing here depends on them. |
| + |
| +## Global Constraints |
| + |
| +- **`CGO_ENABLED=0`** everywhere (`.amadan/ci.d/10-gate:20`). No cgo dependency may enter. |
| +- **Never `yrs::Doc::new()` or `Doc::with_client_id()`** — both draw entropy that is a fixed constant on wasm. Only `Doc::with_options(Options::with_guid_and_client_id(...))`. Enforced by `clippy.toml`. |
| +- **GUIDs and client ids are drawn in Go, never by yrs.** A build's client id is a fresh `uint32` per import, in `[0, 2^32)`. **Never the document row's `ClientID`** — it is 63-bit and 861 of every 1000 such ids throw `Integer out of Range` in the browser (spec §4 D1). |
| +- **Every user-visible string is a catalog key**, resolved at render time. Twelve base locales. A hardcoded English string is a bug. **All new copy goes through the `copy-review` skill before it is written into a template.** |
| +- **WCAG 2.2 AA, no exemptions**: 24×24 targets, 320px reflow, colour never the only carrier of meaning. |
| +- **The bare `rst-*` attribute grammar**, adopted per screen, never mixed within a component. |
| +- **Markup contributed to estilo takes an `estilo-*` prefix**, never `rst-*`. |
| +- **A new browser script is a module under 300 lines**, served the way `jumpToScriptPath()` is. |
| +- **The committed wasm must match a fresh build byte-for-byte** (`10-gate:117`). After any Rust change: `./scripts/build-wasm.sh` then copy to `internal/core/testdata/docs_core.wasm`. |
| +- **Work on this branch; never commit to `main`.** Squash-merge only when done, approved, and `amadan ci status` exits 0. |
| + |
| +## Two rulings this plan needs from Paul before its dependent tasks land |
| + |
| +Both are recorded in the spec; neither is the implementer's to decide. |
| + |
| +1. **`fragment_text` renders marks as literal `<strong>` markup** (spec §4). Fixing it reverses a documented decision in `fixtures/generate-fragment.mjs:99` and means regenerating four `.expected.json` fixtures. Blocks nothing in this plan; decide before it is closed. |
| +2. **Splitting `snapshotEncodingVersion` off the ABI number** (spec §5) overrides the snapshots design's D1. **Task 2 depends on this.** |
| + |
| +--- |
| + |
| +### Task 0: `build_fragment` and the cross-implementation gate — **DONE** |
| + |
| +Landed in commit `ed1a16b`. Recorded here so the plan reads as a whole. |
| + |
| +- `core/src/build.rs` — canonical ProseMirror JSON → `Y.XmlFragment`, with a hand-written JSON reader, content-model enforcement, and a depth cap. |
| +- `core/tests/build.rs` — 33 tests: round trip, every mark, nested lists, refusals, malformed input, and the mark-inheritance regression. |
| +- `core/src/bin/emit_import.rs` + `fixtures/verify-import.mjs`, wired into `scripts/wire-gate.sh` — upstream yjs and y-prosemirror open what the builder built, against the real editor schema. |
| + |
| +**Interfaces produced:** `docs_core::build_fragment(&Doc, &str) -> Result<(), BuildError>`; `BuildError::{Json(String), Schema(String), TooDeep, RootNotEmpty}`. |
| + |
| +--- |
| + |
| +### Task 1: `docs_core_build`, and the snapshot-version split |
| + |
| +**Files:** |
| +- Modify: `core/src/abi.rs` (new export), `core/src/lib.rs:101` (ABI 4 → 5) |
| +- Modify: `internal/core/core.go:79` (`coreABIVersion`), `internal/docs/snapshots.go:139` |
| +- Test: `core/tests/abi.rs` (or the existing ABI test file), `internal/docs/snapshots_test.go:76` |
| + |
| +**Interfaces:** |
| +- Consumes: `build_fragment` (Task 0). |
| +- Produces: `docs_core_build(handle: u32, json_ptr: u32, json_len: u32) -> i32` — `0` on success, `ERR_NO_DOC`, `ERR_INVALID_LENGTH`, or a new `ERR_BUILD = -6`. Go constant `snapshotEncodingVersion = 4`. |
| + |
| +- [ ] **Step 1: Write the failing ABI test** |
| + |
| +```rust |
| +#[test] |
| +fn build_populates_a_document_through_the_abi() { |
| + let wasm = build_wasm(); |
| + let mut h = Harness::new(&wasm); |
| + let handle = h.doc_new("abi-build", 7003).expect("doc_new"); |
| + let tree = r#"{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"Hi."}]}]}"#; |
| + assert_eq!(h.build(handle, tree).expect("build must not trap"), 0); |
| + assert_eq!(h.read_text(handle).expect("read"), b"Hi."); |
| +} |
| + |
| +#[test] |
| +fn build_refuses_a_bad_tree_without_trapping() { |
| + let wasm = build_wasm(); |
| + let mut h = Harness::new(&wasm); |
| + let handle = h.doc_new("abi-build-bad", 7004).expect("doc_new"); |
| + assert_eq!(h.build(handle, r#"{"type":"doc","content":[{"type":"table","content":[]}]}"#).expect("must not trap"), -6); |
| + // The instance still works afterwards. |
| + assert_ne!(h.doc_new("after", 7005).expect("doc_new"), 0); |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `cargo test --manifest-path core/Cargo.toml --test wasm_trap_containment` |
| +Expected: FAIL — no `build` method on the harness, no `docs_core_build` export. |
| + |
| +- [ ] **Step 3: Add the export** |
| + |
| +Follow `docs_core_apply` exactly (`abi.rs`): `in_bounds` first, then a slice, then UTF-8 validation, then the call. Never hold a `DOCS` borrow across `build_fragment`. |
| + |
| +```rust |
| +pub const ERR_BUILD: i32 = -6; |
| + |
| +/// # Safety |
| +/// `ptr`/`len` must describe a byte range inside this module's linear |
| +/// memory, unmutated for the call. Checked by `in_bounds`. |
| +#[no_mangle] |
| +pub unsafe extern "C" fn docs_core_build(handle: u32, ptr: u32, len: u32) -> i32 { |
| + if !in_bounds(ptr, len) { |
| + return ERR_INVALID_LENGTH; |
| + } |
| + let bytes = std::slice::from_raw_parts(ptr as *const u8, len as usize); |
| + let Ok(json) = std::str::from_utf8(bytes) else { |
| + return ERR_BUILD; |
| + }; |
| + match get_doc(handle) { |
| + None => ERR_NO_DOC, |
| + Some(doc) => match crate::build_fragment(&doc, json) { |
| + Ok(()) => 0, |
| + Err(_) => ERR_BUILD, |
| + }, |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Bump the ABI to 5** in `core/src/lib.rs:101` and `internal/core/core.go`'s `coreABIVersion`, extending the doc comment to say what 5 adds. |
| + |
| +- [ ] **Step 5: Split the snapshot version** (needs ruling 2) |
| + |
| +In `internal/docs/snapshots.go`, replace `coreABIVersion` with a new `snapshotEncodingVersion = 4` at both readers (the filter and the row write). Keep the column named `abi_version` with a comment saying why it is now a misnomer. Re-key `TestNewestSnapshotIgnoresAnotherABIsRows`. |
| + |
| +- [ ] **Step 6: Rebuild the wasm and run everything** |
| + |
| +```bash |
| +./scripts/build-wasm.sh |
| +cp core/target/wasm32-unknown-unknown/release/docs_core.wasm internal/core/testdata/docs_core.wasm |
| +cargo test --manifest-path core/Cargo.toml && ./scripts/wire-gate.sh |
| +CGO_ENABLED=0 go test ./... -count=1 |
| +``` |
| + |
| +Expected: all pass; existing snapshots still resolve (the split is what proves this). |
| + |
| +- [ ] **Step 7: Commit** |
| + |
| +--- |
| + |
| +### Task 2: `Handle.Build` in the Go core wrapper |
| + |
| +**Files:** |
| +- Modify: `internal/core/core.go` (after `Apply`) |
| +- Test: `internal/core/core_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `docs_core_build` (Task 1). |
| +- Produces: `func (h *Handle) Build(ctx context.Context, treeJSON string) error`. |
| + |
| +- [ ] **Step 1: Write the failing test** — build a tree through a real wasm handle, then assert `Handle.JSON` returns it; and that a refused tree returns an error without killing the instance. |
| +- [ ] **Step 2: Run it, watch it fail.** |
| +- [ ] **Step 3: Implement `Build`,** structured exactly like `verifySnapshotOn` (`core.go:632`): `allocIn`, write, `callGuest`, `defer freeIn`. Map `-6` to a `*BuildError` carrying the tree's size; treat a trap or timeout exactly as `Apply` does (discard the instance, rebuild, return `*RecoveredError`). |
| +- [ ] **Step 4: Run tests.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 3: `Store.Import` and `Store.Delete` |
| + |
| +**Files:** |
| +- Modify: `internal/docs/store.go` |
| +- Test: `internal/docs/store_test.go` |
| + |
| +**Interfaces:** |
| +- Produces: |
| + - `func (s *Store) Import(ctx context.Context, owner int64, title, treeJSON string) (*Document, error)` |
| + - `func (s *Store) Delete(ctx context.Context, owner int64, id int64) error` — `documents`, `doc_updates` and `doc_import_reports`, all through `owned()`. |
| + - `const maxImportUpdateBytes = 2 << 20` |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +Name them for the properties, not the code: `TestImportCreatesADocumentAnyoneCanEdit`, `TestImportLeavesNoRowWhenTheBuildFails`, `TestImportRefusesAnUpdateOverTheBudget`, `TestImportedDocumentIsScopedToItsOwner`, `TestImportDoesNotUseTheRowsClientID`. |
| + |
| +That last one is the regression for spec §4 D1 and is the important one: |
| + |
| +```go |
| +// The row's ClientID is 63-bit; a client id that large cannot be decoded |
| +// by lib0 in the browser. Whatever authored the imported content must |
| +// have used a fresh id under 2^32. |
| +func TestImportDoesNotUseTheRowsClientID(t *testing.T) { |
| + // ... import a document, read its doc_updates row, decode the update's |
| + // client ids, and assert every one is < 1<<32 and != doc.ClientID. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run them, watch them fail.** |
| +- [ ] **Step 3: Implement the six-step sequence** from spec §5, in order — parse is the caller's, so `Import` does: scratch handle (fresh `uuid.NewString()` GUID and a fresh `uint32` client id) → `Build` → `Snapshot` → budget check → `Create` → `ApplyUpdate` → on failure, `Delete`. No row exists until there is a verified update to write. |
| +- [ ] **Step 4: Run tests.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 4: `internal/importer` — the tree type and its JSON |
| + |
| +**Files:** |
| +- Create: `internal/importer/tree.go`, `internal/importer/importer.go` |
| +- Test: `internal/importer/tree_test.go` |
| + |
| +**Interfaces:** |
| +- Produces: `Node`, `Doc`, `Canonical()`, `MarshalJSON`; `type Importer interface { Import(r io.Reader, size int64) (Doc, Report, error) }`; `Report`, `Warning`, `WarningKind`. |
| + |
| +**Do not reuse `internal/export.Node`** — its `Attrs` is a value struct, so it encodes `"attrs":{"level":0}` on every paragraph plus `"text":""`, `"marks":null`, `"content":null`, none of which `fragment_json` emits and all of which `build_fragment` refuses. |
| + |
| +- [ ] **Step 1: Write the failing golden test** — a tree encodes to exactly the bytes `fragment_json` produces for the same document. Get the expected string from `core/tests/build.rs`'s corpus so the two cannot drift. |
| +- [ ] **Step 2: Run it, watch it fail.** |
| +- [ ] **Step 3: Implement** the types with `omitempty` and pointers where the spec's canonical form requires a key to be absent, plus `Canonical()` — merge consecutive runs with equal mark-name *sets*, drop empty runs, sort marks into `MARKS` order, dedupe. |
| +- [ ] **Step 4: Run tests.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 5: the plain-text importer |
| + |
| +**Files:** Create `internal/importer/text.go`, `internal/importer/text_test.go` |
| + |
| +- [ ] **Step 1: Failing tests** — blank-line-separated paragraphs; CRLF; a BOM stripped; **invalid UTF-8 refused** (validated in Go, before the hand-written Rust parser ever sees it); an empty or whitespace-only file refused with a named error, creating no document. |
| +- [ ] **Step 2: Run, fail.** - [ ] **Step 3: Implement.** - [ ] **Step 4: Run, pass.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 6: the Markdown importer |
| + |
| +**Files:** Create `internal/importer/markdown.go`, `internal/importer/markdown_test.go` |
| + |
| +- [ ] **Step 1: Failing tests.** The subset first (ATX headings, both list kinds, blockquote, `**`/`*`/`` ` `` runs, paragraphs), then **the export→import→export identity test** against `internal/export/markdown.go`, then one test per out-of-subset construct with the answer the spec fixes (link, image, code fence, table, HTML, thematic break, hard break) asserting both the content and the warning. |
| +- [ ] **Step 2: Run, fail.** - [ ] **Step 3: Implement,** unescaping what `escapeMarkdown` wrote. - [ ] **Step 4: Run, pass.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 7: the import report, stored |
| + |
| +**Files:** |
| +- Create: `internal/docs/migrations/0005_import_reports.sql` |
| +- Modify: `internal/docs/models.go`, `internal/docs/store.go` |
| +- Test: `internal/docs/store_test.go` |
| + |
| +**A warning never quotes document text or a link target** (spec §9). `Kind` is an enum resolved to a catalog key; `Where` is a structured locator formatted at render time. Add a test that asserts a report row contains none of the document's own words — it is the kind of rule that decays without one. |
| + |
| +- [ ] Steps: failing test → run → migration + model + `Store.SaveReport`/`Report`/`DismissReport` → run → commit. |
| + |
| +--- |
| + |
| +### Task 8: `POST /documents/import` |
| + |
| +**Files:** Modify `internal/docs/app.go` (route, inside `RequireSession`), `internal/docs/handlers.go`; test `internal/docs/import_test.go` |
| + |
| +- [ ] **Step 1: Failing tests** — single file redirects `303` to the new document; several files redirect to the list; a failing file yields no document and a counted failure; **`ParseMultipartForm` is never called** (it spills plaintext to temp files); each cap in spec §11's table is enforced; a request without a session is refused; the owner comes from the session and never from the body. |
| +- [ ] **Step 2: Run, fail.** |
| +- [ ] **Step 3: Implement** with `r.MultipartReader()`, a per-part `LimitReader`, a whole-body `MaxBytesReader`, a semaphore over concurrent imports, and a wall-clock deadline. Dispatch on extension to the Task 5/6 importers. |
| +- [ ] **Step 4: Run, pass.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 9: the list page — form first, drop zone second |
| + |
| +**Files:** Modify `internal/docs/views/list.html`, `internal/docs/render.go` (catalog keys); create `fixtures/drop-zone.mjs`; modify `fixtures/build-bundles.mjs` |
| + |
| +**`list.html:150` already documents the header as at its 320px reflow limit with five controls.** Solve that layout, do not discover it. |
| + |
| +- [ ] **Step 1: Run the `copy-review` skill on every new string** — the file input's label, the drop-zone prompt, the failure notice, and every `WarningKind`. Do not write copy into the template before this returns. |
| +- [ ] **Step 2: Failing tests** — the form posts to `/documents/import` and works with JavaScript disabled; the page renders at 320px; every string resolves through the catalog. |
| +- [ ] **Step 3: Implement** the form, then `drop-zone.mjs` as progressive enhancement over it (`dragover` reveals the target, `drop` posts the same body by `fetch`). Keep it under 300 lines. Rebuild the bundles and commit the hashed output. |
| +- [ ] **Step 4: Run, pass.** - [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +### Task 10: the report on the document page |
| + |
| +**Files:** Modify `internal/docs/views/show.html`, `internal/docs/handlers.go`; test `internal/docs/import_test.go` |
| + |
| +- [ ] Steps: `copy-review` the strings → failing tests (a clean import shows no empty report; a report with warnings renders each one; dismissal is a POST that deletes the row and is scoped to the owner) → implement → run → commit. |
| + |
| +--- |
| + |
| +### Task 11: the performance gate |
| + |
| +**Files:** Test `internal/core/core_test.go` |
| + |
| +The one number in the spec that is still a guess: that a 2 MB tree builds, snapshots and re-applies inside the 1-second guest-call cap. `find_position` walks from the start of an `XmlText`, so a block with *k* runs costs O(k²). |
| + |
| +- [ ] **Step 1:** Generate a ~2 MB canonical tree (many blocks, and one block with several thousand runs — the quadratic case). |
| +- [ ] **Step 2:** Assert build, `Snapshot` and `ApplyUpdate` each complete in **≤ 250 ms** under wazero. |
| +- [ ] **Step 3:** If it fails, the fix is a smaller update cap, not a longer timeout — say so in the spec and change the number there. |
| +- [ ] **Step 4: Commit.** |
| + |
| +--- |
| + |
| +### Task 12: one schema, not two |
| + |
| +**Files:** Create `fixtures/editor-schema.mjs`; modify `fixtures/editor-entry.js`, `fixtures/verify-import.mjs`, `fixtures/build-bundles.mjs` |
| + |
| +`verify-import.mjs` currently mirrors the editor's schema by hand, and says so loudly. A drift between the two would make the cross-implementation gate test a schema the editor does not use — the gate would pass while the editor broke, which is the exact failure mode it exists to catch. |
| + |
| +- [ ] **Step 1:** Extract the schema (nodes, marks, `spanMark`) into `fixtures/editor-schema.mjs`. |
| +- [ ] **Step 2:** Import it from both `editor-entry.js` and `verify-import.mjs`; delete the duplicate and its warning comment. |
| +- [ ] **Step 3:** Rebuild the bundles; the editor bundle's hash changes, so commit the new `internal/docs/static/editor.<hash>.js` and delete the old one. |
| +- [ ] **Step 4:** `./scripts/wire-gate.sh` and `go test ./...`; the assets test proves the app still finds its bundle. |
| +- [ ] **Step 5: Commit.** |
| + |
| +--- |
| + |
| +## Definition of done |
| + |
| +- `./scripts/wire-gate.sh` passes, including the import check |
| +- `CGO_ENABLED=0 go build ./... && go vet ./... && go test ./... -count=1`, `gofmt -l .` empty |
| +- `cargo test`, `cargo clippy -- -D warnings`, `cargo fmt --check` |
| +- The committed wasm matches a fresh build byte-for-byte |
| +- Dragging a `.md` and a `.txt` onto the list produces documents that open in the editor, with a report where one is warranted |
| +- The same works with JavaScript disabled |
| +- `amadan ci status -branch <n>` exits 0 |
| +- Both rulings above have Paul's answer |