refactor(workspace): timeline API overhaul and extension system cleanup#1528
Merged
refactor(workspace): timeline API overhaul and extension system cleanup#1528
Conversation
applySnapshot() was a no-op—CRDT forward-merge re-applies items that already exist. Replace with delete + insert in a transaction so the document content actually changes. Add deleteSnapshot() RPC and DELETE /documents/:document/snapshots/:id route. Spec: specs/20260313T144805-document-snapshot-fixes.md
The DO's applySnapshot() read from getText('content')—a key nothing
uses after the timeline merge. Remove it and the /apply route. Add
restoreFromSnapshot(handle, binary) in @epicenter/workspace that reads
a snapshot's timeline entry and writes matching content to the live doc
via DocumentHandle. Mode-aware: text restores in-place, sheet/richtext
push new entries. Richtext is flattened to plaintext for now.
- Remove applySnapshot() from DocumentRoom
- Remove POST /documents/:document/snapshots/:id/apply
- Add restoreFromSnapshot in packages/workspace/src/timeline/restore.ts
- 5 new tests (text same-mode, cross-mode, sheet, empty, error cleanup)
Accept Y.Doc instead of DocumentHandle—removes the cross-layer dependency on workspace types and the separate restore.ts file. The text write logic (in-place if same mode, push if different) is inlined directly. Tests no longer need a 35-line createTestHandle helper.
Use Y.XmlElement.clone() and Y.XmlText.clone() instead of flattening to plaintext. Bold, italic, headings, links—all preserved. Verified against Yjs v13.6.29 source: clone() deep-copies attributes, nodeName, and recursively clones children via toDelta()/applyDelta(). Removes populateFragmentFromText import from timeline.ts (still used by create-document.ts for mode conversion).
…extFromFragment
Push methods now return typed objects instead of raw Y.Map<unknown>,
eliminating all unsafe .get('content') as Y.Foo casts in consumers.
Two new Timeline methods encapsulate duplicated patterns:
- replaceCurrentText: in-place text replacement (was duplicated in
handle.write() and restoreFromSnapshot)
- pushRichtextFromFragment: deep-clone with formatting preserved
(was inlined in restoreFromSnapshot)
Push methods now return TextEntry/RichTextEntry/SheetEntry from entries.ts—includes type discriminant and createdAt alongside content. Inlined appendTextEntry/appendRichtextEntry back into the return object as peer methods; replaceCurrentText and pushRichtextFromFragment use this.pushText()/this.pushRichtext() instead of extracted closures.
Draft spec for merging Timeline and DocumentHandle into one object.
Handle becomes Timeline & { exports }—eliminates the escape hatch
pattern where 16% of handle access goes through handle.timeline.
Timeline now owns all content operations—mode conversion, transaction
wrapping, ydoc exposure—that previously lived on DocumentHandle.
DocumentHandle becomes Timeline & { exports }. makeHandle collapses
from 80 lines to Object.assign(createTimeline(ydoc), { exports }).
24 handle.timeline.X → handle.X migrations across filesystem and tests.
17 handle.mode → handle.currentMode renames (Timeline's canonical name).
read() is the canonical name. readAsString() was the old name kept as an alias that nothing called. Removed from type, implementation, and renamed test descriptions.
…omes method Push methods (pushText, pushSheet, pushRichtext, pushSheetFromCsv, replaceCurrentText, pushRichtextFromFragment) are now closure-internal to createTimeline. They compose the content access methods and restore logic but are not on the public Timeline type. restoreFromSnapshot moves from standalone export to Timeline method— handle.restoreFromSnapshot(binary) instead of restoreFromSnapshot(ydoc, binary). Tests migrated from push methods to content access (write, asText, asRichText, asSheet) for setup. Timeline type shrinks from 17 members to 11.
…readEntry currentEntry getter now returns a typed ValidatedEntry (discriminated union on mode) instead of raw Y.Map<unknown>. Consumers no longer need to import readEntry()—the timeline validates its own entries. readEntry becomes closure-internal. xmlFragmentToPlaintext and populateFragmentFromText removed from barrel exports (only used inside timeline.ts). Public export surface shrinks to: createTimeline, parseSheetFromCsv, serializeSheetToCsv, and types.
…scriminant
Absorb entries.ts into timeline.ts as the single source of truth.
Rename discriminant from 'type' to 'mode' in entry types so
push functions and ValidatedEntry share one type hierarchy.
ValidatedEntry is now TimelineEntry | { mode: 'empty' }.
Fires when entries are added to the timeline array. Does NOT fire for content changes within existing entries—those are handled by the Y.js shared types directly (Y.Text, Y.XmlFragment, Y.Map). Callback is () => void; consumer re-reads currentEntry to get the new state. Matches the codebase observe pattern.
… types and API
The Y.Map storage key was already 'type' but the TypeScript types used
'mode', forcing readEntry() to map between them. Aligning eliminates the
mismatch: TextEntry.type, RichTextEntry.type, SheetEntry.type,
ContentType (was ContentMode), currentType (was currentMode),
ValidatedEntry { type: 'empty' }. No storage migration needed—Y.Maps
already store 'type'.
The CSV round-trip in restoreFromSnapshot hardcoded column kind to 'text' and width to '120', dropping any custom column configuration the snapshot had. Use Y.Map.clone() instead—same approach richtext restore already uses for XmlElement/XmlText—to deep-copy columns and rows with all metadata intact.
Add writeSheet(csv) to Timeline API, mirroring write() symmetry for sheet content. Migrate file-system.ts from manual Y.Map manipulation to the new method. Add 22 tests covering mode conversion content verification, observe+restoreFromSnapshot interaction, same-mode restore, cross-mode conversions, writeSheet behavior, and batch coalescing.
Rename internal closure currentEntry() to lastEntry() to eliminate
shadowing with the public getter. Extract validated() helper to DRY
up the repeated readEntry(lastEntry()) pattern across public methods.
Change ValidatedEntry from TimelineEntry | { type: "empty" } to
TimelineEntry | null—"empty" was masquerading as a content type when
it represents absence of content. Update consumers in file-system.ts
and tests to use null checks.
Rewrite JSDoc for currentEntry (fresh object per access), read()
(lossy conversions), write()/writeSheet() (observe does not fire on
in-place replacement), and observe() (conditional firing for
write/writeSheet). Add missing writeSheet in-place observe test.
…to timeline.ts write() claimed to be the counterpart of read() but silently forced text mode. Renamed to writeText() for symmetry with writeSheet()—the name now honestly communicates which mode it targets. Moved SheetBinding type from richtext.ts (where it was misplaced) to timeline.ts next to SheetEntry, which has the same columns/rows shape.
write() now dispatches based on current mode: text replaces in-place, sheet parses CSV in-place, richtext clears and repopulates from plaintext. Eliminates type-branching in file-system.ts consumer. writeText/writeSheet removed from public API—as*() methods handle mode switching, write() handles content updating.
populateFragmentFromMarkdown parses markdown via remark-parse into an mdast, then walks the tree to build Y.XmlElement/Y.XmlText nodes matching Tiptap's document schema. Handles headings, paragraphs, bold, italic, links, inline code, code blocks, blockquotes, lists, and horizontal rules. Zero DOM dependencies—runs headless in Node/Bun.
… public APIs These CSV↔Y.Map helpers are internal to the timeline module—consumers should use handle.write()/handle.read() instead of manipulating raw Y.Maps directly. Exporting them encouraged abstraction leaks. - Remove from @epicenter/workspace root export - Remove from @epicenter/workspace timeline barrel - Remove re-exports from @epicenter/filesystem formats and root - Move CSV test coverage to workspace/src/timeline/sheet.test.ts - Trim filesystem sheet.test.ts to only fractional indexing tests
Codifies the Y.js abstraction leak pattern discovered during the timeline audit: type assertions, mode branching, raw mutations in batch callbacks, and internal helper re-exports are all indicators that consumer code is reaching through typed APIs to manipulate raw CRDTs. - Expand SKILL.md Mistake #6 into a full section with five smell indicators, the three-layer boundary rule, and code examples - Add article with the real appendFile/parseSheetFromCsv examples and the one review question: 'Could this consumer do its job with only the typed API?'
…ture observer handles Add loadSnapshot(update) to WorkspaceClient so consumers hydrate docs without touching the raw Y.Doc. Migrate CLI from Y.applyUpdate(client.ydoc) to client.loadSnapshot(). Store all 13 discarded .observe() return values across tab-manager (8) and honeycrisp (5) to document subscription lifetime and enable future teardown.
Table helper observe() typed its transaction parameter as `unknown`,
forcing consumers to cast to Y.Transaction just to access `origin`.
This leaked Y.js types into consumer code (browser-state.svelte.ts
had 3x `as Transaction` casts).
Add TransactionMeta type exposing only `{ origin: unknown }` — the
minimum surface consumers actually need. Removes the yjs type import
from tab-manager entirely.
file-system.ts appendFile() was branching on entry type and calling raw Y.Text.insert() — both indicators of a Y.js abstraction leak. Add Timeline.appendText(text) that handles text append for all entry types: inserts at end for text entries, converts + concatenates for richtext/sheet, creates new entry when empty. Reduces appendFile from 11 lines of type-checking + raw CRDT mutation to a single method call.
DocumentContext now follows the same "client-so-far" pattern as
ExtensionContext: factories receive everything the document has
minus lifecycle control. The new timeline field gives extensions
access to content abstraction (read/write/observe/mode) without
falling back to raw ydoc.getArray('timeline') internals.
Create the timeline before the extension loop and pass it through
DocumentContext. Extensions can now use timeline.observe(),
timeline.currentType, timeline.read() instead of reaching into
raw ydoc.getArray('timeline') internals.
The same timeline object is reused for the DocumentHandle—no
duplicate creation. makeHandle() now receives the pre-built
timeline instead of creating its own.
…extensions The withExtension() cast from ExtensionContext to DocumentContext was unsound—factories could access workspace-only fields (tables, awareness) that don't exist at document scope. SharedExtensionContext makes workspace-only fields optional, forcing factories to handle their absence. - Add SharedExtensionContext type with optional scope-specific fields - Add DocumentClient type mirroring WorkspaceClient for documents - Redefine DocumentContext as Pick<DocumentClient, ...> (no duplication) - Replace DocumentHandle.exports with .extensions (computed from DocumentClient) - Thread TDocExtensions generics through Documents, DocumentsHelper, WorkspaceClient - Add SharedExtensionFactory for dual-scope factory type convenience - Update createSyncExtension to use SharedExtensionFactory with awareness?.raw - Move createMarkdownPersistence to withWorkspaceExtension (needs guaranteed tables) - Export DocumentClient, SharedExtensionContext, SharedExtensionFactory from barrels - Remove DocumentContext re-export from lifecycle.ts (now lives in types.ts)
…elpers Remove the ValidatedEntry type alias (was just TimelineEntry | null) and inline replaceCurrentSheet/replaceCurrentRichtext into write() since both were single-caller functions.
…erything
The { workspace, document } split was the same code smell as the old
SharedExtensionContext — treating awareness as a sync concern rather
than a separate workspace concern.
createSyncExtension now returns a DualScopeFactory directly. Register
with .withExtension('sync', ...) and both workspace and document Y.Docs
get synced. Awareness can be added later as a separate workspace extension
if cursor position sync is needed.
- Flatten createSyncExtension to return DualScopeFactory<SyncExtensionExports>
- Remove createSyncFactory helper and { workspace, document } split
- Remove awareness from sync provider (optional concern, not sync's job)
- Simplify all call sites to .withExtension('sync', createSyncExtension(...))
…his.currentEntry/this.currentType Public methods now read from the object's own getters via method shorthand this-binding. replaceCurrentText() inlines the type check against lastEntry() directly, collapsing two separate calls into one.
…t, delete DualScopeFactory DualScopeContext was a misnomer — the factory doesn't run in a 'dual scope', it runs twice against the shared subset of two different contexts. SharedExtensionContext names what it actually is. DualScopeFactory was a wrapper around a 2-field context type — not worth the abstraction. Inline the function type at use sites instead. - Rename DualScopeContext → SharedExtensionContext - Delete DualScopeFactory (inline at createSyncExtension) - Update barrel exports, comments, README
…eSheetFromCsv instead pushSheetFromCsv duplicated 90% of pushSheet's body. Call sites in asSheet() now call pushSheet() then parseSheetFromCsv() on the result.
…e param in serialize/parse serializeSheetToCsv and parseSheetFromCsv now take a SheetBinding object instead of separate columns/rows params. Call sites simplify from serializeSheetToCsv(entry.columns, entry.rows) to serializeSheetToCsv(entry).
…Snapshot into restoreFromSnapshot Both were single-use functions. Deep-clone logic now lives directly in the switch arms with condensed inline comments explaining the clone strategy.
Call lastEntry() once and derive type from the raw Y.Map instead of going through this.currentType (which called lastEntry() internally).
readEntry is only called from the currentEntry getter. Moving it inside makes it properly private rather than module-level.
SheetEntry is now SheetBinding & { type: 'sheet'; createdAt: number } instead
of duplicating the columns/rows fields independently.
…inate raw Y.Map access All public methods and replaceCurrentText now use the typed TimelineEntry discriminated union instead of raw Y.Map.get() + as casts. lastEntry() is inlined into the currentEntry getter—readEntry() is the single boundary between untyped Yjs storage and the typed domain.
…ry.type Consistent with read(), asText(), asRichText(), asSheet() which already use switch. Null guard + early return, then exhaustive switch over content types.
write() text case: we know it's text, inline the Y.Text overwrite directly. appendText() richtext/sheet cases: we know it's not text, call pushText() directly. replaceCurrentText survives only in restoreFromSnapshot() where the live doc's type is genuinely unknown at call time.
…s.read() replaceCurrentText had 1 caller—inlined into restoreFromSnapshot's text case. asText/asRichText/asSheet each had duplicate conversion branches that only differed in how they stringified the entry. Since this.read() already does that, the non-matching branches collapse from per-type switches to a single conversion path: read as string → push as target type.
… this.read() Binary decision: text → append in-place, anything else → flatten + push. The switch was pretending 3 cases when richtext and sheet did the identical thing. if/else matches the actual reasoning pattern.
Codifies the systematic audit → diff → implement → commit workflow: caller counting, type safety boundaries, branch collapsing, composition over duplication, parameter objects, and surgical commits.
Link skills that cover complementary patterns so readers can discover related guidance: control-flow↔refactoring, factory↔method-shorthand, services↔errors↔define-errors↔query, typescript↔arktype↔typebox, svelte↔query↔styling, workspace-api↔yjs, git↔incremental-commits.
Standardize frontmatter with author: epicenter, version: '1.0' across all SKILL.md files for consistency. 13 skills that already had metadata blocks were left untouched.
Every skill now has a trigger section with 3-5 concrete bullet points derived from its own description and content. Helps agents decide when to load each skill.
…mentClient JSDoc SharedExtensionContext is the intersection of ExtensionContext and DocumentContext—not a base type. Pick<ExtensionContext, 'ydoc' | 'whenReady'> expresses this relationship explicitly and auto-tracks if those fields change. Also fix DocumentClient JSDoc: said DocumentContext derives 'via Omit' but the implementation uses Pick (IS-A vs HAS-A: handle IS a Timeline, context HAS a timeline field).
# Conflicts: # packages/workspace/src/index.ts
- Rewrite lifecycle.ts module docstring to reflect current architecture (was describing stale Provider/Extension bifurcation with nonexistent ProviderFactory type) - Export DOCUMENTS_ORIGIN from workspace/index.ts and root index.ts to match its consumer-facing JSDoc
# Conflicts: # packages/workspace/src/workspace/table-helper.ts # packages/workspace/src/workspace/types.ts
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.
Overhauls the Timeline from a bag of standalone functions into a cohesive object API, and cleans up the extension system's dual-scope registration model. What started as a snapshot restore bug led to pulling on threads until the whole timeline module was restructured.
Why the timeline needed this
The old timeline had two problems. First,
DocumentHandlewas a wrapper aroundTimelinethat added nothing—consumers had to go through the handle to reach timeline methods, but the handle's only job was forwarding calls. Second, the public API surface was too wide:readEntry,pushText,pushRichtext,pushSheet,parseSheetFromCsv,restoreFromSnapshotwere all standalone exports that leaked implementation details.The snapshot restore bug was the catalyst. Server-side
applySnapshot()was a silent no-op (CRDT idempotency means applying an old state to a newer doc changes nothing), and the richtext path destroyed formatting by extracting text-only content fromXmlFragment. Fixing this properly requiredrestoreFromSnapshotto become a method that understood the timeline's internal state—which meant the standalone-function architecture had to go.Mode-aware write
The old API had
writeText()for text content. Nowwrite()is mode-aware—it checks the current entry type and either replaces in-place (same type, no new entry,observe()doesn't fire) or pushes a new entry (type change,observe()fires).writeSheet()follows the same pattern:Snapshot restore
Moved entirely to client-side. The key fix for richtext: deep-clone the
XmlFragmenttree instead of extracting text.Extension system: dual-scope registration
Extensions that work on both the workspace Y.Doc and content document Y.Docs (persistence, sync) previously had no type-safe way to register once for both scopes. The factory signature for workspace extensions (
ExtensionContextwith tables, kv, awareness) is wider than for document extensions (DocumentContextwith timeline, ydoc).SharedExtensionContextis the intersection—the fields available in both scopes:withExtension()becomes thin sugar—it registers the same factory for both scopes, constrained toSharedExtensionContext. If you need workspace-only or document-only fields, use the scoped methods:The sync extension now returns scoped factories:
Other extension changes
timelinewas added toDocumentContextso document extension factories can bind to the content timeline directly. LIFO teardown logic (destroyLifo/startDestroyLifo) was extracted tolifecycle.tsas shared primitives used by bothcreateWorkspaceandcreateDocuments. The lifecycle.ts module docstring was rewritten—it still described a stale "Providers vs Extensions" architecture with a nonexistentProviderFactorytype.DOCUMENTS_ORIGINis now exported from@epicenter/workspace. It's a sentinel symbol for distinguishing auto-bumps (document edit → rowupdatedAtupdate) from user-initiated row changes intable.observe()callbacks.Smaller changes
table.delete()returnsvoidinstead ofDeleteResult. The discriminated union added no value—Y.Map.delete() is fire-and-forget, and callers never branched on the result.table.observe()callbacks now receive typedTransactionMeta(withorigin) instead of rawunknown.~30 skill files got metadata blocks, cross-reference callouts, and "When to Apply" sections. A refactoring methodology skill was added.