refactor(editor-core): move scene_vars in from op-editor-ui

VariableTable/Value/Scalar (unresolved theme refs + NodeId) now live with
the canonical variable system. NodeId/Color resolve at the op-editor-core
crate root. 26 scene_vars tests green.

crates/op-editor-core/src/lib.rs

@@ -65,6 +65,7 @@ pub mod property_panel_state;
 pub mod ref_resolve;
 pub mod rename;
 pub mod render_backend;
+pub mod scene_vars;
 pub mod selection;
 pub mod selection_resolve;
 pub mod state;

crates/op-editor-core/src/scene_vars.rs

@@ -0,0 +1,516 @@
+//! Paint-time design-variable aggregation — `VariableTable`.
+//!
+//! Mirrors the canonical `jian_ops_schema::variable` model (name →
+//! typed value, theme axes, active-theme selection) but is a
+//! shell-core-local *paint-time* aggregate: it folds the persisted
+//! definitions together with the editor's `fill_refs` / `stroke_refs`
+//! resolution caches so the `LayoutScene` builder can resolve every
+//! `$ref` fill / stroke to a concrete `Color` at scene-build time.
+//!
+//! Lives in `op-editor-core` alongside the canonical editor model +
+//! variable system. Both the scene builder (`op-pen-loader`) and the
+//! widgets (`op-editor-ui`, via re-export) reach it without pulling the
+//! full UI crate.
+
+use std::collections::BTreeMap;
+
+/// Variable type discriminator. Mirrors `VariableKind` in
+/// jian_ops_schema for direct round-trip.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum VariableKind {
+    Color,
+    Number,
+    Boolean,
+    String,
+}
+
+/// Single typed scalar — the leaf value of a variable. Untagged
+/// because `.op` JSON uses raw scalars: `"#ff0000"` / `12.5` / `true`.
+#[derive(Debug, Clone, PartialEq)]
+pub enum VariableScalar {
+    Bool(bool),
+    Num(f64),
+    Str(String),
+}
+
+/// A scalar value paired with the theme combination it applies to.
+/// `theme = None` is the "default" entry used when no themed entry
+/// matches the active selection.
+#[derive(Debug, Clone, PartialEq)]
+pub struct ThemedValue {
+    pub value: VariableScalar,
+    /// Axis → axis-value map. e.g. `{"mode": "dark"}`. Match the active
+    /// theme against this map to pick the right value.
+    pub theme: Option<BTreeMap<String, String>>,
+}
+
+/// Variable value: either a single scalar (theme-agnostic) or a list
+/// of themed alternatives.
+#[derive(Debug, Clone, PartialEq)]
+pub enum VariableValue {
+    Scalar(VariableScalar),
+    Themed(Vec<ThemedValue>),
+}
+
+/// A named variable definition. `name` is the lookup key referenced
+/// by `$color-1` style refs in node fill / stroke fields.
+#[derive(Debug, Clone, PartialEq)]
+pub struct Variable {
+    pub name: String,
+    pub kind: VariableKind,
+    pub value: VariableValue,
+}
+
+impl Variable {
+    /// Resolve the variable's current scalar under `active_theme`.
+    /// Themed values pick the entry whose `theme` map is a subset of
+    /// `active_theme`; falls back to the entry with `theme = None`
+    /// if no match. Returns None for empty `Themed([])`.
+    pub fn resolve<'a>(
+        &'a self,
+        active_theme: &BTreeMap<String, String>,
+    ) -> Option<&'a VariableScalar> {
+        match &self.value {
+            VariableValue::Scalar(s) => Some(s),
+            VariableValue::Themed(entries) => {
+                // First pass: pick the entry whose every theme axis
+                // matches active_theme.
+                for e in entries {
+                    if let Some(t) = &e.theme {
+                        if t.iter().all(|(k, v)| active_theme.get(k) == Some(v)) {
+                            return Some(&e.value);
+                        }
+                    }
+                }
+                // Fallback: entry with theme = None, else the FIRST
+                // entry (TS `resolveThemedValue` falls back to
+                // `values[0]` so fully-themed lists still resolve
+                // before the user picks an axis value).
+                entries
+                    .iter()
+                    .find(|e| e.theme.is_none())
+                    .or_else(|| entries.first())
+                    .map(|e| &e.value)
+            }
+        }
+    }
+}
+
+/// One theme axis (e.g. "mode" with values ["light", "dark"]).
+#[derive(Debug, Clone, PartialEq)]
+pub struct ThemeAxis {
+    pub name: String,
+    pub values: Vec<String>,
+}
+
+/// Per-document variable + theme registry. Populated by the
+/// canonical `.op` loader from `PenDocument.variables` /
+/// `PenDocument.themes`. v1 preserves the data + supports lookup +
+/// active-theme selection; paint-time `$ref` substitution is a
+/// follow-up.
+#[derive(Debug, Clone, Default, PartialEq)]
+pub struct VariableTable {
+    pub variables: Vec<Variable>,
+    pub themes: Vec<ThemeAxis>,
+    /// Current selection per axis. e.g. {"mode": "dark"}.
+    pub active_theme: BTreeMap<String, String>,
+    /// Map of `node_id → variable name` for nodes whose fill is
+    /// `$ref:name`. Paint reads this first, falls back to `node.fill`.
+    /// Side-table avoids touching every `Node { ... }` literal.
+    /// `HashMap` (not `BTreeMap`): `NodeId` is a string with no
+    /// meaningful order, and lookups are point queries.
+    pub fill_refs: std::collections::HashMap<crate::NodeId, String>,
+    /// Map of `node_id → variable name` for nodes whose stroke colour
+    /// follows a `$ref`. Parallel to `fill_refs`; paint pre-resolves
+    /// via `stroke_color_for(node_id)`.
+    pub stroke_refs: std::collections::HashMap<crate::NodeId, String>,
+}
+
+impl VariableTable {
+    /// Register that `node_id`'s fill follows variable `ref_name`.
+    /// Subsequent `fill_for(node_id)` looks the variable up under
+    /// the current `active_theme`.
+    pub fn set_fill_ref(&mut self, node_id: crate::NodeId, ref_name: impl Into<String>) {
+        self.fill_refs.insert(node_id, ref_name.into());
+    }
+    /// Resolve the paint-time fill color for `node_id`. Returns
+    /// None when no `fill_ref` is registered or the referenced
+    /// variable doesn't resolve (paint then falls back to
+    /// `node.fill`).
+    pub fn fill_for(&self, node_id: &crate::NodeId) -> Option<crate::Color> {
+        let name = self.fill_refs.get(node_id)?;
+        self.resolve_color(name)
+    }
+    /// Stroke parallel to `set_fill_ref` — registers the variable
+    /// driving a node's stroke colour at paint time.
+    pub fn set_stroke_ref(&mut self, node_id: crate::NodeId, ref_name: impl Into<String>) {
+        self.stroke_refs.insert(node_id, ref_name.into());
+    }
+    /// Resolve the paint-time stroke color for `node_id`. Same shape
+    /// as `fill_for`; paint falls back to `node.stroke.color` when
+    /// None.
+    pub fn stroke_color_for(&self, node_id: &crate::NodeId) -> Option<crate::Color> {
+        let name = self.stroke_refs.get(node_id)?;
+        self.resolve_color(name)
+    }
+    /// Set the active value of a theme axis (e.g. `("mode", "dark")`).
+    /// Mirrors the TS theme picker — flips colours across every
+    /// themed variable in one call. Paint reflows on the next frame.
+    pub fn set_active_theme(&mut self, axis: impl Into<String>, value: impl Into<String>) {
+        self.active_theme.insert(axis.into(), value.into());
+    }
+    /// Remove an axis from the active theme map; subsequent
+    /// resolutions fall back to the variable's `theme: None` default.
+    pub fn clear_active_axis(&mut self, axis: &str) {
+        self.active_theme.remove(axis);
+    }
+
+    /// Cycle the active value of `axis` to the next entry in the
+    /// `ThemeAxis::values` list. Returns `false` (no-op) when the
+    /// axis isn't in `themes` or has no values. When the axis
+    /// isn't currently in `active_theme`, the first value seeds
+    /// it. When the current value matches the last entry, wraps
+    /// back to the first. Used by the VariablesPanel axis-chip
+    /// click to give users a one-click flip — light↔dark,
+    /// compact↔comfortable, etc.
+    pub fn cycle_active_axis_value(&mut self, axis: &str) -> bool {
+        let Some(theme_axis) = self.themes.iter().find(|t| t.name == axis) else {
+            return false;
+        };
+        if theme_axis.values.is_empty() {
+            return false;
+        }
+        let next = match self.active_theme.get(axis) {
+            None => theme_axis.values[0].clone(),
+            Some(current) => {
+                let idx = theme_axis
+                    .values
+                    .iter()
+                    .position(|v| v == current)
+                    .map(|i| (i + 1) % theme_axis.values.len())
+                    .unwrap_or(0);
+                theme_axis.values[idx].clone()
+            }
+        };
+        self.active_theme.insert(axis.to_string(), next);
+        true
+    }
+
+    /// Look up a variable by name. None if unknown.
+    pub fn find(&self, name: &str) -> Option<&Variable> {
+        self.variables.iter().find(|v| v.name == name)
+    }
+    /// Resolve `$ref` against the table under the active theme. Returns
+    /// the scalar leaf or None on unknown name / empty Themed.
+    pub fn resolve(&self, name: &str) -> Option<&VariableScalar> {
+        self.find(name)?.resolve(&self.active_theme)
+    }
+    /// Resolve a `$ref` into a paintable `Color`. Returns None when
+    /// the variable is unknown, isn't of `Color` kind, or its scalar
+    /// isn't a parseable hex string (`#rgb`, `#rrggbb`, `#rrggbbaa`).
+    /// Used by paint-time `$ref` substitution.
+    pub fn resolve_color(&self, name: &str) -> Option<crate::Color> {
+        let v = self.find(name)?;
+        if !matches!(v.kind, VariableKind::Color) {
+            return None;
+        }
+        let scalar = v.resolve(&self.active_theme)?;
+        if let VariableScalar::Str(s) = scalar {
+            return parse_hex_color(s);
+        }
+        None
+    }
+
+    /// Mutable lookup parallel to `find`. Returns None when no
+    /// variable with that name exists. Editor surfaces use this to
+    /// stage an in-progress write back into the live document.
+    pub fn find_mut(&mut self, name: &str) -> Option<&mut Variable> {
+        self.variables.iter_mut().find(|v| v.name == name)
+    }
+
+    /// Write a new hex string into a `Color`-kind variable. Returns
+    /// `true` when the variable existed AND was Color-kind AND the
+    /// hex parsed cleanly; `false` otherwise (no mutation). For
+    /// themed variables this overwrites the entry matching the
+    /// current `active_theme` (or creates one if absent); for
+    /// scalar variables it overwrites the single value.
+    ///
+    /// The ColorPicker commits through this helper when the
+    /// VariablesPanel routes a row click into the picker — the
+    /// model-layer write path is unified across "edit a node's
+    /// fill" and "edit a variable", so paint sees the change on
+    /// the next frame regardless of which surface the user touched.
+    pub fn set_color_hex(&mut self, name: &str, hex: &str) -> bool {
+        // Validate the hex up front so a malformed input never
+        // corrupts the stored scalar.
+        if parse_hex_color(hex).is_none() {
+            return false;
+        }
+        let active = self.active_theme.clone();
+        let var = match self.find_mut(name) {
+            Some(v) if matches!(v.kind, VariableKind::Color) => v,
+            _ => return false,
+        };
+        let normalized = hex.trim().to_string();
+        match &mut var.value {
+            VariableValue::Scalar(s) => {
+                *s = VariableScalar::Str(normalized);
+                true
+            }
+            VariableValue::Themed(entries) => {
+                // Write-routing rules — the resolve walk picks an
+                // entry; we must update THAT entry when it's a
+                // subset match for the active theme. When no subset
+                // entry matches, the user's intent depends on the
+                // active theme:
+                //
+                //   - active_theme EMPTY: the user is editing with
+                //     no theme axis selected, so the write targets
+                //     the `theme: None` default entry (the resolve
+                //     fallback). Creates one if absent.
+                //
+                //   - active_theme NON-EMPTY: the user is editing
+                //     under a specific theme combo. The edit must
+                //     scope to THAT combo. Pushing a new entry keyed
+                //     to active_theme is correct; mutating the
+                //     `theme: None` default would clobber the value
+                //     under every OTHER theme axis too (codex stop-
+                //     gate flag — fallback writes were silently
+                //     reaching across themes).
+                let subset_idx = entries.iter().position(|e| match &e.theme {
+                    Some(t) => t.iter().all(|(k, v)| active.get(k) == Some(v)),
+                    None => false,
+                });
+                if let Some(i) = subset_idx {
+                    entries[i].value = VariableScalar::Str(normalized);
+                    return true;
+                }
+                if active.is_empty() {
+                    // No theme selected — target the default entry.
+                    let default_idx = entries.iter().position(|e| e.theme.is_none());
+                    if let Some(i) = default_idx {
+                        entries[i].value = VariableScalar::Str(normalized);
+                    } else {
+                        entries.push(ThemedValue {
+                            value: VariableScalar::Str(normalized),
+                            theme: None,
+                        });
+                    }
+                    return true;
+                }
+                // Active theme set + no subset match — append a
+                // new entry keyed to the active theme at the END
+                // of the vec (codex stop-gate: front insertion
+                // shadowed pre-existing themed entries on OTHER
+                // axes — e.g. an existing {density:compact} entry
+                // would never resolve under active=dark+compact if
+                // a {mode:dark} entry was front-inserted because
+                // resolve's first-match walk picks the front entry
+                // for ANY active whose keys are a superset).
+                //
+                // End-push is safe because Step 1 (subset match)
+                // already proved no existing entry is a subset of
+                // `active`. So under the active theme our new
+                // entry is the unique subset match regardless of
+                // position; under OTHER actives, every pre-existing
+                // entry retains its original resolve precedence.
+                entries.push(ThemedValue {
+                    value: VariableScalar::Str(normalized),
+                    theme: Some(active),
+                });
+                true
+            }
+        }
+    }
+
+    /// Set a scalar variable's value with the same theme-routing
+    /// discipline as `set_color_hex`, but generic over any
+    /// `VariableScalar` (Number / Str / Bool). Used by the MCP
+    /// write tools `set_variable_number` / `set_variable_string`
+    /// / `set_variable_boolean`. Caller is responsible for
+    /// matching the scalar's variant to the variable's kind
+    /// (e.g. don't pass a Str to a Number-kind variable) —
+    /// kind-mismatch returns `false`.
+    pub fn set_scalar(&mut self, name: &str, scalar: VariableScalar) -> bool {
+        let active = self.active_theme.clone();
+        let var = match self.find_mut(name) {
+            Some(v) => v,
+            None => return false,
+        };
+        // Reject kind/scalar mismatch — a Number variable must
+        // receive a Num scalar, etc. Color variables are
+        // FORBIDDEN from this path: they need hex validation
+        // before mutation, which only `set_color_hex` provides.
+        // Codex stop-gate flagged that accepting Color+Str here
+        // would let a misrouted `set_variable_string` call write
+        // garbage into a Color variable (the tool snapshot
+        // wouldn't admit the name, but defense in depth says the
+        // apply layer should reject too).
+        let kind_matches = match (&var.kind, &scalar) {
+            (VariableKind::Number, VariableScalar::Num(_)) => true,
+            (VariableKind::String, VariableScalar::Str(_)) => true,
+            (VariableKind::Boolean, VariableScalar::Bool(_)) => true,
+            (VariableKind::Color, _) => false,
+            _ => false,
+        };
+        if !kind_matches {
+            return false;
+        }
+        match &mut var.value {
+            VariableValue::Scalar(s) => {
+                *s = scalar;
+                true
+            }
+            VariableValue::Themed(entries) => {
+                let subset_idx = entries.iter().position(|e| match &e.theme {
+                    Some(t) => t.iter().all(|(k, v)| active.get(k) == Some(v)),
+                    None => false,
+                });
+                if let Some(i) = subset_idx {
+                    entries[i].value = scalar;
+                    return true;
+                }
+                if active.is_empty() {
+                    let default_idx = entries.iter().position(|e| e.theme.is_none());
+                    if let Some(i) = default_idx {
+                        entries[i].value = scalar;
+                    } else {
+                        entries.push(ThemedValue {
+                            value: scalar,
+                            theme: None,
+                        });
+                    }
+                    return true;
+                }
+                entries.push(ThemedValue {
+                    value: scalar,
+                    theme: Some(active),
+                });
+                true
+            }
+        }
+    }
+
+    /// Create a new theme-agnostic scalar variable. Rejects an
+    /// empty (post-trim) name or a name that collides with an
+    /// existing variable. `default` must match `kind`: a Color
+    /// variable takes a `Str` holding a parseable hex; Number /
+    /// String / Boolean take the matching scalar variant.
+    /// Mirrors the TS Variables panel "+ Add" action.
+    pub fn create_variable(
+        &mut self,
+        name: &str,
+        kind: VariableKind,
+        default: VariableScalar,
+    ) -> bool {
+        let trimmed = name.trim();
+        if trimmed.is_empty() {
+            return false;
+        }
+        if self.variables.iter().any(|v| v.name == trimmed) {
+            return false;
+        }
+        let kind_ok = match (&kind, &default) {
+            // Color variables persist as a hex string; validate it
+            // up front so a freshly-created token always resolves.
+            (VariableKind::Color, VariableScalar::Str(s)) => parse_hex_color(s).is_some(),
+            (VariableKind::Number, VariableScalar::Num(_)) => true,
+            (VariableKind::String, VariableScalar::Str(_)) => true,
+            (VariableKind::Boolean, VariableScalar::Bool(_)) => true,
+            _ => false,
+        };
+        if !kind_ok {
+            return false;
+        }
+        self.variables.push(Variable {
+            name: trimmed.to_string(),
+            kind,
+            value: VariableValue::Scalar(default),
+        });
+        true
+    }
+
+    /// Delete a variable by name. Also drops any `fill_refs` /
+    /// `stroke_refs` pointing at it so paint doesn't keep
+    /// resolving a now-dangling `$ref`. Returns false when the
+    /// name doesn't resolve.
+    pub fn delete_variable(&mut self, name: &str) -> bool {
+        let Some(idx) = self.variables.iter().position(|v| v.name == name) else {
+            return false;
+        };
+        self.variables.remove(idx);
+        self.fill_refs.retain(|_, v| v != name);
+        self.stroke_refs.retain(|_, v| v != name);
+        true
+    }
+
+    /// Rename a variable. Rejects an unknown `old` name, an empty
+    /// (post-trim) `new` name, or a `new` name that collides with
+    /// a different existing variable. Rewrites every `fill_refs` /
+    /// `stroke_refs` entry so node `$ref`s follow the rename.
+    /// `old == new` (after trim) is a no-op success.
+    pub fn rename_variable(&mut self, old: &str, new: &str) -> bool {
+        let new_trimmed = new.trim();
+        if new_trimmed.is_empty() {
+            return false;
+        }
+        if old == new_trimmed {
+            return self.variables.iter().any(|v| v.name == old);
+        }
+        if self.variables.iter().any(|v| v.name == new_trimmed) {
+            return false;
+        }
+        let Some(var) = self.variables.iter_mut().find(|v| v.name == old) else {
+            return false;
+        };
+        var.name = new_trimmed.to_string();
+        for v in self.fill_refs.values_mut() {
+            if v == old {
+                *v = new_trimmed.to_string();
+            }
+        }
+        for v in self.stroke_refs.values_mut() {
+            if v == old {
+                *v = new_trimmed.to_string();
+            }
+        }
+        true
+    }
+}
+
+/// Parse `#rgb` / `#rrggbb` / `#rrggbbaa` into a `Color`. Mirrors the
+/// TS paint helpers — lenient on case, requires the leading `#`.
+fn parse_hex_color(s: &str) -> Option<crate::Color> {
+    let s = s.trim().strip_prefix('#')?;
+    let (r, g, b, a) = match s.len() {
+        3 => {
+            let r = u8::from_str_radix(&s[0..1].repeat(2), 16).ok()?;
+            let g = u8::from_str_radix(&s[1..2].repeat(2), 16).ok()?;
+            let b = u8::from_str_radix(&s[2..3].repeat(2), 16).ok()?;
+            (r, g, b, 255)
+        }
+        6 => (
+            u8::from_str_radix(&s[0..2], 16).ok()?,
+            u8::from_str_radix(&s[2..4], 16).ok()?,
+            u8::from_str_radix(&s[4..6], 16).ok()?,
+            255,
+        ),
+        8 => (
+            u8::from_str_radix(&s[0..2], 16).ok()?,
+            u8::from_str_radix(&s[2..4], 16).ok()?,
+            u8::from_str_radix(&s[4..6], 16).ok()?,
+            u8::from_str_radix(&s[6..8], 16).ok()?,
+        ),
+        _ => return None,
+    };
+    Some(crate::Color {
+        r: r as f32 / 255.0,
+        g: g as f32 / 255.0,
+        b: b as f32 / 255.0,
+        a: a as f32 / 255.0,
+    })
+}
+
+#[cfg(test)]
+#[path = "scene_vars_tests.rs"]
+mod scene_vars_tests;

