From b5b46d7fccf0111292e059d4d4bc47782d2fd70f Mon Sep 17 00:00:00 2001 From: malkuthro <166633396+malkuthro@users.noreply.github.com> Date: Tue, 2 Jun 2026 08:22:15 +0200 Subject: [PATCH] =?UTF-8?q?feat(sidebar):=20Snapshot=20Compare=20=E2=80=94?= =?UTF-8?q?=20read-only=20A/B=20side-by-side=20with=20diff=20tint=20(#181)?= =?UTF-8?q?=20(#183)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of #181: read-only side-by-side Compare. A/B toggle renders the existing sidebar twice — live working copy (A) + a loaded snapshot (B), B labeled with its own name + diff-tinted (green=only in comparison, red=same name/graph differs ignoring savedAt). Non-destructive: read-only capture guard on B, onChoose chooser never calls applySnapshot. Phase 2 (pull-in + A<->B swap) tracked in #197. --- tests/sidebar/test_compare_render_source.py | 141 +++++++++++ tests/sidebar/test_snapshot_diff.py | 98 ++++++++ web/koolook_sidebar.js | 4 +- web/sidebar/constants.js | 11 + web/sidebar/modals.js | 29 +++ web/sidebar/picks_store.js | 17 ++ web/sidebar/snapshot_diff.js | 70 ++++++ web/sidebar/tree.js | 261 ++++++++++++++++++-- web/sidebar/workflows_store.js | 40 ++- 9 files changed, 648 insertions(+), 23 deletions(-) create mode 100644 tests/sidebar/test_compare_render_source.py create mode 100644 tests/sidebar/test_snapshot_diff.py create mode 100644 web/sidebar/snapshot_diff.js diff --git a/tests/sidebar/test_compare_render_source.py b/tests/sidebar/test_compare_render_source.py new file mode 100644 index 0000000..37ba533 --- /dev/null +++ b/tests/sidebar/test_compare_render_source.py @@ -0,0 +1,141 @@ +from __future__ import annotations + +import subprocess +import textwrap +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def test_compare_load_path_returns_before_applysnapshot() -> None: + # Spec #181 hard constraint: "no applySnapshot() call on the compare path + # (static guard test)." The Load dialog doubles as the read-only compare + # chooser via `onChoose`; both load functions MUST short-circuit (return) in + # onChoose mode BEFORE reaching the destructive applySnapshot. Pin that + # ordering so a future refactor can't silently reintroduce a destructive + # load on the compare path. + src = (REPO_ROOT / "web" / "sidebar" / "modals.js").read_text(encoding="utf-8") + for fn in ("doNamedLoad", "doAutosaveRestore"): + fn_start = src.index(f"async function {fn}(") + choose_idx = src.index("if (onChoose)", fn_start) + # Match the actual call (with paren), not the word in a comment. + apply_idx = src.index("applySnapshot(", fn_start) + assert choose_idx < apply_idx, ( + f"{fn}: the onChoose compare-mode guard must precede applySnapshot" + ) + assert "return;" in src[choose_idx:apply_idx], ( + f"{fn}: the onChoose guard must return before reaching applySnapshot" + ) + + +def test_enter_compare_mode_uses_readonly_chooser() -> None: + # The compare ENTRY path opens the existing Load dialog in read-only choose + # mode (onChoose), never the destructive apply path. + src = (REPO_ROOT / "web" / "sidebar" / "tree.js").read_text(encoding="utf-8") + start = src.index("function enterCompareMode(") + body = src[start:start + 1400] + assert "showLoadSnapshotDialog(" in body + assert "onChoose:" in body, ( + "enterCompareMode must open the Load dialog in read-only choose mode" + ) + + +def _compare_guard_block(src: str) -> str: + # The read-only capture-guard block lives between the NAV_ALLOW declaration + # and the "Forward declarations" marker that follows the `if (compare)` block. + start = src.index("const NAV_ALLOW =") + return src[start:src.index("// Forward declarations", start)] + + +def test_compare_panel_guard_blocks_drag_drop_fully() -> None: + # #181 read-only guarantee: the comparison panel must neutralize cross-panel + # drag-and-drop, not just dragstart. A drag begun in the live panel and + # dropped on a comparison folder row would otherwise persist a live + # move/archive (handleDndDrop -> persistMutation on the live store). + guard = _compare_guard_block( + (REPO_ROOT / "web" / "sidebar" / "tree.js").read_text(encoding="utf-8") + ) + for evt in ("click", "contextmenu", "dragstart", "dragover", "drop"): + assert f'addEventListener("{evt}"' in guard, ( + f"compare read-only guard is missing a capture handler for '{evt}'" + ) + # Every guard listener must be capture-phase (third arg `true`). + assert guard.count(", true)") >= 5, ( + "all compare-guard listeners must be registered capture-phase" + ) + + +def test_compare_nav_allow_excludes_group_mode_toggle() -> None: + # The grouping mode toggle writes a shared global key (GROUP_MODE_KEY), so it + # must NOT be allow-listed in the read-only comparison panel. + src = (REPO_ROOT / "web" / "sidebar" / "tree.js").read_text(encoding="utf-8") + start = src.index("const NAV_ALLOW =") + nav = src[start:src.index(";", start)] + assert "koolook-mode-toggle" not in nav, ( + "grouping mode toggle writes a shared global key; drop it from NAV_ALLOW" + ) + + +def run_node_scenario(source: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["node", "--input-type=module"], + input=source, + cwd=REPO_ROOT, + text=True, + capture_output=True, + check=False, + ) + + +def test_picks_render_source_override_is_read_only() -> None: + # Compare mode feeds the comparison panel a snapshot's picks via a + # render-source override. It must SERVE the override to loadUserPicks but + # never persist it — clearing returns to live picks (empty under Node, + # which has no localStorage). Proves the non-destructive guarantee. + proc = run_node_scenario( + textwrap.dedent( + """ + import assert from "node:assert/strict"; + import { + setPicksRenderSource, + clearPicksRenderSource, + loadUserPicks, + } from "./web/sidebar/picks_store.js"; + + assert.deepEqual(loadUserPicks(), []); // live (no localStorage in Node) + setPicksRenderSource(["NodeA", "NodeB"]); + assert.deepEqual(loadUserPicks(), ["NodeA", "NodeB"]); // override served + clearPicksRenderSource(); + assert.deepEqual(loadUserPicks(), []); // live again, untouched + """ + ) + ) + assert proc.returncode == 0, proc.stderr + + +def test_workflows_render_source_override_does_not_mutate_live() -> None: + proc = run_node_scenario( + textwrap.dedent( + """ + import assert from "node:assert/strict"; + import { + setWorkflowsRenderSource, + clearWorkflowsRenderSource, + dirOf, + getAllWorkflowsForExport, + } from "./web/sidebar/workflows_store.js"; + + const liveBefore = JSON.stringify(getAllWorkflowsForExport()); + setWorkflowsRenderSource({ + directories: { Cmp: { workflows: { w: { graph: {} } }, directories: {} } }, + }); + assert.ok(dirOf(["Cmp"])); // reads resolve against the override + clearWorkflowsRenderSource(); + // The live export (workflowsCache) is never touched by the override. + assert.equal(JSON.stringify(getAllWorkflowsForExport()), liveBefore); + assert.equal(dirOf(["Cmp"]), undefined); // override gone -> not in live store + """ + ) + ) + assert proc.returncode == 0, proc.stderr diff --git a/tests/sidebar/test_snapshot_diff.py b/tests/sidebar/test_snapshot_diff.py new file mode 100644 index 0000000..fe5b2d4 --- /dev/null +++ b/tests/sidebar/test_snapshot_diff.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +import subprocess +import textwrap +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def run_node_scenario(source: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["node", "--input-type=module"], + input=source, + cwd=REPO_ROOT, + text=True, + capture_output=True, + check=False, + ) + + +def test_diff_picks_partitions_working_comparison_and_shared() -> None: + proc = run_node_scenario( + textwrap.dedent( + """ + import assert from "node:assert/strict"; + import { diffPicks } from "./web/sidebar/snapshot_diff.js"; + + const r = diffPicks(["x", "y"], ["y", "z"]); + assert.deepEqual(r.onlyWorking, ["x"]); + assert.deepEqual(r.onlyComparison, ["z"]); + assert.deepEqual(r.shared, ["y"]); + """ + ) + ) + assert proc.returncode == 0, proc.stderr + + +def test_diff_workflows_classifies_comparison_workflows_by_presence() -> None: + proc = run_node_scenario( + textwrap.dedent( + """ + import assert from "node:assert/strict"; + import { diffWorkflows } from "./web/sidebar/snapshot_diff.js"; + + const working = { + directories: { + Basics: { workflows: { txt2img: { graph: { a: 1 } } }, directories: {} }, + }, + }; + const comparison = { + directories: { + Basics: { + workflows: { + txt2img: { graph: { a: 1 } }, + img2img: { graph: { b: 2 } }, + }, + directories: {}, + }, + }, + }; + + const status = diffWorkflows(working, comparison); + // present only in the comparison -> green/new + assert.equal(status["Basics/img2img"], "new"); + // present in both, identical graph -> plain/same + assert.equal(status["Basics/txt2img"], "same"); + """ + ) + ) + assert proc.returncode == 0, proc.stderr + + +def test_diff_workflows_compares_graph_ignoring_saved_at_and_key_order() -> None: + proc = run_node_scenario( + textwrap.dedent( + """ + import assert from "node:assert/strict"; + import { diffWorkflows } from "./web/sidebar/snapshot_diff.js"; + + const working = { directories: { D: { workflows: { + changed: { graph: { nodes: [1, 2] }, savedAt: "2026-01-01T00:00:00Z" }, + unchanged: { graph: { a: 1, b: 2 }, savedAt: "2026-01-01T00:00:00Z" }, + }, directories: {} } } }; + const comparison = { directories: { D: { workflows: { + changed: { graph: { nodes: [1, 2, 3] }, savedAt: "2026-05-29T00:00:00Z" }, + unchanged: { graph: { b: 2, a: 1 }, savedAt: "2026-05-29T12:34:56Z" }, + }, directories: {} } } }; + + const status = diffWorkflows(working, comparison); + // graph content differs -> red/diff + assert.equal(status["D/changed"], "diff"); + // only savedAt + key order differ -> plain/same (savedAt is volatile) + assert.equal(status["D/unchanged"], "same"); + """ + ) + ) + assert proc.returncode == 0, proc.stderr diff --git a/web/koolook_sidebar.js b/web/koolook_sidebar.js index 9bd6348..5b3feed 100644 --- a/web/koolook_sidebar.js +++ b/web/koolook_sidebar.js @@ -35,7 +35,7 @@ import { } from "./sidebar/workflows_store.js"; import { loadUserPicks } from "./sidebar/picks_store.js"; import { showConfirmModal } from "./sidebar/modals.js"; -import { renderPanel } from "./sidebar/tree.js"; +import { renderSidebar } from "./sidebar/tree.js"; import { patchCanvasMenu } from "./sidebar/canvas_menu.js"; function wireOfflineFallbackRecovery(initialBlob) { @@ -192,7 +192,7 @@ app.registerExtension({ tooltip: TAB_TOOLTIP, icon: TAB_ICON, type: "custom", - render: (el) => renderPanel(el), + render: (el) => renderSidebar(el), }); patchCanvasMenu(); // Periodic defensive auto-save — fires every 5 minutes if state has diff --git a/web/sidebar/constants.js b/web/sidebar/constants.js index f0db200..5d66a45 100644 --- a/web/sidebar/constants.js +++ b/web/sidebar/constants.js @@ -127,6 +127,7 @@ export function ensureStyle() { .koolook-snap-status-autosaved { background: #6db4ff; } .koolook-snap-status-unsaved { background: #e0a64a; } .koolook-snap-status-none { background: rgba(255,255,255,0.28); } +.koolook-snap-status-comparing { background: #6db4ff; } .koolook-tree-divider { margin: 8px 8px; border-top: 1px solid rgba(255,255,255,0.08); flex-shrink: 0; } .koolook-search-wrap { position: relative; width: 100%; } .koolook-search-icon { position: absolute; left: 8px; top: 50%; transform: translateY(-50%); opacity: 0.55; font-size: 11px; pointer-events: none; } @@ -359,6 +360,16 @@ export function ensureStyle() { /* Inline delete state outlines the target row in red. Reminder: no backticks in comments inside this CSS template literal. */ .koolook-snapshot-row-pending-delete { outline: 1.5px solid rgba(220, 80, 80, 0.75); outline-offset: -1px; background: rgba(220, 80, 80, 0.06); } +/* Compare mode (issue 181): two live sidebars side by side, plus a bottom + status bar. The columns reuse the normal koolook-sidebar render verbatim. + Reminder: no backticks in comments inside this CSS template literal. */ +.koolook-compare-host { display: flex; flex-direction: column; height: 100%; min-height: 0; } +.koolook-compare-split { display: flex; flex: 1 1 auto; gap: 8px; min-height: 0; overflow: auto; padding: 0 4px; } +.koolook-compare-col { flex: 1 1 0; min-width: 0; } +.koolook-compare-status { flex: 0 0 auto; padding: 5px 10px; font-size: 11px; font-weight: 700; letter-spacing: 0.08em; text-transform: uppercase; text-align: center; color: rgba(170, 200, 255, 0.95); background: rgba(80, 140, 235, 0.18); border-top: 1px solid var(--border-color, rgba(255, 255, 255, 0.1)); } +/* Text-only diff tint, applied only to the comparison column's rows. */ +.koolook-cmp-new .koolook-name { color: #7be08a; } +.koolook-cmp-diff .koolook-name { color: #ff7d7d; } `; document.head.appendChild(s); } diff --git a/web/sidebar/modals.js b/web/sidebar/modals.js index 4af970f..f014d16 100644 --- a/web/sidebar/modals.js +++ b/web/sidebar/modals.js @@ -1449,6 +1449,10 @@ export function showLoadSnapshotDialog({ listAutosaves, revealPresetFolder, onToast, + // Compare mode (#181): when provided, selecting a preset (or an autosave) + // calls onChoose(snapshot, meta) and closes — instead of the destructive + // applySnapshot load. Default undefined keeps the normal Load flow intact. + onChoose, }) { const toast = onToast || (() => {}); const body = document.createElement("div"); @@ -1606,6 +1610,19 @@ export function showLoadSnapshotDialog({ // case, and no YES/NO choice modal at all). ``close()`` here refers // to the OUTER Load Snapshot dialog's overlay. async function doNamedLoad(preview) { + // Compare mode: read-only — hand the parsed snapshot back and close. + // No pre-load auto-save, no applySnapshot, no tracker change. + if (onChoose) { + try { + const snap = await readPreset(preview.fileName); + close(); + onChoose(snap, { fileName: preview.fileName, displayName: preview.displayName }); + } catch (e) { + console.error("[Koolook] preset read (compare) failed:", e); + toast(`Could not read "${preview.displayName}": ${e.message}`); + } + return; + } try { // Defensive auto-save BEFORE any destructive write — see // `writePreLoadAutosave` rationale in snapshot.js. If the @@ -1667,6 +1684,18 @@ export function showLoadSnapshotDialog({ async function doAutosaveRestore(item) { const tooltipName = `${item.dir}/${item.fileName}`; + // Compare mode: read-only — hand the parsed snapshot back and close. + if (onChoose) { + try { + const snap = await readPreset(item.fileName, { dir: item.dir }); + close(); + onChoose(snap, { fileName: item.fileName, displayName: tooltipName }); + } catch (e) { + console.error("[Koolook] autosave read (compare) failed:", e); + toast(`Could not read "${tooltipName}": ${e.message}`); + } + return; + } try { let backupName = null; if (typeof writePreLoadAutosave === "function") { diff --git a/web/sidebar/picks_store.js b/web/sidebar/picks_store.js index 0419f83..80b9bd7 100644 --- a/web/sidebar/picks_store.js +++ b/web/sidebar/picks_store.js @@ -10,7 +10,24 @@ import { AUTOPULL_HIDDEN_KEY, } from "./constants.js"; +// Read-only render-source override (issue #181, Compare mode). When set, the +// render path (`loadUserPicks`) returns this list instead of the user's live +// localStorage picks, so the Compare view can render a *second* sidebar from a +// loaded snapshot without touching live state. Set only synchronously around a +// read-only render and always cleared afterwards (see `withSnapshotSource` in +// tree.js); never written back to localStorage. +let renderSourcePicks = null; + +export function setPicksRenderSource(picks) { + renderSourcePicks = Array.isArray(picks) ? picks.filter(p => typeof p === "string") : []; +} + +export function clearPicksRenderSource() { + renderSourcePicks = null; +} + export function loadUserPicks() { + if (renderSourcePicks) return [...renderSourcePicks]; try { const raw = localStorage.getItem(STORAGE_KEY); if (!raw) return []; diff --git a/web/sidebar/snapshot_diff.js b/web/sidebar/snapshot_diff.js new file mode 100644 index 0000000..5320c15 --- /dev/null +++ b/web/sidebar/snapshot_diff.js @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: 2025-2026 Kforge Labs +// SPDX-License-Identifier: GPL-3.0-only + +import { sortJsonValue } from "./workflows_store.js"; + +// Pure snapshot-comparison helpers for the Compare-mode sidebar (issue #181). +// No DOM, no globals, no I/O — every function takes plain data and returns +// plain data so it can be unit-tested by piping a scenario through Node. The +// Compare view renders the existing sidebar a second time and uses these +// results purely to tint comparison rows (green = only in comparison, red = +// same key, contents differ); it never mutates the working state. + +// Partition two pick lists (arrays of string identifiers) against each other. +// Order follows the `working` list for `onlyWorking`/`shared` and the +// `comparison` list for `onlyComparison`. Membership is by set, so duplicates +// collapse to a single classification. +export function diffPicks(working, comparison) { + const workingSet = new Set(working); + const comparisonSet = new Set(comparison); + return { + onlyWorking: working.filter((id) => !comparisonSet.has(id)), + onlyComparison: comparison.filter((id) => !workingSet.has(id)), + shared: working.filter((id) => comparisonSet.has(id)), + }; +} + +// Flatten a workflows store ({ directories: { [name]: DirNode } }, the shape +// returned by getAllWorkflowsForExport / stored under snapshot.workflows) into +// a Map of full path ("Basics/txt2img", "Basics/Sub/wf") -> WorkflowEntry. +function collectWorkflows(node, prefix, out) { + const dirs = node && typeof node.directories === "object" && node.directories ? node.directories : {}; + for (const dirName of Object.keys(dirs)) { + const child = dirs[dirName]; + if (!child || typeof child !== "object") continue; + const childPrefix = `${prefix}${dirName}/`; + const wfs = child.workflows && typeof child.workflows === "object" ? child.workflows : {}; + for (const wfName of Object.keys(wfs)) { + out.set(`${childPrefix}${wfName}`, wfs[wfName]); + } + collectWorkflows(child, childPrefix, out); + } + return out; +} + +// Two workflow graphs are equal when their canonical (key-order-normalized) +// JSON matches. Comparing the `graph` subtree alone ignores the sibling +// `savedAt` stamp, which is rewritten on every save and would otherwise make +// every re-saved workflow read as changed. +function graphsEqual(a, b) { + return JSON.stringify(sortJsonValue(a)) === JSON.stringify(sortJsonValue(b)); +} + +// Classify every workflow in the comparison store against the working store, +// keyed by full path: +// "new" -> present only in the comparison (green tint) +// "diff" -> present in both, graph differs ignoring savedAt (red tint) +// "same" -> present in both, identical graph (plain) +export function diffWorkflows(workingStore, comparisonStore) { + const working = collectWorkflows(workingStore, "", new Map()); + const comparison = collectWorkflows(comparisonStore, "", new Map()); + const status = {}; + for (const [path, entry] of comparison) { + if (!working.has(path)) { + status[path] = "new"; + } else { + status[path] = graphsEqual(entry.graph, working.get(path).graph) ? "same" : "diff"; + } + } + return status; +} diff --git a/web/sidebar/tree.js b/web/sidebar/tree.js index dd0465f..2719a89 100644 --- a/web/sidebar/tree.js +++ b/web/sidebar/tree.js @@ -36,6 +36,8 @@ import { addToMyPicks, loadAutoPullHidden, hideAutoPullType, + setPicksRenderSource, + clearPicksRenderSource, } from "./picks_store.js"; import { persistMutation, @@ -58,7 +60,11 @@ import { isWorkflowModule, addTag, removeTag, + getAllWorkflowsForExport, + setWorkflowsRenderSource, + clearWorkflowsRenderSource, } from "./workflows_store.js"; +import { diffPicks, diffWorkflows } from "./snapshot_diff.js"; import { serializeFullCanvas, serializeSelection, @@ -124,6 +130,7 @@ const SVG_NS = "http://www.w3.org/2000/svg"; const TOOLBAR_ICONS = { loadSnapshot: { kind: "letter", text: "L" }, saveSnapshot: { kind: "letter", text: "S" }, + compareSnapshot: { kind: "letter", text: "A/B" }, help: { kind: "letter", text: "H" }, exportStarter: { kind: "letter", text: "E" }, installMissing: { kind: "letter", text: "I" }, @@ -1314,6 +1321,8 @@ function makeNodeLeafRow({ display, type, removable, onClick, unresolved, breadc const row = document.createElement("div"); row.className = "koolook-row koolook-leaf"; if (unresolved) row.classList.add("koolook-leaf-unresolved"); + // Stable identity for Compare-mode tinting (#181). Invisible otherwise. + if (type) row.dataset.koolookNodeType = type; // Breadcrumb-equipped rows under search-flatten mode also surface the // origin path in the row tooltip (`type` is still appended) so a hover // confirms exactly which folder the leaf would live under in the tree. @@ -1383,9 +1392,11 @@ function makeNodeLeafRow({ display, type, removable, onClick, unresolved, breadc return row; } -function makeWorkflowLeafRow({ name, dirName, onClick, onContextMenu, draggablePayload, isModule = false }) { +function makeWorkflowLeafRow({ name, dirName, wfPath, onClick, onContextMenu, draggablePayload, isModule = false }) { const row = document.createElement("div"); row.className = "koolook-row koolook-leaf"; + // Stable identity for Compare-mode tinting (#181). Invisible otherwise. + if (wfPath) row.dataset.koolookWfPath = wfPath; // Module-tagged entries get a distinct hover tooltip so the green icon // isn't the only signal that a left-click does something different. The // `pi pi-plus-circle` class swap is what actually makes them look like @@ -2149,6 +2160,7 @@ addSection({ row: makeWorkflowLeafRow({ name: entry.wfName, dirName: entry.path.join(" / "), + wfPath: [...entry.path, entry.wfName].join("/"), onClick: () => moduleEntry ? insertWorkflowOntoCanvas(entry.path, entry.wfName) : loadWorkflowOntoCanvas(entry.path, entry.wfName), @@ -2200,6 +2212,7 @@ function renderGatheredDir(parentCtx, dir) { row: makeWorkflowLeafRow({ name: wfName, dirName: dirDisplay, + wfPath: [...dirPath, wfName].join("/"), onClick: () => loadWorkflowOntoCanvas(dirPath, wfName), onContextMenu: (e) => workflowRowContextMenu(e, dirPath, wfName, true), draggablePayload: { type: "workflow", path: dirPath, name: wfName }, @@ -2215,6 +2228,7 @@ function renderGatheredDir(parentCtx, dir) { row: makeWorkflowLeafRow({ name: wfName, dirName: dirDisplay, + wfPath: [...dirPath, wfName].join("/"), // Module convention: left-click inserts; non-module // entries left-click loads (replaces canvas, with // confirm modal as before). Both actions are always @@ -2235,10 +2249,190 @@ function renderGatheredDir(parentCtx, dir) { // ============================================================================= // Panel rendering // ============================================================================= -export function renderPanel(container) { +// ============================================================================= +// Compare mode (#181) — host + helpers. +// +// `renderSidebar` is the panel entry point bound by koolook_sidebar.js. With +// compare off it renders the single live `renderPanel`, exactly as before. +// With compare on it lays out two columns — the live working copy and a +// read-only render of a chosen snapshot — plus a bottom "Compare mode" status, +// then tints the comparison column from the snapshot diff. The existing +// `renderPanel` + CSS are reused verbatim; only the data source differs. +// ============================================================================= + +let compareSnapshot = null; // null = compare off; else the chosen snapshot object +let compareMeta = null; // { fileName, displayName } of the chosen snapshot +let sidebarContainer = null; // remembered so a toggle can re-render in place +let liveListeners = null; // AbortController for the live panel's window listeners + +// Run `fn` synchronously with the render path pointed at `snapshot`'s data +// instead of live state. Read-only: always clears the overrides afterwards +// (even if `fn` throws) so later live renders see live data. Must wrap only +// synchronous render work — never an await. +function withSnapshotSource(snapshot, fn) { + setPicksRenderSource(snapshot && Array.isArray(snapshot.picks) ? snapshot.picks : []); + setWorkflowsRenderSource(snapshot && snapshot.workflows ? snapshot.workflows : { directories: {} }); + try { + return fn(); + } finally { + clearPicksRenderSource(); + clearWorkflowsRenderSource(); + } +} + +export function renderSidebar(container) { + sidebarContainer = container; + // Drop the previous mount's live-panel window listeners before re-rendering + // (toggling Compare re-runs renderPanel for the live panel). + if (liveListeners) liveListeners.abort(); + liveListeners = new AbortController(); + const signal = liveListeners.signal; + + if (!compareSnapshot) { + container.classList.remove("koolook-compare-host"); + renderPanel(container, { onToggleCompare: enterCompareMode, signal }); + return; + } + + container.innerHTML = ""; + container.classList.remove("koolook-sidebar"); + container.classList.add("koolook-compare-host"); + + const split = document.createElement("div"); + split.className = "koolook-compare-split"; + const leftCol = document.createElement("div"); + leftCol.className = "koolook-compare-col"; + const rightCol = document.createElement("div"); + rightCol.className = "koolook-compare-col"; + split.appendChild(leftCol); + split.appendChild(rightCol); + container.appendChild(split); + + // Left = live working copy (untouched). Right = the same render, fed the + // chosen snapshot read-only, then tinted. + renderPanel(leftCol, { onToggleCompare: exitCompareMode, signal }); + renderPanel(rightCol, { + compare: true, + snapshot: compareSnapshot, + onToggleCompare: exitCompareMode, + signal, + compareName: compareMeta && compareMeta.displayName ? compareMeta.displayName : "", + }); + applyCompareTint(rightCol, compareSnapshot); + + const status = document.createElement("div"); + status.className = "koolook-compare-status"; + status.textContent = compareMeta && compareMeta.displayName + ? `Compare mode · ${compareMeta.displayName}` + : "Compare mode"; + container.appendChild(status); +} + +function rerenderSidebar() { + if (sidebarContainer) renderSidebar(sidebarContainer); +} + +// Enter Compare mode: reuse the existing Load dialog in read-only "choose" +// mode (`onChoose`) so picking a preset hands back its parsed snapshot WITHOUT +// applying it. Never calls applySnapshot — the non-destructive guarantee. +function enterCompareMode() { + showLoadSnapshotDialog({ + listPresets, + readPreset, + deletePreset, + applySnapshot, + setCurrentPresetName, + getCurrentPresetName, + getLibraryInfo, + saveSettings, + browseDirectories, + createBrowseDirectory, + writePreLoadAutosave, + markStateSaved, + markStateAutosaved, + listAutosaves, + revealPresetFolder, + onToast: toast, + onChoose: (snap, meta) => { + compareSnapshot = snap; + compareMeta = meta || null; + rerenderSidebar(); + }, + }); +} + +function exitCompareMode() { + compareSnapshot = null; + compareMeta = null; + rerenderSidebar(); +} + +// Tint the comparison column from the snapshot diff (text-only, via CSS +// classes on the existing rows). Nodes: a pick only in the comparison → +// green/new. Workflows: per full path → green/new or red/diff; identical +// (ignoring savedAt) stays plain. Runs after the column rendered with the +// store overrides already cleared, so the working-side reads are live. +function applyCompareTint(panelEl, snapshot) { + if (!panelEl || !snapshot) return; + + const pickDiff = diffPicks(loadUserPicks(), Array.isArray(snapshot.picks) ? snapshot.picks : []); + const newPicks = new Set(pickDiff.onlyComparison); + for (const row of panelEl.querySelectorAll("[data-koolook-node-type]")) { + if (newPicks.has(row.dataset.koolookNodeType)) row.classList.add("koolook-cmp-new"); + } + + const snapWorkflows = snapshot.workflows && typeof snapshot.workflows === "object" + ? snapshot.workflows + : { directories: {} }; + const wfStatus = diffWorkflows(getAllWorkflowsForExport(), snapWorkflows); + for (const row of panelEl.querySelectorAll("[data-koolook-wf-path]")) { + const status = wfStatus[row.dataset.koolookWfPath]; + if (status === "new") row.classList.add("koolook-cmp-new"); + else if (status === "diff") row.classList.add("koolook-cmp-diff"); + } +} + +export function renderPanel(container, options = {}) { + const { compare = false, snapshot = null, onToggleCompare = null, signal = null, compareName = "" } = options; + // Compare mode (#181): when `compare` is set, every tree (re-)render in + // this panel reads from the loaded `snapshot` via the read-only store + // overrides, and the live-change listeners at the bottom are skipped so + // the snapshot panel stays static. compare=false is byte-identical to the + // pre-#181 single live panel. + const withSource = compare + ? (fn) => withSnapshotSource(snapshot, fn) + : (fn) => fn(); + // `signal` (from renderSidebar's AbortController) lets a re-render drop the + // previous mount's window listeners, so toggling Compare never leaks a set. + const listenerOpts = signal ? { signal } : undefined; ensureStyle(); container.innerHTML = ""; container.classList.add("koolook-sidebar"); + if (compare) { + container.classList.add("koolook-compare-panel"); + // Read-only comparison panel (#181): the same interactive renderPanel is + // reused (chrome stays visually identical), but the snapshot side must + // never mutate or load live state. Capture-phase guards allow navigation + // only — folder expand/collapse, search, and the A/B exit button — and + // neutralize every mutating/loading affordance: leaf load/insert clicks, + // pick ×/+, the snapshot + tools buttons, the grouping mode toggle + // (which writes the shared global GROUP_MODE_KEY), context menus, and + // drag — dragstart on the source PLUS dragover/drop so a drag begun in + // the live panel can't land on (and mutate via) the snapshot column. + // Buttons stay visible but inert. + const NAV_ALLOW = + ".koolook-search, [data-koolook-compare-exit], .koolook-row:not(.koolook-leaf)"; + container.addEventListener("click", (e) => { + const t = e.target; + if (t instanceof Element && t.closest(NAV_ALLOW)) return; + e.preventDefault(); + e.stopPropagation(); + }, true); + container.addEventListener("contextmenu", (e) => { e.preventDefault(); e.stopPropagation(); }, true); + container.addEventListener("dragstart", (e) => { e.preventDefault(); e.stopPropagation(); }, true); + container.addEventListener("dragover", (e) => { e.preventDefault(); e.stopPropagation(); }, true); + container.addEventListener("drop", (e) => { e.preventDefault(); e.stopPropagation(); }, true); + } // Forward declarations — `tree` and `search` are created several rows // below, but the Nodes-row mode toggle (built mid-renderPanel) needs to @@ -2249,7 +2443,7 @@ export function renderPanel(container) { let tree; let search; const rerenderTree = () => { - if (tree) renderTree({ treeEl: tree, query: search ? search.value : "" }); + if (tree) withSource(() => renderTree({ treeEl: tree, query: search ? search.value : "" })); }; // Three small wrappers over the snapshot dialogs. Each opens directly @@ -2379,10 +2573,21 @@ export function renderPanel(container) { snapshotStatus.title = snapshotTooltip(status); }); } - refreshSnapshotStatus(); - window.addEventListener(PICKS_CHANGED_EVENT, refreshSnapshotStatus); - window.addEventListener(WORKFLOWS_CHANGED_EVENT, refreshSnapshotStatus); - window.addEventListener(SNAPSHOT_STATUS_CHANGED_EVENT, refreshSnapshotStatus); + if (compare) { + // The comparison panel identifies the snapshot it is showing — its own + // name + a static "comparing" state — rather than echoing the live + // working-copy status. It never refreshes off live status events. + snapshotDot.className = "koolook-snap-status-dot koolook-snap-status-comparing"; + snapshotName.classList.remove("koolook-snap-status-name-empty"); + snapshotName.textContent = compareName || "(snapshot)"; + snapshotState.textContent = "· comparing"; + snapshotStatus.title = compareName ? `Comparing: ${compareName}` : "Comparing"; + } else { + refreshSnapshotStatus(); + window.addEventListener(PICKS_CHANGED_EVENT, refreshSnapshotStatus, listenerOpts); + window.addEventListener(WORKFLOWS_CHANGED_EVENT, refreshSnapshotStatus, listenerOpts); + window.addEventListener(SNAPSHOT_STATUS_CHANGED_EVENT, refreshSnapshotStatus, listenerOpts); + } snapshotRow.appendChild(makeToolbarButton({ icon: TOOLBAR_ICONS.loadSnapshot, @@ -2418,13 +2623,29 @@ export function renderPanel(container) { quickSaveBtn.disabled = !getCurrentPresetName(); } refreshQuickSaveDisabled(); - window.addEventListener(SNAPSHOT_STATUS_CHANGED_EVENT, refreshQuickSaveDisabled); + window.addEventListener(SNAPSHOT_STATUS_CHANGED_EVENT, refreshQuickSaveDisabled, listenerOpts); snapshotRow.appendChild(quickSaveBtn); snapshotRow.appendChild(makeToolbarButton({ icon: TOOLBAR_ICONS.saveSnapshot, title: "Save current state — overwrites the last-loaded preset, or prompts for a name", onClick: openSaveDialog, })); + // A/B — Compare mode toggle (#181). Same letter-button style as L/S; the + // host (`renderSidebar`) supplies the toggle handler (enter on the live + // panel, exit on the comparison panels). + if (onToggleCompare) { + const compareBtn = makeToolbarButton({ + icon: TOOLBAR_ICONS.compareSnapshot, + title: compare + ? "Exit Compare mode" + : "Compare mode — open another snapshot side by side", + onClick: onToggleCompare, + }); + // Exempt from the read-only comparison-panel click guard so the user + // can always toggle back out of Compare mode. + compareBtn.dataset.koolookCompareExit = "1"; + snapshotRow.appendChild(compareBtn); + } // Settings cog removed in #137: library-path change now lives inside the // Save dialog ("Save to...") and the Load dialog ("Load from..."), both // backed by the same folder picker. The cog is gone; the snapshot row @@ -2793,7 +3014,7 @@ export function renderPanel(container) { tree.className = "koolook-tree"; container.appendChild(tree); - renderTree({ treeEl: tree, query: "" }); + withSource(() => renderTree({ treeEl: tree, query: "" })); // ---- Build tag (dev-sync verification) ---- // `scripts/sync_to_dev.py` writes `web/_dev_build.json` post-sync so the @@ -2840,15 +3061,21 @@ export function renderPanel(container) { search.addEventListener("input", (e) => { const q = e.target.value; if (debounce) clearTimeout(debounce); - debounce = setTimeout(() => renderTree({ treeEl: tree, query: q }), 60); + debounce = setTimeout(() => withSource(() => renderTree({ treeEl: tree, query: q })), 60); }); // ---- Event subscriptions for live re-rendering ---- - window.addEventListener(PICKS_CHANGED_EVENT, () => renderTree({ treeEl: tree, query: search.value })); - window.addEventListener(WORKFLOWS_CHANGED_EVENT, () => renderTree({ treeEl: tree, query: search.value })); - window.addEventListener("storage", (e) => { - if (e.key === STORAGE_KEY || e.key === WORKFLOWS_FALLBACK_KEY || e.key === GROUP_MODE_KEY) { - renderTree({ treeEl: tree, query: search.value }); - } - }); + // Skipped in compare mode: a comparison panel shows a static snapshot, so + // it must not re-render off live picks/workflows/storage changes (which + // would read live state instead of the snapshot) or leak window listeners + // across compare toggles. + if (!compare) { + window.addEventListener(PICKS_CHANGED_EVENT, () => renderTree({ treeEl: tree, query: search.value }), listenerOpts); + window.addEventListener(WORKFLOWS_CHANGED_EVENT, () => renderTree({ treeEl: tree, query: search.value }), listenerOpts); + window.addEventListener("storage", (e) => { + if (e.key === STORAGE_KEY || e.key === WORKFLOWS_FALLBACK_KEY || e.key === GROUP_MODE_KEY) { + renderTree({ treeEl: tree, query: search.value }); + } + }, listenerOpts); + } } diff --git a/web/sidebar/workflows_store.js b/web/sidebar/workflows_store.js index 472084d..8f2bf6a 100644 --- a/web/sidebar/workflows_store.js +++ b/web/sidebar/workflows_store.js @@ -113,7 +113,12 @@ function cloneJson(value) { return JSON.parse(JSON.stringify(value)); } -function sortJsonValue(value) { +// Exported for reuse by snapshot_diff.js (issue #181) — the Compare view needs +// the same key-order canonicalization to decide whether two workflow graphs +// differ. Kept defined here (next to its `compareNames` dependency and +// `normalizedStoresEqual`) rather than moved, so the store's own callers are +// untouched. +export function sortJsonValue(value) { if (Array.isArray(value)) return value.map(sortJsonValue); if (!value || typeof value !== "object") return value; const out = {}; @@ -538,16 +543,43 @@ export async function seedWorkflowDefaultsIfNeeded() { const ARCHIVE_RESERVED_NAME = "archive"; +// Read-only render-source override (issue #181, Compare mode). When set, +// everything that resolves through `dirOf` — `listDirectoryNames`, +// `getWorkflowGraph`, `getWorkflowTags`, `isWorkflowModule`, and the tree +// gather in tree.js — reads this store instead of the live `workflowsCache`, +// letting the Compare view render a second sidebar from a loaded snapshot +// without mutating live state. Mutators also resolve paths through `dirOf`, so +// the override is scoped to one synchronous render pass and always cleared +// afterwards (see `withSnapshotSource` in tree.js); it must never straddle an +// await or a user interaction. +let renderSourceOverride = null; + +function activeStore() { + return renderSourceOverride || workflowsCache; +} + +export function setWorkflowsRenderSource(store) { + renderSourceOverride = + store && typeof store === "object" && store.directories && typeof store.directories === "object" + ? store + : { directories: {} }; +} + +export function clearWorkflowsRenderSource() { + renderSourceOverride = null; +} + // Resolves to the DirNode at `path`, or `undefined` if any segment is missing. // `[]` returns a synthetic wrapper around the root (with `directories` -// pointing at `workflowsCache.directories`) so callers don't need a special +// pointing at the active store's directories) so callers don't need a special // case for root vs. nested. export function dirOf(path) { if (!Array.isArray(path)) return undefined; + const store = activeStore(); if (path.length === 0) { - return { workflows: {}, directories: workflowsCache.directories || {} }; + return { workflows: {}, directories: store.directories || {} }; } - let node = { directories: workflowsCache.directories || {} }; + let node = { directories: store.directories || {} }; for (const seg of path) { if (typeof seg !== "string" || !seg) return undefined; if (!node.directories || !node.directories[seg]) return undefined;