diff --git a/CLAUDE.md b/CLAUDE.md index f835d98..25fccc4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -662,6 +662,61 @@ documents D4 specifically. `docs/development/v3-d4-implementation-handoff.md`, `docs/development/v3-d4-design-reviews.md`. +## v3 Experiment Designer — Rig-Aware Plugins (#91) + #89 import fix + +`experiment_designer_v3.html` (v0.21+) reads the **rig YAML** the user picks via +Settings → Rig → Browse… and uses its `plugins:` block to drive the plugin UI, and +D4 import binds **canonical** rig plugin names instead of prefixing them (closes #89). + +### The rig→class mapping (in `js/plugin-registry.js`, pure, no YAML dep) +- `WELL_KNOWN_RIG_PLUGIN_NAMES = ['backlight', 'camera', 'temperature']` — the + canonical rig plugin names. **Note** the rig calls the thermometer `temperature` + while the registry key is `thermometer` (class `DAQThermometerPlugin`). +- `mapRigPluginToBuiltin(rigKey, rigType)` — **tolerant**: match the well-known + KEY first (`RIG_PLUGIN_KEY_MAP`), then a normalized `type` (`RIG_PLUGIN_TYPE_MAP`, + handles `"LED Controller"`, `"Bias"`/`"BIAS"`), else `null` ("unknown plugin type"). +- `deriveRigPlugins(rigData)` → `{ plugins:[{key,enabled,type,builtinName,matlabClass,mapped}], unmapped }`. + Never throws on null/partial input. +- `diffRigVsProtocol(derived, experiment.plugins)` → `{ unsupported, unused }`, + **name-based** (a plugin's declared name must equal the rig key to inherit config). +- `createPluginEntry(builtinName, overrideName)` — the optional override lets a rig + plugin be added under its canonical rig key (e.g. `temperature`) while reusing the + built-in's class/defaults. `rigDefined` fields (ip/port) already have empty defaults, + so rig-added plugins come out **config-less** by design. + +### Rig YAML entry point +- `parseRigYAMLText(text)` in `js/protocol-yaml-v3.js` — thin `YAML.parse` wrapper; + malformed/non-mapping input throws a clean `V3ParseError('RIG_PARSE_ERROR')`. The + HTML imports this because it doesn't import the `yaml` package directly. + +### HTML wiring +- Module state `loadedRig = { path, derived }`; `rigIsCurrent()` gates everything on + `loadedRig.path === experiment.rig_path` (a manual path edit makes it stale → ignored). +- The Browse handler is `async` (reads `await f.text()`), then **must call + `renderSettings()` itself** — `onRigEdit` doesn't re-render on success and re-picking + the same path is a no-op there. +- Add-plugin dropdown option values are namespaced: `rig:` (canonical add) vs + `registry:`. `onAddPlugin(value)` parses the prefix. +- Mismatch warnings render **inline in Settings → Plugins** (only when `rigIsCurrent()`). + +### #89 — canonical import binding (in `js/v3-import.js`) +- `_computePluginAction()` has a **canonical-name branch at the top**: if the source + plugin name is in `WELL_KNOWN_RIG_PLUGIN_NAMES` or `staging.rigPluginNames`, never + prefix — merge into an existing same-named target plugin, else add under the + canonical name (`{ action:'add', canonical:true }`). The well-known names are an + **always-on baseline**, so #89 is fixed even when no rig is loaded. +- `createStagingBuffer(src, file, { rigPluginNames })` threads extra loaded-rig names + (the HTML passes them from `loadedRig` when `rigIsCurrent()`). +- A `canonical` entry is **non-renamable** — `setPluginPlannedName` and + `setStagingPrefix` skip it; the import inspector renders "→ binds to rig plugin X". +- **When adding a new entry-point that prefixes plugin names, route through the + canonical-name check** so #89 doesn't regress. + +### Tests +- `tests/test-protocol-roundtrip-v3.js` suites **N12** (rig parse + tolerant map + + `diffRigVsProtocol`, using `tests/fixtures/rigs/*`) and **N13** (#89 canonical + import binding). Existing **N9** was updated to assert `camera` is added unprefixed. + ## Planning Best Practices ### Project Size Assessment diff --git a/experiment_designer_v3.html b/experiment_designer_v3.html index 761cbbb..943381a 100644 --- a/experiment_designer_v3.html +++ b/experiment_designer_v3.html @@ -1332,7 +1332,7 @@

v3 Experiment Designer

@@ -1385,7 +1385,8 @@

Import error

isValidAnchorName, anchorExists, docInsertPluginNode, - docRemovePlugin + docRemovePlugin, + parseRigYAMLText } from './js/protocol-yaml-v3.js'; import { @@ -1394,7 +1395,10 @@

Import error

listV3PluginNames, getV3CommandParams, BUILTIN_PLUGINS, - createPluginEntry + createPluginEntry, + mapRigPluginToBuiltin, + deriveRigPlugins, + diffRigVsProtocol } from './js/plugin-registry.js'; // D4 — cross-library import (Milestone 2 substrate, driven by Milestone 3 UI) @@ -1427,6 +1431,17 @@

Import error

let staging = null; let importMode = false; + // Rig-aware plugin assist (#91). Populated when the user Browse…s a rig + // YAML — browsers can't read the rig from its path string alone, so this + // only reflects a rig loaded in THIS session. `{ path, derived }` where + // `derived` = deriveRigPlugins(...). Treated as current only while + // `path === experiment.rig_path` (a manual path edit to a different file + // makes it stale → ignored). See rigIsCurrent(). + let loadedRig = null; + function rigIsCurrent() { + return !!(loadedRig && experiment && loadedRig.path === experiment.rig_path); + } + // Undo/redo: each snapshot is the YAML text + selection. Restoring // re-parses, so the doc/JS-model stay in sync. _restoring blocks // focus-on-rerender from polluting the stacks. @@ -1900,16 +1915,38 @@