crates/op-editor-core/src/scene_vars_tests.rs

@@ -0,0 +1,613 @@
+//! Tests for `scene_vars`. Kept in a sibling file so the types +
+//! impl stay under the 800-line cap.
+
+use super::{
+    ThemeAxis, ThemedValue, Variable, VariableKind, VariableScalar, VariableTable, VariableValue,
+};
+use std::collections::BTreeMap;
+
+fn axis(name: &str, value: &str) -> BTreeMap<String, String> {
+    let mut m = BTreeMap::new();
+    m.insert(name.to_string(), value.to_string());
+    m
+}
+
+#[test]
+fn scalar_variable_resolves_regardless_of_theme() {
+    let v = Variable {
+        name: "color-1".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("#ff0000".into())),
+    };
+    let theme = axis("mode", "dark");
+    match v.resolve(&theme).unwrap() {
+        VariableScalar::Str(s) => assert_eq!(s, "#ff0000"),
+        _ => panic!(),
+    }
+}
+
+#[test]
+fn themed_variable_picks_active_axis() {
+    let v = Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#ffffff".into()),
+                theme: Some(axis("mode", "light")),
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#000000".into()),
+                theme: Some(axis("mode", "dark")),
+            },
+        ]),
+    };
+    let dark = axis("mode", "dark");
+    let light = axis("mode", "light");
+    match v.resolve(&dark).unwrap() {
+        VariableScalar::Str(s) => assert_eq!(s, "#000000"),
+        _ => panic!(),
+    }
+    match v.resolve(&light).unwrap() {
+        VariableScalar::Str(s) => assert_eq!(s, "#ffffff"),
+        _ => panic!(),
+    }
+}
+
+#[test]
+fn themed_variable_falls_back_to_default_when_no_match() {
+    let v = Variable {
+        name: "accent".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#888888".into()),
+                theme: None,
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#000000".into()),
+                theme: Some(axis("mode", "dark")),
+            },
+        ]),
+    };
+    let light = axis("mode", "light");
+    match v.resolve(&light).unwrap() {
+        VariableScalar::Str(s) => assert_eq!(s, "#888888"),
+        _ => panic!(),
+    }
+}
+
+#[test]
+fn resolve_color_parses_rrggbb_hex() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "accent".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("#ff8040".into())),
+    });
+    let c = tbl.resolve_color("accent").unwrap();
+    assert!((c.r - 1.0).abs() < 0.01);
+    assert!((c.g - (128.0 / 255.0)).abs() < 0.01);
+    assert!((c.b - (64.0 / 255.0)).abs() < 0.01);
+    assert_eq!(c.a, 1.0);
+}
+
+#[test]
+fn resolve_color_picks_themed_active_value() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#ffffff".into()),
+                theme: Some(axis("mode", "light")),
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#000000".into()),
+                theme: Some(axis("mode", "dark")),
+            },
+        ]),
+    });
+    tbl.active_theme = axis("mode", "dark");
+    let c = tbl.resolve_color("bg").unwrap();
+    assert_eq!(c.r, 0.0);
+    assert_eq!(c.g, 0.0);
+    assert_eq!(c.b, 0.0);
+}
+
+#[test]
+fn resolve_color_rejects_non_color_variables() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "spacing".into(),
+        kind: VariableKind::Number,
+        value: VariableValue::Scalar(VariableScalar::Num(12.0)),
+    });
+    assert!(tbl.resolve_color("spacing").is_none());
+}
+
+#[test]
+fn fill_for_resolves_registered_node_ref_to_themed_color() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "accent".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#ff0000".into()),
+                theme: Some(axis("mode", "light")),
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#00ff00".into()),
+                theme: Some(axis("mode", "dark")),
+            },
+        ]),
+    });
+    let node = crate::NodeId::new("n42");
+    tbl.set_fill_ref(node.clone(), "accent");
+    tbl.active_theme = axis("mode", "dark");
+    let c = tbl.fill_for(&node).unwrap();
+    assert!((c.g - 1.0).abs() < 0.01, "dark mode → green; got {c:?}");
+    // Switch theme; same ref resolves differently.
+    tbl.active_theme = axis("mode", "light");
+    let c2 = tbl.fill_for(&node).unwrap();
+    assert!((c2.r - 1.0).abs() < 0.01, "light mode → red; got {c2:?}");
+}
+
+#[test]
+fn stroke_color_for_resolves_registered_ref() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "border".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("#0000ff".into())),
+    });
+    let node = crate::NodeId::new("n7");
+    tbl.set_stroke_ref(node.clone(), "border");
+    let c = tbl.stroke_color_for(&node).unwrap();
+    assert!((c.b - 1.0).abs() < 0.01);
+}
+
+#[test]
+fn set_active_theme_round_trips_through_axis_picker() {
+    let mut tbl = super::VariableTable::default();
+    tbl.set_active_theme("mode", "dark");
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"dark".to_string()));
+    tbl.set_active_theme("density", "comfortable");
+    assert_eq!(tbl.active_theme.len(), 2);
+    tbl.clear_active_axis("mode");
+    assert!(!tbl.active_theme.contains_key("mode"));
+    assert_eq!(tbl.active_theme.len(), 1);
+}
+
+#[test]
+fn fill_for_returns_none_when_no_ref_registered() {
+    let tbl = super::VariableTable::default();
+    assert!(tbl.fill_for(&crate::NodeId::new("n99")).is_none());
+}
+
+#[test]
+fn resolve_color_rejects_invalid_hex() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "broken".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("not-hex".into())),
+    });
+    assert!(tbl.resolve_color("broken").is_none());
+}
+
+#[test]
+fn themed_variable_returns_none_when_empty_and_no_default() {
+    let v = Variable {
+        name: "x".into(),
+        kind: VariableKind::Number,
+        value: VariableValue::Themed(vec![]),
+    };
+    let theme = BTreeMap::new();
+    assert!(v.resolve(&theme).is_none());
+}
+
+#[test]
+fn set_color_hex_writes_scalar_variable() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "color-1".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("#ff0000".into())),
+    });
+    assert!(tbl.set_color_hex("color-1", "#00ff00"));
+    assert_eq!(
+        tbl.resolve_color("color-1"),
+        Some(crate::Color {
+            r: 0.0,
+            g: 1.0,
+            b: 0.0,
+            a: 1.0,
+        })
+    );
+}
+
+#[test]
+fn set_color_hex_rejects_malformed_input() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "color-1".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Scalar(VariableScalar::Str("#ff0000".into())),
+    });
+    assert!(!tbl.set_color_hex("color-1", "not-hex"));
+    // Scalar must be unchanged after a rejected write.
+    assert_eq!(
+        tbl.resolve_color("color-1"),
+        Some(crate::Color {
+            r: 1.0,
+            g: 0.0,
+            b: 0.0,
+            a: 1.0,
+        })
+    );
+}
+
+#[test]
+fn set_color_hex_returns_false_for_unknown_variable() {
+    let mut tbl = super::VariableTable::default();
+    assert!(!tbl.set_color_hex("does-not-exist", "#ffffff"));
+}
+
+#[test]
+fn set_color_hex_returns_false_for_wrong_kind() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "spacing".into(),
+        kind: VariableKind::Number,
+        value: VariableValue::Scalar(VariableScalar::Num(16.0)),
+    });
+    // Number-kind variable: write rejected.
+    assert!(!tbl.set_color_hex("spacing", "#ffffff"));
+}
+
+#[test]
+fn set_color_hex_changes_resolved_value_under_subset_active_theme() {
+    // Codex stop-gate regression: when the themed entry's theme
+    // is a SUBSET of the active theme (e.g. entry only specifies
+    // `mode: dark`; active is `mode: dark, density: compact`),
+    // `resolve` still picks that entry — so a write must update
+    // that entry in place, NOT push a new entry at the end that
+    // `resolve` will never reach.
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![ThemedValue {
+            value: VariableScalar::Str("#111111".into()),
+            theme: Some({
+                let mut m = BTreeMap::new();
+                m.insert("mode".into(), "dark".into());
+                m
+            }),
+        }]),
+    });
+    tbl.set_active_theme("mode", "dark");
+    tbl.set_active_theme("density", "compact");
+    // Sanity: resolves through the subset-matching entry.
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x11 as f32 / 255.0,
+            g: 0x11 as f32 / 255.0,
+            b: 0x11 as f32 / 255.0,
+            a: 1.0,
+        })
+    );
+    // Write under the wider active theme. The original entry
+    // (only mode=dark) is the resolved one — the write MUST go
+    // through it, not append a new entry that resolve never
+    // reaches.
+    assert!(tbl.set_color_hex("bg", "#22ff44"));
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x22 as f32 / 255.0,
+            g: 1.0,
+            b: 0x44 as f32 / 255.0,
+            a: 1.0,
+        }),
+        "themed write must update the entry resolve picks"
+    );
+}
+
+#[test]
+fn set_color_hex_under_active_theme_does_not_clobber_default() {
+    // Codex stop-gate (round 2): when active_theme is non-empty
+    // and no subset-matching themed entry exists, the write
+    // must scope to the active theme (push a new entry), NOT
+    // mutate the `theme: None` default. Mutating the default
+    // would silently change the value under EVERY OTHER theme
+    // axis the user hasn't explicitly authored, which is
+    // never what an editor user clicking a colour swatch in
+    // dark mode means.
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#ffffff".into()),
+                theme: Some({
+                    let mut m = BTreeMap::new();
+                    m.insert("mode".into(), "light".into());
+                    m
+                }),
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#888888".into()),
+                theme: None,
+            },
+        ]),
+    });
+    tbl.set_active_theme("mode", "dark"); // matches neither
+                                          // Pre-write: resolves to the default (#888) under dark.
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x88 as f32 / 255.0,
+            g: 0x88 as f32 / 255.0,
+            b: 0x88 as f32 / 255.0,
+            a: 1.0,
+        })
+    );
+    // Write under active=dark must scope to dark only.
+    assert!(tbl.set_color_hex("bg", "#000000"));
+    // Under dark — new value reads back.
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0.0,
+            g: 0.0,
+            b: 0.0,
+            a: 1.0,
+        }),
+    );
+    // The default entry must be untouched. Flipping to a
+    // never-authored axis value (e.g. mode=sepia) should still
+    // resolve to the original default #888888, NOT the new dark
+    // value.
+    tbl.set_active_theme("mode", "sepia");
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x88 as f32 / 255.0,
+            g: 0x88 as f32 / 255.0,
+            b: 0x88 as f32 / 255.0,
+            a: 1.0,
+        }),
+        "default entry must NOT have been clobbered by the dark write"
+    );
+    // Light entry also untouched.
+    tbl.set_active_theme("mode", "light");
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 1.0,
+            g: 1.0,
+            b: 1.0,
+            a: 1.0,
+        }),
+        "light entry must NOT have been clobbered"
+    );
+}
+
+#[test]
+fn set_color_hex_does_not_shadow_existing_entries_on_other_axes() {
+    // Codex stop-gate (round 3): when the write pushes a new
+    // themed entry, it must NOT shadow pre-existing entries
+    // whose theme references a different axis. The pathological
+    // case is an existing entry keyed to `{density:compact}`
+    // and a write under `{mode:dark}`. A naive front-insert
+    // would mean active=`{mode:dark, density:compact}` resolves
+    // to the new entry instead of the compact entry, silently
+    // changing the resolved colour for the combined axis.
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![ThemedValue {
+            value: VariableScalar::Str("#11ccaa".into()),
+            theme: Some({
+                let mut m = BTreeMap::new();
+                m.insert("density".into(), "compact".into());
+                m
+            }),
+        }]),
+    });
+    // Active theme = mode:dark only (no compact axis set, so the
+    // existing entry doesn't subset-match — the write must push
+    // a new entry).
+    tbl.set_active_theme("mode", "dark");
+    assert!(tbl.set_color_hex("bg", "#000000"));
+    // Under mode:dark — the new entry is reachable.
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0.0,
+            g: 0.0,
+            b: 0.0,
+            a: 1.0,
+        })
+    );
+    // Now flip active to the combined axes — mode:dark +
+    // density:compact. Under that resolve walk:
+    //   - The pre-existing {density:compact} entry must still
+    //     win (it's earlier in the vec AND its theme is a
+    //     subset of the active combo).
+    //   - The new {mode:dark} entry, end-pushed, is also a
+    //     subset of the active combo, but it's later. First
+    //     subset match wins.
+    // Pre-fix (front-insert) this would have read #000000
+    // instead — that was the codex BLOCK.
+    tbl.set_active_theme("density", "compact");
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x11 as f32 / 255.0,
+            g: 0xcc as f32 / 255.0,
+            b: 0xaa as f32 / 255.0,
+            a: 1.0,
+        }),
+        "compact entry must not be shadowed by the end-pushed dark entry"
+    );
+}
+
+#[test]
+fn set_color_hex_empty_active_theme_targets_default_entry() {
+    // The flip side: when active_theme IS empty (no axis
+    // selected), writes target the default `theme: None`
+    // entry — that's what resolve falls back to in this state.
+    // Editing with no theme selected is the explicit "edit the
+    // universal value" path.
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![ThemedValue {
+            value: VariableScalar::Str("#888888".into()),
+            theme: None,
+        }]),
+    });
+    // No active theme set.
+    assert!(tbl.set_color_hex("bg", "#aabbcc"));
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0xaa as f32 / 255.0,
+            g: 0xbb as f32 / 255.0,
+            b: 0xcc as f32 / 255.0,
+            a: 1.0,
+        })
+    );
+}
+
+#[test]
+fn set_color_hex_writes_themed_entry_matching_active_axis() {
+    let mut tbl = super::VariableTable::default();
+    tbl.variables.push(Variable {
+        name: "bg".into(),
+        kind: VariableKind::Color,
+        value: VariableValue::Themed(vec![
+            ThemedValue {
+                value: VariableScalar::Str("#ffffff".into()),
+                theme: Some({
+                    let mut m = BTreeMap::new();
+                    m.insert("mode".into(), "light".into());
+                    m
+                }),
+            },
+            ThemedValue {
+                value: VariableScalar::Str("#000000".into()),
+                theme: Some({
+                    let mut m = BTreeMap::new();
+                    m.insert("mode".into(), "dark".into());
+                    m
+                }),
+            },
+        ]),
+    });
+    tbl.set_active_theme("mode", "dark");
+    // Resolves to dark first.
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0.0,
+            g: 0.0,
+            b: 0.0,
+            a: 1.0,
+        })
+    );
+    // Write under the active theme → updates only the dark entry.
+    assert!(tbl.set_color_hex("bg", "#3344ff"));
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 0x33 as f32 / 255.0,
+            g: 0x44 as f32 / 255.0,
+            b: 1.0,
+            a: 1.0,
+        })
+    );
+    // Flip to light — original light entry untouched.
+    tbl.set_active_theme("mode", "light");
+    assert_eq!(
+        tbl.resolve_color("bg"),
+        Some(crate::Color {
+            r: 1.0,
+            g: 1.0,
+            b: 1.0,
+            a: 1.0,
+        })
+    );
+}
+
+#[test]
+fn cycle_active_axis_seeds_first_value_when_absent() {
+    // Axis defined in `themes` but not yet in active_theme →
+    // cycle plants the first value.
+    let mut tbl = VariableTable::default();
+    tbl.themes.push(ThemeAxis {
+        name: "mode".into(),
+        values: vec!["light".into(), "dark".into()],
+    });
+    assert!(tbl.cycle_active_axis_value("mode"));
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"light".to_string()));
+}
+
+#[test]
+fn cycle_active_axis_advances_to_next_value() {
+    let mut tbl = VariableTable::default();
+    tbl.themes.push(ThemeAxis {
+        name: "mode".into(),
+        values: vec!["light".into(), "dark".into(), "sepia".into()],
+    });
+    tbl.set_active_theme("mode", "light");
+    // light → dark → sepia → light (wrap)
+    assert!(tbl.cycle_active_axis_value("mode"));
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"dark".to_string()));
+    assert!(tbl.cycle_active_axis_value("mode"));
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"sepia".to_string()));
+    assert!(tbl.cycle_active_axis_value("mode"));
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"light".to_string()));
+}
+
+#[test]
+fn cycle_active_axis_returns_false_for_unknown_axis() {
+    let mut tbl = VariableTable::default();
+    assert!(!tbl.cycle_active_axis_value("does-not-exist"));
+    assert!(tbl.active_theme.is_empty());
+}
+
+#[test]
+fn cycle_active_axis_returns_false_for_empty_values() {
+    let mut tbl = VariableTable::default();
+    tbl.themes.push(ThemeAxis {
+        name: "empty".into(),
+        values: Vec::new(),
+    });
+    assert!(!tbl.cycle_active_axis_value("empty"));
+    assert!(!tbl.active_theme.contains_key("empty"));
+}
+
+#[test]
+fn cycle_active_axis_falls_back_to_first_when_current_unknown() {
+    // active_theme carries a value that isn't in themes.values
+    // (e.g. an axis whose options changed since the file loaded).
+    // Cycle wraps to the first known value.
+    let mut tbl = VariableTable::default();
+    tbl.themes.push(ThemeAxis {
+        name: "mode".into(),
+        values: vec!["light".into(), "dark".into()],
+    });
+    tbl.set_active_theme("mode", "ghost"); // not in values
+    assert!(tbl.cycle_active_axis_value("mode"));
+    assert_eq!(tbl.active_theme.get("mode"), Some(&"light".to_string()));
+}