diff --git a/.agents/skills/workspace-api/SKILL.md b/.agents/skills/workspace-api/SKILL.md index 2e8ec0b687..b5367f4a34 100644 --- a/.agents/skills/workspace-api/SKILL.md +++ b/.agents/skills/workspace-api/SKILL.md @@ -273,6 +273,56 @@ return { ...row, views: 0, _v: 2 }; // Works — contextual narrowing return { ...row, views: 0, _v: 2 as const }; // Also works — redundant ``` +## Document Content (Per-Row Y.Docs) + +Tables with `.withDocument()` create a content Y.Doc per row. Content is stored using a **timeline model**: a `Y.Array('timeline')` inside the Y.Doc, where each entry is a typed `Y.Map` supporting text, binary, and sheet modes. + +### Reading and Writing Content + +Use the filesystem package's content helpers, which read/write via the timeline: + +```typescript +import { createYjsFileSystem } from '@epicenter/filesystem'; + +const fs = createYjsFileSystem(ws.tables.files, ws.documents.files.content); + +// Read content (timeline-backed) +const text = await fs.content.read(fileId); + +// Write content (timeline-backed) +await fs.content.write(fileId, 'hello'); +``` + +### Anti-Patterns + +**Do not use `handle.read()`/`handle.write()`** for apps that also use the filesystem API. These methods read/write from `Y.Text('content')`—a different shared type than the timeline—causing silent data loss: + +```typescript +// ❌ BAD: handle uses Y.Text('content'), filesystem uses Y.Array('timeline') +const handle = await ws.documents.files.content.open(id); +handle.read(); // reads from wrong shared type +handle.write('x'); // writes to wrong shared type + +// ✅ GOOD: use fs.content which reads/writes via timeline +await fs.content.read(id); +await fs.content.write(id, 'hello'); +``` + +**Do not access `handle.ydoc` directly for content:** + +```typescript +// ❌ BAD: bypasses all abstractions +const ytext = handle.ydoc.getText('content'); +const fragment = handle.ydoc.getXmlFragment('content'); + +// ✅ GOOD: use timeline abstraction +import { createTimeline } from '@epicenter/filesystem'; +const tl = createTimeline(handle.ydoc); +const text = tl.readAsString(); +``` + +See `specs/20260313T224500-unify-document-content-model.md` for the full unification plan. + ## References - `packages/workspace/src/workspace/define-table.ts` diff --git a/.agents/skills/yjs/SKILL.md b/.agents/skills/yjs/SKILL.md index 786afdf379..74b7a23138 100644 --- a/.agents/skills/yjs/SKILL.md +++ b/.agents/skills/yjs/SKILL.md @@ -207,6 +207,31 @@ yarray.push([sameItem]); // Different Y.Map instance internally Any concurrent edits to the "moved" item are lost because you deleted the original. +### 6. Accessing Raw Y.Doc Shared Types for Document Content + +Document Y.Docs in this codebase use a timeline model (`Y.Array('timeline')` with nested typed entries). Accessing top-level shared types directly writes to a different location than the timeline, causing silent data loss: + +```typescript +// BAD: writes to Y.Text('content'), not the timeline +const ytext = handle.ydoc.getText('content'); +ytext.insert(0, 'hello'); // invisible to fs.readFile() + +// BAD: handle.read/write also uses Y.Text('content') +handle.read(); // reads from wrong shared type +handle.write('x'); // writes to wrong shared type + +// GOOD: use filesystem content helpers (timeline-backed) +await fs.content.read(id); +await fs.content.write(id, 'hello'); + +// GOOD: use timeline abstraction directly if needed +import { createTimeline } from '@epicenter/filesystem'; +const tl = createTimeline(handle.ydoc); +const text = tl.readAsString(); +``` + +See `specs/20260313T224500-unify-document-content-model.md`. + ## Debugging Tips ### Inspect Document State diff --git a/AGENTS.md b/AGENTS.md index 7e0fd83ed1..6da13c96c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,3 +11,5 @@ Destructive actions need approval: Force pushes, hard resets (`--hard`), branch Token-efficient execution: When possible, delegate to sub-agent with only the command. Instruct it to execute without re-analyzing. Writing conventions: Load `writing-voice` skill for any user-facing text—UI strings, tooltips, error messages, docs. Em dashes are always closed (no spaces). + +Content model: Document content uses the timeline model (`Y.Array('timeline')`). Never access `handle.ydoc.getText('content')` or use `handle.read()`/`handle.write()` alongside filesystem APIs—use `fs.content.read/write` or `createTimeline(ydoc)` from `@epicenter/filesystem` instead. See `specs/20260313T224500-unify-document-content-model.md`. diff --git a/apps/opensidian/package.json b/apps/opensidian/package.json index 68e179933b..440de3af55 100644 --- a/apps/opensidian/package.json +++ b/apps/opensidian/package.json @@ -12,13 +12,14 @@ }, "devDependencies": { "@epicenter/filesystem": "workspace:*", - "@epicenter/workspace": "workspace:*", "@epicenter/ui": "workspace:*", + "@epicenter/workspace": "workspace:*", "@sveltejs/adapter-auto": "catalog:", "@sveltejs/kit": "catalog:", "@sveltejs/vite-plugin-svelte": "catalog:", "@tailwindcss/vite": "catalog:", "bun-types": "catalog:", + "lucide-svelte": "^0.577.0", "svelte": "catalog:", "svelte-check": "catalog:", "svelte-sonner": "catalog:", diff --git a/apps/opensidian/src/lib/components/ContentEditor.svelte b/apps/opensidian/src/lib/components/ContentEditor.svelte index a7c68429cd..8f33cb5459 100644 --- a/apps/opensidian/src/lib/components/ContentEditor.svelte +++ b/apps/opensidian/src/lib/components/ContentEditor.svelte @@ -1,7 +1,7 @@ @@ -56,13 +62,13 @@ Loading... {:else} - + /> {/if} diff --git a/apps/opensidian/src/lib/components/CreateDialog.svelte b/apps/opensidian/src/lib/components/CreateDialog.svelte index e8dec14db8..d0f3550cf6 100644 --- a/apps/opensidian/src/lib/components/CreateDialog.svelte +++ b/apps/opensidian/src/lib/components/CreateDialog.svelte @@ -1,6 +1,8 @@ {#if fsState.rootChildIds.length === 0} -
-

