From 259254e4fb8cc37e794ebc84b71a4f1e906b0ab1 Mon Sep 17 00:00:00 2001 From: Michael B Reiser Date: Wed, 27 May 2026 19:57:22 -0400 Subject: [PATCH 1/3] =?UTF-8?q?fix(v3):=20Phase=204=20cleanup=20=E2=80=94?= =?UTF-8?q?=20listener=20accumulation,=20selection=20drift,=20ruler=20tick?= =?UTF-8?q?s,=20convert=20=5FunknownKeys=20(v0.9)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six Codex-flagged bugs in landed Phase 4 work: - A1: #sequenceList drag/drop listeners moved to one-time startup wiring. Previously re-attached on every renderSequence(); a single drop after N renders fired N handlers, inserting N duplicate refs and pushing N undo entries. Surfaced within a normal editing session. - A2: onMoveSequenceEntry and onInsertSequenceRefFromLib now walk selection.index across the full displacement, not just the moved entry. Library drag-drop no longer steals selection focus — it shifts the existing selection through the insert (matches the +Add Ref button's intent of selecting the new entry only for explicit button clicks). - A3: Timeline ruler tick positions are computed piecewise by walking the layout array (rulerXForRealTime), so ticks stay aligned with clamped step widths. Flat pxPerSec drifted as soon as any short step clamped to the 60px / 40px minimum. - A4: isConvertibleBlock rejects entries with non-empty _unknownKeys. Block→ref convert was silently dropping forward-compat fields like retry_on_fail, undermining the Tier 1 unknown-passthrough guarantee. The user-facing error message now enumerates every blocker including forward-compat keys. - A5: _buildSequenceEntry mirrors the parser's positive-integer repetitions validation. Closed a doc/JS-mirror divergence path that D4/paste-import would hit (entry.repetitions = 0 → YAML '0' but mirror = 1 via `|| 1` default). - A6: onMoveCommand defensively early-returns on no-op / out-of-bounds, matching the onMoveSequenceEntry pattern. Avoids pushUndo pollution. Tests: 375/375 (was 369). Added 6 new builder-side validation cases to Suite 10b for A5. Verified in browser: - A1: 30 forced re-renders + 1 simulated library drop adds exactly 1 entry. - A4: Right-click on future_keys block shows full error listing all blockers including `forward-compat keys: retry_on_fail, abort_if`. - A3: Multi_block fixture (mixed durations exercising 60px clamping) shows "1m" tick at x=2447 of a 3516-px ruler — piecewise positioning. Footer bumped to v0.9 | 2026-05-27 19:54 ET. Co-Authored-By: Claude Opus 4.7 (1M context) --- experiment_designer_v3.html | 146 ++++++++++++++++++++-------- js/protocol-yaml-v3.js | 15 ++- tests/test-protocol-roundtrip-v3.js | 32 ++++++ 3 files changed, 151 insertions(+), 42 deletions(-) diff --git a/experiment_designer_v3.html b/experiment_designer_v3.html index 755d672..3b219eb 100644 --- a/experiment_designer_v3.html +++ b/experiment_designer_v3.html @@ -978,7 +978,7 @@

v3 Experiment Designer

@@ -1473,15 +1473,55 @@

Import error

$('addRefBtn').addEventListener('click', onAddSeqRef); $('addBlockBtn').addEventListener('click', onAddSeqBlock); + // List-level drop target for the empty-list / append-past-end case. + // Wired ONCE at startup (not per renderSequence) — the #sequenceList + // element persists across renders, so re-attaching here would stack N + // handlers and cause N duplicate inserts per drop. + (function wireSequenceListDrop() { + const list = $('sequenceList'); + list.addEventListener('dragover', (e) => { + if (!e.dataTransfer.types.includes('text/x-library-cond')) return; + // Only highlight when we're not over an existing entry (entries handle their own) + if (e.target.closest('.seq-entry')) return; + e.preventDefault(); + e.dataTransfer.dropEffect = 'copy'; + list.classList.add('lib-drop-target'); + }); + list.addEventListener('dragleave', (e) => { + if (e.target === list) list.classList.remove('lib-drop-target'); + }); + list.addEventListener('drop', (e) => { + if (e.target.closest('.seq-entry')) return; // handled by entry's own drop + const libCond = e.dataTransfer.getData('text/x-library-cond'); + if (!libCond) return; + e.preventDefault(); + list.classList.remove('lib-drop-target'); + if (!experiment) return; + onInsertSequenceRefFromLib(libCond, experiment.sequence.length); + }); + })(); + function onMoveSequenceEntry(fromIdx, toIdx) { if (fromIdx === toIdx) return; pushUndo(); try { docMoveSequenceEntry(experiment, fromIdx, toIdx); setDirty(true); - // Update selection if it was on the moved entry - if (selection && (selection.kind === 'ref' || selection.kind === 'block') && selection.index === fromIdx) { - selection = { ...selection, index: toIdx }; + // Walk selection.index across the displacement. Three cases: + // 1. The moved entry was selected → follow it to toIdx. + // 2. Selection sits in the range that shifts left because the + // moved entry crossed it going down (fromIdx < sel <= toIdx). + // 3. Selection sits in the range that shifts right because the + // moved entry crossed it going up (toIdx <= sel < fromIdx). + if (selection && (selection.kind === 'ref' || selection.kind === 'block')) { + const sel = selection.index; + if (sel === fromIdx) { + selection = { ...selection, index: toIdx }; + } else if (fromIdx < sel && sel <= toIdx) { + selection = { ...selection, index: sel - 1 }; + } else if (toIdx <= sel && sel < fromIdx) { + selection = { ...selection, index: sel + 1 }; + } } renderAll(); } catch (err) { @@ -1494,8 +1534,14 @@

Import error

try { docInsertSequenceEntry(experiment, atIdx, { kind: 'ref', condition_name: condName }); setDirty(true); - const clamped = Math.max(0, Math.min(atIdx, experiment.sequence.length - 1)); - selection = { kind: 'ref', index: clamped }; + // Walk existing selection through the insert: any entry at or + // after atIdx shifts +1. Drag-drop from the library should not + // steal focus from whatever the user had selected, so we DON'T + // overwrite selection to point to the new entry (unlike the + // explicit "+ Add ref" button, which intentionally selects it). + if (selection && (selection.kind === 'ref' || selection.kind === 'block') && atIdx <= selection.index) { + selection = { ...selection, index: selection.index + 1 }; + } renderAll(); } catch (err) { showError('Add ref failed', err.message); @@ -1538,16 +1584,17 @@

Import error

// Right-click handler — convert a sequence entry between ref and // single-trial block. A block is "convertible" to a ref only when it - // has exactly 1 trial, 1 rep, no randomize, and no intertrial (i.e., - // it's semantically equivalent to a bare ref). Non-convertible blocks - // show a soft error instead. + // has exactly 1 trial, 1 rep, no randomize, no intertrial, AND no + // forward-compat unknown keys (which would be silently dropped by the + // replace path, undermining the Tier 1 unknown-passthrough guarantee). function isConvertibleBlock(entry) { return entry && entry.kind === 'block' && entry.trials.length === 1 && (entry.repetitions || 1) === 1 && !entry.randomize && - !entry.intertrial; + !entry.intertrial && + Object.keys(entry._unknownKeys || {}).length === 0; } function onConvertSequenceEntry(idx) { @@ -1571,12 +1618,17 @@

Import error

} } else if (entry.kind === 'block') { if (!isConvertibleBlock(entry)) { + const unknownKeys = Object.keys(entry._unknownKeys || {}); + const reasons = []; + if (entry.trials.length !== 1) reasons.push(entry.trials.length + ' trial(s)'); + if ((entry.repetitions || 1) !== 1) reasons.push((entry.repetitions || 1) + ' rep(s)'); + if (entry.randomize) reasons.push('randomize=on'); + if (entry.intertrial) reasons.push('intertrial=' + entry.intertrial); + if (unknownKeys.length > 0) reasons.push('forward-compat keys: ' + unknownKeys.join(', ')); showError( 'Cannot convert block to ref', - 'Only blocks with exactly 1 trial, 1 repetition, no randomize, and no intertrial can convert to a bare ref. ' + - 'This block has ' + entry.trials.length + ' trial(s), ' + - (entry.repetitions || 1) + ' rep(s), randomize=' + (entry.randomize ? 'on' : 'off') + ', ' + - (entry.intertrial ? 'intertrial=' + entry.intertrial : 'no intertrial') + '.' + 'Only blocks with exactly 1 trial, 1 repetition, no randomize, no intertrial, and no forward-compat keys can convert to a bare ref. ' + + 'This block has ' + reasons.join(', ') + '.' ); return; } @@ -1805,26 +1857,10 @@

Import error

}); }; - // List-level drop target for the empty-list / append-past-end case. - list.addEventListener('dragover', (e) => { - if (!e.dataTransfer.types.includes('text/x-library-cond')) return; - // Only highlight when we're not over an existing entry (entries handle their own) - if (e.target.closest('.seq-entry')) return; - e.preventDefault(); - e.dataTransfer.dropEffect = 'copy'; - list.classList.add('lib-drop-target'); - }); - list.addEventListener('dragleave', (e) => { - if (e.target === list) list.classList.remove('lib-drop-target'); - }); - list.addEventListener('drop', (e) => { - if (e.target.closest('.seq-entry')) return; // handled by entry's own drop - const libCond = e.dataTransfer.getData('text/x-library-cond'); - if (!libCond) return; - e.preventDefault(); - list.classList.remove('lib-drop-target'); - onInsertSequenceRefFromLib(libCond, experiment.sequence.length); - }); + // List-level drop target listeners live on #sequenceList itself — + // they're wired once at startup (see below), not per-render, because + // innerHTML='' clears children but leaves listeners on the list + // element. Re-binding here would stack N handlers per render. experiment.sequence.forEach((entry, idx) => { const isSelected = selection && ( @@ -2573,6 +2609,15 @@

