feat(workspace): replace markdownMaterializer with createMaterializer factory#1650
Merged
feat(workspace): replace markdownMaterializer with createMaterializer factory#1650
Conversation
…est instead of delete Breddit is a local-first Reddit data browser built on the existing ingest pipeline. The workspace cleanup spec now moves ingest/ to apps/breddit/ instead of deleting it.
…ment access Planning doc for redesigning the markdown materializer API to support document content through the extension closure, eliminating app-specific materializers in fuji and opensidian. Explores typed table helpers, SerializeFn callbacks, and composable serialize presets.
… args pattern The .version() builder API was replaced by variadic args months ago. All 163 call sites in TypeScript already use the correct pattern, but the workspace-api skill doc and 4 articles still showed the old form.
…y pattern
Evolves the spec from markdown-specific to format-agnostic:
- createMaterializer(tables, config) factory with typed .table() chain
- General { filename, content } serialize contract
- markdown() helper for the common frontmatter + body case
- KV materialization to single JSON file
- Default-materialize-all with per-table overrides
- First arg is now ctx ({ tables, kv }) instead of just tables
- Structurally typed so it receives extension context directly
- Added open questions section from abstraction boundary audit:
preset naming, default-materialize-all assumptions, KV observation,
markdown() link conversion opt-out, whenReady timing, MaybePromise
- .kv({ skip: true }) → .skipKv() for consistency with .skip()
- MaybePromise<T> → import from lifecycle.ts (already exists)
- KV observation confirmed: kv.observeAll() exists for live updates
- markdown() opt-out: toMarkdown() is the documented escape hatch
- Serialize presets: JSDoc @remarks documents markdown output
- whenReady: JSDoc documents lazy start after .table()/.skip() calls
- Default-materialize-all: JSDoc documents .skip() for non-content tables
…dd kv serialize
Three design refinements:
- Opt-in over default-materialize-all: .table() and .kv() explicitly
opt in. No .skip()/.skipKv() needed—API is purely additive.
- dir everywhere: consistent short name for base path and subfolder.
- .kv({ serialize }) receives typed KV snapshot, returns SerializeResult.
No global default serialize—would lose row type inference.
Without awaiting ctx.whenReady, the materializer races persistence/sync and reads empty tables on first materialization. Both existing app-specific materializers already do this—the generic one must too.
Skip deleteEntryByKey in set() when inside an outer transaction (batch case). The observer handles conflict resolution in one pass using a lazy entryIndexMap instead of repeated indexOf scans. Individual (non-batched) set() calls keep the eager delete to avoid double observer invocation. - Completed spec items 1.1–1.7 - Deviation: conditional skip vs unconditional (see spec note)
The perf commit (728043f) introduced inline let+null-check caches in the YKeyValueLww observer. This extracts that pattern into a reusable lazy() helper with detailed JSDoc, removing the manual cache bookkeeping.
Covers the sync lazy() helper discovered while optimizing YKeyValueLww, contrasted with the async lazy singleton pattern already documented.
… factory
Implement the new typed createMaterializer(ctx, { dir }) factory with
.table() and .kv() opt-in builder chains. General serialize contract
({ filename, content }) replaces the markdown-specific return shape.
- createMaterializer with TTables/TKv generics for type-safe table names and row inference
- markdown() helper wraps toMarkdown + wikilink conversion
- slugFilename/bodyField presets return SerializeResult via markdown()
- toSlugFilename/toIdFilename standalone utilities
- KV materialization via kv.observeAll() with JSON default
- Completed spec items 1.1-1.11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The markdown materializer started life as a single monolithic function that owned serialization, file writing, and Yjs observation all at once. That was fine while the API was still being figured out, but the spec kept moving—closure-based document access, opt-in materialization, factory composition—so the implementation had to stop pretending the shape was settled.
This PR follows that path and lands on a workspace-bound
createMaterializer(...)factory. The materializer now plugs into the extension chain, waits forwhenReadybefore reading state, and lets each table or KV namespace opt in explicitly instead of forcing everything through one big markdown path.While getting there, the repeated “compute once inside an observer” pattern got pulled into a reusable
lazy()helper so those caches stop being hand-rolled in three different places.That cleanup also exposed a real scaling bug in
YKeyValueLww: bulk updates were re-scanning the array on every write, which made the hot path quadratic. The fix batches the conflict cleanup so the observer resolves the batch once instead of repeatedly walking the same data.There’s also a short article on the lazy initializer pattern, plus doc updates to keep the workspace docs aligned with the current API instead of the older chaining examples.
Stacks on #1649. 13 commits, 14 files changed, +1322/-438.