v3 Designer: validation line numbers + Phase 7 test hardening (v0.12)

docs/development/v3-editor-handoff-2.md

@@ -1,8 +1,8 @@
 # v3 Experiment Designer — Handoff for Next Session (Round 2)
 
-**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)
+**Last updated:** 2026-05-29 (Phase 6 + Phase 7 follow-up)
+**Branch:** `phase7/test-hardening-and-line-numbers` → PR to `main` (Phase 6 already merged via #77)
+**Editor version:** v3 Experiment Designer **v0.12** (validation line numbers + Phase 7 tests)
 **Pinned upstream:** maDisplayTools `origin/version3` at `649d7ef`
 
 This is the second handoff doc for the v3 designer. It supersedes the original
@@ -312,7 +312,16 @@ blocked-when-used / works-when-unused).
 > 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**.
+**Follow-up shipped (v0.12):** the validation modal now reports **source line
+numbers** for anchor errors — e.g. `Duplicate anchor name: "&dup" declared 2
+times (lines 5, 6)`. `collectBlockingErrors` re-parses `_doc.toString()` with a
+`YAML.LineCounter` and maps each node's `range[0]` to a line. Structural
+`validateReferences` errors stay line-less (model-based, no node handle).
+Dangling-alias errors are also line-less by nature: a dangling alias makes the
+doc non-serializable (`toString()` throws "Unresolved alias"), so the
+re-parse can't run — the check falls back to a range-less scan and still
+reports the error, just without a line. Also fixed a double-bullet in the
+modal list (callers prefix `• `, so the `<li>` disc marker is suppressed).
 
 ### Tier 4: D4 — cross-library import (parked — ~5+ days when picked up)
 
@@ -330,9 +339,12 @@ implementation:
 
 ### Tier 5: Phase 7-9 — polish (~1 day total)
 
-- Phase 7: comment-preservation tests at strategic positions, anchor edge
-  cases (numbers, strings, params, two anchors same value), randomized-block
-  semantics tests, validation error cases.
+- Phase 7 ✅ **largely done (v0.12)**: Suite 31 added comment-preservation at
+  strategic positions (head / section / inline / between), anchor edge cases
+  (two anchors same value, binding isolation), randomized-block semantics
+  (`randomize: true`/`false` round-trip), and validation line-number coverage.
+  Still open if wanted: params-level anchor cases, deeper validation-error
+  matrices. v3 suite now **467/467**.
 - Phase 8: write `docs/development/v3-matlab-validation.md` describing
   the MCP-driven MATLAB validation flow.
 - Phase 9: `experiment_designer_v3_quickstart.html` step-by-step

experiment_designer_v3.html