Import error

} function onMoveCommand(condIdx, fromIdx, toIdx) { + // Defensive: docMoveCommand silently no-ops on out-of-bounds / + // identity moves. Without this early-return, pushUndo() would + // record a snapshot for a move that never happened. The up/down + // arrow buttons disable at the edges so this is mostly belt-and- + // suspenders, but matches the onMoveSequenceEntry pattern. + if (fromIdx === toIdx) return; + const cond = experiment.conditions[condIdx]; + if (!cond || fromIdx < 0 || fromIdx >= cond.commands.length || + toIdx < 0 || toIdx >= cond.commands.length) return; pushUndo(); try { docMoveCommand(experiment, condIdx, fromIdx, toIdx); @@ -3180,23 +3225,42 @@

Import error

} const totalWidthPx = cumulativeWidth; - // The ruler maps real time → pixel position using the *actual* step - // strip width (so ticks stay aligned even when min-width clamping - // stretches short steps). 10s tick interval; label every minute - // for <15min totals, every 5min for longer. - const realTotalSec = steps.reduce((s, x) => s + (x.dur || 0), 0) || 1; - const pxPerSecRuler = totalWidthPx / realTotalSec; + // The ruler maps real time → pixel position by walking the layout + // array piecewise. A flat pxPerSec rate would drift as soon as any + // step's pixel width clamped to the minimum, since cumulative pixel + // position would no longer be a linear function of cumulative real + // time. 10s tick interval; label every minute for <15min totals, + // every 5min for longer. + const realTotalSec = layout.reduce((s, x) => s + (x.step.dur || 0), 0); const tickIntervalSec = 10; const labelIntervalSec = realTotalSec < 15 * 60 ? 60 : 5 * 60; const rulerHeight = 18; + // Map real-time t → pixel position. Find the layout entry whose + // real-time range contains t and interpolate proportionally within + // that entry's pixel range. Steps with dur === 0 use their start. + const rulerXForRealTime = (t) => { + let acc = 0; + for (let i = 0; i < layout.length; i++) { + const lay = layout[i]; + const stepDur = lay.step.dur || 0; + const isLast = i === layout.length - 1; + if (t <= acc + stepDur || isLast) { + const within = stepDur > 0 ? (t - acc) / stepDur : 0; + return lay.startPx + Math.max(0, Math.min(1, within)) * (lay.endPx - lay.startPx); + } + acc += stepDur; + } + return totalWidthPx; + }; + const NS = 'http://www.w3.org/2000/svg'; const ruler = document.createElementNS(NS, 'svg'); ruler.setAttribute('class', 'timeline-ruler'); ruler.setAttribute('width', String(totalWidthPx)); ruler.setAttribute('height', String(rulerHeight)); for (let t = 0; t <= realTotalSec; t += tickIntervalSec) { - const x = t * pxPerSecRuler; + const x = rulerXForRealTime(t); const isMajor = (t % labelIntervalSec) === 0 && t > 0; const tick = document.createElementNS(NS, 'line'); tick.setAttribute('class', 'tick' + (isMajor ? ' major' : '')); diff --git a/js/protocol-yaml-v3.js b/js/protocol-yaml-v3.js index c2f9293..1108029 100644 --- a/js/protocol-yaml-v3.js +++ b/js/protocol-yaml-v3.js @@ -1019,7 +1019,20 @@ function _buildSequenceEntry(doc, entry) { } const blockShape = { trials: entry.trials.slice() }; if (typeof entry.name === 'string' && entry.name) blockShape.name = entry.name; - if (typeof entry.repetitions === 'number') blockShape.repetitions = entry.repetitions; + if (entry.repetitions !== undefined) { + // Mirror the parser's validation: positive integer only. The + // parser at lines ~335-349 already rejects 0/negatives/decimals; + // without this guard, programmatic build paths (D4, paste-import) + // would emit invalid YAML and the doc/JS-mirror could diverge + // (entry.repetitions = 0 → YAML `0` but mirror = 1 via `|| 1`). + if (typeof entry.repetitions !== 'number' || !Number.isInteger(entry.repetitions) || entry.repetitions < 1) { + throw new V3ParseError( + '_buildSequenceEntry: invalid repetitions (must be positive integer): ' + JSON.stringify(entry.repetitions), + 'INVALID_SCHEMA' + ); + } + blockShape.repetitions = entry.repetitions; + } if (entry.randomize === true) blockShape.randomize = true; if (typeof entry.intertrial === 'string' && entry.intertrial) blockShape.intertrial = entry.intertrial; return { diff --git a/tests/test-protocol-roundtrip-v3.js b/tests/test-protocol-roundtrip-v3.js index 97d85f7..9c27012 100644 --- a/tests/test-protocol-roundtrip-v3.js +++ b/tests/test-protocol-roundtrip-v3.js @@ -800,6 +800,38 @@ checkThrows( check('reps validation: omitted defaults to 1', exp.sequence[0].repetitions, 1); } +// _buildSequenceEntry (builder path used by D4 / paste-import) must mirror the +// parser's positive-integer validation, so doc/JS-mirror can't diverge. +{ + const exp = parseV3Protocol(v3WithReps('1')); + checkThrows( + 'reps validation: builder rejects 0', + () => docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'], repetitions: 0 }), + 'INVALID_SCHEMA' + ); + checkThrows( + 'reps validation: builder rejects -2', + () => docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'], repetitions: -2 }), + 'INVALID_SCHEMA' + ); + checkThrows( + 'reps validation: builder rejects 1.5', + () => docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'], repetitions: 1.5 }), + 'INVALID_SCHEMA' + ); + checkThrows( + 'reps validation: builder rejects non-number', + () => docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'], repetitions: 'two' }), + 'INVALID_SCHEMA' + ); + // Positive integer accepted on the builder path + docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'], repetitions: 4 }); + check('reps validation: builder accepts 4', exp.sequence[0].repetitions, 4); + // Omitted accepted on the builder path (becomes default 1 on mirror) + docInsertSequenceEntry(exp, 0, { kind: 'block', trials: ['c'] }); + check('reps validation: builder accepts omitted', exp.sequence[0].repetitions, 1); +} + // ─── Test Suite 11: docInsertCommand / docMoveCommand / delete-command ──── console.log('\n--- Suite 11: command add / move / delete ---'); From de31235cb62045f213a6b63bd5df0936e59bbbd1 Mon Sep 17 00:00:00 2001 From: Michael B Reiser Date: Wed, 27 May 2026 20:10:54 -0400 Subject: [PATCH 2/3] =?UTF-8?q?feat(v3):=20Phase=205=20=E2=80=94=20Variabl?= =?UTF-8?q?es=20editor=20+=20anchor=20binding=20popover=20+=20rename=20cas?= =?UTF-8?q?cade=20(v0.10)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the largest user-facing feature gap in the v3 designer. Three pieces, all additive — only one existing function (renderEditableField) gains a 🔗 button. ## Inline-editable Variables table Replaces the read-only Variables section in the Settings drawer with: - Editable anchor-name input on every row (renames cascade via the modal). - Editable value input (numeric or text by inferred type). - Read-only "complex anchor" badge for map/seq anchors. - ✕ delete button — blocked when references exist, with confirmation modal offering cascade-unbind. - "+ Add variable" footer row with name + value + Add button. ## Anchor binding popover Every editable scalar (controller/plugin command fields) gains a 🔗 button. Single global popover positioned beneath the trigger; outside- click and Escape close it. Content varies by alias state: - Literal scalar: "Bind to existing anchor" select (with type-mismatched options visibly disabled with a "(type: X)" suffix) + "Create new anchor from this value" with name input and "Create & bind". - Aliased scalar: summary card "→ &name = value" + "Edit in Variables…" jump button + "Rebind to a different anchor" select + "Unbind (convert to literal)" danger button. Create-and-bind runs as a single pushUndo → so one Ctrl+Z reverts both the variable creation and the binding atomically. ## Rename cascade Confirmation modal lists every reference path that will update; on confirm, a single pushUndo wraps docRenameVariable, which atomically walks every *alias source in one synchronous pass via YAML.visit. One undo step for the whole rename. Empty-refs case skips the modal (no friction for unused anchors). ## New helpers in js/protocol-yaml-v3.js (10 added → 27 total) - docCreateVariable / docDeleteVariable / docRenameVariable - docSetVariableValue (mutates Scalar.value in place — preserves anchor) - docBindToAnchor / docUnbindAnchor - findAliasesTo (recursive walk of doc.contents — simpler/safer than the YAML.visit ancestors-chain reconstruction) - variableIsComplex / isValidAnchorName / anchorExists extractVariables now prefers pair.value.anchor as identity (falls back to map key). This means renaming via the table cascades the anchor without churning the map key — per the plan-mode user decision. ## New UI primitive: confirmModal({title, body, list, confirmLabel}) Promise-based, backdrop-dismiss, Escape/Enter keys. ~70 lines. Phase 6's full pre-export validation modal will reuse it. ## Tests 54 new tests in Suite 29 (Variable lifecycle + anchor binding): - Create / delete (incl. ANCHOR_HAS_REFS + cascade-unbind) - Rename (anchor + every alias, count match, collision rejection, comment preservation, merge-key cascade `<<: *foo`) - Bind literal → alias / unbind alias → literal round-trip - setVariableValue preserves anchor declaration - findAliasesTo count matches grep on canonical fixture - variableIsComplex / isValidAnchorName / anchorExists basics Total: 429/429 (was 375 after cleanup PR). ## Verified in browser - Variables table renders 4 editable rows + Add row for canonical_a - Rename `&dur_long → &duration_long` shows modal listing all 9 references, applies in one undo step - Click 🔗 on aliased duration: popover shows current binding, rebind options (with color_command disabled as type-mismatched string), unbind action - Click 🔗 on literal: bind-existing dropdown + create-new section - Create-and-bind: single Ctrl+Z reverts both ops; Variables table loses the new anchor on undo - Unbind: alias chip replaced by editable input with resolved value ## Updated copy The BETA banner now says "Editing. Scalar fields, sequence layout, and Variables are all editable. Click the 🔗 icon next to any value..." (was "anchor-bound fields are read-only until the Variables editor lands"). Footer bumped to v0.10 | 2026-05-27 20:07 ET. ## Decisions logged - Variables table identity: anchor name only (per plan-mode Q&A). Rename cascades the anchor; the map key is preserved untouched through round-trip but not surfaced. Rare existing-YAML cases where they differ appear "renamed" in the UI. - Codex plan-review pass on Phase 5: skipped (per plan-mode Q&A). ## Open items deferred (not in this PR) - Phase 6 (full validation modal + Reset) - Phase 7 (comment-preservation tests at strategic positions) - Phase 8 (MATLAB-validation flow docs) - Phase 9 (quickstart HTML) - D4 (cross-library import) — now unblocked since Phase 5 closes the prefix-clutter UX gap Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/development/v3-editor-handoff-2.md | 77 ++- experiment_designer_v3.html | 772 +++++++++++++++++++++++- js/protocol-yaml-v3.js | 443 +++++++++++++- tests/test-protocol-roundtrip-v3.js | 319 +++++++++- 4 files changed, 1555 insertions(+), 56 deletions(-) diff --git a/docs/development/v3-editor-handoff-2.md b/docs/development/v3-editor-handoff-2.md index a8354c4..60e5c4c 100644 --- a/docs/development/v3-editor-handoff-2.md +++ b/docs/development/v3-editor-handoff-2.md @@ -1,8 +1,8 @@ # v3 Experiment Designer — Handoff for Next Session (Round 2) -**Last updated:** 2026-05-27 -**Branch:** `main` at `c5cf223` -**Editor version:** v3 Experiment Designer **v0.8** +**Last updated:** 2026-05-27 (Phase 5 session) +**Branch:** `main` at `c5cf223` → cleanup PR `#76` + Phase 5 PR (this session) +**Editor version:** v3 Experiment Designer **v0.10** (Phase 5 shipped this session) **Pinned upstream:** maDisplayTools `origin/version3` at `649d7ef` This is the second handoff doc for the v3 designer. It supersedes the original @@ -227,29 +227,47 @@ already-merged code. **Why first:** the event-listener bug surfaces within a normal editing session. If the lab is actively using v0.8, they'll hit it. Cheap to fix. -### Tier 2: Phase 5 — Variables UX (next big feature — ~4–5 days) - -The handoff plan's Phase 5. Three pieces: - -1. **Inline-editable Variables table** in the Settings drawer (anchor name - + value, both editable). Already exists as read-only. -2. **Anchor binding popover** on every editable scalar in command cards. - Today alias-bound fields are read-only chips. Phase 5 adds a pencil - icon → popover with three actions: Edit literal (unbinds anchor), Bind - to existing anchor (dropdown of available anchors), Create new anchor - (name + use current value). -3. **Rename cascade.** Confirm dialog showing all references that will - update, then the cascade fires as one `pushUndo()` → many `docSet` - calls → single `renderAll`. One undo step for the whole rename. - -**Why before D4:** Codex-adv flagged in the D4 design review that D4 -creates prefix-clutter (`sibling_lab__dur_short`) that Phase 5 is the -exact tool to clean up. Doing Phase 5 first means D4 ships against a -better base. Per `docs/development/v3-d4-design-reviews.md`. +### Tier 2: Phase 5 — Variables UX ✅ SHIPPED (this session, v0.10) + +Phase 5 landed in two PRs this session: cleanup (`#76`, v0.9) then +Phase 5 (this branch, v0.10). All three pieces are in: + +1. **Inline-editable Variables table** in the Settings drawer (anchor + name + value editable, ✕ delete with cascade-unbind, `+ Add` row). + Complex anchors (Map/Seq) render as read-only "complex anchor" + badges. +2. **Anchor binding popover** on every editable scalar (controller and + plugin command fields) via a 🔗 button appended to each input. + - Literal scalars: Bind-to-existing dropdown (with type-mismatched + options visibly disabled) + Create-new-anchor section. + - Aliased scalars: summary card with "Edit in Variables…" jump + button, Rebind-to dropdown, and Unbind action. +3. **Rename cascade.** Modal lists every reference path that will + update; single `pushUndo()` wraps the atomic `docRenameVariable` + call (which walks every `*alias` source in one synchronous pass via + `YAML.visit`). One undo step for the whole rename. + +**New doc helpers** (10 added → 27 total): +`docCreateVariable`, `docDeleteVariable`, `docRenameVariable`, +`docSetVariableValue`, `docBindToAnchor`, `docUnbindAnchor`, +`findAliasesTo`, `variableIsComplex`, `isValidAnchorName`, +`anchorExists`. + +**New UI primitive:** `confirmModal({title, body, list, confirmLabel}) → +Promise` — backdrop-dismiss + Escape/Enter keys. Reusable for +Phase 6's full validation modal. + +**Test coverage:** 54 new tests in Suite 29 (Variable lifecycle + +anchor binding). Total: 429/429. + +**D4 unblocked:** the prefix-clutter mitigation Codex-adv flagged in +the D4 design review now has its UX (Phase 5's Variables table makes +the renames trivial). Complex anchors (maps, lists, merge keys `<<: *foo`) round-trip via -`_doc` but surface as read-only "advanced anchor" badges. User edits -those in YAML. +`_doc`, surface as read-only "complex anchor" badges in the Variables +table, and rename cascades work for them (the test suite covers a +merge-key case). ### Tier 3: Phase 6 — full validation modal + Reset (~1 day) @@ -316,15 +334,18 @@ These should be resolved before the next session picks a direction: ### Current helpers (alphabetical) -`docAddPluginParam`, `docAppendSequenceEntry`, `docCloneCondition`, -`docDelete`, `docDeletePluginParam`, `docInsertCommand`, +`docAddPluginParam`, `docAppendSequenceEntry`, `docBindToAnchor`, +`docCloneCondition`, `docCreateVariable`, `docDelete`, +`docDeletePluginParam`, `docDeleteVariable`, `docInsertCommand`, `docInsertCondition`, `docInsertSequenceEntry`, `docInsertTrialInBlock`, `docMoveCommand`, `docMoveSequenceEntry`, `docMoveTrialInBlock`, `docRemoveSequenceEntry`, `docRemoveTrialFromBlock`, -`docReplaceSequenceEntry`, `docSet`, `docSetPluginCommandHead`. +`docRenameVariable`, `docReplaceSequenceEntry`, `docSet`, +`docSetPluginCommandHead`, `docSetVariableValue`, `docUnbindAnchor`. Plus inspection helpers: `nodeIsAliasAt`, `aliasNameAt`, -`collectExportWarnings`, `validateReferences`. +`anchorExists`, `findAliasesTo`, `isValidAnchorName`, +`variableIsComplex`, `collectExportWarnings`, `validateReferences`. ### Helper anatomy (template for adding new ones) diff --git a/experiment_designer_v3.html b/experiment_designer_v3.html index 3b219eb..4361578 100644 --- a/experiment_designer_v3.html +++ b/experiment_designer_v3.html @@ -189,6 +189,168 @@ .vars-table th { color: var(--text-dim); font-weight: 400; } .vars-table .num { text-align: right; font-family: 'JetBrains Mono', monospace; color: var(--accent); } .vars-table .str { color: var(--accent); } + /* Phase 5 — inline-editable Variables table */ + .var-row input, .var-add-row input { + background: var(--surface); + border: 1px solid var(--border); + color: var(--text); + font: inherit; + padding: 0.15rem 0.35rem; + font-size: 0.75rem; + width: 100%; + box-sizing: border-box; + } + .var-row input.num, .var-add-row input.num { color: var(--accent); } + .var-row input.str, .var-add-row input.str { color: var(--accent); } + .var-row input:focus, .var-add-row input:focus { + outline: 1px solid var(--accent); + outline-offset: -1px; + } + .var-del-btn, .var-add-btn { + background: transparent; + border: 1px solid var(--border); + color: var(--text-dim); + cursor: pointer; + font-size: 0.7rem; + padding: 0.15rem 0.4rem; + } + .var-del-btn:hover { color: var(--text); border-color: #c4514f; } + .var-add-btn:hover { color: var(--accent); border-color: var(--accent); } + .var-add-row { background: rgba(255, 255, 255, 0.02); } + .adv-anchor-badge { + color: var(--text-dim); + font-style: italic; + font-size: 0.7rem; + } + /* Phase 5 — anchor binding popover */ + .bind-icon-btn { + background: transparent; + border: 1px solid transparent; + color: var(--text-dim); + cursor: pointer; + padding: 0 0.25rem; + font-size: 0.75rem; + line-height: 1; + margin-left: 0.25rem; + } + .bind-icon-btn:hover { color: var(--accent); border-color: var(--border); } + .bind-icon-btn.bound { color: var(--accent); } + .anchor-popover { + position: absolute; + z-index: 1000; + background: var(--surface); + border: 1px solid var(--border); + box-shadow: 0 4px 12px rgba(0, 0, 0, 0.5); + min-width: 260px; + max-width: 360px; + padding: 0; + font-size: 0.75rem; + } + .anchor-popover .section { + padding: 0.5rem 0.75rem; + border-top: 1px solid var(--border); + } + .anchor-popover .section:first-child { border-top: none; } + .anchor-popover .section h4 { + margin: 0 0 0.3rem 0; + font-size: 0.7rem; + color: var(--text-dim); + font-weight: 400; + text-transform: uppercase; + letter-spacing: 0.04em; + } + .anchor-popover select, .anchor-popover input { + background: var(--bg); + border: 1px solid var(--border); + color: var(--text); + padding: 0.2rem 0.4rem; + font: inherit; + font-size: 0.75rem; + width: 100%; + box-sizing: border-box; + } + .anchor-popover button { + background: var(--surface); + border: 1px solid var(--border); + color: var(--text); + padding: 0.25rem 0.6rem; + font-size: 0.75rem; + cursor: pointer; + margin-top: 0.3rem; + } + .anchor-popover button:hover { border-color: var(--accent); color: var(--accent); } + .anchor-popover button.danger:hover { border-color: #c4514f; color: #c4514f; } + .anchor-popover .summary { + color: var(--accent); + font-family: 'JetBrains Mono', monospace; + padding: 0.3rem 0; + } + /* Phase 5 — confirmation modal (rename cascade, cascade-unbind) */ + .confirm-modal-backdrop { + position: fixed; + inset: 0; + background: rgba(0, 0, 0, 0.6); + z-index: 2000; + display: flex; + align-items: center; + justify-content: center; + } + .confirm-modal { + background: var(--surface); + border: 1px solid var(--border); + min-width: 360px; + max-width: 560px; + max-height: 80vh; + display: flex; + flex-direction: column; + } + .confirm-modal h3 { + margin: 0; + padding: 0.75rem 1rem; + border-bottom: 1px solid var(--border); + font-size: 0.9rem; + } + .confirm-modal .body { + padding: 0.75rem 1rem; + overflow: auto; + } + .confirm-modal .body p { + margin: 0 0 0.5rem 0; + font-size: 0.8rem; + color: var(--text); + } + .confirm-modal .body ul { + margin: 0; + padding-left: 1rem; + font-size: 0.75rem; + color: var(--text-dim); + max-height: 240px; + overflow: auto; + } + .confirm-modal .body ul li { padding: 0.1rem 0; } + .confirm-modal .actions { + display: flex; + gap: 0.5rem; + padding: 0.6rem 1rem; + border-top: 1px solid var(--border); + justify-content: flex-end; + } + .confirm-modal .actions button { + background: var(--surface); + border: 1px solid var(--border); + color: var(--text); + padding: 0.3rem 0.8rem; + font: inherit; + font-size: 0.8rem; + cursor: pointer; + } + .confirm-modal .actions button.primary { + background: var(--accent); + color: #0f1419; + border-color: var(--accent); + } + .confirm-modal .actions button:hover { border-color: var(--accent); } + .confirm-modal .actions button.primary:hover { background: #00c853; } /* ── 3-zone main area ────────────────────────────────────────── */ .three-zone { @@ -894,9 +1056,9 @@

