INTERNAL FUNCTIONS                          CALLERS
────────────────────────────────────────────────
lastEntry()                                 5  (currentType, validated, 3× replace*)
validated()                                 6  (getter, read, appendText, asText, asRichText, asSheet)
pushText(content)                           8  (write, appendText×3, asText×3, replace)
replaceCurrentText(content)                 1  ← restoreFromSnapshot only ⚠️
pushSheetFromSnapshot(cols, rows)           1  ← restoreFromSnapshot only ⚠️

BAD: Multiple raw access points scattered across methods
─────────────────────────────────────────────────────────
currentType getter  → entry.get('type') as ContentType     ← raw .get() + cast
write()             → entry.get('columns') as Y.Map<...>   ← raw .get() + cast
replaceCurrentText  → entry.get('content') as Y.Text       ← raw .get() + cast

    GOOD: Single boundary, everything else uses typed discriminated union
    ───────────────────────────────────────────────────────────────────
readEntry()         → entry.get('type'), instanceof checks  ← THE boundary
    currentEntry getter → readEntry(last)                       ← typed from here
    write()             → this.currentEntry (discriminated)     ← typed
read()              → this.currentEntry (discriminated)     ← typed

Fix: route all access through the typed getter. Public methods use `this.currentEntry` with type discrimination instead of raw `.get()`.

// BEFORE: "flatten to string and push" appears twice
switch (entry.type) {
	case 'richtext':
		pushText(xmlFragmentToPlaintext(entry.content) + text);
		break;
	case 'sheet':
		pushText(serializeSheetToCsv(entry) + text);
		break;
}

// AFTER: this.read() already does "flatten to string"
} else {
	pushText(this.read() + text);
}

     ```typescript

## Composition Over Duplication

When function B is function A + one extra step, compose instead of duplicating:

```typescript
// BEFORE: pushSheetFromCsv duplicates 90% of pushSheet's body
function pushSheet(): SheetEntry { /* 10 lines of Y.Map setup */ }
function pushSheetFromCsv(csv: string): SheetEntry {
	/* same 10 lines of Y.Map setup */
	parseSheetFromCsv(csv, columns, rows);
}

    // AFTER: Compose
    const result = pushSheet();
parseSheetFromCsv(csv, result);

// 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}`;
}

<!-- 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 a "smart" function branches internally but every caller already knows which branch it takes:

// replaceCurrentText branches on type, but callers know the answer:
function replaceCurrentText(content, current) {
	if (current?.type === 'text') { /* in-place overwrite */ }
	else { pushText(content); }
}

// write() KNOWS it's text → always takes the if-branch
// appendText() KNOWS it's NOT text → always takes the else-branch

Inline the known branch at each call site. Keep the branching function only for callers that genuinely don't know (e.g., `restoreFromSnapshot` where the live doc's type is unknown).

// BEFORE
serializeSheetToCsv(entry.columns, entry.rows)

// AFTER: SheetBinding already bundles columns + rows
serializeSheetToCsv(entry)

// BEFORE: columns/rows duplicated in both types
type SheetBinding = { columns: Y.Map<...>; rows: Y.Map<...> };
type SheetEntry = { type: 'sheet'; columns: Y.Map<...>; rows: Y.Map<...>; createdAt: number };

// AFTER: Intersection
type SheetEntry = SheetBinding & { type: 'sheet'; createdAt: number };

Proposed: inline replaceCurrentSheet into write()

 write(text: string) {
     ydoc.transact(() => {
-        if (type === 'sheet') replaceCurrentSheet(text);
+        if (type === 'sheet') {
+            entry.columns.forEach((_, key) => entry.columns.delete(key));
+            entry.rows.forEach((_, key) => entry.rows.delete(key));
    +            parseSheetFromCsv(text, entry);
+        }

Tradeoff: write() goes from 6 to 14 lines, but is now self-contained.

    edit → diagnostics → test → commit → next edit
    ```

Never mix two unrelated refactors in one commit:

## Anti-Patterns

- **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
- **Batch-committing**: "Cleaned up the module" as one commit with 15 changes—impossible to review or revert
- **Shotgun inlining**: Inlining everything with 1 caller regardless of context. Respect constructor families and complex logic.