@@ -321,7 +321,10 @@
         }
         .confirm-modal .body ul {
             margin: 0;
-            padding-left: 1rem;
+            /* Callers prefix each item with a literal "• ", so suppress the
+               default <li> disc marker to avoid a double bullet. */
+            list-style: none;
+            padding-left: 0.25rem;
             font-size: 0.75rem;
             color: var(--text-dim);
             max-height: 240px;
@@ -1160,7 +1163,7 @@ <h1>v3 Experiment Designer</h1>
     <!-- Footer -->
     <div class="app-footer">
         <a href="https://github.com/reiserlab/webDisplayTools" target="_blank">Reiser Lab</a> |
-        v3 Experiment Designer v0.11 | <span id="footerTimestamp">2026-05-28 22:55 ET</span>
+        v3 Experiment Designer v0.12 | <span id="footerTimestamp">2026-05-29 00:08 ET</span>
     </div>
 
     <!-- Error modal -->

js/protocol-yaml-v3.js

@@ -453,37 +453,77 @@ function validateReferences(experiment) {
  * Returns { ok, errors } — the same shape as validateReferences, so callers can
  * treat the two interchangeably. Never throws (guards on _doc / YAML.visit).
  */
+// Format a " (line N)" / " (lines N, M)" suffix from a list of line numbers,
+// dropping any that couldn't be resolved. Returns '' when none are known.
+function _linesSuffix(lines) {
+    const known = (lines || []).filter((n) => typeof n === 'number');
+    if (known.length === 0) return '';
+    const uniq = [...new Set(known)].sort((a, b) => a - b);
+    return uniq.length === 1 ? ' (line ' + uniq[0] + ')' : ' (lines ' + uniq.join(', ') + ')';
+}
+
 function collectBlockingErrors(experiment) {
     // Fold in all structural reference errors first.
     const base = validateReferences(experiment);
     const errors = base.ok ? [] : base.errors.slice();
 
     // The two anchor checks need the CST; skip gracefully when it's absent.
     if (experiment && experiment._doc && typeof YAML.visit === 'function') {
-        // Duplicate anchor names — count every node.anchor across the doc tree.
-        // The `Node` visitor matches Scalar/Map/Seq (and Alias, which carries
-        // `.source` not `.anchor`, so it never contributes a count).
-        const anchorCounts = new Map();
-        YAML.visit(experiment._doc, {
+        // Re-parse the to-be-exported text with a LineCounter so error
+        // positions map to real line numbers in the export output. Mutated
+        // _doc nodes can lack source ranges; re-parsing _doc.toString() yields
+        // uniform, accurate positions against exactly what export will write.
+        // Falls back to a range-less scan of the live _doc if the yaml build
+        // lacks LineCounter/parseDocument or anything throws.
+        let scanDoc = experiment._doc;
+        let lineOf = () => null;
+        try {
+            if (typeof YAML.LineCounter === 'function' && typeof YAML.parseDocument === 'function') {
+                const lc = new YAML.LineCounter();
+                scanDoc = YAML.parseDocument(experiment._doc.toString(), { lineCounter: lc });
+                lineOf = (node) =>
+                    node && Array.isArray(node.range) && typeof node.range[0] === 'number'
+                        ? lc.linePos(node.range[0]).line
+                        : null;
+            }
+        } catch (_e) {
+            scanDoc = experiment._doc;
+            lineOf = () => null;
+        }
+
+        // Duplicate anchor names — collect every node.anchor across the doc
+        // tree with its line. The `Node` visitor matches Scalar/Map/Seq (and
+        // Alias, which carries `.source` not `.anchor`, so it never counts).
+        const anchorLines = new Map(); // name -> [line, ...]
+        YAML.visit(scanDoc, {
             Node(_, node) {
                 if (node && node.anchor) {
-                    anchorCounts.set(node.anchor, (anchorCounts.get(node.anchor) || 0) + 1);
+                    const lines = anchorLines.get(node.anchor) || [];
+                    lines.push(lineOf(node));
+                    anchorLines.set(node.anchor, lines);
                 }
             }
         });
-        const declaredAnchors = new Set(anchorCounts.keys());
-        for (const [name, count] of anchorCounts) {
-            if (count > 1) {
-                errors.push('Duplicate anchor name: "&' + name + '" declared ' + count + ' times');
+        const declaredAnchors = new Set(anchorLines.keys());
+        for (const [name, lines] of anchorLines) {
+            if (lines.length > 1) {
+                errors.push(
+                    'Duplicate anchor name: "&' +
+                        name +
+                        '" declared ' +
+                        lines.length +
+                        ' times' +
+                        _linesSuffix(lines)
+                );
             }
         }
 
         // Dangling aliases — an alias whose source has no anchor declaration
         // anywhere in the doc (variables:, conditions:, complex nodes — all
         // count as declared). Deduped so N references to one missing anchor
-        // produce a single error.
+        // produce a single error (reporting the first reference's line).
         const danglingSeen = new Set();
-        YAML.visit(experiment._doc, {
+        YAML.visit(scanDoc, {
             Alias(_, node) {
                 if (
                     node &&
@@ -493,7 +533,10 @@ function collectBlockingErrors(experiment) {
                 ) {
                     danglingSeen.add(node.source);
                     errors.push(
-                        'Dangling alias: "*' + node.source + '" has no matching anchor declaration'
+                        'Dangling alias: "*' +
+                            node.source +
+                            '" has no matching anchor declaration' +
+                            _linesSuffix([lineOf(node)])
                     );
                 }
             }

tests/test-protocol-roundtrip-v3.js

@@ -2345,6 +2345,156 @@ console.log('\n--- Suite 30: collectBlockingErrors + library delete ---');
     checkTrue('lib-del: round-trip still validates', collectBlockingErrors(reparsed).ok);
 }
 
+// ─── Test Suite 31: Phase 7 — comments, anchor edge cases, randomize, lines ──
+console.log('\n--- Suite 31: comment preservation + anchor edge cases + randomize + line numbers ---');
+
+// 31.1 — validation error line numbers: duplicate anchor reports its lines
+{
+    const yaml = [
+        'version: 3',
+        'experiment_info: {name: x}',
+        'rig: "/tmp/r.yaml"',
+        'variables:',
+        '  a: &dup 1',
+        '  b: &dup 2',
+        'experiment: [foo]',
+        'conditions:',
+        '  - name: foo',
+        '    commands: [{type: wait, duration: 1}]'
+    ].join('\n') + '\n';
+    const exp = parseV3Protocol(yaml);
+    const dupErr = collectBlockingErrors(exp).errors.find((e) => /Duplicate anchor/.test(e));
+    checkTrue('lines: duplicate-anchor error carries line numbers', /\(lines \d+, \d+\)/.test(dupErr), dupErr);
+}
+
+// 31.2 — dangling alias: still detected via graceful fallback. A dangling
+// alias makes the doc non-serializable (toString throws "Unresolved alias"),
+// so the line-number re-parse can't run — detection must still work without a
+// line number, and collectBlockingErrors must not throw.
+{
+    const yaml = [
+        'version: 3',
+        'experiment_info: {name: x}',
+        'rig: "/tmp/r.yaml"',
+        'variables:',
+        '  d: &d 7',
+        'experiment: [foo]',
+        'conditions:',
+        '  - name: foo',
+        '    commands: [{type: wait, duration: *d}]'
+    ].join('\n') + '\n';
+    const exp = parseV3Protocol(yaml);
+    const varsNode = exp._doc.get('variables', true);
+    for (const pair of varsNode.items) {
+        if (pair.value && pair.value.anchor === 'd') pair.value.anchor = undefined;
+    }
+    let report;
+    let threw = false;
+    try {
+        report = collectBlockingErrors(exp);
+    } catch (_e) {
+        threw = true;
+    }
+    checkTrue('lines: collectBlockingErrors survives unserializable doc', !threw);
+    checkTrue(
+        'lines: dangling alias still detected on fallback',
+        report && !report.ok && report.errors.some((e) => /Dangling alias.*\bd\b/.test(e))
+    );
+}
+
+// 31.3 — clean doc produces no spurious line-suffixed errors
+{
+    const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml'));
+    const report = collectBlockingErrors(exp);
+    checkTrue('lines: clean doc has no errors at all', report.ok && report.errors.length === 0);
+}
+
+// 31.4 — comment preservation at strategic positions (head / section / inline)
+{
+    const yaml = [
+        '# HEAD comment line',
+        'version: 3',
+        'experiment_info: {name: x}',
+        'rig: "/tmp/r.yaml"',
+        '# SECTION comment before variables',
+        'variables:',
+        '  speed: &speed 12  # INLINE trailing comment',
+        'experiment: [foo, bar]',
+        'conditions:',
+        '  # BETWEEN comment before first condition',
+        '  - name: foo',
+        '    commands: [{type: wait, duration: *speed}]',
+        '  - name: bar',
+        '    commands: [{type: wait, duration: 1}]'
+    ].join('\n') + '\n';
+    const regen = generateV3Protocol(parseV3Protocol(yaml));
+    checkTrue('comments: HEAD comment preserved', regen.includes('# HEAD comment line'));
+    checkTrue('comments: SECTION comment preserved', regen.includes('# SECTION comment before variables'));
+    checkTrue('comments: INLINE trailing comment preserved', regen.includes('# INLINE trailing comment'));
+    checkTrue('comments: BETWEEN comment preserved', regen.includes('# BETWEEN comment before first condition'));
+    // round-trip stability: comment count stable across a second pass
+    const c1 = regen.split('\n').filter((l) => l.includes('#')).length;
+    const c2 = generateV3Protocol(parseV3Protocol(regen)).split('\n').filter((l) => l.includes('#')).length;
+    check('comments: count stable on second round-trip', c2, c1);
+}
+
+// 31.5 — two distinct anchors with the SAME value both round-trip independently
+{
+    const yaml = [
+        'version: 3',
+        'experiment_info: {name: x}',
+        'rig: "/tmp/r.yaml"',
+        'variables:',
+        '  first: &first 5',
+        '  second: &second 5',
+        'experiment: [foo]',
+        'conditions:',
+        '  - name: foo',
+        '    commands:',
+        '      - {type: wait, duration: *first}',
+        '      - {type: wait, duration: *second}'
+    ].join('\n') + '\n';
+    const exp = parseV3Protocol(yaml);
+    const regen = generateV3Protocol(exp);
+    checkTrue('anchors-same-value: &first declared', /&first\b/.test(regen));
+    checkTrue('anchors-same-value: &second declared', /&second\b/.test(regen));
+    checkTrue('anchors-same-value: *first referenced', /\*first\b/.test(regen));
+    checkTrue('anchors-same-value: *second referenced', /\*second\b/.test(regen));
+    checkTrue('anchors-same-value: no blocking errors (not treated as dup)', collectBlockingErrors(exp).ok);
+}
+
+// 31.6 — binding one field leaves an unrelated anchor's references intact
+{
+    const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml'));
+    const shortBefore = findAliasesTo(exp, 'dur_short').length;
+    // bind some literal to dur_long; dur_short references must be untouched
+    const longRefs = findAliasesTo(exp, 'dur_long');
+    checkTrue('anchor-isolation: precondition dur_long has refs', longRefs.length > 0);
+    docBindToAnchor(exp, longRefs[0].path.slice(), 'dur_short'); // rebind one to dur_short via path
+    const shortAfter = findAliasesTo(exp, 'dur_short').length;
+    check('anchor-isolation: dur_short ref count increased by exactly 1', shortAfter, shortBefore + 1);
+}
+
+// 31.7 — randomize:true round-trips (canonical_a block)
+{
+    const exp = parseV3Protocol(readFixture('v3_canonical_a.yaml'));
+    const block = exp.sequence.find((e) => e.kind === 'block');
+    checkTrue('randomize: canonical block has randomize true', block && block.randomize === true);
+    const regen = generateV3Protocol(exp);
+    checkTrue('randomize: true survives in regen', /randomize:\s*true/.test(regen));
+}
+
+// 31.8 — randomize:false explicit is NOT dropped on round-trip
+{
+    const exp = parseV3Protocol(readFixture('v3_no_randomize.yaml'));
+    const block = exp.sequence.find((e) => e.kind === 'block');
+    checkTrue('randomize: no_randomize block parses randomize false', block && block.randomize === false);
+    const regen = generateV3Protocol(exp);
+    checkTrue('randomize: explicit false preserved in regen', /randomize:\s*false/.test(regen));
+    // repetitions survive too
+    checkTrue('randomize: repetitions preserved', block.repetitions === 2);
+}
+
 // ─── Results ────────────────────────────────────────────────────────────────
 console.log('\n=== Results: ' + passedTests + '/' + totalTests + ' passed ===');
 if (failedTests.length > 0) {