v3 Experiment Designer

- Editing (early). Scalar fields in command cards are editable; anchor-bound fields ( - → &name) are read-only until the Variables editor lands. - Drag/drop, undo, and sequence editing arrive in later updates. + Editing. Scalar fields, sequence layout, and Variables are all editable. Click the + 🔗 icon next to any value to bind it to an anchor, or open + Settings to manage the Variables table directly. Need v2? Open the v2 Experiment Designer.
@@ -978,7 +1140,7 @@

v3 Experiment Designer

@@ -1017,7 +1179,18 @@

Import error

docAddPluginParam, docDeletePluginParam, nodeIsAliasAt, - aliasNameAt + aliasNameAt, + // Phase 5 — Variables (anchor) lifecycle + docCreateVariable, + docDeleteVariable, + docRenameVariable, + docSetVariableValue, + docBindToAnchor, + docUnbindAnchor, + findAliasesTo, + variableIsComplex, + isValidAnchorName, + anchorExists } from './js/protocol-yaml-v3.js'; import { @@ -1295,6 +1468,70 @@

Import error

if (e.target.id === 'errorModal') $('errorModal').classList.remove('open'); }); + /** + * confirmModal({title, body, list, confirmLabel, cancelLabel}) -> Promise + * + * Generic confirmation modal for destructive / wide-reaching operations. + * Used by the Variables rename cascade and cascade-unbind delete flows + * (Phase 5). Phase 6's full pre-export validation modal will reuse + * this primitive. + * + * - title: short heading + * - body: plain-text paragraph + * - list: optional array of strings rendered as a bulleted list (the + * affected references, etc.). Pass [] or undefined to omit. + * - confirmLabel: label on the primary action button ("Rename", "Delete") + * - cancelLabel: defaults to "Cancel" + */ + function confirmModal(opts) { + return new Promise((resolve) => { + const backdrop = el('div', { class: 'confirm-modal-backdrop' }); + const modal = el('div', { class: 'confirm-modal' }); + modal.appendChild(el('h3', {}, opts.title || 'Confirm')); + const body = el('div', { class: 'body' }); + if (opts.body) body.appendChild(el('p', {}, opts.body)); + if (opts.list && opts.list.length > 0) { + const ul = el('ul'); + for (const item of opts.list) ul.appendChild(el('li', {}, item)); + body.appendChild(ul); + } + modal.appendChild(body); + const actions = el('div', { class: 'actions' }); + let resolved = false; + const finish = (result) => { + if (resolved) return; + resolved = true; + document.removeEventListener('keydown', onKey); + backdrop.remove(); + resolve(result); + }; + const cancelBtn = el('button', { + onClick: () => finish(false), + }, opts.cancelLabel || 'Cancel'); + const confirmBtn = el('button', { + class: 'primary', + onClick: () => finish(true), + }, opts.confirmLabel || 'Confirm'); + actions.appendChild(cancelBtn); + actions.appendChild(confirmBtn); + modal.appendChild(actions); + backdrop.appendChild(modal); + // Backdrop click → cancel + backdrop.addEventListener('click', (e) => { + if (e.target === backdrop) finish(false); + }); + // Esc → cancel; Enter → confirm + const onKey = (e) => { + if (e.key === 'Escape') { e.preventDefault(); finish(false); } + else if (e.key === 'Enter') { e.preventDefault(); finish(true); } + }; + document.addEventListener('keydown', onKey); + document.body.appendChild(backdrop); + // Autofocus primary + setTimeout(() => confirmBtn.focus(), 0); + }); + } + // ═══════════════════════════════════════════════════════════════ // Settings drawer // ═══════════════════════════════════════════════════════════════ @@ -1358,26 +1595,238 @@

Import error

root.appendChild(pluginBox); } - // Variables - if (experiment.variables.length > 0) { - const varBox = el('div', { class: 'settings-section' }); - varBox.appendChild(el('h3', {}, 'Variables (' + experiment.variables.length + ')')); - const table = el('table', { class: 'vars-table' }); - const thead = el('thead', {}, el('tr', {}, - el('th', {}, 'Anchor'), el('th', {}, 'Value') - )); - table.appendChild(thead); - const tbody = el('tbody'); - for (const v of experiment.variables) { - const valClass = typeof v.value === 'number' ? 'num' : (typeof v.value === 'string' ? 'str' : ''); - tbody.appendChild(el('tr', {}, - el('td', {}, '&' + v.name), - el('td', { class: valClass }, typeof v.value === 'string' ? JSON.stringify(v.value) : String(v.value)) + // Variables — inline-editable (Phase 5, v0.10) + // Always render the section even when empty so the user can add + // the first variable. Complex anchors (maps/seqs) render as + // read-only badges; scalar anchors are name + value editable. + const varBox = el('div', { class: 'settings-section' }); + const varCount = experiment.variables ? experiment.variables.length : 0; + varBox.appendChild(el('h3', {}, 'Variables (' + varCount + ')')); + const table = el('table', { class: 'vars-table' }); + const thead = el('thead', {}, el('tr', {}, + el('th', {}, 'Anchor'), + el('th', {}, 'Value'), + el('th', { style: 'width: 1.5em;' }, '') + )); + table.appendChild(thead); + const tbody = el('tbody'); + for (const v of experiment.variables || []) { + const isComplex = variableIsComplex(experiment, v.name); + const row = el('tr', { class: 'var-row' }); + + // Name cell — always editable (rename cascades) + const nameInput = el('input', { + type: 'text', + value: v.name, + class: 'var-name-input', + title: 'Rename anchor. Renaming cascades to every *alias reference.', + }); + // change fires on blur, so each visit to the input produces + // at most one pushUndo (inside onVariableRename). + nameInput.addEventListener('change', () => onVariableRename(v.name, nameInput.value.trim())); + row.appendChild(el('td', {}, nameInput)); + + // Value cell — editable scalar OR read-only badge for complex + if (isComplex) { + row.appendChild(el('td', {}, + el('span', { + class: 'adv-anchor-badge', + title: 'Map or sequence anchor — edit the YAML directly to change the value. Renaming still works.', + }, 'complex anchor (edit YAML)') )); + } else { + const isNum = typeof v.value === 'number'; + const valInput = el('input', { + type: isNum ? 'number' : 'text', + value: isNum ? String(v.value) : String(v.value ?? ''), + class: 'var-value-input ' + (isNum ? 'num' : 'str'), + title: 'Edit the anchor value. Every *alias to this anchor will resolve to the new value.', + }); + valInput.addEventListener('change', () => onVariableValueEdit(v.name, valInput.value, isNum)); + row.appendChild(el('td', {}, valInput)); } - table.appendChild(tbody); - varBox.appendChild(table); - root.appendChild(varBox); + + // Delete cell + const delBtn = el('button', { + class: 'var-del-btn', + title: 'Delete this anchor. Blocked if it has *alias references; the prompt will offer cascade-unbind.', + onClick: () => onVariableDelete(v.name), + }, '✕'); + row.appendChild(el('td', {}, delBtn)); + + tbody.appendChild(row); + } + + // + Add variable row (always at end) + const addRow = el('tr', { class: 'var-add-row' }); + const addNameInput = el('input', { + type: 'text', + placeholder: 'new_anchor_name', + class: 'var-name-input', + title: 'Anchor name (letters, digits, _, -). Pressing Enter or Tab creates the anchor with the value to the right.', + }); + const addValInput = el('input', { + type: 'text', + placeholder: 'value', + class: 'var-value-input', + title: 'Initial value. Numeric strings become numbers; everything else stays a string.', + }); + const addBtn = el('button', { + class: 'var-add-btn', + title: 'Create the new anchor.', + onClick: () => { + const name = addNameInput.value.trim(); + const val = addValInput.value; + if (!name) return; + onVariableAdd(name, val); + addNameInput.value = ''; + addValInput.value = ''; + addNameInput.focus(); + }, + }, '+ Add'); + // Enter in either input submits the add + const submitOnEnter = (e) => { if (e.key === 'Enter') { e.preventDefault(); addBtn.click(); } }; + addNameInput.addEventListener('keydown', submitOnEnter); + addValInput.addEventListener('keydown', submitOnEnter); + addRow.appendChild(el('td', {}, addNameInput)); + addRow.appendChild(el('td', {}, addValInput)); + addRow.appendChild(el('td', {}, addBtn)); + tbody.appendChild(addRow); + + table.appendChild(tbody); + varBox.appendChild(table); + root.appendChild(varBox); + } + + // ─── Variables editor handlers ───────────────────────────────────── + + // Coerce a string from a text input into a number when it parses + // cleanly. Used by the Add row (which doesn't pre-know the type). + function _coerceVarValue(raw) { + const trimmed = String(raw).trim(); + if (trimmed === '') return ''; + const n = Number(trimmed); + if (!Number.isNaN(n) && /^-?\d+(\.\d+)?$/.test(trimmed)) return n; + return trimmed; + } + + async function onVariableRename(oldName, newName) { + if (!experiment) return; + if (oldName === newName) return; + if (!newName) { + showError('Rename failed', 'Anchor name cannot be empty.'); + renderSettings(); + return; + } + if (!isValidAnchorName(newName)) { + showError( + 'Rename failed', + 'Invalid anchor name: must contain only letters, digits, underscores, and hyphens.' + ); + renderSettings(); + return; + } + if (anchorExists(experiment, newName)) { + showError('Rename failed', 'Anchor name "' + newName + '" is already in use.'); + renderSettings(); + return; + } + const refs = findAliasesTo(experiment, oldName); + if (refs.length > 0) { + const proceed = await confirmModal({ + title: 'Rename &' + oldName + ' to &' + newName + '?', + body: 'This will update ' + refs.length + ' reference' + (refs.length === 1 ? '' : 's') + ':', + list: refs.map((r) => '• ' + r.humanLabel), + confirmLabel: 'Rename', + }); + if (!proceed) { renderSettings(); return; } + } + pushUndo(); + try { + docRenameVariable(experiment, oldName, newName); + setDirty(true); + renderAll(); + } catch (err) { + showError('Rename failed', err.message); + renderSettings(); + } + } + + function onVariableValueEdit(name, rawValue, isNumericInput) { + if (!experiment) return; + const coerced = isNumericInput ? Number(rawValue) : _coerceVarValue(rawValue); + if (isNumericInput && Number.isNaN(coerced)) { + showError('Update failed', 'Value must be a number.'); + renderSettings(); + return; + } + pushUndo(); + try { + docSetVariableValue(experiment, name, coerced); + setDirty(true); + renderAll(); + } catch (err) { + showError('Update failed', err.message); + renderSettings(); + } + } + + async function onVariableDelete(name) { + if (!experiment) return; + const refs = findAliasesTo(experiment, name); + if (refs.length === 0) { + if (!confirm('Delete anchor "&' + name + '"?')) return; + pushUndo(); + try { + docDeleteVariable(experiment, name); + setDirty(true); + renderAll(); + } catch (err) { + showError('Delete failed', err.message); + } + return; + } + const proceed = await confirmModal({ + title: 'Anchor &' + name + ' is in use', + body: 'It has ' + refs.length + ' reference' + (refs.length === 1 ? '' : 's') + + '. Cascade-unbind will replace each reference with the current literal value (' + + JSON.stringify((experiment.variables.find((v) => v.name === name) || {}).value) + + ') before deleting:', + list: refs.map((r) => '• ' + r.humanLabel), + confirmLabel: 'Cascade-unbind & delete', + }); + if (!proceed) return; + pushUndo(); + try { + docDeleteVariable(experiment, name, { cascadeUnbind: true }); + setDirty(true); + renderAll(); + } catch (err) { + showError('Delete failed', err.message); + } + } + + function onVariableAdd(name, rawValue) { + if (!experiment) return; + if (!isValidAnchorName(name)) { + showError( + 'Add failed', + 'Invalid anchor name: must contain only letters, digits, underscores, and hyphens.' + ); + return; + } + if (anchorExists(experiment, name)) { + showError('Add failed', 'Anchor name "' + name + '" is already in use.'); + return; + } + const value = _coerceVarValue(rawValue); + pushUndo(); + try { + docCreateVariable(experiment, name, value); + setDirty(true); + renderAll(); + } catch (err) { + showError('Add failed', err.message); } } @@ -2288,16 +2737,31 @@

Import error

typeHint = schema.type === 'number' ? 'number' : (schema.type === 'select' ? 'select' : 'string'); } + // Build the 🔗 anchor-binding button — appended to every editable + // scalar regardless of alias state. Click opens a popover that + // offers bind / rebind / unbind / create-new based on context. + const bindBtn = el('button', { + class: 'bind-icon-btn' + (aliasNameAt(experiment, path) ? ' bound' : ''), + title: aliasNameAt(experiment, path) + ? 'Edit anchor binding (rebind, unbind, or edit anchor value)' + : 'Bind this value to an anchor variable', + onClick: (e) => { + e.stopPropagation(); + openAnchorPopover(e.currentTarget, path, value, schema, typeHint); + } + }, '🔗'); + const aliasName = aliasNameAt(experiment, path); if (aliasName) { const chip = el('span', { class: 'alias-chip', - title: 'Bound to anchor &' + aliasName + ' (resolved: ' + JSON.stringify(value) + '). Anchor binding editor arrives in a later update.' + title: 'Bound to anchor &' + aliasName + ' (resolved: ' + JSON.stringify(value) + '). Click 🔗 to rebind or unbind.' }, '→ &' + aliasName, el('span', { class: 'resolved-val' }, '= ' + (typeof value === 'string' ? JSON.stringify(value) : String(value))) ); wrapper.appendChild(chip); + wrapper.appendChild(bindBtn); return wrapper; } @@ -2334,6 +2798,7 @@

Import error

} }); wrapper.appendChild(select); + wrapper.appendChild(bindBtn); return wrapper; } @@ -2379,9 +2844,268 @@