No files yet

-

Use the toolbar to create files and folders

-
+ + + No files yet + Use the toolbar to create files and folders + + {:else} -
+ {#each fsState.rootChildIds as childId (childId)} - + {/each} -
+ {/if} diff --git a/apps/opensidian/src/lib/components/FileTreeItem.svelte b/apps/opensidian/src/lib/components/FileTreeItem.svelte new file mode 100644 index 0000000000..e41840323f --- /dev/null +++ b/apps/opensidian/src/lib/components/FileTreeItem.svelte @@ -0,0 +1,104 @@ + + +{#if row} + + + {#snippet child({ props })} + {#if isFolder} +
+ fsState.actions.toggleExpand(id)} + class="w-full rounded-sm px-2 py-1 text-sm hover:bg-accent {isSelected + ? 'bg-accent text-accent-foreground' + : ''}" + > + {#each children as childId (childId)} + + {/each} + +
+ {:else} + fsState.actions.selectFile(id)} + onkeydown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + fsState.actions.selectFile(id); + } + }} + role="treeitem" + > + {#snippet icon()} + + {/snippet} + + {/if} + {/snippet} +
+ + {#if isFolder} + selectAndOpenCreate('file')}> + New File + + selectAndOpenCreate('folder')}> + New Folder + + + {/if} + Rename + + Delete + + +
+ + + + +{/if} diff --git a/apps/opensidian/src/lib/components/RenameDialog.svelte b/apps/opensidian/src/lib/components/RenameDialog.svelte index c7303b67fe..81325b8842 100644 --- a/apps/opensidian/src/lib/components/RenameDialog.svelte +++ b/apps/opensidian/src/lib/components/RenameDialog.svelte @@ -1,6 +1,8 @@ -
- - - - - -
- + +
+ + + {#snippet child({ props })} + + {/snippet} + + Create a new file + + + + {#snippet child({ props })} + + {/snippet} + + Create a new folder + + + + + {#snippet child({ props })} + + {/snippet} + + Rename selected item + + + + {#snippet child({ props })} + + {/snippet} + + Delete selected item + +
+ + + {#snippet child({ props })} + + {/snippet} + + Load example files and folders + +
-
+ diff --git a/apps/opensidian/src/lib/components/TreeNode.svelte b/apps/opensidian/src/lib/components/TreeNode.svelte deleted file mode 100644 index 91d63b3600..0000000000 --- a/apps/opensidian/src/lib/components/TreeNode.svelte +++ /dev/null @@ -1,177 +0,0 @@ - - -{#if row} - - - {#snippet child({ props })} - {#if isFolder} -
- - - {#snippet child({ props: collapsibleProps })} - - {/snippet} - - - {#each children as childId (childId)} - - {/each} - - -
- {:else} - - {/if} - {/snippet} -
- - {#if isFolder} - selectAndOpenCreate('file')}> - New File - - selectAndOpenCreate('folder')}> - New Folder - - - {/if} - Rename - - Delete - - -
- - - - -{/if} diff --git a/apps/opensidian/src/lib/fs/fs-state.svelte.ts b/apps/opensidian/src/lib/fs/fs-state.svelte.ts index b5d7f8f1ca..1be1a6da32 100644 --- a/apps/opensidian/src/lib/fs/fs-state.svelte.ts +++ b/apps/opensidian/src/lib/fs/fs-state.svelte.ts @@ -223,15 +223,14 @@ function createFsState() { }, /** - * Read file content as string via the documents manager. + * Read file content as string via the filesystem's timeline-backed content helpers. * - * Opens (or reuses) the per-file Y.Doc via the document handle, - * then reads the text content synchronously. + * Uses `fs.content.read()` which reads from the `Y.Array('timeline')` in the + * per-file content Y.Doc—the same shared type that `fs.writeFile()` writes to. */ async readContent(id: FileId): Promise { try { - const handle = await ws.documents.files.content.open(id); - return handle.read(); + return await fs.content.read(id); } catch (err) { console.error('Failed to read content:', err); return null; @@ -239,15 +238,15 @@ function createFsState() { }, /** - * Write file content via the documents manager. + * Write file content via the filesystem's timeline-backed content helpers. * - * The documents manager automatically bumps `updatedAt` on the file row - * when content changes. Toasts only on error. + * Uses `fs.content.write()` which writes to the `Y.Array('timeline')` in the + * per-file content Y.Doc. The documents manager's `onUpdate` callback still + * fires (it watches all Y.Doc changes) and bumps `updatedAt` on the file row. */ async writeContent(id: FileId, data: string): Promise { try { - const handle = await ws.documents.files.content.open(id); - handle.write(data); + await fs.content.write(id, data); } catch (err) { toast.error( err instanceof Error ? err.message : 'Failed to save file', diff --git a/bun.lock b/bun.lock index fa8966fbda..5b9637d712 100644 --- a/bun.lock +++ b/bun.lock @@ -156,6 +156,7 @@ "@sveltejs/vite-plugin-svelte": "catalog:", "@tailwindcss/vite": "catalog:", "bun-types": "catalog:", + "lucide-svelte": "^0.577.0", "svelte": "catalog:", "svelte-check": "catalog:", "svelte-sonner": "catalog:", @@ -2087,6 +2088,8 @@ "lru-cache": ["lru-cache@11.2.7", "", {}, "sha512-aY/R+aEsRelme17KGQa/1ZSIpLpNYYrhcrepKTZgE+W3WM16YMCaPwOHLHsmopZHELU0Ojin1lPVxKR0MihncA=="], + "lucide-svelte": ["lucide-svelte@0.577.0", "", { "peerDependencies": { "svelte": "^3 || ^4 || ^5.0.0-next.42" } }, "sha512-0i88o57KsaHWnc80J57fY99CWzlZsSdtH5kKjLUJa7z8dum/9/AbINNLzJ7NiRFUdOgMnfAmJt8jFbW2zeC5qQ=="], + "lz-string": ["lz-string@1.5.0", "", { "bin": { "lz-string": "bin/bin.js" } }, "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ=="], "magic-string": ["magic-string@0.30.21", "", { "dependencies": { "@jridgewell/sourcemap-codec": "^1.5.5" } }, "sha512-vd2F4YUyEXKGcLHoq+TEyCjxueSeHnFxyyjNp80yg0XV4vUhnDer/lvvlqM/arB5bXQN5K2/3oinyCRyx8T2CQ=="], diff --git a/packages/ui/src/tree-view/index.ts b/packages/ui/src/tree-view/index.ts new file mode 100644 index 0000000000..2f069d31bd --- /dev/null +++ b/packages/ui/src/tree-view/index.ts @@ -0,0 +1,13 @@ +import TreeView from './tree-view.svelte'; +import TreeViewFile from './tree-view-file.svelte'; +import TreeViewFolder from './tree-view-folder.svelte'; + +export { + TreeView, + TreeViewFile, + TreeViewFolder, + // ... + TreeView as Root, + TreeViewFile as File, + TreeViewFolder as Folder, +}; diff --git a/packages/ui/src/tree-view/tree-view-file.svelte b/packages/ui/src/tree-view/tree-view-file.svelte new file mode 100644 index 0000000000..b31d29ad58 --- /dev/null +++ b/packages/ui/src/tree-view/tree-view-file.svelte @@ -0,0 +1,26 @@ + + + diff --git a/packages/ui/src/tree-view/tree-view-folder.svelte b/packages/ui/src/tree-view/tree-view-folder.svelte new file mode 100644 index 0000000000..7c1757c558 --- /dev/null +++ b/packages/ui/src/tree-view/tree-view-folder.svelte @@ -0,0 +1,35 @@ + + + + + {#if icon} + {@render icon({ name, open })} + {:else if open} + + {:else} + + {/if} + {name} + + +
+
+
{@render children?.()}
+
+
+
diff --git a/packages/ui/src/tree-view/tree-view.svelte b/packages/ui/src/tree-view/tree-view.svelte new file mode 100644 index 0000000000..018964c852 --- /dev/null +++ b/packages/ui/src/tree-view/tree-view.svelte @@ -0,0 +1,10 @@ + + +
+ {@render children?.()} +
diff --git a/packages/ui/src/tree-view/types.ts b/packages/ui/src/tree-view/types.ts new file mode 100644 index 0000000000..5aaea8b294 --- /dev/null +++ b/packages/ui/src/tree-view/types.ts @@ -0,0 +1,21 @@ +import type { WithChildren, WithoutChildren } from 'bits-ui'; +import type { Snippet } from 'svelte'; +import type { HTMLAttributes, HTMLButtonAttributes } from 'svelte/elements'; + +export type TreeViewRootProps = HTMLAttributes; + +export type TreeViewFolderProps = WithChildren<{ + name: string; + open?: boolean; + onOpenChange?: (open: boolean) => void; + class?: string; + icon?: Snippet<[{ name: string; open: boolean }]>; +}>; + +export type TreeViewFilePropsWithoutHTML = WithChildren<{ + name: string; + icon?: Snippet<[{ name: string }]>; +}>; + +export type TreeViewFileProps = WithoutChildren & + TreeViewFilePropsWithoutHTML; diff --git a/packages/ui/src/utils.ts b/packages/ui/src/utils.ts index 50338df890..848565bb60 100644 --- a/packages/ui/src/utils.ts +++ b/packages/ui/src/utils.ts @@ -1,13 +1,9 @@ -/* - Installed from @ieedan/shadcn-svelte-extras -*/ - import { type ClassValue, clsx } from 'clsx'; import { twMerge } from 'tailwind-merge'; -export type WithElementRef = T & { - ref?: null | U; -}; +export function cn(...inputs: ClassValue[]) { + return twMerge(clsx(inputs)); +} // biome-ignore lint/suspicious/noExplicitAny: inherited from shadcn-svelte export type WithoutChild = T extends { child?: any } ? Omit : T; @@ -16,6 +12,6 @@ export type WithoutChildren = T extends { children?: any } ? Omit : T; export type WithoutChildrenOrChild = WithoutChildren>; -export function cn(...inputs: ClassValue[]) { - return twMerge(clsx(inputs)); -} +export type WithElementRef = T & { + ref?: U | null; +}; diff --git a/packages/workspace/AGENTS.md b/packages/workspace/AGENTS.md index c48c88dfd0..c2c8a7ec4d 100644 --- a/packages/workspace/AGENTS.md +++ b/packages/workspace/AGENTS.md @@ -8,6 +8,10 @@ Core library shared across apps. - All functions return `Result` types - Use Bun for everything (see below) +## Content Model + +`handle.read()`/`handle.write()` on `DocumentHandle` use raw `Y.Text('content')`, which is NOT the same shared type as the filesystem's timeline (`Y.Array('timeline')`). Apps using `@epicenter/filesystem` must use `fs.content.read/write` for timeline-backed content access. Direct `handle.ydoc.getText('content')` access is an anti-pattern. See `specs/20260313T224500-unify-document-content-model.md`. + ## Bun Usage Default to Bun instead of Node.js: diff --git a/packages/workspace/README.md b/packages/workspace/README.md index cd82ceacf0..3704dc7183 100644 --- a/packages/workspace/README.md +++ b/packages/workspace/README.md @@ -1477,6 +1477,11 @@ client.whenReady; // Promise for async initialization await client.destroy(); // Cleanup resources ``` +### Document Content Model + +> **Important**: Tables with `.withDocument()` create per-row content Y.Docs. These use a **timeline model** (`Y.Array('timeline')`) in `@epicenter/filesystem` for multi-format content (text, binary, sheet). `DocumentHandle.read()`/`write()` use a different shared type (`Y.Text('content')`) and are not timeline-aware. Apps using the filesystem package should use `fs.content.read()`/`fs.content.write()` for content access. Accessing `handle.ydoc.getText('content')` or `handle.ydoc.getXmlFragment('content')` directly is discouraged—use `createTimeline(ydoc)` from `@epicenter/filesystem` instead. See `specs/20260313T224500-unify-document-content-model.md`. + + ### Column Schemas ```typescript diff --git a/packages/workspace/src/workspace/README.md b/packages/workspace/src/workspace/README.md index 3444b538b1..ebbff74bc9 100644 --- a/packages/workspace/src/workspace/README.md +++ b/packages/workspace/src/workspace/README.md @@ -103,6 +103,14 @@ if (result.status === 'valid') { For detailed rationale on all of this, see [the guide](docs/articles/20260127T120000-static-workspace-api-guide.md). +## Document Content + +Tables with `.withDocument()` create per-row Y.Docs for content. These Y.Docs use a **timeline model** (`Y.Array('timeline')` with nested typed entries) in the filesystem package. + +**Important**: `DocumentHandle.read()`/`write()` currently use `Y.Text('content')`, which is a different shared type than the timeline. If your app uses `@epicenter/filesystem`, prefer `fs.content.read()`/`fs.content.write()` for timeline-backed access. Accessing `handle.ydoc.getText('content')` directly is also discouraged. + +See `specs/20260313T224500-unify-document-content-model.md` for the full unification plan and anti-pattern reference. + ## Testing The tests are in `*.test.ts` files next to the implementation. Use `new Y.Doc()` for in-memory tests. Migrations are validated by reading old data and checking the result. Look at existing tests for patterns. diff --git a/packages/workspace/src/workspace/types.ts b/packages/workspace/src/workspace/types.ts index 32bb396ad9..613b83d558 100644 --- a/packages/workspace/src/workspace/types.ts +++ b/packages/workspace/src/workspace/types.ts @@ -251,15 +251,45 @@ export type ClaimedDocumentColumns< * All operations are scoped to this specific document. Content methods * (read, write) are synchronous because the Y.Doc is already open. * Exports are a property, not a function, because they belong to this doc. + * + * **Content model warning**: `read()` and `write()` operate on a raw + * `Y.Text('content')` shared type, which is NOT the same as the filesystem's + * timeline (`Y.Array('timeline')`). If your app uses the filesystem package, + * prefer `fs.content.read()`/`fs.content.write()` instead. Direct `handle.ydoc` + * access for content (e.g. `handle.ydoc.getText('content')`) is also discouraged— + * use `createTimeline(handle.ydoc)` from `@epicenter/filesystem` instead. + * + * See `specs/20260313T224500-unify-document-content-model.md` for the unification plan. */ export type DocumentHandle = { - /** The raw Y.Doc — escape hatch for custom operations (timelines, binary, sheets). */ + /** + * The underlying Y.Doc for this document. + * + * Use for genuinely custom shared types (awareness, cursors, non-content data). + * For content operations, prefer the filesystem's timeline-backed helpers + * (`fs.content.read/write` or `createTimeline(ydoc)`) over raw shared type + * access like `ydoc.getText('content')` or `ydoc.getXmlFragment('content')`. + */ ydoc: Y.Doc; - /** Read the document's text content (from `ydoc.getText('content')`). */ + /** + * Read the document's text content from `ydoc.getText('content')`. + * + * **Warning**: This reads from a raw `Y.Text('content')` shared type, NOT the + * filesystem's timeline. If your app uses `@epicenter/filesystem`, prefer + * `fs.content.read(id)` which reads from the timeline. This method will be + * unified with the timeline in a future version. + */ read(): string; - /** Replace the document's text content. */ + /** + * Replace the document's text content in `ydoc.getText('content')`. + * + * **Warning**: This writes to a raw `Y.Text('content')` shared type, NOT the + * filesystem's timeline. If your app uses `@epicenter/filesystem`, prefer + * `fs.content.write(id, text)` which writes to the timeline. This method will + * be unified with the timeline in a future version. + */ write(text: string): void; /** diff --git a/specs/20260219T094400-migrate-filesystem-to-document-binding.md b/specs/20260219T094400-migrate-filesystem-to-document-binding.md index e0ac4e18bb..04db63dcf2 100644 --- a/specs/20260219T094400-migrate-filesystem-to-document-binding.md +++ b/specs/20260219T094400-migrate-filesystem-to-document-binding.md @@ -1,6 +1,7 @@ # Migrate Filesystem Package to Document Binding API > **Note**: The `.docs` access pattern described here was replaced by `client.documents` — see specs/20260221T204200-documents-top-level-namespace.md +> **Content model note**: The "two content access paths" design described in this spec's review section is superseded by `specs/20260313T224500-unify-document-content-model.md`. The dual model (handle Y.Text vs filesystem timeline) causes silent data loss and is being unified on the timeline model. **Date**: 2026-02-19 **Status**: Complete diff --git a/specs/20260313T143100-opensidian-ui-idiomaticity.md b/specs/20260313T143100-opensidian-ui-idiomaticity.md new file mode 100644 index 0000000000..dea4b96d39 --- /dev/null +++ b/specs/20260313T143100-opensidian-ui-idiomaticity.md @@ -0,0 +1,187 @@ +# OpenSidian UI Idiomaticity + +**Date**: 2026-03-13 +**Status**: Implemented +**Author**: AI-assisted + +## Overview + +Fix specific non-idiomatic patterns in OpenSidian's UI where hand-written markup reimplements existing shadcn-svelte primitives from `@epicenter/ui`. These are targeted, mechanical fixes—not new features. + +## Motivation + +### Current State + +OpenSidian has 10 components built on `@epicenter/ui` primitives (Collapsible, ContextMenu, Dialog, AlertDialog, Breadcrumb, Button, Resizable, ScrollArea, Separator). The overall structure is sound, but several components bypass available primitives with manual implementations. + +**Problem 1: Raw `` with reimplemented styling** + +`CreateDialog.svelte` and `RenameDialog.svelte` use raw `` elements with manually copied shadcn classes: + +```svelte + + +``` + +This is literally the `Input` component's class string copy-pasted. If the design system updates Input styling, these dialogs won't pick up the change. + +**Problem 2: Inline SVG icons** + +`TreeNode.svelte` defines SVG icons inline (ChevronRight, Folder, FolderOpen, File). Each is 5–10 lines of SVG markup hardcoded in the template. The monorepo already uses icon components elsewhere. + +**Problem 3: Missing `Textarea` component** + +`ContentEditor.svelte` uses a raw `