refactor(opensidian): consolidate state, eliminate duplication, and reorganize by concern

.agents/skills/refactoring/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: refactoring
-description: Systematic code audit and refactoring methodology—caller counting, type safety boundaries, inlining single-use extractions, collapsing duplicate branches, and surgical commits. Use when cleaning up code, auditing for code smells, refactoring modules, or reviewing internal function structure.
+description: Systematic code audit and refactoring methodology—caller counting, type safety boundaries, inlining single-use extractions, keeping trivial duplications inline over premature extraction, collapsing duplicate branches, and surgical commits. Use when cleaning up code, auditing for code smells, refactoring modules, or reviewing internal function structure.
 metadata:
   author: epicenter
   version: '1.0'
@@ -154,6 +154,63 @@ function pushSheetFromCsv(csv: string): SheetEntry {
 parseSheetFromCsv(csv, result);
 ```
 
+## Prefer Inline for Trivial Duplications
+
+When a duplicated block is 1–3 lines and appears 2–3 times within the same file, keeping it inline is usually more readable than extracting a helper. The helper adds a name to learn, a definition to jump to, and an abstraction boundary to reason about—all of which cost more than the duplication saves.
+
+**The readability test:** Does a reader need to leave the callsite to understand what's happening? If the inline code is self-explanatory, extraction hurts more than it helps.
+
+```typescript
+// Two adjacent functions with identical 2-line path construction.
+// A buildChildPath() helper saves zero cognitive load—readers
+// understand the inline version instantly.
+
+// GOOD: Keep inline. The repetition is obvious and local.
+async createFile(parentId: FileId | null, name: string) {
+	const parentPath = parentId ? (this.getPath(parentId) ?? '/') : '/';
+	const path = parentPath === '/' ? `/${name}` : `${parentPath}/${name}`;
+	await fs.writeFile(path, '');
+}
+
+async createFolder(parentId: FileId | null, name: string) {
+	const parentPath = parentId ? (this.getPath(parentId) ?? '/') : '/';
+	const path = parentPath === '/' ? `/${name}` : `${parentPath}/${name}`;
+	await fs.mkdir(path);
+}
+
+// BAD: Extracted for 2 callers in the same file.
+// Reader now has to find buildChildPath() to understand either function.
+function buildChildPath(parentId: FileId | null, name: string): string {
+	const parentPath = parentId ? (this.getPath(parentId) ?? '/') : '/';
+	return parentPath === '/' ? `/${name}` : `${parentPath}/${name}`;
+}
+```
+
+Same principle in templates—a class string used twice is easier to scan inline than to chase to a `$derived` variable:
+
+```svelte
+<!-- GOOD: Both usages visible at a glance while scanning the template -->
+<TreeView.Folder
+	class="w-full rounded-sm px-2 py-1 text-sm hover:bg-accent
+		{isHighlighted ? 'bg-accent text-accent-foreground' : ''}"
+/>
+<!-- ...later in the same template... -->
+<TreeView.File
+	class="w-full rounded-sm px-2 py-1 text-sm hover:bg-accent
+		{isHighlighted ? 'bg-accent text-accent-foreground' : ''}"
+/>
+
+<!-- BAD: Extracted to a variable—reader leaves the template to understand styling -->
+<TreeView.Folder class={itemClass} />
+```
+
+### When extraction IS worth it
+
+- The block is 5+ lines (the abstraction pays for itself)
+- It appears 4+ times (pattern, not coincidence)
+- The callers are in different files (no local context to rely on)
+- The logic is non-obvious and the function name documents intent
+
 ## Inline Known-Behavior Calls
 
     When a "smart" function branches internally but every caller already knows which branch it takes:
@@ -238,7 +295,7 @@ c4f8ddc  refactor: move SheetBinding to sheet.ts, accept as single param
 
 ## Anti-Patterns
 
-- **Premature extraction**: Extracting a function used once that doesn't add clarity over inline code
+- **Premature extraction**: Extracting a 1–3 line block used 2–3 times into a named helper. The indirection costs more than the duplication. See "Prefer Inline for Trivial Duplications" above.
 - **Abstracting away differences**: Three push constructors with different fields share boilerplate, but a `pushEntry(type, fields: Record<string, unknown>)` helper loses all type safety. The duplication communicates structure.
 - **Type-erasing helpers**: Any helper that accepts `unknown` or `Record<string, any>` to "reduce duplication"
 - **Refactoring while fixing bugs**: Fix the bug minimally first, refactor in a separate commit

apps/opensidian/src/lib/components/AppShell.svelte

@@ -1,10 +1,11 @@
 <script lang="ts">
 	import * as Resizable from '@epicenter/ui/resizable';
 	import { ScrollArea } from '@epicenter/ui/scroll-area';
-	import ContentPanel from './ContentPanel.svelte';
-	import FileTree from './FileTree.svelte';
-	import Toolbar from './Toolbar.svelte';
 	import CommandPalette from './CommandPalette.svelte';
+	import ContentPanel from './editor/ContentPanel.svelte';
+	import DeleteConfirmation from './dialogs/DeleteConfirmation.svelte';
+	import FileTree from './tree/FileTree.svelte';
+	import Toolbar from './Toolbar.svelte';
 </script>
 
 <div class="flex h-screen flex-col">
@@ -20,3 +21,5 @@
 	</Resizable.PaneGroup>
 	<CommandPalette />
 </div>
+
+<DeleteConfirmation />

apps/opensidian/src/lib/components/CommandPalette.svelte

@@ -4,8 +4,8 @@
 		CommandPalette,
 		type CommandPaletteItem,
 	} from '@epicenter/ui/command-palette';
-	import { getFileIcon } from '$lib/fs/file-icons';
-	import { fsState } from '$lib/fs/fs-state.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
+	import { getFileIcon } from '$lib/utils/file-icons';
 
 	let open = $state(false);
 	let searchQuery = $state('');
@@ -16,25 +16,15 @@
 
 	const allFiles = $derived.by((): FileEntry[] => {
 		if (!open) return [];
-		void fsState.version;
-
-		const files: FileEntry[] = [];
-		function collect(parentId: FileId | null) {
-			for (const childId of fsState.getChildIds(parentId)) {
-				const row = fsState.getRow(childId);
-				if (!row) continue;
-				if (row.type === 'file') {
-					const fullPath = fsState.getPathForId(childId) ?? '';
-					const lastSlash = fullPath.lastIndexOf('/');
-					const parentDir = lastSlash > 0 ? fullPath.slice(1, lastSlash) : '';
-					files.push({ id: childId, name: row.name, parentDir });
-				} else if (row.type === 'folder') {
-					collect(childId);
-				}
+		return fsState.walkTree<FileEntry>((id, row) => {
+			if (row.type === 'file') {
+				const fullPath = fsState.getPathForId(id) ?? '';
+				const lastSlash = fullPath.lastIndexOf('/');
+				const parentDir = lastSlash > 0 ? fullPath.slice(1, lastSlash) : '';
+				return { collect: { id, name: row.name, parentDir }, descend: false };
 			}
-		}
-		collect(null);
-		return files;
+			return { descend: true };
+		});
 	});
 
 	// ── Debounce search input at 150ms ───────────────────────────────

apps/opensidian/src/lib/components/Toolbar.svelte

@@ -1,19 +1,17 @@
 <script lang="ts">
 	import { Button } from '@epicenter/ui/button';
 	import { Separator } from '@epicenter/ui/separator';
-	import * as Tooltip from '@epicenter/ui/tooltip';
 	import { Spinner } from '@epicenter/ui/spinner';
+	import * as Tooltip from '@epicenter/ui/tooltip';
 	import { toast } from 'svelte-sonner';
-	import { fsState } from '$lib/fs/fs-state.svelte';
-	import DeleteConfirmation from './DeleteConfirmation.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
+	import { fs } from '$lib/workspace';
 
-	let deleteDialogOpen = $state(false);
 	let seeding = $state(false);
 
 	async function loadSampleData() {
 		seeding = true;
 		try {
-			const { fs } = fsState;
 			await fs.mkdir('/docs');
 			await fs.mkdir('/src');
 			await fs.mkdir('/src/utils');
@@ -108,7 +106,7 @@
 						variant="ghost"
 						size="sm"
 						onclick={() => {
-							if (fsState.activeFileId) deleteDialogOpen = true;
+							if (fsState.activeFileId) fsState.openDelete();
 						}}
 						disabled={!fsState.activeFileId}
 					>
@@ -129,7 +127,11 @@
 							onclick={loadSampleData}
 							disabled={seeding}
 						>
-							{#if seeding}<Spinner class="size-3.5" />{:else}Load Sample Data{/if}
+							{#if seeding}
+								<Spinner class="size-3.5" />
+							{:else}
+								Load Sample Data
+							{/if}
 						</Button>
 					{/snippet}
 				</Tooltip.Trigger>
@@ -138,5 +140,3 @@
 		</div>
 	</div>
 </Tooltip.Provider>
-
-<DeleteConfirmation bind:open={deleteDialogOpen} />

apps/opensidian/src/lib/components/DeleteConfirmation.svelte → apps/opensidian/src/lib/components/dialogs/DeleteConfirmation.svelte

@@ -1,20 +1,26 @@
 <script lang="ts">
 	import * as AlertDialog from '@epicenter/ui/alert-dialog';
 	import { Button } from '@epicenter/ui/button';
-	import { fsState } from '$lib/fs/fs-state.svelte';
-
-	type Props = {
-		open: boolean;
-	};
-
-	let { open = $bindable(false) }: Props = $props();
+	import { fsState } from '$lib/state/fs-state.svelte';
 
 	const nodeName = $derived(fsState.selectedNode?.name ?? 'this item');
 	const nodeType = $derived(fsState.selectedNode?.type ?? 'file');
 
+	async function handleDelete() {
+		if (!fsState.activeFileId) return;
+		await fsState.deleteFile(fsState.activeFileId);
+		fsState.closeDelete();
+	}
+
+	function handleOpenChange(isOpen: boolean) {
+		if (!isOpen) fsState.closeDelete();
+	}
 </script>
 
-<AlertDialog.Root bind:open>
+<AlertDialog.Root
+	open={fsState.deleteDialogOpen}
+	onOpenChange={handleOpenChange}
+>
 	<AlertDialog.Content>
 		<AlertDialog.Header>
 			<AlertDialog.Title>Delete {nodeName}?</AlertDialog.Title>
@@ -29,7 +35,7 @@
 		</AlertDialog.Header>
 		<AlertDialog.Footer>
 			<AlertDialog.Cancel>Cancel</AlertDialog.Cancel>
-			<Button variant="destructive" onclick={async () => { if (!fsState.activeFileId) return; await fsState.deleteFile(fsState.activeFileId); open = false; }}>Delete</Button>
+			<Button variant="destructive" onclick={handleDelete}>Delete</Button>
 		</AlertDialog.Footer>
 	</AlertDialog.Content>
 </AlertDialog.Root>

apps/opensidian/src/lib/components/ContentEditor.svelte → apps/opensidian/src/lib/components/editor/ContentEditor.svelte

@@ -2,7 +2,8 @@
 	import type { FileId } from '@epicenter/filesystem';
 	import { Spinner } from '@epicenter/ui/spinner';
 	import type { DocumentHandle } from '@epicenter/workspace';
-	import { fsState } from '$lib/fs/fs-state.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
+	import { ws } from '$lib/workspace';
 	import CodeMirrorEditor from './CodeMirrorEditor.svelte';
 
 	let {
@@ -16,7 +17,7 @@
 	$effect(() => {
 		const id = fileId;
 		handle = null;
-		fsState.documents.open(id).then((h) => {
+		ws.documents.files.content.open(id).then((h) => {
 			// Guard against race condition -- if file changed while loading, ignore
 			if (fsState.activeFileId !== id) return;
 			handle = h;

apps/opensidian/src/lib/components/ContentPanel.svelte → apps/opensidian/src/lib/components/editor/ContentPanel.svelte

@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { fsState } from '$lib/fs/fs-state.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
 	import ContentEditor from './ContentEditor.svelte';
 	import PathBreadcrumb from './PathBreadcrumb.svelte';
 	import TabBar from './TabBar.svelte';

apps/opensidian/src/lib/components/PathBreadcrumb.svelte → apps/opensidian/src/lib/components/editor/PathBreadcrumb.svelte

@@ -1,6 +1,6 @@
 <script lang="ts">
 	import * as Breadcrumb from '@epicenter/ui/breadcrumb';
-	import { fsState } from '$lib/fs/fs-state.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
 
 	const pathSegments = $derived.by(() => {
 		const path = fsState.selectedPath;

apps/opensidian/src/lib/components/TabBar.svelte → apps/opensidian/src/lib/components/editor/TabBar.svelte

@@ -2,10 +2,9 @@
 	import type { FileId } from '@epicenter/filesystem';
 	import * as Tabs from '@epicenter/ui/tabs';
 	import XIcon from '@lucide/svelte/icons/x';
-	import { fsState } from '$lib/fs/fs-state.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
 
 	const hasOpenFiles = $derived(fsState.openFileIds.length > 0);
-
 </script>
 
 {#if hasOpenFiles}
@@ -23,13 +22,13 @@
 					<Tabs.Trigger
 						value={fileId}
 						class="relative flex-none rounded-none border-0 text-muted-foreground hover:bg-accent hover:text-accent-foreground data-[state=active]:bg-muted data-[state=active]:text-foreground data-[state=active]:shadow-none"
-					onauxclick={(e) => { if (e.button === 1) { e.preventDefault(); fsState.closeFile(fileId); } }}
+						onauxclick={(e) => { if (e.button === 1) { e.preventDefault(); fsState.closeFile(fileId); } }}
 					>
 						<span class="mr-4">{row.name}</span>
 						<button
 							type="button"
 							class="absolute right-1 top-1/2 -translate-y-1/2 rounded-sm p-0.5 opacity-50 hover:opacity-100 hover:bg-accent"
-					onclick={(e) => { e.stopPropagation(); e.preventDefault(); fsState.closeFile(fileId); }}
+							onclick={(e) => { e.stopPropagation(); e.preventDefault(); fsState.closeFile(fileId); }}
 							aria-label="Close {row.name}"
 						>
 							<XIcon class="h-3 w-3" />

apps/opensidian/src/lib/components/FileTree.svelte → apps/opensidian/src/lib/components/tree/FileTree.svelte

@@ -2,32 +2,19 @@
 	import type { FileId } from '@epicenter/filesystem';
 	import * as Empty from '@epicenter/ui/empty';
 	import * as TreeView from '@epicenter/ui/tree-view';
-	import { fsState } from '$lib/fs/fs-state.svelte';
-	import DeleteConfirmation from './DeleteConfirmation.svelte';
+	import { fsState } from '$lib/state/fs-state.svelte';
 	import FileTreeItem from './FileTreeItem.svelte';
 	import InlineNameInput from './InlineNameInput.svelte';
 
-	let deleteDialogOpen = $state(false);
-
 	/**
 	 * Flat list of visible item IDs in visual order.
 	 * Respects folder expansion state—collapsed folders hide their descendants.
 	 */
 	const visibleIds = $derived.by(() => {
-		void fsState.version;
-		const ids: FileId[] = [];
-		function walk(parentId: FileId | null) {
-			for (const childId of fsState.getChildIds(parentId)) {
-				const row = fsState.getRow(childId);
-				if (!row) continue;
-				ids.push(childId);
-				if (row.type === 'folder' && fsState.expandedIds.has(childId)) {
-					walk(childId);
-				}
-			}
-		}
-		walk(null);
-		return ids;
+		return fsState.walkTree<FileId>((id, row) => ({
+			collect: id,
+			descend: row.type === 'folder' && fsState.expandedIds.has(id),
+		}));
 	});
 
 	/** Whether an inline create/rename is active (suppresses tree keyboard shortcuts). */
@@ -128,7 +115,7 @@
 				if (!current) break;
 				// Select the focused item so DeleteConfirmation reads the right target
 				fsState.selectFile(current);
-				deleteDialogOpen = true;
+				fsState.openDelete();
 				break;
 			}
 			default:
@@ -161,5 +148,3 @@
 		{/if}
 	</TreeView.Root>
 {/if}
-
-<DeleteConfirmation bind:open={deleteDialogOpen} />