Import error

title: 'Pick a .yaml file to fill in its name. The directory prefix is kept; browsers cannot read full filesystem paths.', onClick: () => rigFile.click(), }, 'Browse…'); - rigFile.addEventListener('change', () => { + rigFile.addEventListener('change', async () => { const f = rigFile.files && rigFile.files[0]; if (!f) return; const cur = experiment.rig_path || ''; const slash = cur.lastIndexOf('/'); const dir = slash >= 0 ? cur.slice(0, slash + 1) : ''; const newPath = dir + f.name; + // Parse the rig's plugins: block so the plugin UI can become + // rig-aware (#91). Assist, never block — on any failure we fall + // back to plain filename-fill and clear any cached rig. + let rigWarn = null; + try { + const text = await f.text(); + const rigData = parseRigYAMLText(text); + loadedRig = { path: newPath, derived: deriveRigPlugins(rigData) }; + } catch (err) { + loadedRig = null; + rigWarn = err && err.message ? err.message : String(err); + } rigInput.value = newPath; - onRigEdit(newPath); + onRigEdit(newPath); // commits the path (no-op if unchanged) rigFile.value = ''; + // onRigEdit doesn't re-render on success, and re-picking the same + // path is a no-op there — so refresh the drawer ourselves to reflect + // the freshly parsed rig (rig-aware dropdown + mismatch warnings). + renderSettings(); + if (rigWarn) { + showError('Could not read rig plugins', + 'Filled in the filename, but couldn\'t parse "' + f.name + + '" as a rig YAML:\n\n' + rigWarn + + '\n\nPlugin suggestions from the rig are unavailable.'); + } }); rigInner.appendChild(rigInput); rigInner.appendChild(browseBtn); @@ -1959,36 +1996,92 @@

Import error

pluginBox.appendChild(pluginCard); } + const declared = new Set(experiment.plugins.map((p) => p.name)); + + // Rig-vs-protocol mismatch warnings (#91). Non-blocking — assists, + // never rewrites. Only shown when a rig is loaded in this session + // and still matches the current rig path. + if (rigIsCurrent()) { + const diff = diffRigVsProtocol(loadedRig.derived.plugins, experiment.plugins); + if (diff.unsupported.length || diff.unused.length) { + const warnBox = el('div', { + style: 'margin: 0.4rem 0 0.2rem; padding: 0.4rem 0.55rem; ' + + 'background: rgba(255,152,0,0.08); border: 1px solid var(--warn); ' + + 'border-radius: 4px; color: var(--warn); font-size: 0.72rem; line-height: 1.5;' + }); + warnBox.appendChild(el('div', { style: 'font-weight: 600; margin-bottom: 0.2rem;' }, + '⚠ Rig mismatch (non-blocking)')); + for (const nm of diff.unsupported) { + warnBox.appendChild(el('div', { title: 'The loaded rig does not enable a plugin with this name' }, + '• "' + nm + '" is declared but not enabled on the loaded rig.')); + } + for (const key of diff.unused) { + warnBox.appendChild(el('div', { title: 'The rig enables this plugin but the protocol never declares it' }, + '• rig enables "' + key + '" but this protocol does not use it.')); + } + pluginBox.appendChild(warnBox); + } + } + // Add-plugin control (hidden in import mode — target doc locked). if (!importMode) { - const declared = new Set(experiment.plugins.map((p) => p.name)); - const available = Object.keys(BUILTIN_PLUGINS).filter((n) => !declared.has(n)); + // Rig-aware options (#91): when a rig is loaded, offer its + // enabled + mapped plugins under their canonical rig key + // (value "rig:"); built-in registry plugins fall to a + // second group (value "registry:"). With no rig loaded, + // it's the flat registry list (value "registry:"). + const rigCurrent = rigIsCurrent(); + const rigOpts = rigCurrent + ? loadedRig.derived.plugins.filter((p) => + p.enabled && p.mapped && !declared.has(p.key) && !declared.has(p.builtinName)) + : []; + const offeredBuiltins = new Set(rigOpts.map((p) => p.builtinName)); + const registryOpts = Object.keys(BUILTIN_PLUGINS).filter( + (n) => !declared.has(n) && !offeredBuiltins.has(n)); + const totalAvail = rigOpts.length + registryOpts.length; + const addRow = el('div', { style: 'display:flex; gap:0.3rem; margin-top:0.5rem;' }); const sel = el('select', { style: 'flex:1; background: var(--bg); color: var(--text); ' + 'border:1px solid var(--border); border-radius:4px; ' + 'padding:0.3rem 0.4rem; font: inherit;', - title: 'Choose a supported plugin to add to this protocol' + title: rigCurrent + ? 'Add a plugin — rig-supported plugins are listed first' + : 'Choose a supported plugin to add to this protocol' }); - if (available.length === 0) { + if (totalAvail === 0) { sel.appendChild(el('option', { value: '' }, 'All supported plugins already added')); sel.disabled = true; } else { sel.appendChild(el('option', { value: '' }, '+ Add plugin…')); - for (const n of available) { - const def = BUILTIN_PLUGINS[n]; - const cls = def.matlab ? def.matlab.class - : (def.python ? def.python.class : ''); - sel.appendChild(el('option', { value: n }, - n + (cls ? ' — ' + cls : ''))); + if (rigOpts.length) { + const grp = el('optgroup', { label: 'Supported by rig' }); + for (const p of rigOpts) { + grp.appendChild(el('option', { value: 'rig:' + p.key }, + p.key + (p.matlabClass ? ' — ' + p.matlabClass : ''))); + } + sel.appendChild(grp); + } + if (registryOpts.length) { + const groupEl = rigCurrent + ? el('optgroup', { label: 'Registry (not in rig)' }) + : sel; + for (const n of registryOpts) { + const def = BUILTIN_PLUGINS[n]; + const cls = def.matlab ? def.matlab.class + : (def.python ? def.python.class : ''); + groupEl.appendChild(el('option', { value: 'registry:' + n }, + n + (cls ? ' — ' + cls : ''))); + } + if (rigCurrent) sel.appendChild(groupEl); } } addRow.appendChild(sel); addRow.appendChild(el('button', { class: 'header-btn', - disabled: available.length === 0, + disabled: totalAvail === 0, title: 'Add the selected plugin (its commands become available immediately)', onClick: () => { if (sel.value) onAddPlugin(sel.value); } }, 'Add')); @@ -2752,8 +2845,14 @@

Import error

function enterImportMode(srcExperiment, filename) { let buf; + // Extra canonical plugin names from a loaded TARGET rig (#89). The + // well-known names (camera/backlight/temperature) are baked into + // v3-import; this only adds any further names the loaded rig declares. + const rigPluginNames = rigIsCurrent() + ? loadedRig.derived.plugins.map((p) => p.key) + : null; try { - buf = createStagingBuffer(srcExperiment, filename); + buf = createStagingBuffer(srcExperiment, filename, { rigPluginNames }); } catch (err) { // Duplicate-anchor source (and any other preflight failure) is a // hard block — surface it and stay in normal mode. @@ -2985,6 +3084,11 @@

Import error

if (entry.action === 'merge') { pluginSec.appendChild(el('div', { class: 'si-merge', title: 'Same class + config — reuses your existing plugin' }, srcName + ' → merges with your "' + entry.mergeWith + '"')); + } else if (entry.canonical) { + // Canonical rig name — bound un-prefixed so it inherits the + // rig's config; not renamable (would re-break binding, #89). + pluginSec.appendChild(el('div', { class: 'si-merge', title: 'Canonical rig plugin name — bound directly so it inherits the rig config' }, + srcName + ' → binds to rig plugin "' + entry.plannedName + '"')); } else { const row = el('div', { class: 'si-row' }); row.appendChild(el('span', { class: 'si-key' }, srcName + ' →')); @@ -4373,16 +4477,36 @@

Import error

// Add a supported plugin (from the registry) to the protocol's plugins: // list. Its commands become available in the "Add command" picker as // soon as it lands in experiment.plugins[] (listV3PluginNames reads it). - function onAddPlugin(pluginName) { - if (!experiment || importMode || !pluginName) return; - if (experiment.plugins.some((p) => p.name === pluginName)) { + // `value` is "rig:" (add under the canonical rig name, with the + // mapped built-in's class) or "registry:" (add the built-in + // under its own name). A bare "" is accepted as registry for safety. + function onAddPlugin(value) { + if (!experiment || importMode || !value) return; + let builtinName, entryName; + const ci = value.indexOf(':'); + const kind = ci >= 0 ? value.slice(0, ci) : 'registry'; + const ident = ci >= 0 ? value.slice(ci + 1) : value; + if (kind === 'rig') { + const mapped = mapRigPluginToBuiltin(ident); + if (!mapped) { + showError('Add plugin failed', + 'The rig plugin "' + ident + '" has no known plugin class.'); + return; + } + builtinName = mapped.builtinName; + entryName = ident; // bind to the canonical rig name + } else { + builtinName = ident; + entryName = ident; + } + if (experiment.plugins.some((p) => p.name === entryName)) { showError('Add plugin failed', - 'A plugin named "' + pluginName + '" is already declared.'); + 'A plugin named "' + entryName + '" is already declared.'); return; } - const entry = createPluginEntry(pluginName); + const entry = createPluginEntry(builtinName, entryName); if (!entry) { - showError('Add plugin failed', 'Unknown plugin: ' + pluginName); + showError('Add plugin failed', 'Unknown plugin: ' + builtinName); return; } pushUndo(); diff --git a/js/plugin-registry.js b/js/plugin-registry.js index 5c0946c..fe73d2d 100644 --- a/js/plugin-registry.js +++ b/js/plugin-registry.js @@ -552,15 +552,18 @@ function getAllCommandOptions(enabledPlugins) { /** * Create a default plugin config object from a builtin plugin definition. * - * @param {string} pluginName - builtin plugin name + * @param {string} pluginName - builtin plugin name (registry key) + * @param {string} [overrideName] - optional name for the entry. Used when adding + * a plugin under its canonical RIG key (e.g. the rig calls the thermometer + * `temperature` while the registry key is `thermometer`). Defaults to def.name. * @returns {object} plugin definition suitable for experiment.plugins[] */ -function createPluginEntry(pluginName) { +function createPluginEntry(pluginName, overrideName) { var def = BUILTIN_PLUGINS[pluginName]; if (!def) return null; var entry = { - name: def.name, + name: overrideName != null && overrideName !== '' ? String(overrideName) : def.name, type: def.type }; if (def.matlab) { @@ -670,6 +673,136 @@ function getV3CommandParams(experiment, type, pluginName, commandName) { return null; } +// ════════════════════════════════════════════════════ +// Rig-aware plugin mapping (#91 / #89) +// ════════════════════════════════════════════════════ +// Rig YAMLs declare the plugins a rig provides under a `plugins:` map, keyed by +// canonical names (`backlight`, `camera`, `temperature`). An experiment's +// `plugins:` MUST use those same names to inherit the rig's connection config — +// so these are also the names D4 import must never prefix. Rig schemas drift +// (`type: "LED Controller"` vs `com_port`; camera `"Bias"` vs `"BIAS"`), so the +// rig→class mapping is tolerant: match the well-known key first, fall back to a +// normalized `type`, and degrade gracefully ("unknown plugin type") when unmapped. + +// Canonical rig plugin names — never namespaced on import (the #89 baseline). +var WELL_KNOWN_RIG_PLUGIN_NAMES = ['backlight', 'camera', 'temperature']; + +// Well-known rig KEY → built-in registry key. Note the rig calls the thermometer +// `temperature`; the registry key is `thermometer`. +var RIG_PLUGIN_KEY_MAP = { + backlight: 'backlight', + camera: 'camera', + temperature: 'thermometer', + thermometer: 'thermometer' +}; + +// Normalized `type` string → built-in registry key (fallback when the rig key +// isn't well-known). Keys are lowercased + stripped of spaces/punctuation. +var RIG_PLUGIN_TYPE_MAP = { + ledcontroller: 'backlight', + bias: 'camera', + daqthermometer: 'thermometer', + thermometer: 'thermometer', + temperature: 'thermometer' +}; + +function _normalizeRigType(rigType) { + if (rigType == null) return ''; + return String(rigType) + .toLowerCase() + .replace(/[^a-z0-9]/g, ''); +} + +/** + * Map a rig plugin (by its key and optional `type`) to a built-in registry + * definition. Key match first, then normalized type. Returns + * `{ builtinName, def }` or null when unmapped. + */ +function mapRigPluginToBuiltin(rigKey, rigType) { + var key = (rigKey == null ? '' : String(rigKey)).toLowerCase(); + if (RIG_PLUGIN_KEY_MAP[key]) { + var bn = RIG_PLUGIN_KEY_MAP[key]; + return { builtinName: bn, def: BUILTIN_PLUGINS[bn] }; + } + var t = _normalizeRigType(rigType); + if (t && RIG_PLUGIN_TYPE_MAP[t]) { + var bn2 = RIG_PLUGIN_TYPE_MAP[t]; + return { builtinName: bn2, def: BUILTIN_PLUGINS[bn2] }; + } + return null; +} + +/** + * Derive the plugins a rig supports from a parsed rig YAML object. + * + * Tolerant by design — a null/partial `rigData`, or a missing/non-object + * `plugins:` block, yields an empty result rather than throwing. + * + * @param {object} rigData - parsed rig YAML (e.g. from parseRigYAMLText) + * @returns {{ plugins: Array, unmapped: string[] }} one entry per declared rig + * plugin: { key, enabled, type, builtinName, matlabClass, mapped }. `unmapped` + * lists keys with no recognized class. + */ +function deriveRigPlugins(rigData) { + var result = { plugins: [], unmapped: [] }; + if (!rigData || typeof rigData !== 'object') return result; + var pluginsObj = rigData.plugins; + if (!pluginsObj || typeof pluginsObj !== 'object') return result; + var keys = Object.keys(pluginsObj); + for (var i = 0; i < keys.length; i++) { + var key = keys[i]; + var cfg = pluginsObj[key] && typeof pluginsObj[key] === 'object' ? pluginsObj[key] : {}; + var type = cfg.type != null ? cfg.type : null; + var mapped = mapRigPluginToBuiltin(key, type); + result.plugins.push({ + key: key, + enabled: cfg.enabled === true, + type: type, + builtinName: mapped ? mapped.builtinName : null, + matlabClass: mapped && mapped.def.matlab ? mapped.def.matlab.class : null, + mapped: !!mapped + }); + if (!mapped) result.unmapped.push(key); + } + return result; +} + +/** + * Compare a rig's derived plugins against the protocol's declared plugins. + * Name-based, because a plugin's declared name MUST equal the rig key to inherit + * config (so a name mismatch is a real, actionable flag). + * + * @param {Array} derivedPlugins - deriveRigPlugins(...).plugins + * @param {Array} experimentPlugins - experiment.plugins + * @returns {{ unsupported: string[], unused: string[] }} + * unsupported = declared plugin names (excluding `log`) that aren't an ENABLED rig key; + * unused = ENABLED rig keys the protocol never declares. + */ +function diffRigVsProtocol(derivedPlugins, experimentPlugins) { + var derived = Array.isArray(derivedPlugins) ? derivedPlugins : []; + var declared = Array.isArray(experimentPlugins) ? experimentPlugins : []; + var enabledRigKeys = {}; + for (var i = 0; i < derived.length; i++) { + if (derived[i] && derived[i].enabled) enabledRigKeys[derived[i].key] = true; + } + var declaredNames = {}; + for (var j = 0; j < declared.length; j++) { + if (declared[j] && declared[j].name != null) declaredNames[declared[j].name] = true; + } + var unsupported = []; + for (var k = 0; k < declared.length; k++) { + var nm = declared[k] && declared[k].name; + if (nm == null || nm === 'log') continue; + if (!enabledRigKeys[nm]) unsupported.push(nm); + } + var unused = []; + for (var m = 0; m < derived.length; m++) { + var d = derived[m]; + if (d && d.enabled && !declaredNames[d.key]) unused.push(d.key); + } + return { unsupported: unsupported, unused: unused }; +} + // ════════════════════════════════════════════════════ // Exports // ════════════════════════════════════════════════════ @@ -678,6 +811,7 @@ var PluginRegistry = { BUILTIN_PLUGINS: BUILTIN_PLUGINS, CONTROLLER_COMMANDS: CONTROLLER_COMMANDS, LOG_PLUGIN: LOG_PLUGIN, + WELL_KNOWN_RIG_PLUGIN_NAMES: WELL_KNOWN_RIG_PLUGIN_NAMES, getPluginCommands: getPluginCommands, getCommandParams: getCommandParams, getAllCommandOptions: getAllCommandOptions, @@ -686,7 +820,10 @@ var PluginRegistry = { getCommandsForClass: getCommandsForClass, getV3PluginCommands: getV3PluginCommands, listV3PluginNames: listV3PluginNames, - getV3CommandParams: getV3CommandParams + getV3CommandParams: getV3CommandParams, + mapRigPluginToBuiltin: mapRigPluginToBuiltin, + deriveRigPlugins: deriveRigPlugins, + diffRigVsProtocol: diffRigVsProtocol }; // Browser global @@ -704,6 +841,7 @@ export { BUILTIN_PLUGINS, CONTROLLER_COMMANDS, LOG_PLUGIN, + WELL_KNOWN_RIG_PLUGIN_NAMES, getPluginCommands, getCommandParams, getAllCommandOptions, @@ -712,6 +850,9 @@ export { getCommandsForClass, getV3PluginCommands, listV3PluginNames, - getV3CommandParams + getV3CommandParams, + mapRigPluginToBuiltin, + deriveRigPlugins, + diffRigVsProtocol }; export default PluginRegistry; diff --git a/js/protocol-yaml-v3.js b/js/protocol-yaml-v3.js index b47a7bb..78fbf2d 100644 --- a/js/protocol-yaml-v3.js +++ b/js/protocol-yaml-v3.js @@ -196,6 +196,36 @@ function parseV3Protocol(yamlText) { return experiment; } +/** + * Parse a RIG YAML string into a plain JS object (rig-aware plugin support, #91). + * + * Rig files are a different schema from protocols — this is deliberately a thin, + * tolerant wrapper over YAML.parse that returns the raw object so callers + * (deriveRigPlugins) can read its `plugins:` block. The HTML imports this because + * it doesn't import the `yaml` package directly. Malformed YAML throws a clean + * V3ParseError (RIG_PARSE_ERROR) instead of a raw module error. + * + * @param {string} yamlText - raw rig YAML content + * @returns {object} parsed rig object (may be {} for empty/comment-only files) + * @throws V3ParseError + */ +function parseRigYAMLText(yamlText) { + if (typeof yamlText !== 'string') { + throw new V3ParseError('Expected rig YAML text, got ' + typeof yamlText, 'RIG_PARSE_ERROR'); + } + let data; + try { + data = YAML.parse(yamlText); + } catch (err) { + throw new V3ParseError('Rig YAML parse error: ' + err.message, 'RIG_PARSE_ERROR'); + } + if (data == null) return {}; + if (typeof data !== 'object' || Array.isArray(data)) { + throw new V3ParseError('Rig YAML root must be a mapping', 'RIG_PARSE_ERROR'); + } + return data; +} + function extractExperimentInfo(info) { const out = {}; for (const k of KNOWN_EXPERIMENT_INFO_KEYS) { @@ -2094,6 +2124,7 @@ function docRemovePlugin(experiment, pluginName) { const ProtocolV3 = { parseV3Protocol, + parseRigYAMLText, generateV3Protocol, validateReferences, collectBlockingErrors, @@ -2150,6 +2181,7 @@ if (typeof module !== 'undefined' && module.exports) { // ES module export export { parseV3Protocol, + parseRigYAMLText, generateV3Protocol, validateReferences, collectBlockingErrors, diff --git a/js/v3-import.js b/js/v3-import.js index 56e04f5..753bdd7 100644 --- a/js/v3-import.js +++ b/js/v3-import.js @@ -38,6 +38,7 @@ import { isValidAnchorName, anchorExists } from './protocol-yaml-v3.js'; +import { WELL_KNOWN_RIG_PLUGIN_NAMES } from './plugin-registry.js'; // Command types the v3 schema knows (mirrors KNOWN_COMMAND_KEYS_BY_TYPE in // protocol-yaml-v3.js). Anything else in a condition is an "unknown command @@ -362,12 +363,17 @@ function createStagingBuffer(srcExperiment, srcFilename, opts) { } const options = opts || {}; const prefix = options.prefix !== undefined ? options.prefix : derivePrefix(srcFilename); + // Extra canonical plugin names from a loaded TARGET rig (#89). The well-known + // names (camera/backlight/temperature) are always treated as canonical; this + // set adds any further plugin names the user's loaded rig declares. + const rigPluginNames = new Set(Array.isArray(options.rigPluginNames) ? options.rigPluginNames : []); return { src: { doc: srcExperiment._doc, experiment: srcExperiment, filename: srcFilename || '' }, prefix: prefix, + rigPluginNames: rigPluginNames, batch: { anchorRegistry: new Map(), // srcName → { srcValueNode, plannedName, override } - pluginRegistry: new Map(), // srcName → { srcEntryNode, plannedName, override, action, mergeWith } + pluginRegistry: new Map(), // srcName → { srcEntryNode, plannedName, override, action, mergeWith, canonical } visitedAnchors: new Set() }, items: [], @@ -376,6 +382,13 @@ function createStagingBuffer(srcExperiment, srcFilename, opts) { }; } +// True when `name` is a rig-canonical plugin name that must never be prefixed on +// import — a well-known rig key, or one the loaded target rig declares (#89). +function _isCanonicalRigName(staging, name) { + if (WELL_KNOWN_RIG_PLUGIN_NAMES.indexOf(name) !== -1) return true; + return !!(staging.rigPluginNames && staging.rigPluginNames.has(name)); +} + // Walk a source value/condition node, registering every transitively-referenced // anchor into the shared registry (visited-set closure, #5). function _closeAnchors(staging, seedNode) { @@ -403,6 +416,17 @@ function _closeAnchors(staging, seedNode) { // Decide merge-vs-namespace for a source plugin against yours (broadened identity). function _computePluginAction(staging, srcPluginMirror, yoursExperiment) { + const name = srcPluginMirror.name; + // #89: the rig defines canonical plugin names; never prefix one. Bind to the + // canonical name so the plugin inherits the target rig's config at runtime — + // even when the target protocol doesn't declare it yet. + if (_isCanonicalRigName(staging, name)) { + const existing = (yoursExperiment.plugins || []).find((p) => p.name === name); + if (existing) { + return { action: 'merge', mergeWith: existing.name, plannedName: existing.name }; + } + return { action: 'add', mergeWith: null, plannedName: name, canonical: true }; + } const srcProj = sortedJson(_pluginProjection(srcPluginMirror)); const srcRig = staging.src.experiment.rig_path; const yoursRig = yoursExperiment.rig_path; @@ -480,7 +504,8 @@ function addToStaging(staging, sourceCondIdx, yoursExperiment) { plannedName: act.plannedName, override: false, action: act.action, - mergeWith: act.mergeWith + mergeWith: act.mergeWith, + canonical: !!act.canonical }); // plugin config may reference anchors the condition doesn't if (act.action === 'add') _closeAnchors(staging, srcEntryNode); @@ -558,7 +583,8 @@ function _rebuildRegistry(staging) { plannedName: act.plannedName, override: false, action: act.action, - mergeWith: act.mergeWith + mergeWith: act.mergeWith, + canonical: !!act.canonical }); if (act.action === 'add') _closeAnchors(staging, srcEntryNode); } @@ -592,7 +618,10 @@ function setStagingPrefix(staging, newPrefix) { if (!entry.override) entry.plannedName = staging.prefix + name; } for (const [name, entry] of staging.batch.pluginRegistry) { - if (entry.action === 'add' && !entry.override) entry.plannedName = staging.prefix + name; + // Canonical rig binds keep their (un-prefixed) name regardless of prefix (#89). + if (entry.action === 'add' && !entry.override && !entry.canonical) { + entry.plannedName = staging.prefix + name; + } } if (staging._yours) refreshStagingValidation(staging, staging._yours); return staging; @@ -622,6 +651,8 @@ function setAnchorPlannedName(staging, srcAnchorName, newName) { function setPluginPlannedName(staging, srcPluginName, newName) { const entry = staging.batch.pluginRegistry.get(srcPluginName); if (!entry || entry.action !== 'add') return staging; + // Canonical rig binds are non-renamable — renaming would re-break rig binding (#89). + if (entry.canonical) return staging; entry.plannedName = String(newName); entry.override = true; if (staging._yours) refreshStagingValidation(staging, staging._yours); diff --git a/tests/test-protocol-roundtrip-v3.js b/tests/test-protocol-roundtrip-v3.js index f93173f..a1fa29c 100644 --- a/tests/test-protocol-roundtrip-v3.js +++ b/tests/test-protocol-roundtrip-v3.js @@ -63,7 +63,8 @@ const { docInsertConditionNode, docInsertVariableNode, docInsertPluginNode, - docRemovePlugin + docRemovePlugin, + parseRigYAMLText } = require('../js/protocol-yaml-v3.js'); const { @@ -73,7 +74,10 @@ const { listV3PluginNames, getV3CommandParams, createPluginEntry, - LOG_PLUGIN + LOG_PLUGIN, + mapRigPluginToBuiltin, + deriveRigPlugins, + diffRigVsProtocol } = require('../js/plugin-registry.js'); // D4 — cross-document primitives (M1) + staging/commit pipeline (M2) @@ -3149,7 +3153,10 @@ console.log('\n--- Suite N9: edge cases ---'); const st2 = createStagingBuffer(src, 'sib.yaml'); addToStaging(st2, 0, noPlugins); // sibling check uses camera commitStaging(st2, noPlugins); - checkTrue('N9: plugins: section created + camera added', noPlugins.plugins.some((p) => p.name === 'sib__camera' || p.name === 'sibling_source__camera')); + // #89: `camera` is a canonical rig name — it must be added UN-prefixed so it + // inherits the target rig's config at runtime (was sib__camera). + checkTrue('N9: plugins: section created + camera added (canonical, unprefixed)', noPlugins.plugins.some((p) => p.name === 'camera')); + checkTrue('N9: camera NOT namespaced (#89)', !noPlugins.plugins.some((p) => /__camera$/.test(p.name))); checkTrue('N9: no-plugins target re-parses', (() => { try { parseV3Protocol(noPlugins._doc.toString()); return true; } catch (e) { return false; } })()); // zero-alias condition → imports clean, no anchors @@ -3327,6 +3334,167 @@ console.log('\n--- Suite N11: add/remove plugin from registry ---'); pass('N11: no-plugins-key protocol re-parses after add'); } +// ════════════════════════════════════════════════════════════════════════════ +// Rig-aware plugin assist (#91) + canonical-name import binding (#89) +// ════════════════════════════════════════════════════════════════════════════ + +// ─── Suite N12: rig YAML parse + tolerant rig→class mapping ────────────────── +console.log('\n--- Suite N12: rig-aware plugin parse + mapping ---'); +{ + const findKey = (derived, key) => derived.plugins.find((p) => p.key === key); + + // (a) test_rig_1 — richest case: backlight + camera ENABLED with type/config. + const r1 = deriveRigPlugins(parseRigYAMLText(readFixture('rigs/test_rig_1.yaml'))); + check('N12a: test_rig_1 has 3 plugins', r1.plugins.length, 3); + check('N12a: backlight → LEDControllerPlugin', findKey(r1, 'backlight').matlabClass, 'LEDControllerPlugin'); + checkTrue('N12a: backlight enabled', findKey(r1, 'backlight').enabled === true); + check('N12a: camera (type "Bias") → BiasPlugin', findKey(r1, 'camera').matlabClass, 'BiasPlugin'); + checkTrue('N12a: camera enabled', findKey(r1, 'camera').enabled === true); + check('N12a: temperature → DAQThermometerPlugin', findKey(r1, 'temperature').matlabClass, 'DAQThermometerPlugin'); + checkTrue('N12a: temperature disabled', findKey(r1, 'temperature').enabled === false); + check('N12a: nothing unmapped', r1.unmapped.length, 0); + + // (b) example_rig — schema variation: backlight has NO type (com_port/ir_power), + // camera type is "BIAS" (different casing). Tolerant mapping must still resolve. + const r2 = deriveRigPlugins(parseRigYAMLText(readFixture('rigs/example_rig.yaml'))); + check('N12b: backlight maps via KEY despite missing type', findKey(r2, 'backlight').matlabClass, 'LEDControllerPlugin'); + check('N12b: camera "BIAS" (casing) → BiasPlugin', findKey(r2, 'camera').matlabClass, 'BiasPlugin'); + check('N12b: temperature maps via key', findKey(r2, 'temperature').matlabClass, 'DAQThermometerPlugin'); + checkTrue('N12b: all disabled in example_rig', r2.plugins.every((p) => p.enabled === false)); + + // (c) test_rig_2 — minimal: all disabled, no config, still mapped via key. + const r3 = deriveRigPlugins(parseRigYAMLText(readFixture('rigs/test_rig_2.yaml'))); + check('N12c: test_rig_2 has 3 plugins', r3.plugins.length, 3); + checkTrue('N12c: all mapped via key', r3.plugins.every((p) => p.mapped)); + checkTrue('N12c: all disabled', r3.plugins.every((p) => p.enabled === false)); + + // (d) tolerant fallback + unknown degradation + check('N12d: mapRigPluginToBuiltin("camera") → camera', mapRigPluginToBuiltin('camera').builtinName, 'camera'); + check('N12d: mapRigPluginToBuiltin("temperature") → thermometer', mapRigPluginToBuiltin('temperature').builtinName, 'thermometer'); + check('N12d: unknown key + known TYPE falls back', mapRigPluginToBuiltin('cam2', 'LED Controller').builtinName, 'backlight'); + checkTrue('N12d: unknown key + unknown type → null', mapRigPluginToBuiltin('zorp', 'Quantum Flux') === null); + const weird = deriveRigPlugins(parseRigYAMLText('plugins:\n zorp:\n enabled: true\n type: "Quantum Flux"\n')); + checkTrue('N12d: unmapped plugin surfaced (mapped=false)', findKey(weird, 'zorp').mapped === false); + checkTrue('N12d: unmapped key listed in .unmapped', weird.unmapped.includes('zorp')); + + // (e) graceful degradation — never throws on null/partial input + check('N12e: deriveRigPlugins(null) → empty', deriveRigPlugins(null).plugins.length, 0); + check('N12e: deriveRigPlugins({}) → empty', deriveRigPlugins({}).plugins.length, 0); + check('N12e: deriveRigPlugins({plugins:null}) → empty', deriveRigPlugins({ plugins: null }).plugins.length, 0); + check('N12e: empty rig text parses to {}', JSON.stringify(parseRigYAMLText('# only a comment\n')), '{}'); + checkThrows('N12e: malformed rig YAML throws RIG_PARSE_ERROR', () => parseRigYAMLText('a: [b: c: d'), 'RIG_PARSE_ERROR'); + checkThrows('N12e: non-mapping rig root throws', () => parseRigYAMLText('- a\n- b\n'), 'RIG_PARSE_ERROR'); + + // (f) diffRigVsProtocol — mismatch detection (name-based) + const d1 = diffRigVsProtocol(r1.plugins, [{ name: 'backlight' }]); + check('N12f: backlight declared → not unsupported', d1.unsupported.length, 0); + checkTrue('N12f: camera enabled-but-unused flagged', d1.unused.includes('camera')); + checkTrue('N12f: disabled temperature not flagged as unused', !d1.unused.includes('temperature')); + const d2 = diffRigVsProtocol(r1.plugins, [{ name: 'thermometer' }]); + checkTrue('N12f: thermometer name not an enabled rig key → unsupported', d2.unsupported.includes('thermometer')); + const d3 = diffRigVsProtocol(r1.plugins, [{ name: 'log' }, { name: 'camera' }, { name: 'backlight' }]); + checkTrue('N12f: log excluded from unsupported', !d3.unsupported.includes('log')); + check('N12f: all enabled rig plugins declared → none unused', d3.unused.length, 0); +} + +// ─── Suite N13: D4 import binds canonical rig plugin names (#89) ───────────── +console.log('\n--- Suite N13: import canonical plugin binding (#89) ---'); +{ + const camSrc = (name) => + mkProtocol({ + name: name || 'theirs', + rig: '/their/rig.yaml', + plugins: 'plugins:\n - {name: camera, type: class, matlab: {class: BiasPlugin}}', + conditions: + ' - name: cam trial\n commands:\n - {type: plugin, plugin_name: camera, command_name: getTimestamp}' + }); + + // (a) camera-less target: canonical `camera` is added UN-prefixed, never namespaced. + const src = parseV3Protocol(camSrc()); + const yours = parseV3Protocol( + mkProtocol({ name: 'yours', rig: '/your/rig.yaml', conditions: ' - name: x\n commands: [{type: wait, duration: 1}]' }) + ); + const st = createStagingBuffer(src, 'their_lib.yaml'); + addToStaging(st, 0, yours); + const entry = st.batch.pluginRegistry.get('camera'); + check('N13a: action is add (target lacks camera)', entry.action, 'add'); + checkTrue('N13a: marked canonical', entry.canonical === true); + check('N13a: planned name is canonical (unprefixed)', entry.plannedName, 'camera'); + const sum = commitStaging(st, yours); + checkTrue('N13a: camera reported added', sum.pluginsAdded.includes('camera')); + checkTrue('N13a: camera declared with canonical name', yours.plugins.some((p) => p.name === 'camera')); + const outA = yours._doc.toString(); + checkTrue('N13a: plugin_name stays "camera"', /plugin_name:\s*"?camera"?/.test(outA)); + checkTrue('N13a: NEVER prefixed (#89)', !/their_lib__camera/.test(outA) && !yours.plugins.some((p) => /__camera$/.test(p.name))); + + // (b) target already declares camera → merge (protocol authoritative), no new plugin. + const src2 = parseV3Protocol(camSrc()); + const yoursDecl = parseV3Protocol( + mkProtocol({ + name: 'yours', + rig: '/your/rig.yaml', + plugins: 'plugins:\n - {name: camera, type: class, matlab: {class: BiasPlugin}, config: {frame_rate: 60}}', + experiment: 'existing', + conditions: ' - name: existing\n commands: [{type: wait, duration: 1}]' + }) + ); + const st2 = createStagingBuffer(src2, 'their_lib.yaml'); + addToStaging(st2, 0, yoursDecl); + const e2 = st2.batch.pluginRegistry.get('camera'); + check('N13b: canonical name present in target → merge', e2.action, 'merge'); + check('N13b: merges with existing camera', e2.mergeWith, 'camera'); + const pcount = yoursDecl.plugins.length; + const sum2 = commitStaging(st2, yoursDecl); + check('N13b: no plugin added on canonical merge', yoursDecl.plugins.length, pcount); + checkTrue('N13b: merge recorded', sum2.pluginsMerged.includes('camera')); + checkTrue('N13b: target camera config preserved (authoritative)', yoursDecl.plugins.find((p) => p.name === 'camera').config.frame_rate === 60); + + // (c) non-canonical custom plugin with no rig support → still namespaced (regression guard). + const srcCustom = parseV3Protocol( + mkProtocol({ + name: 'theirs', + rig: '/their/rig.yaml', + plugins: 'plugins:\n - {name: widget, type: class, matlab: {class: WidgetPlugin}, config: {chan: 2}}', + conditions: + ' - name: w trial\n commands:\n - {type: plugin, plugin_name: widget, command_name: go}' + }) + ); + const yoursC = parseV3Protocol( + mkProtocol({ name: 'yours', conditions: ' - name: x\n commands: [{type: wait, duration: 1}]' }) + ); + const stC = createStagingBuffer(srcCustom, 'their_lib.yaml'); + addToStaging(stC, 0, yoursC); + const eC = stC.batch.pluginRegistry.get('widget'); + check('N13c: non-canonical custom plugin → add', eC.action, 'add'); + checkTrue('N13c: not marked canonical', !eC.canonical); + check('N13c: still namespaced with prefix', eC.plannedName, 'their_lib__widget'); + + // (d) opts.rigPluginNames extends the canonical set with loaded-rig names. + const srcRig = parseV3Protocol( + mkProtocol({ + name: 'theirs', + rig: '/their/rig.yaml', + plugins: 'plugins:\n - {name: specialCam, type: class, matlab: {class: BiasPlugin}}', + conditions: + ' - name: s trial\n commands:\n - {type: plugin, plugin_name: specialCam, command_name: getTimestamp}' + }) + ); + const yoursR = parseV3Protocol( + mkProtocol({ name: 'yours', conditions: ' - name: x\n commands: [{type: wait, duration: 1}]' }) + ); + const stR = createStagingBuffer(srcRig, 'their_lib.yaml', { rigPluginNames: ['specialCam'] }); + addToStaging(stR, 0, yoursR); + const eR = stR.batch.pluginRegistry.get('specialCam'); + checkTrue('N13d: rig-declared name marked canonical', eR.canonical === true); + check('N13d: rig-declared name not prefixed', eR.plannedName, 'specialCam'); + // setPluginPlannedName must be a no-op on a canonical bind (can't re-break it). + setPluginPlannedName(stR, 'specialCam', 'their_lib__specialCam'); + check('N13d: canonical bind is non-renamable', stR.batch.pluginRegistry.get('specialCam').plannedName, 'specialCam'); + // prefix change must not re-prefix a canonical bind either. + setStagingPrefix(stR, 'zzz__'); + check('N13d: prefix change leaves canonical bind alone', stR.batch.pluginRegistry.get('specialCam').plannedName, 'specialCam'); +} + // ─── Results ──────────────────────────────────────────────────────────────── console.log('\n=== Results: ' + passedTests + '/' + totalTests + ' passed ==='); if (failedTests.length > 0) {