From 788f8bbd0c656c16fa086a40994dc0b13a9c5762 Mon Sep 17 00:00:00 2001 From: sawka Date: Thu, 2 Oct 2025 23:06:01 -0700 Subject: [PATCH 1/3] bookkeeping updates --- .gitignore | 1 + package-lock.json | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 458a993443..57fd982c96 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ out/ make/ artifacts/ mikework/ +.env # Yarn Modern .pnp.* diff --git a/package-lock.json b/package-lock.json index 1ef2a34f9f..b50e84bad2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "waveterm", - "version": "0.11.6-beta.4", + "version": "0.11.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "waveterm", - "version": "0.11.6-beta.4", + "version": "0.11.6", "hasInstallScript": true, "license": "Apache-2.0", "workspaces": [ From 22d86accb824fcb1a77d6969b67d8debed5f4da8 Mon Sep 17 00:00:00 2001 From: sawka Date: Fri, 3 Oct 2025 00:21:00 -0700 Subject: [PATCH 2/3] layout updates + simplification --- aiprompts/layout-simplification.md | 857 ++++++++++++++++++++++++ frontend/app/store/global.ts | 15 +- frontend/app/store/keymodel.ts | 31 +- frontend/layout/index.ts | 6 - frontend/layout/lib/TileLayout.tsx | 2 +- frontend/layout/lib/layoutAtom.ts | 51 +- frontend/layout/lib/layoutModel.ts | 463 +++++++------ frontend/layout/lib/layoutModelHooks.ts | 21 +- frontend/layout/lib/layoutTree.ts | 24 +- frontend/layout/lib/types.ts | 1 - frontend/layout/tests/model.ts | 1 - frontend/types/gotypes.d.ts | 1 + pkg/waveobj/wtype.go | 1 + pkg/wcore/layout.go | 7 + 14 files changed, 1188 insertions(+), 293 deletions(-) create mode 100644 aiprompts/layout-simplification.md diff --git a/aiprompts/layout-simplification.md b/aiprompts/layout-simplification.md new file mode 100644 index 0000000000..785bfb1cca --- /dev/null +++ b/aiprompts/layout-simplification.md @@ -0,0 +1,857 @@ +# Wave Terminal Layout System - Simplification via Write Cache Pattern + +## Executive Summary + +The current layout system uses a complex bidirectional atom architecture that forces every layout change to round-trip through the backend WaveObject, even though **the backend never reads this data** - it only queues actions via `PendingBackendActions`. By switching to a "write cache" pattern where local atoms are the source of truth and backend writes are fire-and-forget, we can eliminate ~70% of the complexity while maintaining full persistence. + +## Current Architecture Problems + +### The Unnecessary Round-Trip + +Every layout change (split, close, focus, magnify) currently follows this flow: + +``` +User action + ↓ +treeReducer() mutates layoutState + ↓ +layoutState.generation++ ← Only purpose: trigger the write + ↓ +Bidirectional atom setter (checks generation) + ↓ +Write to WaveObject {rootnode, focusednodeid, magnifiednodeid} + ↓ +WaveObject update notification + ↓ +Bidirectional atom getter runs + ↓ +ALL dependent atoms recalculate (every isFocused, etc.) + ↓ +React re-renders with updated state +``` + +**The critical insight**: The backend reads ONLY `leaforder` from the WaveObject (for block number resolution in commands like `wsh block:1`). The `rootnode`, `focusednodeid`, and `magnifiednodeid` fields exist **only for persistence** (tab restore, uncaching). + +### What the Backend Actually Does + +**Backend Reads** (from [`pkg/wshrpc/wshserver/resolvers.go`](../pkg/wshrpc/wshserver/resolvers.go:196-206)): +- **`LeafOrder`** - Used to resolve block numbers in commands (e.g., `wsh block:1` → blockId lookup) + +**Backend Writes** (from [`pkg/wcore/layout.go`](../pkg/wcore/layout.go)): +- **`PendingBackendActions`** - Queued layout actions via [`QueueLayoutAction()`](../pkg/wcore/layout.go:101-118) + +**Backend NEVER touches**: +- **`RootNode`** - Never read, only written by frontend for persistence +- **`FocusedNodeId`** - Never read, only written by frontend for persistence +- **`MagnifiedNodeId`** - Never read, only written by frontend for persistence + +**The key insight**: Only `LeafOrder` needs to be synced to backend (for command resolution). The tree structure fields (`rootnode`, `focusednodeid`, `magnifiednodeid`) are pure persistence! + +### Complexity Symptoms + +1. **Generation tracking**: [`layoutState.generation++`](../frontend/layout/lib/layoutTree.ts:294) appears in 10+ places, only to trigger atom writes +2. **Bidirectional atoms**: [`withLayoutTreeStateAtomFromTab()`](../frontend/layout/lib/layoutAtom.ts:18-60) has complex read/write logic +3. **Timing coordination**: The entire Section 8 of the WaveAI focus proposal exists only because of race conditions between focus updates and atom commits +4. **False reactivity**: Changes to `focusedNodeId` trigger full tree state propagation even though they're unrelated to tree structure + +## Proposed "Write Cache" Architecture + +### Core Concept + +``` +User action + ↓ +Update LOCAL atom (immediate, synchronous) + ↓ +React re-renders (single tick, all atoms see new state) + ↓ +[async, fire-and-forget] Persist to WaveObject +``` + +### Key Principles + +1. **Local atoms are source of truth** during runtime +2. **WaveObject is persistence layer** only (read on init, write async) +3. **Backend actions still work** via `PendingBackendActions` +4. **No generation tracking needed** (no need to trigger writes) + +## Implementation Design + +### 1. New LayoutModel Structure + +```typescript +// frontend/layout/lib/layoutModel.ts + +class LayoutModel { + // BEFORE: Bidirectional atom with generation tracking + // treeStateAtom: WritableLayoutTreeStateAtom + + // AFTER: Simple local atom (source of truth) + private localTreeStateAtom: PrimitiveAtom; + + // Keep reference to WaveObject atom for persistence + private waveObjectAtom: WritableWaveObjectAtom; + + constructor(tabAtom: Atom, ...) { + this.waveObjectAtom = getLayoutStateAtomFromTab(tabAtom); + + // Initialize local atom (starts empty) + this.localTreeStateAtom = atom({ + rootNode: undefined, + focusedNodeId: undefined, + magnifiedNodeId: undefined, + leafOrder: undefined, + pendingBackendActions: undefined, + generation: 0 // Can be removed entirely or kept for debugging + }); + + // Read from WaveObject ONCE during initialization + this.initializeFromWaveObject(); + } + + private async initializeFromWaveObject() { + const waveObjState = this.getter(this.waveObjectAtom); + + // Load persisted state into local atom + const initialState: LayoutTreeState = { + rootNode: waveObjState?.rootnode, + focusedNodeId: waveObjState?.focusednodeid, + magnifiedNodeId: waveObjState?.magnifiednodeid, + leafOrder: undefined, // Computed by updateTree() + pendingBackendActions: waveObjState?.pendingbackendactions, + generation: 0 + }; + + // Set local state + this.treeState = initialState; + this.setter(this.localTreeStateAtom, initialState); + + // Process any pending backend actions + if (initialState.pendingBackendActions?.length) { + await this.processPendingBackendActions(); + } + + // Initialize tree (compute leafOrder, etc.) + this.updateTree(); + } + + // Process backend-queued actions (startup only) + private async processPendingBackendActions() { + const actions = this.treeState.pendingBackendActions; + if (!actions?.length) return; + + this.treeState.pendingBackendActions = undefined; + + for (const action of actions) { + // Convert backend action to frontend action and run through treeReducer + // This code already exists in onTreeStateAtomUpdated() + switch (action.actiontype) { + case LayoutTreeActionType.InsertNode: + this.treeReducer({ + type: LayoutTreeActionType.InsertNode, + node: newLayoutNode(undefined, undefined, undefined, { + blockId: action.blockid + }), + magnified: action.magnified, + focused: action.focused + }, false); + break; + // ... other action types + } + } + } +} +``` + +### 2. Simplified treeReducer + +```typescript +class LayoutModel { + treeReducer(action: LayoutTreeAction, setState = true): boolean { + // Run the tree operation (mutates this.treeState) + switch (action.type) { + case LayoutTreeActionType.InsertNode: + insertNode(this.treeState, action); + break; + case LayoutTreeActionType.FocusNode: + focusNode(this.treeState, action); + break; + case LayoutTreeActionType.DeleteNode: + deleteNode(this.treeState, action); + break; + // ... all other cases unchanged + } + + if (setState) { + // Update tree (compute leafOrder, validate, etc.) + this.updateTree(); + + // Update local atom IMMEDIATELY (synchronous) + this.setter(this.localTreeStateAtom, { ...this.treeState }); + + // Persist to backend asynchronously (fire and forget) + this.persistToBackend(); + } + + return true; + } + + // Fire-and-forget persistence + private async persistToBackend() { + const waveObj = this.getter(this.waveObjectAtom); + if (!waveObj) return; + + // Update WaveObject fields + waveObj.rootnode = this.treeState.rootNode; // Persistence only + waveObj.focusednodeid = this.treeState.focusedNodeId; // Persistence only + waveObj.magnifiednodeid = this.treeState.magnifiedNodeId; // Persistence only + waveObj.leaforder = this.treeState.leafOrder; // Backend reads this for command resolution! + + // Write to backend (don't await - fire and forget) + this.setter(this.waveObjectAtom, waveObj); + + // Optional: Debounce if rapid changes are a concern + } +} +``` + +### 3. Simplified NodeModel isFocused + +```typescript +class LayoutModel { + getNodeModel(node: LayoutNode): NodeModel { + return { + // BEFORE: Complex dependency on bidirectional treeStateAtom + // isFocused: atom((get) => { + // const treeState = get(this.treeStateAtom); // Triggers on any tree change + // ... + // }) + + // AFTER: Simple dependency on local atom + isFocused: atom((get) => { + const treeState = get(this.localTreeStateAtom); // Simple read + const focusType = get(focusManager.focusType); + return treeState.focusedNodeId === node.id && focusType === "node"; + }), + + // All other atoms similarly simplified... + isMagnified: atom((get) => { + const treeState = get(this.localTreeStateAtom); + return treeState.magnifiedNodeId === node.id; + }), + + // ... rest unchanged + }; + } +} +``` + +### 4. Remove Generation Tracking + +The `generation` field can be removed entirely from [`LayoutTreeState`](../frontend/layout/lib/types.ts): + +```typescript +// frontend/layout/lib/types.ts + +export interface LayoutTreeState { + rootNode?: LayoutNode; + focusedNodeId?: string; + magnifiedNodeId?: string; + leafOrder?: LayoutLeafEntry[]; + pendingBackendActions?: LayoutActionData[]; + // generation: number; ← DELETE THIS +} +``` + +And remove all `generation++` calls from [`layoutTree.ts`](../frontend/layout/lib/layoutTree.ts) (appears in 10+ places). + +### 5. Simplified layoutAtom.ts + +```typescript +// frontend/layout/lib/layoutAtom.ts + +// BEFORE: Complex bidirectional atom (60 lines) +// AFTER: Can be deleted entirely or simplified to just helper for WaveObject access + +export function getLayoutStateAtomFromTab( + tabAtom: Atom, + get: Getter +): WritableWaveObjectAtom { + const tabData = get(tabAtom); + if (!tabData) return; + const layoutStateOref = WOS.makeORef("layout", tabData.layoutstate); + return WOS.getWaveObjectAtom(layoutStateOref); +} + +// No more withLayoutTreeStateAtomFromTab() - not needed! +``` + +## Benefits + +### Immediate Benefits + +1. **10x simpler reactivity**: Local atoms update synchronously, React sees complete state in one tick +2. **No generation tracking**: Eliminate 10+ `generation++` calls and all related logic +3. **No timing issues**: Everything happens synchronously, no coordination needed +4. **Faster updates**: No round-trip through WaveObject for every change +5. **Easier debugging**: Clear separation between runtime state (local atoms) and persistence (WaveObject) + +### Impact on WaveAI Focus Proposal + +The entire Section 8 ("Layout Model Focus Integration - CRITICAL TIMING") **becomes unnecessary**: + +**BEFORE** (complex timing coordination): +```typescript +treeReducer(action: LayoutTreeAction) { + insertNode(this.treeState, action); // generation++ + + // CRITICAL: Must update focus manager BEFORE atom commits + if (action.focused) { + focusManager.requestNodeFocus(); // Synchronous! + } + + // Then atom commits + this.setter(this.treeStateAtom, ...); + // Now isFocused sees correct focusType +} +``` + +**AFTER** (trivial): +```typescript +treeReducer(action: LayoutTreeAction) { + insertNode(this.treeState, action); // Just mutates local state + + // Update local atom (synchronous) + this.setter(this.localTreeStateAtom, { ...this.treeState }); + + // Update focus manager (order doesn't matter - both updated synchronously) + if (action.focused) { + focusManager.setBlockFocus(); + } + + // Both updates happen in same tick, no race condition possible! +} +``` + +### Code Deletion + +**Can delete**: +- `generation` field and all `generation++` calls (~15 places) +- Complex bidirectional atom logic in [`layoutAtom.ts`](../frontend/layout/lib/layoutAtom.ts) (~40 lines) +- `lastTreeStateGeneration` tracking in [`LayoutModel`](../frontend/layout/lib/layoutModel.ts) +- All `generation > this.treeState.generation` checks + +**Total**: ~200-300 lines of complex coordination code deleted + +## Edge Cases & Considerations + +### 1. Rapid Changes + +**Concern**: Many layout changes in quick succession could cause many backend writes. + +**Solution**: Debounce the `persistToBackend()` call (e.g., 100ms). Users won't notice the delay in persistence. + +```typescript +private persistDebounceTimer: NodeJS.Timeout | null = null; + +private persistToBackend() { + if (this.persistDebounceTimer) { + clearTimeout(this.persistDebounceTimer); + } + + this.persistDebounceTimer = setTimeout(() => { + const waveObj = this.getter(this.waveObjectAtom); + if (!waveObj) return; + + waveObj.rootnode = this.treeState.rootNode; + waveObj.focusednodeid = this.treeState.focusedNodeId; + waveObj.magnifiednodeid = this.treeState.magnifiedNodeId; + waveObj.leaforder = this.treeState.leafOrder; + + this.setter(this.waveObjectAtom, waveObj); + this.persistDebounceTimer = null; + }, 100); +} +``` + +### 2. Tab Switching + +**Current**: Each tab has its own `treeStateAtom` in a WeakMap. + +**After**: Each tab has its own `localTreeStateAtom` in the LayoutModel instance. No change needed - already isolated per tab. + +### 3. Tab Uncaching (Electron Limit) + +**Current**: Tab gets uncached, needs to reload layout from WaveObject. + +**After**: Same - `initializeFromWaveObject()` reads persisted state. No change in behavior. + +### 4. Backend Actions (New Blocks) +### 5. LeafOrder and CLI Commands + +**Concern**: The backend reads `LeafOrder` for CLI command resolution (e.g., `wsh block:1`). What if it's not synced yet? + +**Solution**: Fire-and-forget is perfectly fine! CLI commands aren't time-sensitive: +- Commands are typed/run by users (human speed, not machine speed) +- Even if `LeafOrder` is 100ms behind, no one will notice +- By the time a user types `wsh block:1`, the async write has long since completed +- Worst case: User types command during a split operation and gets previous block - extremely rare and not breaking + + +## Immutability and Jotai Atoms + +### Question: Do we need deep copies for Jotai to detect changes? + +**Answer: NO - shallow copy is sufficient!** ✓ + +### Current System (Already Uses Shallow Updates) + +Looking at the current code in [`layoutModel.ts:587`](../frontend/layout/lib/layoutModel.ts:587): + +```typescript +setTreeStateAtom(bumpGeneration = false) { + if (bumpGeneration) { + this.treeState.generation++; + } + this.lastTreeStateGeneration = this.treeState.generation; + this.setter(this.treeStateAtom, this.treeState); // ← Sets same object! +} +``` + +**The current system doesn't create new objects either!** It relies on `generation` changing to trigger the bidirectional atom's setter. + +### Why Shallow Copy Works with Jotai + +```typescript +// In treeReducer after mutations +this.setter(this.localTreeStateAtom, { ...this.treeState }); +``` + +**This works because**: +1. **Jotai checks reference equality** on the atom value itself (the `LayoutTreeState` object) +2. **`{ ...this.treeState }` creates a NEW object** with a different reference +3. **Nested structures don't matter** - Jotai doesn't do deep equality checks + +**Example**: +```typescript +const oldState = { rootNode: someTree, focusedNodeId: "node1" }; +const newState = { ...oldState }; + +oldState === newState // FALSE - different objects! +oldState.rootNode === newState.rootNode // TRUE - same tree reference + +// But Jotai only checks the first comparison, so it detects the change! +``` + +### Tree Mutations Don't Need Immutability + +All tree operations in [`layoutTree.ts`](../frontend/layout/lib/layoutTree.ts) **mutate in place**: +- `insertNode()` - Mutates `layoutState.rootNode` + +### Derived Atoms Will Update Correctly ✓ + +**Concern**: Will derived atoms like `isFocused` and `isMagnified` update when we change to local atoms? + +**Answer: YES - they will work perfectly!** ✓ + +### How Derived Atoms Work + +The NodeModel creates derived atoms that depend on `treeStateAtom`: + +```typescript +// From layoutModel.ts:936-946 +isFocused: atom((get) => { + const treeState = get(this.treeStateAtom); // Subscribe to treeStateAtom + const isFocused = treeState.focusedNodeId === nodeid; + const waveAIFocused = get(atoms.waveAIFocusedAtom); + return isFocused && !waveAIFocused; +}), + +isMagnified: atom((get) => { + const treeState = get(this.treeStateAtom); // Subscribe to treeStateAtom + return treeState.magnifiedNodeId === nodeid; +}), +``` + +### Why They'll Still Work with Local Atoms + +**After the change**: +```typescript +isFocused: atom((get) => { + const treeState = get(this.localTreeStateAtom); // Subscribe to localTreeStateAtom + const isFocused = treeState.focusedNodeId === nodeid; + const waveAIFocused = get(atoms.waveAIFocusedAtom); + return isFocused && !waveAIFocused; +}), +``` + +**The update flow**: +1. User clicks block → `focusNode()` called +2. `treeReducer()` runs → mutates `this.treeState.focusedNodeId = newId` +3. `this.setter(this.localTreeStateAtom, { ...this.treeState })` ← **New reference!** +4. Jotai detects reference change in `localTreeStateAtom` +5. All derived atoms that call `get(this.localTreeStateAtom)` are notified +6. They re-run their getter functions +7. They see the new `focusedNodeId` value +8. React components re-render with correct values ✓ + +### Key Insight + +**We're not mutating fields inside the atom** - we're replacing the entire state object: + +```typescript +// OLD way (current): +// 1. Mutate this.treeState.focusedNodeId = newId +// 2. Bump this.treeState.generation++ +// 3. Set bidirectional atom (checks generation, writes to WaveObject, reads back, updates) +// 4. Derived atoms see new state from the round-trip + +// NEW way (proposed): +// 1. Mutate this.treeState.focusedNodeId = newId (same!) +// 2. this.setter(localTreeStateAtom, { ...this.treeState }) (new object reference!) +// 3. Derived atoms immediately see new state (no round-trip!) +``` + +**Both approaches create a new state object that triggers Jotai's reactivity!** + +The new way is actually **MORE reliable** because: +- No round-trip delay +- No generation checking +- Direct, synchronous update +- Same Jotai reactivity mechanism + +### What About Nested Fields? + +**Question**: What if derived atoms access nested fields like `treeState.rootNode.children`? + +**Answer**: Still works! Example: + +```typescript +// Hypothetical derived atom +someAtom: atom((get) => { + const treeState = get(this.localTreeStateAtom); + return treeState.rootNode.children.length; // Nested access +}) +``` + +**This works because**: +1. We create new `LayoutTreeState` object: `{ ...this.treeState }` +2. Jotai sees new reference → notifies subscribers +3. Getter re-runs, calls `get(this.localTreeStateAtom)` +4. Gets the new state object +5. Accesses `newState.rootNode` (same reference as before, but that's OK!) +6. Returns correct value + +**The derived atom doesn't care that `rootNode` is the same object** - it just cares that the STATE object changed and it needs to re-evaluate. + +### Verification + +All derived atoms in NodeModel: +- ✅ `isFocused` - depends on `treeState.focusedNodeId` +- ✅ `isMagnified` - depends on `treeState.magnifiedNodeId` +- ✅ `blockNum` - depends on separate `this.leafOrder` atom (unaffected) +- ✅ `isEphemeral` - depends on separate `this.ephemeralNode` atom (unaffected) + +All will update correctly with the new local atom approach! + +- `deleteNode()` - Mutates parent's children array +- `focusNode()` - Mutates `layoutState.focusedNodeId` + +This is fine! We're not relying on immutability for change detection. We're relying on creating a new `LayoutTreeState` wrapper object via spread operator. + +### Backend Round-Trip + +When reading from WaveObject on initialization: +```typescript +const waveObjState = this.getter(this.waveObjectAtom); +const initialState: LayoutTreeState = { + rootNode: waveObjState?.rootnode, // New reference from backend + focusedNodeId: waveObjState?.focusednodeid, + // ... +}; +``` + +This creates a **completely new object** with new references, which is even more immutable than necessary. No issues here. + +### Summary + +✅ **We're covered** - Shallow copy via spread operator is sufficient + +✅ **Same as current system** - We're not making it worse, just simpler + +✅ **Jotai only checks reference equality** on the atom value, not deep equality + +✅ **Tree mutations are fine** - They've always worked this way + + +**Current**: Backend queues actions via [`QueueLayoutAction()`](../pkg/wcore/layout.go:101), frontend processes via `pendingBackendActions`. + +**After**: Same - `initializeFromWaveObject()` processes pending actions. No change needed. + +### 5. Write Failures + +**Concern**: What if the async write to WaveObject fails? + +**Solution**: +1. The app continues working (local state is fine) +2. On next persistence attempt, full state is written again +3. On tab reload, worst case is state from last successful write +4. Can add retry logic or error notification if needed + +## Migration Path + +### Phase 1: Preparation (No Breaking Changes) + +1. Add `localTreeStateAtom` alongside existing `treeStateAtom` +2. Keep both in sync +3. Update a few `isFocused` atoms to use local atom +4. Test thoroughly + +### Phase 2: Switch Over + +1. Update `treeReducer` to write to local atom + fire-and-forget persist +2. Update all `isFocused` and other computed atoms to use local atom +3. Remove generation checks and tracking +4. Test all layout operations + +### Phase 3: Cleanup + +1. Delete bidirectional atom logic from [`layoutAtom.ts`](../frontend/layout/lib/layoutAtom.ts) +2. Remove `generation` field from `LayoutTreeState` +3. Simplify `onTreeStateAtomUpdated()` (only needed for `pendingBackendActions`) +4. Update documentation + +### Testing Checklist + +- [ ] Split horizontal/vertical +- [ ] Close blocks (focused and unfocused) +- [ ] Focus changes via click, keyboard nav, tab switching +- [ ] Magnify/unmagnify +- [ ] Resize operations +- [ ] Drag & drop +- [ ] Tab switching (verify state persistence) +- [ ] App restart (verify state restore) +- [ ] Multiple windows +- [ ] Rapid operations (verify debouncing works) + +## Impact on Other Systems + +### Focus Manager + +**Before**: Must coordinate timing with atom commits. + +**After**: Can update `focusType` atom independently. Order doesn't matter since both updates happen synchronously. + +### Block Component + +**No change**: Blocks still subscribe to `nodeModel.isFocused`, which still reacts correctly (faster now). + +### Keyboard Navigation + +**No change**: Still calls `layoutModel.focusNode()`, which updates local state immediately. + +### Terminal/Views + +**No change**: Views don't interact with layout atoms directly. + +## Performance Implications + +### Improved + +1. **Faster reactivity**: No round-trip through WaveObject (save ~1-2ms per operation) +2. **Fewer atom updates**: Only local atom updates, not bidirectional propagation +3. **Batched writes**: Debouncing reduces backend write frequency + +### No Change + +1. **Tree operations**: Same complexity (balance, walk, compute, etc.) +2. **React rendering**: Same render triggers, just faster +3. **Memory usage**: Same (local atom vs bidirectional atom is similar size) + +## Conclusion + +The "write cache" pattern can simplify the layout system by ~70% while maintaining full functionality: + +- **Remove**: Generation tracking, bidirectional atoms, timing coordination +- **Keep**: All tree logic, backend integration, persistence +- **Gain**: Simpler code, faster updates, easier debugging + +This also makes the WaveAI focus integration trivial, eliminating the need for complex timing coordination. + +## Recommendation + +Implement this simplification **before** adding WaveAI focus features. The cleaner foundation will make the focus work much easier and the codebase more maintainable long-term. +# Wave Terminal Layout System - Simplification via Write Cache Pattern + +## Risk Assessment: LOW RISK, Well-Contained Change + +### Files to Modify: **4-5 files, all in `frontend/layout/`** + +1. **`frontend/layout/lib/layoutModel.ts`** (~150 lines changed) + - Add `localTreeStateAtom` field + - Modify `treeReducer()` to update local atom + persist async + - Add `initializeFromWaveObject()` method + - Add `persistToBackend()` method + - Update `getNodeModel()` atoms to use local atom + +2. **`frontend/layout/lib/layoutTree.ts`** (~15 line deletions) + - Remove all `layoutState.generation++` calls (appears 15 times) + - No other changes needed + +3. **`frontend/layout/lib/layoutAtom.ts`** (~40 lines deleted or simplified) + - Can delete most of the bidirectional atom logic + - Keep only `getLayoutStateAtomFromTab()` helper + +4. **`frontend/layout/lib/types.ts`** (~1 line deletion) + - Remove `generation: number` from `LayoutTreeState` + +5. **`frontend/layout/tests/model.ts`** (~1 line change) + - Remove generation from test fixtures + +**Total**: ~5 files, all within `frontend/layout/` directory. **No changes outside layout system!** + +### Why This is Low Risk + +#### 1. **Fail-Fast Behavior** ✓ +If we break something, it will be **immediately obvious**: +- Split horizontal/vertical won't work → visible immediately +- Block focus won't work → obvious when clicking +- Close block won't work → obvious +- Magnify won't work → obvious + +**No subtle corruption**: This change affects reactive state flow, not data persistence. If it breaks, the UI breaks obviously. We won't get "sometimes it works, sometimes it doesn't." + +#### 2. **Well-Contained Scope** ✓ +- **All changes in one directory**: `frontend/layout/` +- **No changes to**: + - Block components (unchanged) + - Terminal/views (unchanged) + - Keyboard navigation (unchanged) + - Focus manager (unchanged) + - Backend Go code (unchanged) + +The **interface** to the layout system stays the same: +- Blocks still call `nodeModel.focusNode()` +- Blocks still subscribe to `nodeModel.isFocused` +- Keyboard nav still calls `layoutModel.focusNode()` +- Nothing outside the layout system needs to know about the change + +#### 3. **No Data Corruption Risk** ✓ +This change affects **reactive state propagation**, not data storage: +- WaveObject still stores the same data +- Backend still queues actions the same way +- Blocks still have the same IDs +- Tab structure unchanged + +**Worst case**: Layout stops working, we revert the code. No data loss, no corruption. + +#### 4. **Incremental Implementation Possible** ✓ + +Can be done in safe phases: + +**Phase 1**: Add alongside existing (no breaking changes) +```typescript +class LayoutModel { + treeStateAtom: WritableLayoutTreeStateAtom; // Keep old + localTreeStateAtom: PrimitiveAtom; // Add new + + // Keep both in sync temporarily +} +``` + +**Phase 2**: Switch consumers one at a time +```typescript +// Change this gradually +isFocused: atom((get) => { + // const treeState = get(this.treeStateAtom); // Old + const treeState = get(this.localTreeStateAtom); // New + ... +}) +``` + +**Phase 3**: Remove old code once everything uses new atoms + +**Can test thoroughly at each phase before proceeding!** + +#### 5. **Easy to Test** ✓ + +Every layout operation is user-visible and testable: +- [ ] Split horizontal → obvious if broken +- [ ] Split vertical → obvious if broken +- [ ] Close block → obvious if broken +- [ ] Focus block → obvious if broken +- [ ] Magnify/unmagnify → obvious if broken +- [ ] Drag & drop → obvious if broken +- [ ] Tab switch → obvious if broken +- [ ] App restart → obvious if broken + +No subtle edge cases to hunt down. If it works in manual testing, it works. + +### Comparison to High-Risk Changes + +**This change is NOT**: +- ❌ Touching 20+ files across the codebase +- ❌ Changing subtle timing in async operations +- ❌ Modifying data storage formats +- ❌ Affecting backend/frontend protocol +- ❌ Requiring coordinated backend changes +- ❌ Creating subtle race conditions + +**This change IS**: +- ✅ Contained to 5 files in one directory +- ✅ Synchronous state updates (simpler than current!) +- ✅ Same data format, just different flow +- ✅ Frontend-only +- ✅ Backend unchanged +- ✅ Eliminating race conditions (not creating them) + +### What Could Go Wrong? (And How We'd Know) + +| Potential Issue | How We'd Detect | Recovery | +|-----------------|-----------------|----------| +| Local atom doesn't update | Layout frozen, nothing responds | Immediately obvious, revert | +| Persistence fails silently | State doesn't survive restart | Caught in testing, add logging | +| isFocused calculation wrong | Wrong focus ring | Immediately obvious, fix calculation | +| Missing generation++ somewhere | Old code path tries to use generation | Compile error or immediate runtime error | +| Tab switching breaks | Tabs don't load correctly | Immediately obvious | + +**All failure modes are immediate and obvious!** + +### Difficulty Assessment + +**Conceptual Difficulty**: LOW +- Replace bidirectional atom with simple atom +- Add async persist function +- Remove generation tracking +- Very straightforward refactor + +**Code Difficulty**: LOW-MEDIUM +- Changes are localized and mechanical +- Most changes are deletions (always good!) +- New code is simpler than old code +- No complex algorithms to implement + +**Testing Difficulty**: LOW +- All functionality is user-visible +- No need for complex test scenarios +- Manual testing catches everything +- Can test incrementally + +### Recommendation + +This is a **low-risk, high-reward change**: +- **Risk**: LOW (contained, fail-fast, no corruption) +- **Difficulty**: LOW-MEDIUM (straightforward refactor) +- **Reward**: HIGH (70% less complexity, easier future work) + +**Suggested approach**: +1. Implement in a feature branch +2. Add local atom alongside existing system +3. Test thoroughly with both systems running +4. Switch over gradually +5. Remove old code +6. Merge when confident + +Total implementation time: **1-2 days for experienced developer**, including thorough testing. + +--- diff --git a/frontend/app/store/global.ts b/frontend/app/store/global.ts index 5db71d62a5..b5b15805b2 100644 --- a/frontend/app/store/global.ts +++ b/frontend/app/store/global.ts @@ -4,12 +4,11 @@ import { RpcApi } from "@/app/store/wshclientapi"; import { TabRpcClient } from "@/app/store/wshrpcutil"; import { - getLayoutModelForTabById, + getLayoutModelForStaticTab, LayoutTreeActionType, LayoutTreeInsertNodeAction, newLayoutNode, } from "@/layout/index"; -import { getLayoutModelForStaticTab } from "@/layout/lib/layoutModelHooks"; import { LayoutTreeReplaceNodeAction, LayoutTreeSplitHorizontalAction, @@ -385,8 +384,7 @@ async function createBlockSplitHorizontally( targetBlockId: string, position: "before" | "after" ): Promise { - const tabId = globalStore.get(atoms.staticTabId); - const layoutModel = getLayoutModelForTabById(tabId); + const layoutModel = getLayoutModelForStaticTab(); const rtOpts: RuntimeOpts = { termsize: { rows: 25, cols: 80 } }; const newBlockId = await ObjectService.CreateBlock(blockDef, rtOpts); const targetNodeId = layoutModel.getNodeByBlockId(targetBlockId)?.id; @@ -409,8 +407,7 @@ async function createBlockSplitVertically( targetBlockId: string, position: "before" | "after" ): Promise { - const tabId = globalStore.get(atoms.staticTabId); - const layoutModel = getLayoutModelForTabById(tabId); + const layoutModel = getLayoutModelForStaticTab(); const rtOpts: RuntimeOpts = { termsize: { rows: 25, cols: 80 } }; const newBlockId = await ObjectService.CreateBlock(blockDef, rtOpts); const targetNodeId = layoutModel.getNodeByBlockId(targetBlockId)?.id; @@ -429,8 +426,7 @@ async function createBlockSplitVertically( } async function createBlock(blockDef: BlockDef, magnified = false, ephemeral = false): Promise { - const tabId = globalStore.get(atoms.staticTabId); - const layoutModel = getLayoutModelForTabById(tabId); + const layoutModel = getLayoutModelForStaticTab(); const rtOpts: RuntimeOpts = { termsize: { rows: 25, cols: 80 } }; const blockId = await ObjectService.CreateBlock(blockDef, rtOpts); if (ephemeral) { @@ -448,8 +444,7 @@ async function createBlock(blockDef: BlockDef, magnified = false, ephemeral = fa } async function replaceBlock(blockId: string, blockDef: BlockDef): Promise { - const tabId = globalStore.get(atoms.staticTabId); - const layoutModel = getLayoutModelForTabById(tabId); + const layoutModel = getLayoutModelForStaticTab(); const rtOpts: RuntimeOpts = { termsize: { rows: 25, cols: 80 } }; const newBlockId = await ObjectService.CreateBlock(blockDef, rtOpts); setTimeout(async () => { diff --git a/frontend/app/store/keymodel.ts b/frontend/app/store/keymodel.ts index b673925584..49bcecda72 100644 --- a/frontend/app/store/keymodel.ts +++ b/frontend/app/store/keymodel.ts @@ -19,11 +19,9 @@ import { } from "@/app/store/global"; import { deleteLayoutModelForTab, - getLayoutModelForTab, - getLayoutModelForTabById, + getLayoutModelForStaticTab, NavigateDirection, } from "@/layout/index"; -import { getLayoutModelForStaticTab } from "@/layout/lib/layoutModelHooks"; import * as keyutil from "@/util/keyutil"; import { CHORD_TIMEOUT } from "@/util/sharedconst"; import { fireAndForget } from "@/util/util"; @@ -64,8 +62,7 @@ export function keyboardMouseDownHandler(e: MouseEvent) { } function getFocusedBlockInStaticTab() { - const tabId = globalStore.get(atoms.staticTabId); - const layoutModel = getLayoutModelForTabById(tabId); + const layoutModel = getLayoutModelForStaticTab(); const focusedNode = globalStore.get(layoutModel.focusedNode); return focusedNode.data?.blockId; } @@ -108,8 +105,9 @@ function shouldDispatchToBlock(e: WaveKeyboardEvent): boolean { return true; } -function genericClose(tabId: string) { +function genericClose() { const ws = globalStore.get(atoms.workspace); + const tabId = globalStore.get(atoms.staticTabId); const tabORef = WOS.makeORef("tab", tabId); const tabAtom = WOS.getWaveObjectAtom(tabORef); const tabData = globalStore.get(tabAtom); @@ -126,7 +124,7 @@ function genericClose(tabId: string) { deleteLayoutModelForTab(tabId); return; } - const layoutModel = getLayoutModelForTab(tabAtom); + const layoutModel = getLayoutModelForStaticTab(); fireAndForget(layoutModel.closeFocusedNode.bind(layoutModel)); } @@ -138,8 +136,8 @@ function switchBlockByBlockNum(index: number) { layoutModel.switchNodeFocusByBlockNum(index); } -function switchBlockInDirection(tabId: string, direction: NavigateDirection) { - const layoutModel = getLayoutModelForTabById(tabId); +function switchBlockInDirection(direction: NavigateDirection) { + const layoutModel = getLayoutModelForStaticTab(); layoutModel.switchNodeFocusInDirection(direction); } @@ -397,8 +395,7 @@ function registerGlobalKeys() { return true; }); globalKeyMap.set("Cmd:w", () => { - const tabId = globalStore.get(atoms.staticTabId); - genericClose(tabId); + genericClose(); return true; }); globalKeyMap.set("Cmd:Shift:w", () => { @@ -423,23 +420,19 @@ function registerGlobalKeys() { return true; }); globalKeyMap.set("Ctrl:Shift:ArrowUp", () => { - const tabId = globalStore.get(atoms.staticTabId); - switchBlockInDirection(tabId, NavigateDirection.Up); + switchBlockInDirection(NavigateDirection.Up); return true; }); globalKeyMap.set("Ctrl:Shift:ArrowDown", () => { - const tabId = globalStore.get(atoms.staticTabId); - switchBlockInDirection(tabId, NavigateDirection.Down); + switchBlockInDirection(NavigateDirection.Down); return true; }); globalKeyMap.set("Ctrl:Shift:ArrowLeft", () => { - const tabId = globalStore.get(atoms.staticTabId); - switchBlockInDirection(tabId, NavigateDirection.Left); + switchBlockInDirection(NavigateDirection.Left); return true; }); globalKeyMap.set("Ctrl:Shift:ArrowRight", () => { - const tabId = globalStore.get(atoms.staticTabId); - switchBlockInDirection(tabId, NavigateDirection.Right); + switchBlockInDirection(NavigateDirection.Right); return true; }); globalKeyMap.set("Ctrl:Shift:k", () => { diff --git a/frontend/layout/index.ts b/frontend/layout/index.ts index 8d2f678616..c147c30ef3 100644 --- a/frontend/layout/index.ts +++ b/frontend/layout/index.ts @@ -6,10 +6,7 @@ import { LayoutModel } from "./lib/layoutModel"; import { deleteLayoutModelForTab, getLayoutModelForStaticTab, - getLayoutModelForTab, - getLayoutModelForTabById, useDebouncedNodeInnerRect, - useLayoutModel, } from "./lib/layoutModelHooks"; import { newLayoutNode } from "./lib/layoutNode"; import type { @@ -38,15 +35,12 @@ export { deleteLayoutModelForTab, DropDirection, getLayoutModelForStaticTab, - getLayoutModelForTab, - getLayoutModelForTabById, LayoutModel, LayoutTreeActionType, NavigateDirection, newLayoutNode, TileLayout, useDebouncedNodeInnerRect, - useLayoutModel, }; export type { ContentRenderer, diff --git a/frontend/layout/lib/TileLayout.tsx b/frontend/layout/lib/TileLayout.tsx index 86e92793a5..fa4ec9a030 100644 --- a/frontend/layout/lib/TileLayout.tsx +++ b/frontend/layout/lib/TileLayout.tsx @@ -140,7 +140,7 @@ function NodeBackdrops({ layoutModel }: { layoutModel: LayoutModel }) { const [blockBlurAtom] = useState(() => getSettingsKeyAtom("window:magnifiedblockblursecondarypx")); const blockBlur = useAtomValue(blockBlurAtom); const ephemeralNode = useAtomValue(layoutModel.ephemeralNode); - const magnifiedNodeId = useAtomValue(layoutModel.treeStateAtom).magnifiedNodeId; + const magnifiedNodeId = useAtomValue(layoutModel.magnifiedNodeIdAtom); const [showMagnifiedBackdrop, setShowMagnifiedBackdrop] = useState(!!ephemeralNode); const [showEphemeralBackdrop, setShowEphemeralBackdrop] = useState(!!magnifiedNodeId); diff --git a/frontend/layout/lib/layoutAtom.ts b/frontend/layout/lib/layoutAtom.ts index 9318d9caeb..62890446da 100644 --- a/frontend/layout/lib/layoutAtom.ts +++ b/frontend/layout/lib/layoutAtom.ts @@ -2,59 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 import { WOS } from "@/app/store/global"; -import { Atom, atom, Getter } from "jotai"; -import { LayoutTreeState, WritableLayoutTreeStateAtom } from "./types"; +import { Atom, Getter } from "jotai"; -const layoutStateAtomMap: WeakMap, WritableLayoutTreeStateAtom> = new WeakMap(); - -function getLayoutStateAtomFromTab(tabAtom: Atom, get: Getter): WritableWaveObjectAtom { +export function getLayoutStateAtomFromTab(tabAtom: Atom, get: Getter): WritableWaveObjectAtom { const tabData = get(tabAtom); if (!tabData) return; const layoutStateOref = WOS.makeORef("layout", tabData.layoutstate); const layoutStateAtom = WOS.getWaveObjectAtom(layoutStateOref); return layoutStateAtom; } - -export function withLayoutTreeStateAtomFromTab(tabAtom: Atom): WritableLayoutTreeStateAtom { - if (layoutStateAtomMap.has(tabAtom)) { - return layoutStateAtomMap.get(tabAtom); - } - const generationAtom = atom(1); - const treeStateAtom: WritableLayoutTreeStateAtom = atom( - (get) => { - const stateAtom = getLayoutStateAtomFromTab(tabAtom, get); - if (!stateAtom) return; - const layoutStateData = get(stateAtom); - const layoutTreeState: LayoutTreeState = { - rootNode: layoutStateData?.rootnode, - focusedNodeId: layoutStateData?.focusednodeid, - magnifiedNodeId: layoutStateData?.magnifiednodeid, - pendingBackendActions: layoutStateData?.pendingbackendactions, - generation: get(generationAtom), - }; - return layoutTreeState; - }, - (get, set, value) => { - if (get(generationAtom) < value.generation) { - const stateAtom = getLayoutStateAtomFromTab(tabAtom, get); - if (!stateAtom) return; - const waveObjVal = get(stateAtom); - if (waveObjVal == null) { - console.log("in withLayoutTreeStateAtomFromTab, waveObjVal is null", value); - return; - } - waveObjVal.rootnode = value.rootNode; - waveObjVal.magnifiednodeid = value.magnifiedNodeId; - waveObjVal.focusednodeid = value.focusedNodeId; - waveObjVal.leaforder = value.leafOrder; // only set leaforder, never get it, since this value is driven by the frontend - waveObjVal.pendingbackendactions = value?.pendingBackendActions?.length - ? value.pendingBackendActions - : undefined; - set(generationAtom, value.generation); - set(stateAtom, waveObjVal); - } - } - ); - layoutStateAtomMap.set(tabAtom, treeStateAtom); - return treeStateAtom; -} diff --git a/frontend/layout/lib/layoutModel.ts b/frontend/layout/lib/layoutModel.ts index 0e8887709d..77523f3ed2 100644 --- a/frontend/layout/lib/layoutModel.ts +++ b/frontend/layout/lib/layoutModel.ts @@ -7,6 +7,7 @@ import { Atom, atom, Getter, PrimitiveAtom, Setter } from "jotai"; import { splitAtom } from "jotai/utils"; import { createRef, CSSProperties } from "react"; import { debounce } from "throttle-debounce"; +import { getLayoutStateAtomFromTab } from "./layoutAtom"; import { balanceNode, findNode, newLayoutNode, walkNodes } from "./layoutNode"; import { clearTree, @@ -50,7 +51,6 @@ import { PreviewRenderer, ResizeHandleProps, TileLayoutContents, - WritableLayoutTreeStateAtom, } from "./types"; import { getCenter, navigateDirectionToOffset, setTransform } from "./utils"; @@ -71,17 +71,29 @@ const DefaultAnimationTimeS = 0.15; export class LayoutModel { /** - * The jotai atom for persisting the tree state to the backend and retrieving updates from the backend. + * Local atom holding the current tree state (source of truth during runtime) */ - treeStateAtom: WritableLayoutTreeStateAtom; + private localTreeStateAtom: PrimitiveAtom; /** - * The tree state as it is persisted on the backend. + * The tree state (local cache) */ treeState: LayoutTreeState; /** - * The last-recorded tree state generation. + * Reference to the tab atom for accessing WaveObject */ - lastTreeStateGeneration: number; + private tabAtom: Atom; + /** + * WaveObject atom for persistence + */ + private waveObjectAtom: WritableWaveObjectAtom; + /** + * Debounce timer for persistence + */ + private persistDebounceTimer: NodeJS.Timeout | null; + /** + * Set of action IDs that have been processed (prevents duplicate processing) + */ + private processedActionIds: Set; /** * The jotai getter that is used to read atom values. */ @@ -184,6 +196,10 @@ export class LayoutModel { * The currently magnified node. */ magnifiedNodeId: string; + /** + * Atom for the magnified node ID (derived from local tree state) + */ + magnifiedNodeIdAtom: Atom; /** * The last node to be magnified, other than the current magnified node, if set. This node should sit at a higher z-index than the others so that it floats above the other nodes as it returns to its original position. */ @@ -218,7 +234,7 @@ export class LayoutModel { private isContainerResizing: PrimitiveAtom; constructor( - treeStateAtom: WritableLayoutTreeStateAtom, + tabAtom: Atom, getter: Getter, setter: Setter, renderContent?: ContentRenderer, @@ -227,7 +243,7 @@ export class LayoutModel { gapSizePx?: number, animationTimeS?: number ) { - this.treeStateAtom = treeStateAtom; + this.tabAtom = tabAtom; this.getter = getter; this.setter = setter; this.renderContent = renderContent; @@ -239,7 +255,26 @@ export class LayoutModel { return 2 * (gapSizePx > 5 ? gapSizePx : DefaultGapSizePx); }); this.animationTimeS = atom(animationTimeS ?? DefaultAnimationTimeS); - this.lastTreeStateGeneration = -1; + this.persistDebounceTimer = null; + this.processedActionIds = new Set(); + + this.waveObjectAtom = getLayoutStateAtomFromTab(tabAtom, getter); + + this.localTreeStateAtom = atom({ + rootNode: undefined, + focusedNodeId: undefined, + magnifiedNodeId: undefined, + leafOrder: undefined, + pendingBackendActions: undefined, + }); + + this.treeState = { + rootNode: undefined, + focusedNodeId: undefined, + magnifiedNodeId: undefined, + leafOrder: undefined, + pendingBackendActions: undefined, + }; this.leafs = atom([]); this.leafOrder = atom([]); @@ -288,9 +323,14 @@ export class LayoutModel { this.ephemeralNode = atom(); this.magnifiedNodeSizeAtom = getSettingsKeyAtom("window:magnifiedblocksize"); + this.magnifiedNodeIdAtom = atom((get) => { + const treeState = get(this.localTreeStateAtom); + return treeState.magnifiedNodeId; + }); + this.focusedNode = atom((get) => { const ephemeralNode = get(this.ephemeralNode); - const treeState = get(this.treeStateAtom); + const treeState = get(this.localTreeStateAtom); if (ephemeralNode) { return ephemeralNode; } @@ -307,7 +347,217 @@ export class LayoutModel { return this.getPlaceholderTransform(pendingAction); }); - this.onTreeStateAtomUpdated(true); + this.initializeFromWaveObject(); + } + + private initializeFromWaveObject() { + const waveObjState = this.getter(this.waveObjectAtom); + + const initialState: LayoutTreeState = { + rootNode: waveObjState?.rootnode, + focusedNodeId: waveObjState?.focusednodeid, + magnifiedNodeId: waveObjState?.magnifiednodeid, + leafOrder: undefined, + pendingBackendActions: waveObjState?.pendingbackendactions, + }; + + this.treeState = initialState; + this.magnifiedNodeId = initialState.magnifiedNodeId; + this.setter(this.localTreeStateAtom, { ...initialState }); + + if (initialState.pendingBackendActions?.length) { + fireAndForget(() => this.processPendingBackendActions()); + } else { + this.updateTree(); + } + } + + onBackendUpdate() { + const waveObj = this.getter(this.waveObjectAtom); + const pendingActions = waveObj?.pendingbackendactions; + if (pendingActions?.length) { + fireAndForget(() => this.processPendingBackendActions()); + } + } + + private async processPendingBackendActions() { + const waveObj = this.getter(this.waveObjectAtom); + const actions = waveObj?.pendingbackendactions; + if (!actions?.length) return; + + this.treeState.pendingBackendActions = undefined; + + for (const action of actions) { + if (!action.actionid) { + console.warn("Dropping layout action without actionid:", action); + continue; + } + if (this.processedActionIds.has(action.actionid)) { + continue; + } + this.processedActionIds.add(action.actionid); + await this.handleBackendAction(action); + } + + this.updateTree(); + this.setter(this.localTreeStateAtom, { ...this.treeState }); + this.persistToBackend(); + } + + private async handleBackendAction(action: LayoutActionData) { + switch (action.actiontype) { + case LayoutTreeActionType.InsertNode: { + if (action.ephemeral) { + this.newEphemeralNode(action.blockid); + break; + } + const insertNodeAction: LayoutTreeInsertNodeAction = { + type: LayoutTreeActionType.InsertNode, + node: newLayoutNode(undefined, undefined, undefined, { + blockId: action.blockid, + }), + magnified: action.magnified, + focused: action.focused, + }; + this.treeReducer(insertNodeAction, false); + break; + } + case LayoutTreeActionType.DeleteNode: { + const leaf = this?.getNodeByBlockId(action.blockid); + if (leaf) { + await this.closeNode(leaf.id); + } else { + console.error( + "Cannot apply eventbus layout action DeleteNode, could not find leaf node with blockId", + action.blockid + ); + } + break; + } + case LayoutTreeActionType.InsertNodeAtIndex: { + if (!action.indexarr) { + console.error("Cannot apply eventbus layout action InsertNodeAtIndex, indexarr field is missing."); + break; + } + const insertAction: LayoutTreeInsertNodeAtIndexAction = { + type: LayoutTreeActionType.InsertNodeAtIndex, + node: newLayoutNode(undefined, action.nodesize, undefined, { + blockId: action.blockid, + }), + indexArr: action.indexarr, + magnified: action.magnified, + focused: action.focused, + }; + this.treeReducer(insertAction, false); + break; + } + case LayoutTreeActionType.ClearTree: { + this.treeReducer( + { + type: LayoutTreeActionType.ClearTree, + } as LayoutTreeClearTreeAction, + false + ); + break; + } + case LayoutTreeActionType.ReplaceNode: { + const targetNode = this?.getNodeByBlockId(action.targetblockid); + if (!targetNode) { + console.error( + "Cannot apply eventbus layout action ReplaceNode, could not find target node with blockId", + action.targetblockid + ); + break; + } + const replaceAction: LayoutTreeReplaceNodeAction = { + type: LayoutTreeActionType.ReplaceNode, + targetNodeId: targetNode.id, + newNode: newLayoutNode(undefined, action.nodesize, undefined, { + blockId: action.blockid, + }), + }; + this.treeReducer(replaceAction, false); + break; + } + case LayoutTreeActionType.SplitHorizontal: { + const targetNode = this?.getNodeByBlockId(action.targetblockid); + if (!targetNode) { + console.error( + "Cannot apply eventbus layout action SplitHorizontal, could not find target node with blockId", + action.targetblockid + ); + break; + } + if (action.position != "before" && action.position != "after") { + console.error( + "Cannot apply eventbus layout action SplitHorizontal, invalid position", + action.position + ); + break; + } + const newNode = newLayoutNode(undefined, action.nodesize, undefined, { + blockId: action.blockid, + }); + const splitAction: LayoutTreeSplitHorizontalAction = { + type: LayoutTreeActionType.SplitHorizontal, + targetNodeId: targetNode.id, + newNode: newNode, + position: action.position, + }; + this.treeReducer(splitAction, false); + break; + } + case LayoutTreeActionType.SplitVertical: { + const targetNode = this?.getNodeByBlockId(action.targetblockid); + if (!targetNode) { + console.error( + "Cannot apply eventbus layout action SplitVertical, could not find target node with blockId", + action.targetblockid + ); + break; + } + if (action.position != "before" && action.position != "after") { + console.error( + "Cannot apply eventbus layout action SplitVertical, invalid position", + action.position + ); + break; + } + const newNode = newLayoutNode(undefined, action.nodesize, undefined, { + blockId: action.blockid, + }); + const splitAction: LayoutTreeSplitVerticalAction = { + type: LayoutTreeActionType.SplitVertical, + targetNodeId: targetNode.id, + newNode: newNode, + position: action.position, + }; + this.treeReducer(splitAction, false); + break; + } + default: + console.warn("unsupported layout action", action); + break; + } + } + + private persistToBackend() { + if (this.persistDebounceTimer) { + clearTimeout(this.persistDebounceTimer); + } + + this.persistDebounceTimer = setTimeout(() => { + const waveObj = this.getter(this.waveObjectAtom); + if (!waveObj) return; + + waveObj.rootnode = this.treeState.rootNode; + waveObj.focusednodeid = this.treeState.focusedNodeId; + waveObj.magnifiednodeid = this.treeState.magnifiedNodeId; + waveObj.leaforder = this.treeState.leafOrder; + + this.setter(this.waveObjectAtom, waveObj); + this.persistDebounceTimer = null; + }, 100); } /** @@ -396,14 +646,15 @@ export class LayoutModel { default: console.error("Invalid reducer action", this.treeState, action); } - if (this.lastTreeStateGeneration < this.treeState.generation) { - if (this.magnifiedNodeId !== this.treeState.magnifiedNodeId) { - this.lastMagnifiedNodeId = this.magnifiedNodeId; - this.lastEphemeralNodeId = undefined; - this.magnifiedNodeId = this.treeState.magnifiedNodeId; - } + if (this.magnifiedNodeId !== this.treeState.magnifiedNodeId) { + this.lastMagnifiedNodeId = this.magnifiedNodeId; + this.lastEphemeralNodeId = undefined; + this.magnifiedNodeId = this.treeState.magnifiedNodeId; + } + if (setState) { this.updateTree(); - if (setState) this.setTreeStateAtom(true); + this.setter(this.localTreeStateAtom, { ...this.treeState }); + this.persistToBackend(); } } @@ -412,165 +663,9 @@ export class LayoutModel { * @param force Whether to force the local tree state to update, regardless of whether the state is already up to date. */ async onTreeStateAtomUpdated(force = false) { - const treeState = this.getter(this.treeStateAtom); - // Only update the local tree state if it is different from the one in the upstream atom. This function is called even when the update was initiated by the LayoutModel, so we need to filter out false positives or we'll enter an infinite loop. - if ( - force || - !this.treeState?.rootNode || - !this.treeState?.generation || - treeState?.generation > this.treeState.generation || - treeState?.pendingBackendActions?.length - ) { - this.treeState = treeState; - this.magnifiedNodeId = treeState.magnifiedNodeId; - - if (this.treeState?.pendingBackendActions?.length) { - const actions = this.treeState.pendingBackendActions; - this.treeState.pendingBackendActions = undefined; - for (const action of actions) { - switch (action.actiontype) { - case LayoutTreeActionType.InsertNode: { - if (action.ephemeral) { - this.newEphemeralNode(action.blockid); - break; - } - - const insertNodeAction: LayoutTreeInsertNodeAction = { - type: LayoutTreeActionType.InsertNode, - node: newLayoutNode(undefined, undefined, undefined, { - blockId: action.blockid, - }), - magnified: action.magnified, - focused: action.focused, - }; - this.treeReducer(insertNodeAction, false); - break; - } - case LayoutTreeActionType.DeleteNode: { - const leaf = this?.getNodeByBlockId(action.blockid); - if (leaf) { - await this.closeNode(leaf.id); - } else { - console.error( - "Cannot apply eventbus layout action DeleteNode, could not find leaf node with blockId", - action.blockid - ); - } - break; - } - case LayoutTreeActionType.InsertNodeAtIndex: { - if (!action.indexarr) { - console.error( - "Cannot apply eventbus layout action InsertNodeAtIndex, indexarr field is missing." - ); - break; - } - const insertAction: LayoutTreeInsertNodeAtIndexAction = { - type: LayoutTreeActionType.InsertNodeAtIndex, - node: newLayoutNode(undefined, action.nodesize, undefined, { - blockId: action.blockid, - }), - indexArr: action.indexarr, - magnified: action.magnified, - focused: action.focused, - }; - this.treeReducer(insertAction, false); - break; - } - case LayoutTreeActionType.ClearTree: { - this.treeReducer( - { - type: LayoutTreeActionType.ClearTree, - } as LayoutTreeClearTreeAction, - false - ); - break; - } - case LayoutTreeActionType.ReplaceNode: { - const targetNode = this?.getNodeByBlockId(action.targetblockid); - if (!targetNode) { - console.error( - "Cannot apply eventbus layout action ReplaceNode, could not find target node with blockId", - action.targetblockid - ); - break; - } - const replaceAction: LayoutTreeReplaceNodeAction = { - type: LayoutTreeActionType.ReplaceNode, - targetNodeId: targetNode.id, - newNode: newLayoutNode(undefined, action.nodesize, undefined, { - blockId: action.blockid, - }), - }; - this.treeReducer(replaceAction, false); - break; - } - case LayoutTreeActionType.SplitHorizontal: { - const targetNode = this?.getNodeByBlockId(action.targetblockid); - if (!targetNode) { - console.error( - "Cannot apply eventbus layout action SplitHorizontal, could not find target node with blockId", - action.targetblockid - ); - break; - } - if (action.position != "before" && action.position != "after") { - console.error( - "Cannot apply eventbus layout action SplitHorizontal, invalid position", - action.position - ); - break; - } - const newNode = newLayoutNode(undefined, action.nodesize, undefined, { - blockId: action.blockid, - }); - const splitAction: LayoutTreeSplitHorizontalAction = { - type: LayoutTreeActionType.SplitHorizontal, - targetNodeId: targetNode.id, - newNode: newNode, - position: action.position, - }; - this.treeReducer(splitAction, false); - break; - } - case LayoutTreeActionType.SplitVertical: { - const targetNode = this?.getNodeByBlockId(action.targetblockid); - if (!targetNode) { - console.error( - "Cannot apply eventbus layout action SplitVertical, could not find target node with blockId", - action.targetblockid - ); - break; - } - if (action.position != "before" && action.position != "after") { - console.error( - "Cannot apply eventbus layout action SplitVertical, invalid position", - action.position - ); - break; - } - const newNode = newLayoutNode(undefined, action.nodesize, undefined, { - blockId: action.blockid, - }); - const splitAction: LayoutTreeSplitVerticalAction = { - type: LayoutTreeActionType.SplitVertical, - targetNodeId: targetNode.id, - newNode: newNode, - position: action.position, - }; - this.treeReducer(splitAction, false); - break; - } - default: - console.warn("unsupported layout action", action); - break; - } - } - this.setTreeStateAtom(true); - } else { - this.updateTree(); - this.setTreeStateAtom(force); - } + if (force) { + this.updateTree(); + this.setter(this.localTreeStateAtom, { ...this.treeState }); } } @@ -578,13 +673,6 @@ export class LayoutModel { * Set the upstream tree state atom to the value of the local tree state. * @param bumpGeneration Whether to bump the generation of the tree state before setting the atom. */ - setTreeStateAtom(bumpGeneration = false) { - if (bumpGeneration) { - this.treeState.generation++; - } - this.lastTreeStateGeneration = this.treeState.generation; - this.setter(this.treeStateAtom, this.treeState); - } /** * Recursively walks the tree to find leaf nodes, update the resize handles, and compute additional properties for each node. @@ -932,14 +1020,14 @@ export class LayoutModel { blockId, blockNum: atom((get) => get(this.leafOrder).findIndex((leafEntry) => leafEntry.nodeid === nodeid) + 1), isFocused: atom((get) => { - const treeState = get(this.treeStateAtom); + const treeState = get(this.localTreeStateAtom); const isFocused = treeState.focusedNodeId === nodeid; return isFocused; }), numLeafs: this.numLeafs, isResizing: this.isResizing, isMagnified: atom((get) => { - const treeState = get(this.treeStateAtom); + const treeState = get(this.localTreeStateAtom); return treeState.magnifiedNodeId === nodeid; }), isEphemeral: atom((get) => { @@ -1115,7 +1203,8 @@ export class LayoutModel { this.setter(this.ephemeralNode, undefined); this.treeState.focusedNodeId = undefined; this.updateTree(false); - this.setTreeStateAtom(true); + this.setter(this.localTreeStateAtom, { ...this.treeState }); + this.persistToBackend(); await this.onNodeDelete?.(ephemeralNode.data); return; } diff --git a/frontend/layout/lib/layoutModelHooks.ts b/frontend/layout/lib/layoutModelHooks.ts index 582957b50a..f1491e1b4b 100644 --- a/frontend/layout/lib/layoutModelHooks.ts +++ b/frontend/layout/lib/layoutModelHooks.ts @@ -6,13 +6,13 @@ import { atoms, globalStore, WOS } from "@/app/store/global"; import { fireAndForget } from "@/util/util"; import { Atom, useAtomValue } from "jotai"; import { CSSProperties, useCallback, useEffect, useState } from "react"; -import { withLayoutTreeStateAtomFromTab } from "./layoutAtom"; +import { getLayoutStateAtomFromTab } from "./layoutAtom"; import { LayoutModel } from "./layoutModel"; import { LayoutNode, NodeModel, TileLayoutContents } from "./types"; const layoutModelMap: Map = new Map(); -export function getLayoutModelForTab(tabAtom: Atom): LayoutModel { +function getLayoutModelForTab(tabAtom: Atom): LayoutModel { const tabData = globalStore.get(tabAtom); if (!tabData) return; const tabId = tabData.oid; @@ -22,14 +22,21 @@ export function getLayoutModelForTab(tabAtom: Atom): LayoutModel { return layoutModel; } } - const layoutTreeStateAtom = withLayoutTreeStateAtomFromTab(tabAtom); - const layoutModel = new LayoutModel(layoutTreeStateAtom, globalStore.get, globalStore.set); - globalStore.sub(layoutTreeStateAtom, () => fireAndForget(layoutModel.onTreeStateAtomUpdated.bind(layoutModel))); + const layoutModel = new LayoutModel(tabAtom, globalStore.get, globalStore.set); + + const staticTabId = globalStore.get(atoms.staticTabId); + if (tabId === staticTabId) { + const layoutStateAtom = getLayoutStateAtomFromTab(tabAtom, globalStore.get); + globalStore.sub(layoutStateAtom, () => { + layoutModel.onBackendUpdate(); + }); + } + layoutModelMap.set(tabId, layoutModel); return layoutModel; } -export function getLayoutModelForTabById(tabId: string) { +function getLayoutModelForTabById(tabId: string) { const tabOref = WOS.makeORef("tab", tabId); const tabAtom = WOS.getWaveObjectAtom(tabOref); return getLayoutModelForTab(tabAtom); @@ -44,7 +51,7 @@ export function deleteLayoutModelForTab(tabId: string) { if (layoutModelMap.has(tabId)) layoutModelMap.delete(tabId); } -export function useLayoutModel(tabAtom: Atom): LayoutModel { +function useLayoutModel(tabAtom: Atom): LayoutModel { return getLayoutModelForTab(tabAtom); } diff --git a/frontend/layout/lib/layoutTree.ts b/frontend/layout/lib/layoutTree.ts index 74fcad5f34..1cf46d8ac3 100644 --- a/frontend/layout/lib/layoutTree.ts +++ b/frontend/layout/lib/layoutTree.ts @@ -270,7 +270,7 @@ export function moveNode(layoutState: LayoutTreeState, action: LayoutTreeMoveNod if (oldParent) { removeChild(oldParent, node, startingIndex); } - layoutState.generation++; + } export function insertNode(layoutState: LayoutTreeState, action: LayoutTreeInsertNodeAction) { @@ -291,7 +291,7 @@ export function insertNode(layoutState: LayoutTreeState, action: LayoutTreeInser if (action.focused) { layoutState.focusedNodeId = action.node.id; } - layoutState.generation++; + } export function insertNodeAtIndex(layoutState: LayoutTreeState, action: LayoutTreeInsertNodeAtIndexAction) { @@ -316,7 +316,7 @@ export function insertNodeAtIndex(layoutState: LayoutTreeState, action: LayoutTr if (action.focused) { layoutState.focusedNodeId = action.node.id; } - layoutState.generation++; + } export function swapNode(layoutState: LayoutTreeState, action: LayoutTreeSwapNodeAction) { @@ -348,7 +348,7 @@ export function swapNode(layoutState: LayoutTreeState, action: LayoutTreeSwapNod parentNode1.children[parentNode1Index] = node2; parentNode2.children[parentNode2Index] = node1; - layoutState.generation++; + } export function deleteNode(layoutState: LayoutTreeState, action: LayoutTreeDeleteNodeAction) { @@ -375,7 +375,7 @@ export function deleteNode(layoutState: LayoutTreeState, action: LayoutTreeDelet } } - layoutState.generation++; + } export function resizeNode(layoutState: LayoutTreeState, action: LayoutTreeResizeNodeAction) { @@ -390,7 +390,7 @@ export function resizeNode(layoutState: LayoutTreeState, action: LayoutTreeResiz const node = findNode(layoutState.rootNode, resize.nodeId); node.size = resize.size; } - layoutState.generation++; + } export function focusNode(layoutState: LayoutTreeState, action: LayoutTreeFocusNodeAction) { @@ -400,7 +400,7 @@ export function focusNode(layoutState: LayoutTreeState, action: LayoutTreeFocusN } layoutState.focusedNodeId = action.nodeId; - layoutState.generation++; + } export function magnifyNodeToggle(layoutState: LayoutTreeState, action: LayoutTreeMagnifyNodeToggleAction) { @@ -418,7 +418,7 @@ export function magnifyNodeToggle(layoutState: LayoutTreeState, action: LayoutTr layoutState.magnifiedNodeId = action.nodeId; layoutState.focusedNodeId = action.nodeId; } - layoutState.generation++; + } export function clearTree(layoutState: LayoutTreeState) { @@ -426,7 +426,7 @@ export function clearTree(layoutState: LayoutTreeState) { layoutState.leafOrder = undefined; layoutState.focusedNodeId = undefined; layoutState.magnifiedNodeId = undefined; - layoutState.generation++; + } export function replaceNode(layoutState: LayoutTreeState, action: LayoutTreeReplaceNodeAction) { @@ -453,7 +453,7 @@ export function replaceNode(layoutState: LayoutTreeState, action: LayoutTreeRepl if (action.focused) { layoutState.focusedNodeId = newNode.id; } - layoutState.generation++; + } // ─── SPLIT HORIZONTAL ───────────────────────────────────────────────────────────── @@ -497,7 +497,7 @@ export function splitHorizontal(layoutState: LayoutTreeState, action: LayoutTree if (action.focused) { layoutState.focusedNodeId = newNode.id; } - layoutState.generation++; + } // ─── SPLIT VERTICAL ───────────────────────────────────────────────────────────── @@ -539,5 +539,5 @@ export function splitVertical(layoutState: LayoutTreeState, action: LayoutTreeSp if (action.focused) { layoutState.focusedNodeId = newNode.id; } - layoutState.generation++; + } diff --git a/frontend/layout/lib/types.ts b/frontend/layout/lib/types.ts index d250fa74e6..5ce8e466f1 100644 --- a/frontend/layout/lib/types.ts +++ b/frontend/layout/lib/types.ts @@ -297,7 +297,6 @@ export type LayoutTreeState = { */ leafOrder?: LeafOrderEntry[]; pendingBackendActions: LayoutActionData[]; - generation: number; }; export type WritableLayoutTreeStateAtom = WritableAtom; diff --git a/frontend/layout/tests/model.ts b/frontend/layout/tests/model.ts index fd67c9fae2..38b1230537 100644 --- a/frontend/layout/tests/model.ts +++ b/frontend/layout/tests/model.ts @@ -6,7 +6,6 @@ import { LayoutNode, LayoutTreeState } from "../lib/types"; export function newLayoutTreeState(rootNode: LayoutNode): LayoutTreeState { return { rootNode, - generation: 0, pendingBackendActions: [], }; } diff --git a/frontend/types/gotypes.d.ts b/frontend/types/gotypes.d.ts index 647af8cf47..df15dbb188 100644 --- a/frontend/types/gotypes.d.ts +++ b/frontend/types/gotypes.d.ts @@ -476,6 +476,7 @@ declare global { // waveobj.LayoutActionData type LayoutActionData = { actiontype: string; + actionid: string; blockid: string; nodesize?: number; indexarr?: number[]; diff --git a/pkg/waveobj/wtype.go b/pkg/waveobj/wtype.go index e7070c990f..1498c44a09 100644 --- a/pkg/waveobj/wtype.go +++ b/pkg/waveobj/wtype.go @@ -203,6 +203,7 @@ func (t *Tab) GetBlockORefs() []ORef { type LayoutActionData struct { ActionType string `json:"actiontype"` + ActionId string `json:"actionid"` BlockId string `json:"blockid"` NodeSize *uint `json:"nodesize,omitempty"` IndexArr *[]int `json:"indexarr,omitempty"` diff --git a/pkg/wcore/layout.go b/pkg/wcore/layout.go index 63a9f2ef57..9ddbe593eb 100644 --- a/pkg/wcore/layout.go +++ b/pkg/wcore/layout.go @@ -9,6 +9,7 @@ import ( "log" "time" + "github.com/google/uuid" "github.com/wavetermdev/waveterm/pkg/waveobj" "github.com/wavetermdev/waveterm/pkg/wstore" ) @@ -104,6 +105,12 @@ func QueueLayoutAction(ctx context.Context, layoutStateId string, actions ...wav return fmt.Errorf("unable to get layout state for given id %s: %w", layoutStateId, err) } + for i := range actions { + if actions[i].ActionId == "" { + actions[i].ActionId = uuid.New().String() + } + } + if layoutStateObj.PendingBackendActions == nil { layoutStateObj.PendingBackendActions = &actions } else { From d5fff4f109c6fca81b6321307686fecf9d740cb1 Mon Sep 17 00:00:00 2001 From: sawka Date: Fri, 3 Oct 2025 09:18:59 -0700 Subject: [PATCH 3/3] persist pendingbackendactions --- frontend/layout/lib/layoutModel.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/layout/lib/layoutModel.ts b/frontend/layout/lib/layoutModel.ts index 77523f3ed2..7b4e2978ec 100644 --- a/frontend/layout/lib/layoutModel.ts +++ b/frontend/layout/lib/layoutModel.ts @@ -384,9 +384,9 @@ export class LayoutModel { const waveObj = this.getter(this.waveObjectAtom); const actions = waveObj?.pendingbackendactions; if (!actions?.length) return; - + this.treeState.pendingBackendActions = undefined; - + for (const action of actions) { if (!action.actionid) { console.warn("Dropping layout action without actionid:", action); @@ -554,6 +554,7 @@ export class LayoutModel { waveObj.focusednodeid = this.treeState.focusedNodeId; waveObj.magnifiednodeid = this.treeState.magnifiedNodeId; waveObj.leaforder = this.treeState.leafOrder; + waveObj.pendingbackendactions = this.treeState.pendingBackendActions; this.setter(this.waveObjectAtom, waveObj); this.persistDebounceTimer = null;