From 6c2afb1cbbd5af8fcd42047283cd1eb0cb75c646 Mon Sep 17 00:00:00 2001 From: Ethanfel Date: Mon, 29 Jun 2026 19:28:43 +0200 Subject: [PATCH] Non-destructive timeline: semantic diffs, quieter autosave, switch-spam fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks snapshot capture and navigation toward a Fusion-360-style non-destructive timeline, addressing three pain points. - Workflow-switch spam: re-seed the dedup/change-detection baseline for the newly-opened tab (seedWorkflowBaseline) and suppress auto-capture for a short guard window (suppressAutoCapture/SWITCH_GUARD_MS), so the graphChanged fired by loadGraphData no longer spawns a redundant snapshot of a workflow you only just opened. - Quieter autosave: detectChangeType now recognises resize, collapse and pin (previously saved as "unknown") and classifies pure move/resize/collapse as "cosmetic"; a cosmetic flag never escalates a real edit. Auto-capture skips cosmetic-only changes; mode (mute/bypass) is treated as a meaningful change. The cosmetic gate applies to the auto path only — manual saves and the pre-swap/pre-restore "Current" capture still preserve everything (no silent layout loss). - Semantic "what changed": getLiveWidgetNames/widgetNameFor map widgets_values indices to widget names (by exact node id), so diffs and tooltips read "seed", "text", "cfg" instead of "Value[6]"; the diff modal shows meaningful params first and collapses position/size into a single muted "Layout" line. - Non-destructive navigation: Alt+Left/Right step through history via stepToSnapshot (quiet, re-entrancy-guarded swap with a position toast); jumping between saved states is a storage no-op and never deletes later snapshots. Includes research report + implementation plan under docs/plans. Verified: node --check passes; 19 unit tests on the diff/classification logic pass. Co-Authored-By: Claude Opus 4.8 --- ...ndestructive-timeline-research-and-plan.md | 258 ++++++++++++++++++ js/snapshot_manager.js | 256 +++++++++++++---- 2 files changed, 459 insertions(+), 55 deletions(-) create mode 100644 docs/plans/2026-06-29-nondestructive-timeline-research-and-plan.md diff --git a/docs/plans/2026-06-29-nondestructive-timeline-research-and-plan.md b/docs/plans/2026-06-29-nondestructive-timeline-research-and-plan.md new file mode 100644 index 0000000..7287561 --- /dev/null +++ b/docs/plans/2026-06-29-nondestructive-timeline-research-and-plan.md @@ -0,0 +1,258 @@ +# Non-Destructive Timeline — Research Report & Implementation Plan + +**Date:** 2026-06-29 +**Goal:** Make the Snapshot Manager timeline behave like Autodesk Fusion 360's parametric +timeline — jumping back and forth is easy, fast, and **non-destructive** — while fixing the +three reported pain points: (1) weak "what changed" info, (2) spammy autosave, (3) workflow +switching spams "now save". + +This document has two parts: +- **Part 1 — Research report** (web research, 25 claims survived 3-vote adversarial verification). +- **Part 2 — Implementation plan** (each recommendation mapped to concrete changes in `js/snapshot_manager.js`). + +--- + +# Part 1 — Research Report + +## Executive summary + +Across Fusion 360 (CAD), Houdini (node graph), Unreal (Blueprint diff), Final Cut Pro (NLE), +Figma and Google Docs (history panels), the tools that feel non-destructive share four moves: +**(a)** jumping back never deletes later work — it *disables and recomputes* it (Fusion) or keeps +the live state untouched while you preview (Final Cut Pro skimmer); **(b)** *preview* is decoupled +from *commit* — you can hover/scrub other states and still "return to where you were"; **(c)** a +restore/jump records the pre-jump state **once** as a checkpoint (Figma) rather than spamming +checkpoints; and **(d)** "what changed" is shown **semantically** (named parameters, fixed +color-coded change types — Unreal's red/green/cyan/grey), with trivial moves *de-emphasized* and +auto-history *coalesced* by time or by filtering out streaming/cosmetic edits (Figma's 30-min +cadence; redux-undo's `filter`/`excludeAction`/`groupBy`; Houdini's explicit warning against +over-automatic capture). These map cleanly onto a snapshot-per-state model and onto all three pain +points. + +## Findings + +### F1 — Non-destructive "jump back" = disable + recompute, never delete (Fusion 360) — **HIGH** +Fusion records every step in creation order and lets you "go back in time" to edit earlier +decisions without starting over. Mechanically, editing/rolling to a past feature **rolls the +history marker to just before it and disables (not deletes) all downstream nodes**, then +**recomputes them forward** when you roll back to the end — "all the features that were after that +point are preserved and will reappear." Rolling the marker back "has not deleted the features in +front of the marker." This is the load-bearing property: navigating history is a *reversible +editing surface*, not a destructive log. +*Sources: autodesk.com Fusion blog (timeline-edits, beginners-guide-part-4) — primary; help.autodesk.com Fusion-360-API CustomFeatures_UM — primary; productdesignonline.com; Ace Makerspace. Votes 3-0 (×5), 2-1 (×1).* + +### F2 — Navigation gesture == edit gesture, plus a draggable marker with playback controls (Fusion) — **HIGH** +A past step is edited in place by **double-clicking** it, or **right-click → Edit Sketch/Edit +Feature**. Pure navigation is a **draggable history-marker slider** ("rolling back the design") +with **playback controls** (move-to-beginning, previous step, play, next step, move-to-end) and a +right-click **"Roll History Marker Here"** to jump the marker directly to any step. So back/forth +is a one-gesture, low-friction interaction distinct from editing. +*Sources: autodesk.com Fusion blog — primary; productdesignonline.com (blog); Noble Desktop. Votes 3-0.* + +### F3 — Decouple *preview* from *committed position* — the skimmer pattern (Final Cut Pro) — **HIGH** +Final Cut Pro uses **two indicators**: a persistent **playhead** (your committed position, fixed +unless you move it) and a transient **skimmer** (a preview cursor that follows the pointer). The +skimmer "lets you preview clips… **without affecting the playhead position**," so you can "skim to +see what's in other clips but still keep your playhead position." This is the canonical +"scrub-to-preview, keep your place / return to where I was" pattern — non-destructive preview +decoupled from the committed edit position. +*Sources: support.apple.com Final Cut Pro (skimmer, intro-to-playback) — primary; Larry Jordan; Ripple Training. Votes 3-0 (×3), 2-1 (×1).* + +### F4 — Semantic, color-coded "what changed" + difference navigation (Unreal Blueprint diff) — **HIGH** +Unreal's Blueprint Diff Tool communicates **change type visually** with a fixed legend: +**red = removed, green = added, cyan = changed, grey = moved nodes/comments.** It provides +**Next/Previous** buttons to cycle differences one at a time and a **clickable navigation tree** to +jump to a specific difference. The lesson: show the *kind* of change semantically (not raw +indices), and explicitly give **moves their own subdued category** (grey). +*Sources: dev.epicgames.com UE Diff Tool docs (UE 5.7) — primary. Votes 2-1 (color legend), 3-0 (navigation).* + +### F5 — Scope change capture explicitly; warn against over-automatic capture (Houdini takes) — **HIGH** +Houdini "takes" are **hierarchically overlaid sets of parameter changes**: any parameter you don't +explicitly change is **inherited from the parent**, enabling non-destructive parallel variations +that preserve the original. Only **explicitly included** parameters are editable in a take (others +appear disabled), and the takes pane **shows which parameters changed** for the selected take. +Critically, SideFX **explicitly warns against overusing Auto-take mode** because it "makes it easy +to unintentionally include parameters… which… can make diagnosing problems difficult" — a +first-party anti-pattern for automatic change capture that mirrors this tool's autosave spam. +*Sources: sidefx.com/docs/houdini takes + ref/panes/takes — primary. Votes 3-0.* + +### F6 — Restore/jump is non-destructive *because it checkpoints the pre-jump state once* (Figma) — **HIGH** +Restoring a previous version in Figma "is a **non-destructive action**, so you can still access the +current version." It works by adding **two autosave checkpoints**: one preserving the +current/pre-restore state, one marking the restored version. So later work is never silently lost — +it's captured as a checkpoint exactly once at the moment of restore. +*Sources: help.figma.com version-history — primary. Votes 3-0.* + +### F7 — Coalesce auto-history by time; separate viewing from restoring; named vs auto (Figma, Google Docs) — **HIGH** +Figma records an auto **checkpoint every 30 minutes** (time-based coalescing) while keeping the +live "current version" continuously up to date — it does **not** snapshot on every edit. Google +Docs separates **viewing** an earlier version (read-only; a **"Back"** control returns you to +current, no state change) from **restoring** it (explicit "Restore this version"). It also supports +**named versions** distinct from auto-saved revisions, with an **"Only show named versions"** filter +to cut noise (caps: 40 named/doc, 15/spreadsheet). +*Sources: help.figma.com; support.google.com/docs/answer/190843 — primary. Votes 3-0.* + +### F8 — History-noise control patterns: filter, exclude streaming actions, group into chunks (redux-undo) — **HIGH** +A `filter` keeps intermediate state changes **out of history** (`state.past`) **without affecting +the live state**. For drag-like continuous edits, `excludeAction(['MOVE_CURSOR','UPDATE_OBJECT_POS'])` +records **only the final committed state**. And undo/redo in "reasonable chunks" needs deliberate +**custom filters + `groupBy`** rather than recording every micro-action. This is the concrete +recipe for de-noising autosave: gate out cosmetic/streaming edits, coalesce a burst into one entry. +*Sources: redux-undo.js.org/main/faq — primary; GitHub README. Votes 3-0 (×2), 2-1 (×1).* + +## Confidence & caveats +- **High confidence** on every finding above (primary vendor docs; mostly unanimous 3-0 votes). +- **Vendor-doc framing:** Fusion's "non-destructive" language partly comes from Autodesk marketing + blogs, but the *mechanism* (disable + recompute, marker preserves downstream) is independently + corroborated by API docs and third parties — solid. +- **Edge cases noted but not refuting:** editing a far-upstream Fusion feature can break downstream + dependency references (inherent to any dependency graph; the design is still never destroyed); + Figma history retention is tier-bounded (30 days on free); redux-undo `filter` is technically a + warning-context API, not an endorsement (the *pattern* still holds). +- **Coverage gaps:** DAW (Ableton/Logic), DaVinci/Premiere timeline scrubbing, Photoshop history, + and Git-GUI detached-HEAD UX were searched but produced no *separately verified* surviving claims + beyond what Final Cut Pro / Figma / Google Docs already cover. The FCP skimmer is the strongest + scrubbing-ergonomics result. + +## Open questions +1. For a **snapshot-per-state** model (vs Fusion's parametric feature graph), should "jump back and + edit" create a **branch** automatically, or keep the linear list and rely on non-destructive + "Current" checkpoints? (Branching is currently hard-disabled in the build.) +2. What's the right **auto-snapshot cadence** — pure event-debounce (today: 3 s), a Figma-style + time floor (e.g. ≥1 per N minutes), or both? +3. Should **scrub/preview** load the graph at all (expensive), or only show the SVG/thumbnail + preview until the user explicitly commits — to keep back/forth instant on large graphs? + +--- + +# Part 2 — Implementation Plan + +Each workstream cites concrete code in `js/snapshot_manager.js`. Ordered by impact; +**Must-have → Should-have → Nice-to-have**. + +## Implementation status (2026-06-29) +**Done** (all four batches landed, 19/19 unit tests on the extracted diff logic pass): +- **C1 + C2** — `seedWorkflowBaseline()` + `suppressAutoCapture(SWITCH_GUARD_MS)` on `openWorkflow`; `scheduleCaptureSnapshot` honours the suppression window. +- **B1** — `detectChangeType` now classifies move/resize/collapse(+pin) as `"cosmetic"` and never lets a cosmetic flag escalate a real edit; `mode` (mute/bypass) treated as meaningful. `_captureCore` skips `changeType==="cosmetic"` for auto-captures (`skipCosmetic`). +- **A1 + A3** — `getLiveWidgetNames()`/`widgetNameFor()` map `widgets_values` indices → names at capture; `computeDetailedDiff`/`computeCaptureMetaDiff` carry names; diff modal shows `seed:`/`text:` (meaningful first) and collapses position/size into one muted "Layout: moved, resized" line; tooltips read `KSampler (seed, cfg)`. +- **D1 + D3** — non-destructive jump confirmed (swap re-seeds hash + dedup → repeat steps are storage no-ops); `stepToSnapshot()` + **Alt+◀ / Alt+▶** keyboard step nav with a quiet swap and a `N/total · label` position toast. + +**Deferred (nice-to-have, not yet built):** A2 full Unreal-style color legend per change type; B2 time-based checkpoint floor; B3 Google-Docs "only show manual/named" filter; D2 drag-scrub skimmer mode (hover preview already exists); D4 explicit "return to where I was" affordance. + +## Pain-point ↔ finding map +| Pain point | Backed by | Workstreams | +|---|---|---| +| Weak "what changed" info | F4, F5, F7 | **A** | +| Spammy autosave | F5, F7, F8 | **B** | +| WF-switch spams "now save" | (codebase bug) F6 | **C** | +| Fusion-360 non-destructive feel | F1, F2, F3, F6 | **D** | + +--- + +## Workstream A — Semantic "what changed" (MUST) — *F4, F5, F7* + +**Problem (code):** `computeDetailedDiff` (`:565`) compares `widgets_values` positionally and emits +`Value[6]: "a cat" → "a dog"` (`:1370`). `getGraphData()` is `app.graph.serialize()` (`:438`), so +at capture time the **live** `app.graph._nodes[i].widgets[]` array — each with a `.name` in the +same order as `widgets_values` — is available but never used. `detectChangeType` (`:454`) only +yields a coarse single bucket. + +**A1. Capture a widget index→name map (MUST).** At capture in `_captureCore` (`:1574`), walk +`app.graph._nodes`, build `{nodeId: [widgetName,…]}`, and use it so diffs read `text:`, `seed:`, +`cfg:`, `sampler_name:` instead of `Value[i]`. Store a compact, named `captureDiff` (extend +`computeCaptureMetaDiff` `:697`). Persisted per snapshot so old snapshots without it degrade +gracefully to the current `Value[i]` form. + +**A2. Semantic change classification + Unreal-style legend (SHOULD).** Replace the single +`changeType` with a small set the user cares about: `prompt`, `param` (seed/cfg/sampler/steps/…), +`model` (checkpoint/LoRA names), `connection`, `node_add`/`node_remove`, and a subordinate +`cosmetic` (move/resize/collapse). Reuse `CHANGE_TYPE_ICONS` (`:2755`) with the red=removed / +green=added / cyan=changed / grey=cosmetic palette (F4). **Moves/resizes get the grey, de-emphasized +treatment** — exactly what you asked for. + +**A3. De-noise the diff modal + tooltip (SHOULD).** In `showDiffModal` (`:1232`) and +`formatCaptureDiffLines` (`:720`), put position/size/move rows in a **collapsed "Cosmetic" +section** and surface prompt/param/model/connection changes first with their widget names. The +hover tooltip headline should read e.g. `~ KSampler (seed, cfg) · + CLIPTextEncode` rather than +`~ 2 values`. + +## Workstream B — Tame autosave noise (MUST) — *F5, F7, F8* + +**Problem (code):** every `graphChanged` schedules a capture (`:4236`→`scheduleCaptureSnapshot` +`:1671`, 3 s debounce). `_captureCore` only skips `changeType === "move"` (`:1589`), so **resize and +collapse/expand fall through as `"unknown"` and ARE saved** as snapshots — pure visual noise. No +floor on auto-snapshot frequency. + +**B1. Cosmetic-change gate (MUST).** Generalize `skipMove` → `skipCosmetic`: skip auto-capture when +the *only* changes are position/size/collapse/pin (redux-undo `filter`/`excludeAction`; Houdini +Auto-take warning F5/F8). Manual snapshots (Ctrl+S `:4297`, Snapshot button `:3901`) and node-trigger +captures still save everything. This alone removes most of the spam. + +**B2. Coalesce bursts; optional time floor (SHOULD).** Keep the 3 s event-debounce (already +coalesces a typing burst). Add an optional minimum interval between *auto* snapshots and/or a +Figma-style time-based fallback checkpoint (F7), configurable via the existing settings +(`debounceSeconds` lives at `:4130`). + +**B3. Make auto vs manual legible + filterable (NICE).** Auto-snapshots already carry labels; +borrow Google Docs' **"Only show named/manual versions"** filter (F7) in the sidebar so the auto +stream can be hidden. The existing search/filter UI (`:2934`+) is the natural home. + +## Workstream C — Fix workflow-switch "now save" spam (MUST) — *codebase bug, F6* + +**Root cause (code):** the `openWorkflow` handler (`:4252`) resets state and seeds +`lastCapturedIdMap` for the new tab but **never seeds `lastCapturedHashMap` / `lastGraphData`** for +it. ComfyUI's `loadGraphData` for the freshly-opened workflow then fires `graphChanged` → +`scheduleCaptureSnapshot` → 3 s later `captureSnapshot("Auto")` runs, finds no seeded hash, **can't +dedupe**, and saves a redundant snapshot of a workflow you only just opened. + +**C1. Re-seed hash on switch (MUST).** In the `openWorkflow` `after()` block (`:4255`), after the +new graph is live, set `lastCapturedHashMap`/`setLastGraphData` for `newKey` (mirror the setup +seeding at `:4309-4322`). The post-load `graphChanged` then dedupes to a no-op. + +**C2. Programmatic-load suppression window (MUST).** Add a short-lived `loadingLock` flag (sibling +to `restoreLock` `:35`/`:1170`) set around tab switches and snapshot loads so `scheduleCaptureSnapshot` +(`:1671`) ignores the `graphChanged` events that *we* caused. Belt-and-suspenders with C1. + +## Workstream D — Fusion-360 non-destructive navigation (SHOULD) — *F1, F2, F3, F6* + +**Problem (code):** every timeline marker click calls `swapSnapshot(rec)` (`:3955`), which +`captureSnapshot("Current")` **before** loading (`:1728`). So *navigating* is what generates "now +save" and it feels destructive. There's no prev/next step nav and the current-position indicator is +subtle. + +**D1. Non-destructive jump = checkpoint-once-then-load (SHOULD).** Adopt Figma's model (F6): when +leaving a dirty live state, capture the pre-jump state **once** (already hash-deduped, so browsing +between *saved* states is a no-op), then load the target. Confirm/tighten that repeated back/forth +between existing snapshots creates **zero** new "Current" snapshots. Never delete later snapshots +(we already don't) — that's the non-destructive guarantee (F1). + +**D2. Preview vs commit — skimmer pattern (SHOULD).** The hover tooltip already previews via +SVG/thumbnail (`:3793`). Lean into it: hovering = preview (skimmer), clicking = commit (playhead) +(F3). Optionally a "scrub" mode where dragging along the timeline updates only the preview, and you +commit on release — keeps back/forth instant on big graphs (open question 3). + +**D3. Prev/Next step + clear current marker (SHOULD).** Add **keyboard step navigation** +(`[` / `]` or arrow keys) and Fusion-style playback buttons (begin/prev/next/end) to the timeline +(`buildTimeline` `:3869`), cycling snapshots one at a time (Unreal Next/Prev F4; Fusion controls +F2). Strengthen the current-position indicator (`marker-current`/`marker-active` `:3940`) so "where +am I" is obvious. + +**D4. "Return to where I was" (NICE).** Remember the snapshot you were on before scrubbing and offer +a one-click jump back (FCP playhead F3; Houdini go-back-and-forth F5). Lightweight: a single +`preScrubSnapshotId` + a "↩ return" affordance. + +--- + +## Suggested sequencing +1. **C1 + C2** (kill wf-switch spam) — small, isolated, immediate relief. +2. **B1** (cosmetic gate) — biggest reduction in autosave noise, low risk. +3. **A1 + A3** (named semantic diffs) — the "what changed" payoff you asked for. +4. **D1 + D3** (non-destructive jump + step nav) — the Fusion-360 feel. +5. **A2, B2/B3, D2, D4** — polish / configurable refinements. + +## Risks +- Widget-name mapping (A1) depends on live `_nodes`/`widgets` internals — guard for nodes whose + widget count ≠ `widgets_values` length and for headless/serialize-only paths. +- The cosmetic gate (B1) must not swallow a *meaningful* change that co-occurs with a move — gate + only when changes are **exclusively** cosmetic. +- Suppression window (C2) must auto-release even if a load throws (mirror `withRestoreLock` `:1170`). diff --git a/js/snapshot_manager.js b/js/snapshot_manager.js index cbd2a57..66607fc 100644 --- a/js/snapshot_manager.js +++ b/js/snapshot_manager.js @@ -12,6 +12,10 @@ import { api } from "../../scripts/api.js"; const EXTENSION_NAME = "ComfyUI.SnapshotManager"; const RESTORE_GUARD_MS = 500; const INITIAL_CAPTURE_DELAY_MS = 1500; +// Window after a programmatic workflow switch during which auto-capture is +// suppressed — ComfyUI's loadGraphData fires graphChanged for the freshly +// opened workflow, which must not be mistaken for a user edit. +const SWITCH_GUARD_MS = 2000; const MIGRATE_BATCH_SIZE = 10; const OLD_DB_NAME = "ComfySnapshotManager"; const OLD_STORE_NAME = "snapshots"; @@ -34,6 +38,7 @@ const lastCapturedHashMap = new Map(); const lastGraphDataMap = new Map(); // workflowKey -> previous graphData for change-type detection let restoreLock = null; let captureTimer = null; +let suppressAutoCaptureUntil = 0; // timestamp; auto-capture is ignored before it let sidebarRefresh = null; // callback set by sidebar render let viewingWorkflowKey = null; // null = follow active workflow; string = override let pickerDirty = true; // forces workflow picker to re-fetch on next expand @@ -381,6 +386,22 @@ function setLastGraphData(workflowKey, graphData) { } } +// Record the current graph as the dedup/change-detection baseline for a +// workflow without taking a snapshot. Used after a programmatic load (initial +// load, workflow switch) so the trailing graphChanged dedupes to a no-op. +function seedWorkflowBaseline(workflowKey) { + const graphData = getGraphData(); + if (!graphData) return; + lastCapturedHashMap.set(workflowKey, quickHash(JSON.stringify(graphData))); + setLastGraphData(workflowKey, graphData); +} + +// Ignore auto-captures for the next `ms` milliseconds (e.g. while a workflow +// switch settles). Manual/explicit captures are unaffected. +function suppressAutoCapture(ms) { + suppressAutoCaptureUntil = Date.now() + ms; +} + // SVG previews are immutable per snapshot, so the cache can persist across // refreshes. Drop entries for snapshots that no longer exist and cap the size, // instead of clearing the whole cache on every refresh. @@ -441,6 +462,36 @@ function getGraphData() { } } +// Widget values serialize as a positional array (`widgets_values`), so a diff +// can only say "Value[6] changed" unless we recover the widget *names*. The +// live graph holds them: app.graph._nodes[i].widgets[j].name aligns (by index) +// with widgets_values[j]. We map by node *id* only — an exact match — so the +// name is always either correct or absent. (A by-type fallback would let a node +// not on the canvas be labelled from a same-type node with a different widget +// layout, which is worse than showing the bare index.) When a node isn't live +// (e.g. diffing two old snapshots) the lookup misses and we fall back to +// "Value[i]". For the capture-time diff and "snapshot vs current" the target +// node is on the canvas, so names resolve correctly there. +function getLiveWidgetNames() { + const byId = new Map(); + try { + const nodes = app.graph?._nodes || []; + for (const n of nodes) { + if (!n || !Array.isArray(n.widgets) || n.id == null) continue; + byId.set(n.id, n.widgets.map((w) => (w && w.name != null ? String(w.name) : null))); + } + } catch {} + return byId; +} + +// Resolve a human widget name for a node's widgets_values[index], or null. +function widgetNameFor(widgetNames, node, index) { + if (!widgetNames || !node) return null; + const names = widgetNames.get(node.id); + const nm = names && names[index]; + return nm || null; +} + function generateId() { return `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`; } @@ -474,12 +525,19 @@ function detectChangeType(prevGraph, currGraph) { } // Node sets identical (same length, all curr IDs exist in prev) - // — check links, params, positions with early exits + // — check links, params, positions with early exits. + // Cosmetic flags (move/size/collapse) describe canvas-only changes the user + // doesn't care to version; meaningful flags (connection/param) do. let flags = 0; const FLAG_CONNECTION = 1; const FLAG_PARAM = 2; const FLAG_MOVE = 4; - const ALL_FLAGS = FLAG_CONNECTION | FLAG_PARAM | FLAG_MOVE; + const FLAG_SIZE = 8; + const FLAG_COLLAPSE = 16; + const FLAG_MODE = 32; + const MEANINGFUL = FLAG_CONNECTION | FLAG_PARAM | FLAG_MODE; + const COSMETIC = FLAG_MOVE | FLAG_SIZE | FLAG_COLLAPSE; + const ALL_FLAGS = MEANINGFUL | COSMETIC; // Compare links — check length first to avoid stringify when possible const prevLinks = prevGraph.links || []; @@ -529,22 +587,38 @@ function detectChangeType(prevGraph, currGraph) { if (cp?.[0] !== pp?.[0] || cp?.[1] !== pp?.[1]) flags |= FLAG_MOVE; } + // Compare size (manual node resize) + if (!(flags & FLAG_SIZE)) { + const cs = cn.size, ps = pn.size; + if (cs?.[0] !== ps?.[0] || cs?.[1] !== ps?.[1]) flags |= FLAG_SIZE; + } + + // Compare collapse/pin state (node.flags.{collapsed,pinned}) + if (!(flags & FLAG_COLLAPSE)) { + const cf = cn.flags || {}, pf = pn.flags || {}; + if (!!cf.collapsed !== !!pf.collapsed || !!cf.pinned !== !!pf.pinned) flags |= FLAG_COLLAPSE; + } + + // Compare mode (mute/bypass) — a functional change, not cosmetic + if (!(flags & FLAG_MODE)) { + if ((cn.mode || 0) !== (pn.mode || 0)) flags |= FLAG_MODE; + } + if (flags === ALL_FLAGS) break; } if (flags === 0) return "unknown"; - // Count set flags - const count = ((flags & FLAG_CONNECTION) ? 1 : 0) - + ((flags & FLAG_PARAM) ? 1 : 0) - + ((flags & FLAG_MOVE) ? 1 : 0); - if (count > 1) return "mixed"; + // Only canvas-cosmetic changes (move/resize/collapse) → "cosmetic". + if (!(flags & MEANINGFUL)) return "cosmetic"; + // A meaningful change is present; cosmetic flags don't escalate to "mixed". + const meaningfulCount = ((flags & FLAG_CONNECTION) ? 1 : 0) + + ((flags & FLAG_PARAM) ? 1 : 0) + + ((flags & FLAG_MODE) ? 1 : 0); + if (meaningfulCount > 1) return "mixed"; if (flags & FLAG_CONNECTION) return "connection"; - if (flags & FLAG_PARAM) return "param"; - if (flags & FLAG_MOVE) return "move"; - - return "unknown"; + return "param"; // param or mode-only → treated as a parameter change } // ─── Detailed Diff ────────────────────────────────────────────────── @@ -562,7 +636,7 @@ function buildNodeLookup(...graphs) { return map; } -function computeDetailedDiff(baseGraph, targetGraph) { +function computeDetailedDiff(baseGraph, targetGraph, widgetMaps = null) { const empty = { addedNodes: [], removedNodes: [], modifiedNodes: [], addedLinks: [], removedLinks: [], @@ -630,7 +704,7 @@ function computeDetailedDiff(baseGraph, targetGraph) { if (bv !== tv) { const bs = typeof bv === "object" ? JSON.stringify(bv) : String(bv ?? ""); const ts = typeof tv === "object" ? JSON.stringify(tv) : String(tv ?? ""); - if (bs !== ts) diffs.push({ index: i, from: bs, to: ts }); + if (bs !== ts) diffs.push({ index: i, name: widgetNameFor(widgetMaps, tn, i), from: bs, to: ts }); } } if (diffs.length > 0) changes.widgetValues = diffs; @@ -693,10 +767,11 @@ function computeDetailedDiff(baseGraph, targetGraph) { }; } -// Compact diff stored in snapshot metadata for hover display -function computeCaptureMetaDiff(prevGraph, currGraph) { +// Compact diff stored in snapshot metadata for hover display. widgetMaps comes +// from the live graph at capture time so changed parameters are named. +function computeCaptureMetaDiff(prevGraph, currGraph, widgetMaps = null) { if (!prevGraph || !currGraph) return null; - const diff = computeDetailedDiff(prevGraph, currGraph); + const diff = computeDetailedDiff(prevGraph, currGraph, widgetMaps); const result = {}; if (diff.addedNodes.length > 0) result.added = diff.addedNodes.map(n => n.title); @@ -708,6 +783,17 @@ function computeCaptureMetaDiff(prevGraph, currGraph) { ); if (paramChanged.length > 0) result.params = paramChanged.map(n => { + // Prefer naming the changed widgets/props; fall back to a count. + const names = []; + if (Array.isArray(n.changes.widgetValues)) { + for (const wv of n.changes.widgetValues) if (wv.name) names.push(wv.name); + } + if (Array.isArray(n.changes.properties)) { + for (const pv of n.changes.properties) if (pv.key) names.push(pv.key); + } + if (n.changes.title) names.push("title"); + if (n.changes.mode) names.push("mode"); + if (names.length > 0) return `${n.title} (${names.join(", ")})`; const wvCount = Array.isArray(n.changes.widgetValues) ? n.changes.widgetValues.length : (n.changes.widgetValues ? 1 : 0); const count = wvCount + (n.changes.properties?.length ?? 0); return count > 0 ? `${n.title} (${count} value${count > 1 ? "s" : ""})` : n.title; @@ -1334,40 +1420,13 @@ function showDiffModal(baseLabel, targetLabel, diff, allNodes, baseGraphData, ta wrap.appendChild(header); const { changes } = n; - if (changes.position) { - const d = document.createElement("div"); - d.className = "snap-diff-change-detail"; - const from = changes.position.from || [0, 0]; - const to = changes.position.to || [0, 0]; - d.appendChild(makeValueChange("Position", `[${Math.round(from[0])}, ${Math.round(from[1])}]`, `[${Math.round(to[0])}, ${Math.round(to[1])}]`)); - wrap.appendChild(d); - } - if (changes.size) { - const d = document.createElement("div"); - d.className = "snap-diff-change-detail"; - const from = changes.size.from || [0, 0]; - const to = changes.size.to || [0, 0]; - d.appendChild(makeValueChange("Size", `[${Math.round(from[0])}, ${Math.round(from[1])}]`, `[${Math.round(to[0])}, ${Math.round(to[1])}]`)); - wrap.appendChild(d); - } - if (changes.title) { - const d = document.createElement("div"); - d.className = "snap-diff-change-detail"; - d.appendChild(makeValueChange("Title", changes.title.from, changes.title.to)); - wrap.appendChild(d); - } - if (changes.mode) { - const d = document.createElement("div"); - d.className = "snap-diff-change-detail"; - d.appendChild(makeValueChange("Mode", String(changes.mode.from), String(changes.mode.to))); - wrap.appendChild(d); - } + // Meaningful changes first: named parameters, properties, title, mode. if (changes.widgetValues) { if (Array.isArray(changes.widgetValues)) { for (const wv of changes.widgetValues) { const d = document.createElement("div"); d.className = "snap-diff-change-detail"; - d.appendChild(makeValueChange(`Value[${wv.index}]`, wv.from, wv.to)); + d.appendChild(makeValueChange(wv.name || `Value[${wv.index}]`, wv.from, wv.to)); wrap.appendChild(d); } } else { @@ -1381,10 +1440,33 @@ function showDiffModal(baseLabel, targetLabel, diff, allNodes, baseGraphData, ta for (const pv of changes.properties) { const d = document.createElement("div"); d.className = "snap-diff-change-detail"; - d.appendChild(makeValueChange(`prop.${pv.key}`, pv.from, pv.to)); + d.appendChild(makeValueChange(pv.key, pv.from, pv.to)); wrap.appendChild(d); } } + if (changes.title) { + const d = document.createElement("div"); + d.className = "snap-diff-change-detail"; + d.appendChild(makeValueChange("Title", changes.title.from, changes.title.to)); + wrap.appendChild(d); + } + if (changes.mode) { + const d = document.createElement("div"); + d.className = "snap-diff-change-detail"; + d.appendChild(makeValueChange("Mode", String(changes.mode.from), String(changes.mode.to))); + wrap.appendChild(d); + } + // Cosmetic (position/size) collapsed into one muted line at the end. + const cosmeticBits = []; + if (changes.position) cosmeticBits.push("moved"); + if (changes.size) cosmeticBits.push("resized"); + if (cosmeticBits.length) { + const d = document.createElement("div"); + d.className = "snap-diff-change-detail"; + d.style.opacity = "0.5"; + d.textContent = `Layout: ${cosmeticBits.join(", ")}`; + wrap.appendChild(d); + } return wrap; }); if (modSec) body.appendChild(modSec); @@ -1564,14 +1646,18 @@ async function showPreviewModal(record) { let captureInProgress = false; -async function captureSnapshot(label = "Auto") { +// skipCosmetic is an AUTO-capture concern only: the debounced auto path passes +// true so node moves/resizes/collapses don't spawn snapshots. Manual saves and +// the pre-swap/pre-restore "Current" capture pass false so an explicit save — +// or preserving unsaved layout work before a load — is never silently dropped. +async function captureSnapshot(label = "Auto", { skipCosmetic = false } = {}) { if (restoreLock) return false; if (captureInProgress) return false; captureInProgress = true; - try { return await _captureCore({ label, dedupe: true, skipMove: true }); } finally { captureInProgress = false; } + try { return await _captureCore({ label, dedupe: true, skipCosmetic }); } finally { captureInProgress = false; } } -async function _captureCore({ label, source = null, thumbnail = null, dedupe = false, skipMove = false }) { +async function _captureCore({ label, source = null, thumbnail = null, dedupe = false, skipCosmetic = false }) { const graphData = getGraphData(); if (!graphData) return false; @@ -1586,7 +1672,10 @@ async function _captureCore({ label, source = null, thumbnail = null, dedupe = f const prevGraph = lastGraphDataMap.get(workflowKey); const changeType = detectChangeType(prevGraph, graphData); - if (skipMove && changeType === "move") return false; + // Auto-captures ignore canvas-cosmetic changes (move/resize/collapse); the + // cosmetic edit will ride along with the next meaningful snapshot. Manual + // and node-triggered captures (skipCosmetic=false) always save. + if (skipCosmetic && changeType === "cosmetic") return false; // Determine parentId for branching let parentId = null; @@ -1598,7 +1687,7 @@ async function _captureCore({ label, source = null, thumbnail = null, dedupe = f } } - const captureDiff = computeCaptureMetaDiff(prevGraph, graphData); + const captureDiff = computeCaptureMetaDiff(prevGraph, graphData, getLiveWidgetNames()); const record = { id: generateId(), workflowKey, @@ -1671,10 +1760,11 @@ async function captureNodeSnapshot(label = "Node Trigger", thumbnail = null) { function scheduleCaptureSnapshot() { if (!autoCaptureEnabled) return; if (restoreLock) return; + if (Date.now() < suppressAutoCaptureUntil) return; if (captureTimer) clearTimeout(captureTimer); captureTimer = setTimeout(() => { captureTimer = null; - captureSnapshot("Auto").catch((err) => { + captureSnapshot("Auto", { skipCosmetic: true }).catch((err) => { console.warn(`[${EXTENSION_NAME}] Auto-capture failed:`, err); }); }, debounceMs); @@ -1709,7 +1799,7 @@ async function restoreSnapshot(record) { }); } -async function swapSnapshot(record) { +async function swapSnapshot(record, { quiet = false } = {}) { // Warn when swapping in a snapshot from a different workflow const currentKey = getWorkflowKey(); if (record.workflowKey && record.workflowKey !== currentKey) { @@ -1746,7 +1836,7 @@ async function swapSnapshot(record) { lastCapturedHashMap.set(wfKey, quickHash(JSON.stringify(record.graphData))); setLastGraphData(wfKey, record.graphData); activeSnapshotId = record.id; - showToast("Snapshot swapped", "success"); + if (!quiet) showToast("Snapshot swapped", "success"); } catch (err) { console.warn(`[${EXTENSION_NAME}] Swap failed:`, err); showToast("Failed to swap snapshot", "error"); @@ -1754,6 +1844,40 @@ async function swapSnapshot(record) { }); } +// Non-destructive step to the previous (-1) or next (+1) snapshot in +// chronological order. Browsing between already-saved states is a storage +// no-op (captureSnapshot dedupes) and never deletes later snapshots, so +// back/forth is cheap and reversible — the Fusion-360 "roll the marker" feel. +let stepInProgress = false; +async function stepToSnapshot(direction) { + const wfKey = getWorkflowKey(); + // Don't hijack the keys while browsing another workflow's history. + if (viewingWorkflowKey != null && viewingWorkflowKey !== wfKey) return; + // Re-entrancy guard: holding Alt+Arrow must not launch overlapping swaps + // (which would race and spam the "please wait" toast from withRestoreLock). + if (stepInProgress || restoreLock) return; + stepInProgress = true; + try { + let recs; + try { recs = await db_getAllForWorkflow(wfKey); } catch { return; } + if (!recs || recs.length === 0) return; + recs.sort((a, b) => a.timestamp - b.timestamp); + const currentId = activeSnapshotId ?? currentSnapshotId ?? lastCapturedIdMap.get(wfKey); + let idx = recs.findIndex(r => r.id === currentId); + if (idx === -1) idx = recs.length - 1; // unknown position → treat as latest + const nextIdx = idx + direction; + if (nextIdx < 0 || nextIdx >= recs.length) { + showToast(direction < 0 ? "At earliest snapshot" : "At latest snapshot", "info"); + return; + } + const target = recs[nextIdx]; + await swapSnapshot(target, { quiet: true }); + showToast(`${nextIdx + 1}/${recs.length} · ${target.label}`, "info"); + } finally { + stepInProgress = false; + } +} + // ─── Sidebar UI ────────────────────────────────────────────────────── const CSS = ` @@ -2783,6 +2907,11 @@ const CHANGE_TYPE_ICONS = { color: "#64748b", label: "Nodes repositioned", }, + cosmetic: { + svg: '', + color: "#64748b", + label: "Layout only", + }, mixed: { svg: '', color: "#f97316", @@ -3767,7 +3896,7 @@ async function buildSidebar(el) { baseLabel = rec.label; targetLabel = "Current Workflow"; } - const diff = computeDetailedDiff(baseGraph, targetGraph); + const diff = computeDetailedDiff(baseGraph, targetGraph, getLiveWidgetNames()); const allNodes = buildNodeLookup(baseGraph, targetGraph); showDiffModal(baseLabel, targetLabel, diff, allNodes, baseGraph, targetGraph); })(); @@ -4267,6 +4396,12 @@ if (window.__COMFYUI_FRONTEND_VERSION__) { activeBranchSelections.clear(); // Seed active ring for the new workflow tab const newKey = getWorkflowKey(); + // Re-seed the dedup/change-detection baseline for the new + // tab and suppress auto-capture briefly, so the graphChanged + // fired by loading this workflow doesn't spawn a redundant + // "Auto" snapshot of a workflow the user only just opened. + seedWorkflowBaseline(newKey); + suppressAutoCapture(SWITCH_GUARD_MS); trackSessionWorkflow(newKey); db_getAllForWorkflow(newKey).then(recs => { if (recs.length > 0 && !lastCapturedIdMap.has(newKey)) { @@ -4301,6 +4436,17 @@ if (window.__COMFYUI_FRONTEND_VERSION__) { } }); + // Alt+Left / Alt+Right step non-destructively through snapshot history + // (Fusion-360 "roll the history marker" feel — jump back/forth freely). + document.addEventListener("keydown", (e) => { + if (!e.altKey || e.ctrlKey || e.metaKey || e.shiftKey) return; + if (e.key !== "ArrowLeft" && e.key !== "ArrowRight") return; + const t = e.target; + if (t && (t.tagName === "INPUT" || t.tagName === "TEXTAREA" || t.isContentEditable)) return; + e.preventDefault(); + stepToSnapshot(e.key === "ArrowLeft" ? -1 : 1).catch(() => {}); + }); + // Build the timeline bar on the canvas buildTimeline();