Import error

} }); wrapper.appendChild(input); + wrapper.appendChild(bindBtn); return wrapper; } + // ═══════════════════════════════════════════════════════════════ + // Phase 5 — Anchor binding popover + // ═══════════════════════════════════════════════════════════════ + + // Single global popover. Created lazily on first open; one instance + // shared across all 🔗 buttons. The outside-click and Escape handlers + // are wired once on first open and persist for the page lifetime. + let _anchorPopover = null; + let _anchorPopoverState = null; // { triggerEl, path, schema } + function _closeAnchorPopover() { + if (_anchorPopover) { + _anchorPopover.remove(); + _anchorPopover = null; + _anchorPopoverState = null; + } + } + // Outside-click & Escape — wired once at module init. + document.addEventListener('click', (e) => { + if (!_anchorPopover) return; + if (_anchorPopover.contains(e.target)) return; + if (_anchorPopoverState?.triggerEl?.contains(e.target)) return; + _closeAnchorPopover(); + }); + document.addEventListener('keydown', (e) => { + if (e.key === 'Escape' && _anchorPopover) { + e.preventDefault(); + _closeAnchorPopover(); + } + }); + + function openAnchorPopover(triggerEl, path, currentValue, schema, typeHint) { + // Re-open / re-position if already open + _closeAnchorPopover(); + const aliasName = aliasNameAt(experiment, path); + const popover = el('div', { class: 'anchor-popover' }); + + // Sectioned content varies by alias state. + const buildLiteralContent = () => { + // Section A: Bind to existing anchor + const allAnchors = (experiment.variables || []).map((v) => v.name); + const sectionBind = el('div', { class: 'section' }); + sectionBind.appendChild(el('h4', {}, 'Bind to existing anchor')); + if (allAnchors.length === 0) { + sectionBind.appendChild(el('p', { style: 'color: var(--text-dim);' }, + 'No anchors declared yet — create one below.')); + } else { + const select = el('select'); + select.appendChild(el('option', { value: '' }, '— pick an anchor —')); + // Sort: same-type first, then disabled mismatched + const sameType = []; + const otherType = []; + for (const name of allAnchors) { + const v = experiment.variables.find((x) => x.name === name); + if (!v) continue; + if (variableIsComplex(experiment, name)) continue; + const sameTypeFlag = _anchorTypeCompatible(v.value, schema, typeHint); + const labelStr = '&' + name + ' = ' + JSON.stringify(v.value) + + (sameTypeFlag ? '' : ' (type: ' + (typeof v.value) + ')'); + const opt = el('option', { value: name, disabled: !sameTypeFlag }, labelStr); + if (sameTypeFlag) sameType.push(opt); + else otherType.push(opt); + } + for (const o of sameType) select.appendChild(o); + for (const o of otherType) select.appendChild(o); + sectionBind.appendChild(select); + const bindBtn = el('button', { + class: 'primary', + onClick: () => { + if (!select.value) return; + _closeAnchorPopover(); + _doBindToAnchor(path, select.value); + }, + }, 'Bind'); + sectionBind.appendChild(bindBtn); + } + popover.appendChild(sectionBind); + + // Section B: Create new anchor from current value + const sectionNew = el('div', { class: 'section' }); + sectionNew.appendChild(el('h4', {}, 'Create new anchor from this value')); + const nameInput = el('input', { type: 'text', placeholder: 'new_anchor_name' }); + sectionNew.appendChild(nameInput); + const summary = el('div', { class: 'summary' }, + '= ' + (typeof currentValue === 'string' ? JSON.stringify(currentValue) : String(currentValue)) + ); + sectionNew.appendChild(summary); + const createBtn = el('button', { + class: 'primary', + onClick: () => { + const newName = nameInput.value.trim(); + if (!newName) return; + _closeAnchorPopover(); + _doCreateAndBind(path, newName, currentValue); + }, + }, 'Create & bind'); + nameInput.addEventListener('keydown', (e) => { + if (e.key === 'Enter') { e.preventDefault(); createBtn.click(); } + }); + sectionNew.appendChild(createBtn); + popover.appendChild(sectionNew); + }; + + const buildAliasContent = () => { + const anchorVar = (experiment.variables || []).find((v) => v.name === aliasName); + + // Section A: Summary + jump to Variables + const sectionSummary = el('div', { class: 'section' }); + sectionSummary.appendChild(el('h4', {}, 'Currently bound')); + sectionSummary.appendChild(el('div', { class: 'summary' }, + '→ &' + aliasName + ' = ' + + (anchorVar ? (typeof anchorVar.value === 'string' ? JSON.stringify(anchorVar.value) : String(anchorVar.value)) : '?') + )); + const goBtn = el('button', { + onClick: () => { + _closeAnchorPopover(); + // Open settings drawer if closed; the Variables table is inside it. + const drawer = $('settingsDrawer'); + if (!drawer.classList.contains('open')) drawer.classList.add('open'); + // Best-effort focus on the matching var row + setTimeout(() => { + const inputs = drawer.querySelectorAll('.var-name-input'); + for (const inp of inputs) { + if (inp.value === aliasName) { inp.focus(); inp.select(); break; } + } + }, 50); + }, + }, 'Edit anchor value in Variables…'); + sectionSummary.appendChild(goBtn); + popover.appendChild(sectionSummary); + + // Section B: Rebind to a different anchor + const others = (experiment.variables || []) + .filter((v) => v.name !== aliasName && !variableIsComplex(experiment, v.name)); + if (others.length > 0) { + const sectionRebind = el('div', { class: 'section' }); + sectionRebind.appendChild(el('h4', {}, 'Rebind to a different anchor')); + const select = el('select'); + select.appendChild(el('option', { value: '' }, '— pick an anchor —')); + for (const v of others) { + const sameTypeFlag = _anchorTypeCompatible(v.value, schema, typeHint); + select.appendChild(el('option', { + value: v.name, + disabled: !sameTypeFlag, + }, '&' + v.name + ' = ' + JSON.stringify(v.value) + + (sameTypeFlag ? '' : ' (type: ' + (typeof v.value) + ')'))); + } + sectionRebind.appendChild(select); + const rebindBtn = el('button', { + class: 'primary', + onClick: () => { + if (!select.value) return; + _closeAnchorPopover(); + _doBindToAnchor(path, select.value); + }, + }, 'Rebind'); + sectionRebind.appendChild(rebindBtn); + popover.appendChild(sectionRebind); + } + + // Section C: Unbind + const sectionUnbind = el('div', { class: 'section' }); + sectionUnbind.appendChild(el('h4', {}, 'Convert to literal')); + sectionUnbind.appendChild(el('p', {}, + 'Replace the alias with the current resolved value. Other aliases to &' + + aliasName + ' are unaffected.' + )); + const unbindBtn = el('button', { + class: 'danger', + onClick: () => { + _closeAnchorPopover(); + _doUnbindAnchor(path); + }, + }, 'Unbind'); + sectionUnbind.appendChild(unbindBtn); + popover.appendChild(sectionUnbind); + }; + + if (aliasName) buildAliasContent(); + else buildLiteralContent(); + + document.body.appendChild(popover); + // Position beneath the trigger, clamped to viewport + const rect = triggerEl.getBoundingClientRect(); + const pw = popover.offsetWidth; + const ph = popover.offsetHeight; + const vw = window.innerWidth; + const vh = window.innerHeight; + let left = rect.left; + let top = rect.bottom + 4; + if (left + pw > vw - 8) left = vw - pw - 8; + if (left < 8) left = 8; + if (top + ph > vh - 8) top = rect.top - ph - 4; // flip above if overflow below + popover.style.left = left + 'px'; + popover.style.top = top + 'px'; + + _anchorPopover = popover; + _anchorPopoverState = { triggerEl, path, schema }; + } + + // Type compatibility between an anchor's resolved value and a target + // field's expected type. Booleans aren't aliasable in the v3 spec + // (no boolean fields in plugin schemas yet), so we only worry about + // number-vs-string and select.options enumeration. + function _anchorTypeCompatible(anchorValue, schema, typeHint) { + const aType = typeof anchorValue; + if (schema && schema.type === 'select' && Array.isArray(schema.options)) { + return schema.options.some((o) => o.value === anchorValue); + } + if (typeHint === 'number') return aType === 'number'; + // string fields accept string anchors (and we allow numeric anchors + // since YAML scalars are stringified on serialize either way). + return aType === 'string' || aType === 'number'; + } + + function _doBindToAnchor(path, anchorName) { + pushUndo(); + try { + docBindToAnchor(experiment, path, anchorName); + setDirty(true); + renderAll(); + } catch (err) { + showError('Bind failed', err.message); + } + } + + function _doUnbindAnchor(path) { + pushUndo(); + try { + docUnbindAnchor(experiment, path); + setDirty(true); + renderAll(); + } catch (err) { + showError('Unbind failed', err.message); + } + } + + // Create-and-bind: one pushUndo wraps both mutations so a single Ctrl+Z + // reverts both operations. + function _doCreateAndBind(path, anchorName, currentValue) { + if (!isValidAnchorName(anchorName)) { + showError('Create & bind failed', 'Invalid anchor name.'); + return; + } + if (anchorExists(experiment, anchorName)) { + showError('Create & bind failed', 'Anchor name "' + anchorName + '" is already in use.'); + return; + } + pushUndo(); + try { + docCreateVariable(experiment, anchorName, currentValue); + docBindToAnchor(experiment, path, anchorName); + setDirty(true); + renderAll(); + } catch (err) { + showError('Create & bind failed', err.message); + } + } + // ── Block property row renderers ───────────────────────────────── // After every commit, re-render sequence (block cards reflect new // reps / randomize badge / iti label) and timeline (step count and diff --git a/js/protocol-yaml-v3.js b/js/protocol-yaml-v3.js index 1108029..988d760 100644 --- a/js/protocol-yaml-v3.js +++ b/js/protocol-yaml-v3.js @@ -214,7 +214,14 @@ function extractVariables(doc) { const varsNode = doc.get('variables', true); if (!varsNode || !varsNode.items) return out; for (const pair of varsNode.items) { - const name = pair.key && pair.key.value !== undefined ? pair.key.value : String(pair.key); + // Identity preference: the anchor name on pair.value (the v3-canonical + // form `name_key: &anchor_name value`). Falls back to the map key when + // there's no anchor — rare in real fixtures, but tolerated. Phase 5's + // Variables editor surfaces the anchor name as the rename target and + // expects this to be the identity used throughout the JS mirror. + const anchorName = pair.value && pair.value.anchor ? pair.value.anchor : null; + const mapKey = pair.key && pair.key.value !== undefined ? pair.key.value : String(pair.key); + const name = anchorName || mapKey; let value; if (pair.value && pair.value.value !== undefined) { value = pair.value.value; @@ -1351,6 +1358,414 @@ function docDelete(experiment, path) { } } +// ════════════════════════════════════════════════════ +// Phase 5 — Variables (anchor) lifecycle helpers +// +// Anchors are first-class YAML CST nodes on `_doc`. The JS mirror +// (`experiment.variables[]`) always holds the *resolved* values; the +// `_doc` is the only place that knows about aliasing. These helpers +// preserve that invariant. +// +// Helper inventory: +// docCreateVariable(exp, name, value) +// docDeleteVariable(exp, name, opts?) +// docRenameVariable(exp, oldName, newName) +// docSetVariableValue(exp, name, newValue) +// docBindToAnchor(exp, path, anchorName) +// docUnbindAnchor(exp, path) +// findAliasesTo(exp, anchorName) +// variableIsComplex(exp, name) +// isValidAnchorName(name) +// anchorExists(exp, name) +// ════════════════════════════════════════════════════ + +const _ANCHOR_NAME_RE = /^[A-Za-z0-9_-]+$/; + +function isValidAnchorName(name) { + return typeof name === 'string' && _ANCHOR_NAME_RE.test(name); +} + +// Internal: locate the defining Pair of `name` in the variables: map. +// Prefers anchor identity (pair.value.anchor), falls back to map key. +function _findVariablePair(experiment, name) { + if (!experiment || !experiment._doc) return null; + const varsNode = experiment._doc.get('variables', true); + if (!varsNode || !Array.isArray(varsNode.items)) return null; + for (let i = 0; i < varsNode.items.length; i++) { + const pair = varsNode.items[i]; + const anchorName = pair.value && pair.value.anchor ? pair.value.anchor : null; + const mapKey = + pair.key && pair.key.value !== undefined ? pair.key.value : String(pair.key); + const identity = anchorName || mapKey; + if (identity === name) return { pair, index: i, varsNode }; + } + return null; +} + +// True if `name` is in use as an anchor anywhere in the document. +// Walks every Scalar/Map/Seq node; needs to be doc-wide because anchors +// can legally sit on non-variables values (rare but legal). +function anchorExists(experiment, name) { + if (!experiment || !experiment._doc) return false; + if (!isValidAnchorName(name)) return false; + let found = false; + if (typeof YAML.visit !== 'function') return false; + YAML.visit(experiment._doc, { + Node(_, node) { + if (node && node.anchor === name) { + found = true; + return YAML.visit.BREAK; + } + } + }); + return found; +} + +// True if the anchor's defining value node is a Map or Seq (not a Scalar). +// The Variables UI renders complex anchors as read-only badges. +function variableIsComplex(experiment, name) { + const found = _findVariablePair(experiment, name); + if (!found) return false; + const v = found.pair.value; + return !!(v && (YAML.isMap?.(v) || YAML.isSeq?.(v))); +} + +// Walk the doc collecting every Alias whose `.source` matches anchorName. +// Returns [{ path, humanLabel }] in document order. Recursive walk because +// YAML.visit's ancestors chain mixes Pair/Map/Seq/Scalar in ways that are +// fiddly to translate into a flat path; doing the walk ourselves with an +// explicit accumulator is simpler and easier to reason about. +function findAliasesTo(experiment, anchorName) { + const out = []; + if (!experiment || !experiment._doc) return out; + const root = experiment._doc.contents; + if (!root) return out; + _walkForAliases(root, [], anchorName, out); + return out; +} + +function _walkForAliases(node, path, anchorName, out) { + if (!node) return; + if (YAML.isAlias?.(node)) { + if (node.source === anchorName) { + out.push({ path: path.slice(), humanLabel: _humanLabelForPath(path) }); + } + return; + } + if (YAML.isMap?.(node)) { + for (const pair of node.items) { + const key = + pair.key && pair.key.value !== undefined ? pair.key.value : String(pair.key); + _walkForAliases(pair.value, [...path, key], anchorName, out); + } + return; + } + if (YAML.isSeq?.(node)) { + for (let i = 0; i < node.items.length; i++) { + _walkForAliases(node.items[i], [...path, i], anchorName, out); + } + return; + } + // Scalars / unknown: no recursion. +} + +// Best-effort human label for an alias path — e.g. "conditions[2].commands[1].duration". +function _humanLabelForPath(path) { + if (!path || path.length === 0) return ''; + const parts = []; + for (let i = 0; i < path.length; i++) { + const p = path[i]; + if (typeof p === 'number') { + parts[parts.length - 1] = (parts[parts.length - 1] || '') + '[' + p + ']'; + } else { + parts.push(String(p)); + } + } + return parts.join('.'); +} + +/** + * docCreateVariable(experiment, name, value) + * + * Append `&name: value` to the variables: map. If no variables: section + * exists, create one at the top-level. Mirror into experiment.variables. + * + * Throws V3ParseError(INVALID_INPUT) on: + * - invalid anchor name (regex) + * - duplicate anchor (anywhere in the doc, not just variables) + * - non-scalar value (caller should construct via setIn for maps/seqs) + */ +function docCreateVariable(experiment, name, value) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docCreateVariable: experiment has no _doc handle', 'NO_DOC'); + } + if (!isValidAnchorName(name)) { + throw new V3ParseError( + 'docCreateVariable: invalid anchor name ' + JSON.stringify(name) + + ' (must match /^[A-Za-z0-9_-]+$/)', + 'INVALID_INPUT' + ); + } + if (anchorExists(experiment, name)) { + throw new V3ParseError( + 'docCreateVariable: anchor name "' + name + '" is already in use', + 'INVALID_INPUT' + ); + } + + // Find or create the variables: map. yaml@2's setIn replaces nodes + // wholesale, so we create a fresh YAMLMap only if absent. + let varsNode = experiment._doc.get('variables', true); + if (!varsNode || !Array.isArray(varsNode.items)) { + const newMap = experiment._doc.createNode({}); + experiment._doc.set('variables', newMap); + varsNode = experiment._doc.get('variables', true); + } + + // Build the scalar with the anchor attached, then assemble the pair. + const valueNode = experiment._doc.createNode(value); + valueNode.anchor = name; + const keyNode = experiment._doc.createNode(name); + varsNode.items.push(new YAML.Pair(keyNode, valueNode)); + + // Mirror + if (!Array.isArray(experiment.variables)) experiment.variables = []; + experiment.variables.push({ name: name, value: value }); +} + +/** + * docDeleteVariable(experiment, name, opts = { cascadeUnbind: false }) + * + * Remove the anchor's defining pair from the variables: map. If aliases + * reference this anchor: + * - opts.cascadeUnbind === false (default) → throws ANCHOR_HAS_REFS + * - opts.cascadeUnbind === true → unbinds each reference first (each + * becomes a literal with the resolved value) + * + * Mirrors the JS-side variable list. Aliased path mirrors are unchanged + * (the JS mirror always holds resolved values). + */ +function docDeleteVariable(experiment, name, opts) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docDeleteVariable: experiment has no _doc handle', 'NO_DOC'); + } + const cascade = !!(opts && opts.cascadeUnbind); + const found = _findVariablePair(experiment, name); + if (!found) { + throw new V3ParseError( + 'docDeleteVariable: no anchor named "' + name + '"', + 'BAD_PATH' + ); + } + const refs = findAliasesTo(experiment, name); + if (refs.length > 0 && !cascade) { + const err = new V3ParseError( + 'docDeleteVariable: anchor "' + name + '" still has ' + refs.length + + ' reference(s); pass {cascadeUnbind: true} to unbind them first', + 'ANCHOR_HAS_REFS' + ); + err.refCount = refs.length; + err.refs = refs; + throw err; + } + // Cascade-unbind: convert every alias to its literal value before + // removing the anchor. Walk the refs list AFTER capturing it (the doc + // walks are stable across mutations of unrelated nodes). + if (cascade) { + for (const ref of refs) { + docUnbindAnchor(experiment, ref.path); + } + } + found.varsNode.items.splice(found.index, 1); + + // Mirror + if (Array.isArray(experiment.variables)) { + const mirrorIdx = experiment.variables.findIndex((v) => v.name === name); + if (mirrorIdx >= 0) experiment.variables.splice(mirrorIdx, 1); + } +} + +/** + * docRenameVariable(experiment, oldName, newName) + * + * Atomic rename in a single synchronous pass: + * 1. Rewrite the anchor at the defining Scalar (`pair.value.anchor`). + * 2. Walk every Alias and rewrite `.source` where it matches oldName. + * 3. Update the JS mirror entry. + * + * Throws V3ParseError(INVALID_INPUT) on invalid newName or collision. + * BAD_PATH if oldName is not declared. + */ +function docRenameVariable(experiment, oldName, newName) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docRenameVariable: experiment has no _doc handle', 'NO_DOC'); + } + if (oldName === newName) return; + if (!isValidAnchorName(newName)) { + throw new V3ParseError( + 'docRenameVariable: invalid new anchor name ' + JSON.stringify(newName), + 'INVALID_INPUT' + ); + } + if (anchorExists(experiment, newName)) { + throw new V3ParseError( + 'docRenameVariable: anchor name "' + newName + '" is already in use', + 'INVALID_INPUT' + ); + } + const found = _findVariablePair(experiment, oldName); + if (!found) { + throw new V3ParseError( + 'docRenameVariable: no anchor named "' + oldName + '"', + 'BAD_PATH' + ); + } + // 1. Rename at the definition site. + if (found.pair.value) found.pair.value.anchor = newName; + + // 2. Cascade to every alias. Single YAML.visit pass. + if (typeof YAML.visit === 'function') { + YAML.visit(experiment._doc, { + Alias(_, node) { + if (node && node.source === oldName) { + node.source = newName; + } + } + }); + } + + // 3. Mirror. + if (Array.isArray(experiment.variables)) { + const mirrorIdx = experiment.variables.findIndex((v) => v.name === oldName); + if (mirrorIdx >= 0) experiment.variables[mirrorIdx].name = newName; + } +} + +/** + * docSetVariableValue(experiment, name, newValue) + * + * Change the scalar value AT the anchor's definition site. Does NOT + * touch aliases (they resolve dynamically). Critically, this MUST mutate + * the existing Scalar's `.value` in place rather than swap the value + * node — otherwise the anchor is lost. + * + * Updates the JS mirror (both the variable entry and every aliased + * path's resolved value). + */ +function docSetVariableValue(experiment, name, newValue) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docSetVariableValue: experiment has no _doc handle', 'NO_DOC'); + } + const found = _findVariablePair(experiment, name); + if (!found) { + throw new V3ParseError( + 'docSetVariableValue: no anchor named "' + name + '"', + 'BAD_PATH' + ); + } + if (variableIsComplex(experiment, name)) { + throw new V3ParseError( + 'docSetVariableValue: anchor "' + name + '" is complex (Map/Seq); ' + + 'use scalar values only or edit YAML directly', + 'INVALID_INPUT' + ); + } + // Mutate the Scalar in place so the anchor stays attached. + if (found.pair.value) found.pair.value.value = newValue; + + // Mirror the variable entry. + if (Array.isArray(experiment.variables)) { + const v = experiment.variables.find((v) => v.name === name); + if (v) v.value = newValue; + } + + // Mirror every aliased path's resolved value (JS mirror holds resolved). + const refs = findAliasesTo(experiment, name); + for (const ref of refs) { + mirrorIntoModel(experiment, ref.path, newValue); + } +} + +/** + * docBindToAnchor(experiment, path, anchorName) + * + * Replace the literal scalar at `path` with an Alias pointing to + * `anchorName`. The anchor must already exist (use docCreateVariable + * first to make a fresh one). Updates the JS mirror to the resolved + * value (the mirror never sees aliases). + */ +function docBindToAnchor(experiment, path, anchorName) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docBindToAnchor: experiment has no _doc handle', 'NO_DOC'); + } + if (!Array.isArray(path) || path.length === 0) { + throw new V3ParseError('docBindToAnchor: path must be a non-empty array', 'BAD_PATH'); + } + if (!isValidAnchorName(anchorName)) { + throw new V3ParseError( + 'docBindToAnchor: invalid anchor name ' + JSON.stringify(anchorName), + 'INVALID_INPUT' + ); + } + const varPair = _findVariablePair(experiment, anchorName); + if (!varPair) { + throw new V3ParseError( + 'docBindToAnchor: anchor "' + anchorName + '" is not declared in variables', + 'BAD_PATH' + ); + } + // Build the Alias node and stash it at path. + const alias = new YAML.Alias(anchorName); + experiment._doc.setIn(path, alias); + + // Mirror: resolved value from the anchor's scalar. + let resolved; + const v = varPair.pair.value; + if (v && v.value !== undefined) resolved = v.value; + else if (v && typeof v.toJSON === 'function') resolved = v.toJSON(); + else resolved = v; + mirrorIntoModel(experiment, path, resolved); +} + +/** + * docUnbindAnchor(experiment, path) + * + * If the node at `path` is an Alias, replace it with its resolved + * literal value. Throws NOT_ALIAS if the path doesn't currently hold + * an Alias — caller should check `nodeIsAliasAt` first. + */ +function docUnbindAnchor(experiment, path) { + if (!experiment || !experiment._doc) { + throw new V3ParseError('docUnbindAnchor: experiment has no _doc handle', 'NO_DOC'); + } + if (!Array.isArray(path) || path.length === 0) { + throw new V3ParseError('docUnbindAnchor: path must be a non-empty array', 'BAD_PATH'); + } + const node = experiment._doc.getIn(path, true); + const isAlias = node && (YAML.isAlias?.(node) || node.constructor?.name === 'Alias'); + if (!node || !isAlias) { + throw new V3ParseError( + 'docUnbindAnchor: path does not point to an Alias', + 'NOT_ALIAS' + ); + } + const anchorName = node.source; + const varPair = _findVariablePair(experiment, anchorName); + let resolved; + if (varPair) { + const v = varPair.pair.value; + if (v && v.value !== undefined) resolved = v.value; + else if (v && typeof v.toJSON === 'function') resolved = v.toJSON(); + else resolved = v; + } else { + // Dangling alias (shouldn't happen post-parse). Fall back to the + // path's current resolved JS-mirror value, if any. + resolved = experiment._doc.getIn(path); + } + experiment._doc.setIn(path, resolved); + // JS mirror was already holding the resolved value — no-op. +} + // ════════════════════════════════════════════════════ // Exports // ════════════════════════════════════════════════════ @@ -1379,7 +1794,18 @@ const ProtocolV3 = { docAddPluginParam, docDeletePluginParam, nodeIsAliasAt, - aliasNameAt + aliasNameAt, + // Phase 5 + docCreateVariable, + docDeleteVariable, + docRenameVariable, + docSetVariableValue, + docBindToAnchor, + docUnbindAnchor, + findAliasesTo, + variableIsComplex, + isValidAnchorName, + anchorExists }; // Browser global @@ -1417,6 +1843,17 @@ export { docAddPluginParam, docDeletePluginParam, nodeIsAliasAt, - aliasNameAt + aliasNameAt, + // Phase 5 + docCreateVariable, + docDeleteVariable, + docRenameVariable, + docSetVariableValue, + docBindToAnchor, + docUnbindAnchor, + findAliasesTo, + variableIsComplex, + isValidAnchorName, + anchorExists }; export default ProtocolV3; diff --git a/tests/test-protocol-roundtrip-v3.js b/tests/test-protocol-roundtrip-v3.js index 9c27012..0a418ae 100644 --- a/tests/test-protocol-roundtrip-v3.js +++ b/tests/test-protocol-roundtrip-v3.js @@ -45,7 +45,18 @@ const { docAddPluginParam, docDeletePluginParam, nodeIsAliasAt, - aliasNameAt + aliasNameAt, + // Phase 5 — anchor lifecycle + docCreateVariable, + docDeleteVariable, + docRenameVariable, + docSetVariableValue, + docBindToAnchor, + docUnbindAnchor, + findAliasesTo, + variableIsComplex, + isValidAnchorName, + anchorExists } = require('../js/protocol-yaml-v3.js'); const { @@ -1883,6 +1894,312 @@ checkThrows( 'INVALID_INPUT' ); +// ─── Test Suite 29: Variable lifecycle + anchor binding (Phase 5) ───────── +console.log('\n--- Suite 29: Variable lifecycle + anchor binding ---'); + +// Helper: canonical_a has 4 variables with anchors that match their map keys: +// dur_long: &dur_long 10 +// dur_short: &dur_short 3 +// color_command: &color_command "setRedLEDPower" +// color_power: &color_power 5 +// `*dur_long` is referenced in the trialParams duration for several trials. + +// 29.1 — docCreateVariable emits `&name: value` line in regen +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + docCreateVariable(exp, 'gain_low', 2); + const regen = generateV3Protocol(exp); + checkTrue('var-create: anchor line in regen', /&gain_low\b/.test(regen)); + checkTrue('var-create: value in regen', / 2\b/.test(regen)); + check('var-create: mirror has new entry name', exp.variables[exp.variables.length - 1].name, 'gain_low'); + check('var-create: mirror has new entry value', exp.variables[exp.variables.length - 1].value, 2); + // Re-parse confirms round-trip integrity + const exp2 = parseV3Protocol(regen); + checkTrue( + 'var-create: round-trip preserves variable', + exp2.variables.some((v) => v.name === 'gain_low' && v.value === 2) + ); +} + +// 29.2 — docCreateVariable rejects duplicate name +checkThrows( + 'var-create: rejects duplicate name', + () => { + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + docCreateVariable(exp, 'dur_long', 999); + }, + 'INVALID_INPUT' +); + +// 29.3 — docCreateVariable rejects invalid name +checkThrows( + 'var-create: rejects name with space', + () => { + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + docCreateVariable(exp, 'bad name', 1); + }, + 'INVALID_INPUT' +); +checkThrows( + 'var-create: rejects empty name', + () => { + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + docCreateVariable(exp, '', 1); + }, + 'INVALID_INPUT' +); + +// 29.4 — docDeleteVariable blocks when references exist +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + const refs = findAliasesTo(exp, 'dur_long'); + checkTrue('var-delete: dur_long has refs (precondition)', refs.length > 0, refs.length + ' refs'); + let threw = null; + try { + docDeleteVariable(exp, 'dur_long'); + } catch (e) { + threw = e; + } + checkTrue('var-delete: throws on referenced anchor', !!threw); + check('var-delete: error code', threw && threw.code, 'ANCHOR_HAS_REFS'); + check('var-delete: refCount on error', threw && threw.refCount, refs.length); +} + +// 29.5 — docDeleteVariable({cascadeUnbind:true}) cascades correctly +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + const refs = findAliasesTo(exp, 'dur_long'); + const refCount = refs.length; + docDeleteVariable(exp, 'dur_long', { cascadeUnbind: true }); + const regen = generateV3Protocol(exp); + checkTrue('var-delete-cascade: no anchor in regen', !/\*dur_long\b/.test(regen)); + checkTrue('var-delete-cascade: no anchor decl in regen', !/&dur_long\b/.test(regen)); + // Mirror: variable entry removed + checkTrue( + 'var-delete-cascade: mirror entry removed', + !exp.variables.some((v) => v.name === 'dur_long') + ); + // Reparse + count literal `duration: 10`s — should match the alias count + const exp2 = parseV3Protocol(regen); + checkTrue('var-delete-cascade: round-trip parses', exp2 != null); + // refCount-many former aliases now hold literal 10 + let literalCount = 0; + for (const cond of exp2.conditions) { + for (const cmd of cond.commands || []) { + if (cmd.duration === 10) literalCount++; + } + } + checkTrue( + 'var-delete-cascade: literals replaced aliases', + literalCount >= refCount, + literalCount + ' literals found, ' + refCount + ' refs originally' + ); +} + +// 29.6 / 29.7 — docRenameVariable updates anchor at definition + all aliases +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + const origRefs = findAliasesTo(exp, 'dur_long'); + docRenameVariable(exp, 'dur_long', 'duration_long'); + const regen = generateV3Protocol(exp); + checkTrue('var-rename: new anchor declared', /&duration_long\b/.test(regen)); + checkTrue('var-rename: old anchor gone', !/&dur_long\b/.test(regen)); + const oldAliasCount = (regen.match(/\*dur_long\b/g) || []).length; + const newAliasCount = (regen.match(/\*duration_long\b/g) || []).length; + check('var-rename: old aliases all gone', oldAliasCount, 0); + check('var-rename: new aliases match orig count', newAliasCount, origRefs.length); + // Mirror + check( + 'var-rename: mirror name updated', + exp.variables.find((v) => v.name === 'duration_long')?.value, + 10 + ); + checkTrue('var-rename: old name gone from mirror', !exp.variables.some((v) => v.name === 'dur_long')); +} + +// 29.8 — docRenameVariable rejects collision with existing anchor +checkThrows( + 'var-rename: blocks newName collision', + () => { + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + docRenameVariable(exp, 'dur_long', 'dur_short'); + }, + 'INVALID_INPUT' +); + +// 29.9 — docBindToAnchor converts literal to alias +{ + // Take an unbound literal scalar: conditions[0].commands[?].duration + // The first condition is "arena_check". Find a literal duration there. + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + // Find a literal-bearing path + let targetPath = null; + for (let ci = 0; ci < exp.conditions.length && !targetPath; ci++) { + const cmds = exp.conditions[ci].commands || []; + for (let cmdi = 0; cmdi < cmds.length; cmdi++) { + const p = ['conditions', ci, 'commands', cmdi, 'duration']; + if (cmds[cmdi].duration !== undefined && !nodeIsAliasAt(exp, p)) { + targetPath = p; + break; + } + } + } + checkTrue('var-bind: found a literal duration path (precondition)', !!targetPath); + if (targetPath) { + const before = (generateV3Protocol(exp).match(/\*dur_short\b/g) || []).length; + docBindToAnchor(exp, targetPath, 'dur_short'); + checkTrue('var-bind: path now Alias', nodeIsAliasAt(exp, targetPath)); + check('var-bind: aliasNameAt returns anchor', aliasNameAt(exp, targetPath), 'dur_short'); + const regen = generateV3Protocol(exp); + const after = (regen.match(/\*dur_short\b/g) || []).length; + check('var-bind: regen has one more alias', after, before + 1); + // Mirror: resolved to anchor value (3) + // Walk JS-mirror path + let cur = exp; + for (const k of targetPath) { + cur = cur[k === 'conditions' ? k : k]; // identity + // The above is a defensive walk + } + // Direct readout + const ci = targetPath[1], cmdi = targetPath[3]; + check('var-bind: mirror resolved to anchor value', exp.conditions[ci].commands[cmdi].duration, 3); + } +} + +// 29.10 — bind → unbind round-trip restores literal +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + // Find a literal duration path + let targetPath = null; + for (let ci = 0; ci < exp.conditions.length && !targetPath; ci++) { + const cmds = exp.conditions[ci].commands || []; + for (let cmdi = 0; cmdi < cmds.length; cmdi++) { + const p = ['conditions', ci, 'commands', cmdi, 'duration']; + if (cmds[cmdi].duration !== undefined && !nodeIsAliasAt(exp, p)) { + targetPath = p; + break; + } + } + } + if (targetPath) { + const origVal = exp._doc.getIn(targetPath); // resolved literal + docBindToAnchor(exp, targetPath, 'dur_short'); + checkTrue('var-bind-unbind: bound to alias', nodeIsAliasAt(exp, targetPath)); + docUnbindAnchor(exp, targetPath); + checkTrue('var-bind-unbind: alias removed', !nodeIsAliasAt(exp, targetPath)); + // After unbind, scalar holds the resolved value (3 for dur_short) + check('var-bind-unbind: final literal is anchor value', exp._doc.getIn(targetPath), 3); + } +} + +// 29.11 — docSetVariableValue preserves anchor; aliases still point to anchor +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + const origAliasCount = (generateV3Protocol(exp).match(/\*dur_long\b/g) || []).length; + docSetVariableValue(exp, 'dur_long', 99); + const regen = generateV3Protocol(exp); + checkTrue('var-setval: anchor declaration preserved', /&dur_long\b/.test(regen)); + checkTrue('var-setval: new value in regen', /&dur_long\s*99\b/.test(regen)); + const aliasCount = (regen.match(/\*dur_long\b/g) || []).length; + check('var-setval: alias count unchanged', aliasCount, origAliasCount); + // Mirror: variable + every aliased path got the new value + check('var-setval: variable mirror', exp.variables.find((v) => v.name === 'dur_long').value, 99); + // At least one aliased path: walk conditions for any duration === 99 + let found99 = false; + for (const c of exp.conditions) { + for (const cmd of c.commands || []) { + if (cmd.duration === 99) found99 = true; + } + } + checkTrue('var-setval: aliased mirror values updated to 99', found99); +} + +// 29.12 — findAliasesTo count matches manual grep +{ + const yamlText = readFixture('v3_canonical_a.yaml'); + const exp = parseV3Protocol(yamlText); + const refs = findAliasesTo(exp, 'dur_long'); + const grepCount = (yamlText.match(/\*dur_long\b/g) || []).length; + check('findAliasesTo: count matches grep', refs.length, grepCount); + checkTrue('findAliasesTo: each ref has a path', refs.every((r) => Array.isArray(r.path) && r.path.length > 0)); + checkTrue('findAliasesTo: each ref has a humanLabel', refs.every((r) => typeof r.humanLabel === 'string')); +} + +// 29.13 — Comments survive a rename +{ + const orig = readFixture('v3_canonical_a.yaml'); + const exp = parseV3Protocol(orig); + docRenameVariable(exp, 'color_command', 'led_command'); + const regen = generateV3Protocol(exp); + const origCommentLines = (orig.match(/^\s*#.*$/gm) || []).length; + const regenCommentLines = (regen.match(/^\s*#.*$/gm) || []).length; + check('var-rename: comment line count preserved', regenCommentLines, origCommentLines); +} + +// 29.14 — Merge keys cascade on rename (`<<: *foo`) +{ + // Construct a small YAML inline with a merge-key alias + const yamlText = [ + 'version: 3', + 'experiment_info: {name: x}', + 'rig: "/tmp/r.yaml"', + 'variables:', + ' base: &base_cfg {a: 1, b: 2}', + 'experiment:', + ' - "arena check"', + 'conditions:', + ' - name: "arena check"', + ' commands:', + ' - type: wait', + ' duration: 1', + ' params:', + ' <<: *base_cfg', + ' c: 3' + ].join('\n') + '\n'; + const exp = parseV3Protocol(yamlText); + // Confirm the alias is reachable via findAliasesTo + const refs = findAliasesTo(exp, 'base_cfg'); + checkTrue('var-merge: findAliasesTo finds merge-key alias', refs.length >= 1); + // Rename and verify cascade + docRenameVariable(exp, 'base_cfg', 'new_base'); + const regen = generateV3Protocol(exp); + checkTrue('var-merge: anchor renamed', /&new_base\b/.test(regen)); + checkTrue('var-merge: merge-key alias renamed', /\*new_base\b/.test(regen)); + checkTrue('var-merge: old anchor gone', !/&base_cfg\b/.test(regen)); + checkTrue('var-merge: old alias gone', !/\*base_cfg\b/.test(regen)); +} + +// 29.15 — variableIsComplex detects map/seq anchors +{ + // The merge-key fixture's `&base_cfg` is a map → complex + const yamlText = [ + 'version: 3', + 'experiment_info: {name: x}', + 'rig: "/tmp/r.yaml"', + 'variables:', + ' base: &base_cfg {a: 1}', + ' simple: &simple_n 42', + 'experiment: ["c"]', + 'conditions:', + ' - name: c', + ' commands: [{type: wait, duration: 1}]' + ].join('\n') + '\n'; + const exp = parseV3Protocol(yamlText); + checkTrue('var-complex: map anchor is complex', variableIsComplex(exp, 'base_cfg')); + checkTrue('var-complex: scalar anchor is not complex', !variableIsComplex(exp, 'simple_n')); +} + +// 29.16 — isValidAnchorName / anchorExists basics +checkTrue('valid-name: alphanum_dashes', isValidAnchorName('foo_bar-1')); +checkTrue('valid-name: rejects space', !isValidAnchorName('foo bar')); +checkTrue('valid-name: rejects empty', !isValidAnchorName('')); +checkTrue('valid-name: rejects non-string', !isValidAnchorName(42)); +{ + const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml')); + checkTrue('anchorExists: existing returns true', anchorExists(exp, 'dur_long')); + checkTrue('anchorExists: nonexistent returns false', !anchorExists(exp, 'no_such_anchor')); +} + // ─── Results ──────────────────────────────────────────────────────────────── console.log('\n=== Results: ' + passedTests + '/' + totalTests + ' passed ==='); if (failedTests.length > 0) { From 75a4b87d24d6fe16792fd790a9874d5ec49f120c Mon Sep 17 00:00:00 2001 From: Michael B Reiser Date: Thu, 28 May 2026 23:26:13 -0400 Subject: [PATCH 3/3] =?UTF-8?q?feat(v3):=20Phase=206=20=E2=80=94=20pre-exp?= =?UTF-8?q?ort=20validation=20modal,=20Reset,=20library=20delete=20(v0.11)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the Phase 5 Variables editor (de31235). Adds the deferred Phase 6 variables-adjacent work, all browser-verified this session. - collectBlockingErrors(experiment) in protocol-yaml-v3.js: a blocking sibling to collectExportWarnings. Composes validateReferences and adds two CST checks via YAML.visit — duplicate anchor names (yaml@2 accepts them silently, so they're counted) and dangling aliases (safety net for the in-memory mutation model). Exported from all three surfaces. - Pre-export validation modal: Export runs collectBlockingErrors first; on errors it shows a confirmModal listing each one with an "Export anyway" escape hatch. Soft warnings stay in the non-blocking banner. - Reset button (header): confirms, then loads the blank skeleton. Reversible via a new loadYamlText({ keepUndo: true }) option + pushUndo, so one Undo restores the prior doc. - Library-row delete (✕): blocked while usage > 0 ("remove from sequence first"); when unused, confirm then docDelete(['conditions', idx]). Clears selection if the deleted condition was selected. - Suite 30 (17 checks) covering the validator + library delete. Full suite 446/446 (arena 10, v2 137, v3 446). Footer bumped v0.10 -> v0.11. Browser-verified: the 6 Phase 5 checks (rename cascade, bind, unbind, create-and-bind, cascade-unbind delete, complex-anchor badge) plus the 3 Phase 6 checks (validation modal block/cancel, Reset + Undo, library delete blocked-when-used / works-when-unused). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/development/v3-editor-handoff-2.md | 76 +++++++++---- docs/development/v3-editor-handoff.md | 5 + experiment_designer_v3.html | 117 ++++++++++++++++++- js/protocol-yaml-v3.js | 68 +++++++++++ tests/test-protocol-roundtrip-v3.js | 145 ++++++++++++++++++++++++ 5 files changed, 386 insertions(+), 25 deletions(-) diff --git a/docs/development/v3-editor-handoff-2.md b/docs/development/v3-editor-handoff-2.md index 60e5c4c..e854622 100644 --- a/docs/development/v3-editor-handoff-2.md +++ b/docs/development/v3-editor-handoff-2.md @@ -1,8 +1,8 @@ # v3 Experiment Designer — Handoff for Next Session (Round 2) -**Last updated:** 2026-05-27 (Phase 5 session) -**Branch:** `main` at `c5cf223` → cleanup PR `#76` + Phase 5 PR (this session) -**Editor version:** v3 Experiment Designer **v0.10** (Phase 5 shipped this session) +**Last updated:** 2026-05-28 (Phase 6 session) +**Branch:** `phase5/variables-ux` (Phase 5 `de31235` + Phase 6 this session) → PR to `main` +**Editor version:** v3 Experiment Designer **v0.11** (Phase 6 shipped this session) **Pinned upstream:** maDisplayTools `origin/version3` at `649d7ef` This is the second handoff doc for the v3 designer. It supersedes the original @@ -194,15 +194,15 @@ fixtures, tests, and docs. ### What the editor still can't do (deferred work) -- Edit the **variables** section (anchor inline-edit, rename cascade, anchor - binding popover). - **Cross-library import** (D4) — design is on the shelf with a known fix list; deferred per Codex-adv concerns. -- **Pre-export blocking validation modal** with line numbers (currently - only the soft-warn banner). -- **Library row delete** (when usage = 0). -- **Reset button** (clear sequence + conditions + variables to defaults). +- **Pre-export validation line numbers** — the Phase 6 modal aggregates + blocking errors but does not yet map them to source line numbers. - **MATLAB-validation flow documentation** (Phase 8 in the original plan). + +*(Shipped since the original deferred list: variables editing — Phase 5; +pre-export blocking validation modal, library-row delete, Reset button — +Phase 6.)* - **Quickstart HTML doc** (Phase 9 in the original plan). - Comment preservation tests at strategic positions (Phase 7 in the plan). @@ -269,17 +269,50 @@ Complex anchors (maps, lists, merge keys `<<: *foo`) round-trip via table, and rename cascades work for them (the test suite covers a merge-key case). -### Tier 3: Phase 6 — full validation modal + Reset (~1 day) - -- Promote the soft-warn export banner to a full pre-export modal that - aggregates all errors (dup condition names, missing intertrial - referents, missing trial referents, alias references to missing - variables, duplicate anchor names, name-identity normalization). - Display with line numbers where possible. -- Library: delete blocked when usage > 0; "remove from sequence first?" - affordance. -- Reset button: clear sequence + conditions + variables to defaults - (one empty condition, sequence = `[ref to it]`). +### Tier 3: Phase 6 — validation modal + Reset + library delete ✅ SHIPPED (this session, v0.11) + +All three pieces landed on `phase5/variables-ux` after the Phase 5 commit: + +1. **Pre-export validation modal.** The Export button now runs a blocking + validator before writing. On errors it shows a `confirmModal` listing + each error with an "Export anyway" escape hatch (Cancel aborts the + download). Soft warnings stay in the non-blocking banner — the two + tiers are distinct. +2. **Library-row delete.** Each library row has a `✕` button. Deletion is + blocked while usage > 0 (shows "remove from sequence first"); when + unused, a confirm modal precedes `docDelete(['conditions', idx])`. + Selection clears if the deleted condition was selected. +3. **Reset button.** Header toolbar `⟲ Reset` confirms, then loads the + blank skeleton. Reversible: `pushUndo()` runs first and `loadYamlText` + gained a `{ keepUndo: true }` option, so a single Undo restores the + prior doc. + +**New validator:** `collectBlockingErrors(experiment)` in +`js/protocol-yaml-v3.js` — a blocking sibling to `collectExportWarnings`. +It *composes* `validateReferences` (folds in dup-condition-name and +missing-ref errors) and adds two CST checks via `YAML.visit`: **duplicate +anchor names** (yaml@2 silently accepts `&dup` twice, so it's counted) and +**dangling aliases** (an `*alias` whose anchor is gone — a safety net for +the in-memory mutation model, since a fully-dangling alias throws at +import). Returns `{ ok, errors }`, the same shape as `validateReferences`. +Exported from all three surfaces (ProtocolV3 object, named export, ESM +import in the HTML). + +**Test coverage:** Suite 30 (17 checks) in `tests/test-protocol-roundtrip-v3.js`. +Total: **446/446**. + +**Browser-verified (this session):** the 6 Phase 5 checks (rename cascade, +bind, unbind, create-and-bind, cascade-unbind delete, complex-anchor badge) +plus the 3 Phase 6 checks (validation modal blocks/cancels on duplicate +anchor, Reset collapses to skeleton and Undo restores, library delete +blocked-when-used / works-when-unused). + +> **Preview caching note:** `python -m http.server` lets the browser +> heuristically cache `js/*.js`, which breaks ES-module verification after +> edits (a stale import silently kills the whole module). A no-cache static +> server (`.claude/nocache-server.py`, untracked) on a fresh port avoids it. + +**Still deferred:** mapping validation errors to source **line numbers**. ### Tier 4: D4 — cross-library import (parked — ~5+ days when picked up) @@ -345,7 +378,8 @@ These should be resolved before the next session picks a direction: Plus inspection helpers: `nodeIsAliasAt`, `aliasNameAt`, `anchorExists`, `findAliasesTo`, `isValidAnchorName`, -`variableIsComplex`, `collectExportWarnings`, `validateReferences`. +`variableIsComplex`, `collectExportWarnings`, `validateReferences`, +`collectBlockingErrors` (Phase 6 — blocking pre-export validation). ### Helper anatomy (template for adding new ones) diff --git a/docs/development/v3-editor-handoff.md b/docs/development/v3-editor-handoff.md index 33d0d76..18eb7e4 100644 --- a/docs/development/v3-editor-handoff.md +++ b/docs/development/v3-editor-handoff.md @@ -1,5 +1,10 @@ # v3 Experiment Designer — Handoff for Next Session +> **⚠️ SUPERSEDED (2026-05-28).** This is the original handoff (covers up to +> v0.2). Most of its items have shipped. Read +> [`v3-editor-handoff-2.md`](v3-editor-handoff-2.md) instead — it has the +> current picture through Phase 6 (v0.11). This file is kept for history only. + **Last updated:** 2026-05-27 **Branch:** `main` at `6bacb46` **Pinned upstream:** maDisplayTools `origin/version3` at `649d7ef` diff --git a/experiment_designer_v3.html b/experiment_designer_v3.html index 4361578..f945699 100644 --- a/experiment_designer_v3.html +++ b/experiment_designer_v3.html @@ -429,6 +429,25 @@ color: var(--accent); border-color: var(--border); } + .lib-row .lib-row-del { + background: transparent; + border: 1px solid transparent; + color: var(--text-dim); + font-family: inherit; + font-size: 0.6rem; + padding: 0.05rem 0.3rem; + margin-left: 0.15rem; + border-radius: 3px; + cursor: pointer; + opacity: 0; + transition: opacity 0.1s; + } + .lib-row:hover .lib-row-del { opacity: 1; } + .lib-row .lib-row-del:hover { + background: var(--surface-2); + color: var(--error); + border-color: #c4514f; + } .zone-body { flex: 1; overflow-y: auto; @@ -1032,6 +1051,7 @@

v3 Experiment Designer

+