|
| 1 | +# Design: Open-Closed typed nodes — attribute & field value behavior on the class |
| 2 | + |
| 3 | +**Date:** 2026-05-22 |
| 4 | +**Status:** Approved (design) |
| 5 | +**Author:** Doug Mealing (with Claude) |
| 6 | +**Branch:** `feat/typed-node-open-closed` (off `main` @ `1cfdfbf`, which includes the attr.filter/properties feature) |
| 7 | + |
| 8 | +## Problem |
| 9 | + |
| 10 | +Adding a new value-shaped attribute subtype to the TS reference is shotgun surgery across central |
| 11 | +files. Measured on the just-shipped `attr.filter` + `attr.properties` feature: **TS = 11 source |
| 12 | +files, C# = 9 (mirrored TS), Java = 3** (one self-registering `FilterAttribute` + a registration |
| 13 | +line + a serializer branch). Java stayed cheap because it kept the **polymorphic, self-registering |
| 14 | +`MetaAttribute<T>`** pattern; the TS rewrite replaced it with **a single `MetaAttr`/`MetaField` |
| 15 | +class dispatched through central tables**: |
| 16 | + |
| 17 | +- value type → the closed `AttrValue` union (`meta/meta-data.ts`) |
| 18 | +- value coercion → the `convertToDataType` switch (`data-converter.ts`) |
| 19 | +- normalization/desugar → the central parser (`parser-core.ts`) |
| 20 | +- value validation → hardcoded subtype-sets in `attr-schema-validate.ts` |
| 21 | +- subtype → datatype → the `ATTR_DATA_TYPE` / `FIELD_DATA_TYPE` maps (`core-types.ts`) |
| 22 | + |
| 23 | +This is the expression problem: TS optimized for adding new *operations* cheaply at the cost of |
| 24 | +adding new *types* — the opposite of what a metamodel framework wants. It is systemic (`MetaField` |
| 25 | +has the same shape) and it multiplies across every language port. |
| 26 | + |
| 27 | +**Root cause, deeper than the central switches:** in TS, inline `@`-attributes are stored as raw |
| 28 | +values in the owning node's flat `_attrs: Map<string, AttrValue>` and are **never materialized as |
| 29 | +`MetaAttr` instances** (only the rarer child-form `{ "attr.*": {...} }` creates instances). So there |
| 30 | +is no per-attribute object to carry behavior. In Java, the canonical parser materializes **every** |
| 31 | +attribute — inline included — as a `MetaAttribute` instance and dispatches `setValueAsString` |
| 32 | +polymorphically. "Inline vs child" is a **serialization concern owned by the parser**, not a property |
| 33 | +of the constructed model. Baking it into the TS model was the original mistake. |
| 34 | + |
| 35 | +## Goal |
| 36 | + |
| 37 | +Restore Open-Closed for typed nodes: **adding an attribute or field subtype is one class + one |
| 38 | +registration line, with zero edits to any central file.** The model represents every attribute |
| 39 | +uniformly as a `MetaAttr` instance; the parser is the sole owner of inline-vs-child syntax. TS |
| 40 | +converges structurally onto Java (and what C# mirrors), so future metamodel features cost ~1 class |
| 41 | +per language instead of ~9–11 central edits. |
| 42 | + |
| 43 | +## Decisions |
| 44 | + |
| 45 | +- **D1 — Scope: attributes AND fields.** Both use the single-class + central-dispatch pattern; fix |
| 46 | + both. (Fields are already instances, so their share is mostly moving datatype/value behavior onto |
| 47 | + the class.) |
| 48 | +- **D2 — Full materialization.** Attributes become `MetaAttr` instances in the model. Inline |
| 49 | + `@`-syntax is parse-time sugar only. No flat raw-value attr map in the constructed model. |
| 50 | +- **D3 — Behavior on the class, polymorphic.** `MetaAttr`/`MetaField` own `dataType` / `coerce` / |
| 51 | + `validateValue` / `desugar`, resolving by subtype; subclasses override (`FilterAttr`, |
| 52 | + `StringArrayAttr`, `PropertiesAttr`). The registry `factory` maps each `(type, subType)` to its |
| 53 | + class. Parser and validator dispatch to the instance. |
| 54 | +- **D4 — Keep the value-accessor API; reimplement over instances.** `ownAttr`/`attr`/`attrs`/ |
| 55 | + `ownAttrs`/`hasAttr`/`setAttr` stay (they are the correct convenience layer, same as Java's |
| 56 | + `MetaData`), reimplemented to read/write through `MetaAttr` instances. Add `ownMetaAttr(name)` / |
| 57 | + `ownMetaAttrs()` for callers needing the instance. |
| 58 | +- **D5 — Canonical rule: attributes always emit inline `@name`.** Attributes have no children, so |
| 59 | + there is one canonical output form. No provenance flag on the model. The single child-form fixture |
| 60 | + in the corpus (`attr-properties-basic`) regenerates to inline; all other canonical output stays |
| 61 | + byte-identical. |
| 62 | +- **D6 — Straight to the end state. No backwards compatibility.** Delete the `_attrs` raw map and the |
| 63 | + legacy `_effectiveAttrs` raw-map merge outright. No delegation shims, no dual-path, no compat flags. |
| 64 | + (Per CLAUDE.md "No backwards-compat hacks.") |
| 65 | +- **D7 — Consumers.** No external TS adopters except downstream-consumer; fix it after the change. Internal |
| 66 | + monorepo consumers keep working through the reimplemented value accessors; update any that break. |
| 67 | + |
| 68 | +## Design |
| 69 | + |
| 70 | +### 1. Model & storage (`meta/meta-data.ts`) |
| 71 | + |
| 72 | +Replace `private _attrs: Map<string, AttrValue>` with a name-indexed collection of `MetaAttr` |
| 73 | +instances (e.g. `private _attrNodes: Map<string, MetaAttr>`, insertion-ordered). Attributes are owned |
| 74 | +nodes (parented), not flat values. |
| 75 | + |
| 76 | +- `setAttr(name, value)`: resolve the attr's class via the registry (declared subtype) or infer it |
| 77 | + via the existing `inferAttrSubType` (undeclared); create/update the `MetaAttr` instance; the |
| 78 | + instance coerces/validates its own value. |
| 79 | +- `ownAttr(name)` → `this._attrNodes.get(name)?.value`. |
| 80 | +- `attrs()` / `ownAttrs()` → value maps built from instances (effective resolution walks the super |
| 81 | + chain collecting instances by name, own wins). The legacy `_effectiveAttrs` raw-map implementation |
| 82 | + is deleted and replaced by instance-based effective resolution. |
| 83 | +- New: `ownMetaAttr(name): MetaAttr | undefined`, `ownMetaAttrs(): readonly MetaAttr[]`. |
| 84 | + |
| 85 | +### 2. Behavior hierarchy |
| 86 | + |
| 87 | +`MetaAttr` (base) gains: |
| 88 | +- `get dataType(): DataType` — resolves by `this.subType` (replaces `ATTR_DATA_TYPE`). |
| 89 | +- `coerce(raw: unknown): AttrValue` — replaces the `convertToDataType` body for the base subtypes. |
| 90 | +- `validateValue(value: AttrValue): ValueError[]` — replaces `valueMatchesType` + the subtype-sets. |
| 91 | +- `desugar(value: AttrValue): AttrValue` — default identity. |
| 92 | + |
| 93 | +Subclasses override only what differs: |
| 94 | +- `FilterAttr` — object validation + the filter desugar (scalar→eq, array→in, null→isNull, or/and |
| 95 | + recurse), moved out of `parser-core.ts`. |
| 96 | +- `StringArrayAttr` — bare-string→one-element-array coercion, moved out of `normalizeStringArrayAttr`. |
| 97 | +- `PropertiesAttr` — object/string-bag validation. |
| 98 | + |
| 99 | +`MetaField` (base) gains `get dataType()` (replaces `FIELD_DATA_TYPE`) and any field value behavior; |
| 100 | +subclass only where a field's *value* handling diverges (none required today — `currency` is a |
| 101 | +`long` carrying extra metadata attrs, not a distinct value shape). |
| 102 | + |
| 103 | +The registry `TypeDefinition.factory` already creates a node per `(type, subType)`; register the |
| 104 | +specific class for each subtype so the right class is instantiated. |
| 105 | + |
| 106 | +### 3. What collapses centrally (deleted) |
| 107 | + |
| 108 | +- `ATTR_DATA_TYPE`, `FIELD_DATA_TYPE` maps and `dataTypeFor` (`core-types.ts`). |
| 109 | +- `convertToDataType` datatype switch (`data-converter.ts`) — folded into per-class `coerce`. (The |
| 110 | + `toAttrValue` undeclared-value path likewise becomes the base/inferred class's `coerce`.) |
| 111 | +- `valueMatchesType` + `STRING/NUMERIC/OBJECT_ATTR_SUBTYPES` sets (`attr-schema-validate.ts`) — |
| 112 | + folded into per-class `validateValue`. The attr-schema pass iterates materialized instances and |
| 113 | + calls `validateValue`. |
| 114 | +- `normalizeStringArrayAttr`, `normalizeFilterAttr` (`parser-core.ts`) — folded into the relevant |
| 115 | + subclasses. |
| 116 | + |
| 117 | +### 4. What stays central (correctly) |
| 118 | + |
| 119 | +Cross-node loader passes — e.g. `validateDataGridFilterValues` (a `@filter` references *sibling* |
| 120 | +filterable fields) — cannot live on a single node; they remain loader passes. This is the only |
| 121 | +legitimately-central category. |
| 122 | + |
| 123 | +### 5. Parser & serializer |
| 124 | + |
| 125 | +- **Parser** (`parser-core.ts`) becomes the sole owner of inline-vs-child syntax: inline `@`-keys and |
| 126 | + child-form `attr.*` both materialize into `MetaAttr` instances (right class via registry / inferred |
| 127 | + subtype), and the instance coerces+desugars its own value. The central coercion/normalization code |
| 128 | + is gone. |
| 129 | +- **Serializer** (`serializer-json.ts`): canonical rule D5 — attributes always emit inline `@name` |
| 130 | + (attrs have no children). Remove the child-node attr emission path from canonical output. Reading is |
| 131 | + from instances via `ownMetaAttrs()`. |
| 132 | +- **Overlay/merge + super chain**: merge `MetaAttr` instances by name (own/last-writer wins) — same |
| 133 | + semantics as the current raw-map merge, now over instances. |
| 134 | + |
| 135 | +### 6. Cross-language |
| 136 | + |
| 137 | +Java is already fully instance-based; C# mirrors it. This makes TS structurally identical. The |
| 138 | +conformance corpus is the guardrail: canonical output stays byte-identical except `attr-properties-basic`, |
| 139 | +which regenerates from child-form to inline `@config` (a deliberate, deterministic canonical change), |
| 140 | +regenerated for all languages. |
| 141 | + |
| 142 | +### 7. Open-Closed proof (executable definition of done) |
| 143 | + |
| 144 | +A test registers a throwaway `attr.fizz` + `field.fizz` subtype via **only** a new class + a single |
| 145 | +registration line, then asserts load → coerce → validate → canonical-serialize all work, with an |
| 146 | +assertion that no central file (datatype map, converter switch, validator set, value union) was |
| 147 | +touched. This is the regression guard for the property the refactor exists to create. |
| 148 | + |
| 149 | +## Migration (direct to end state) |
| 150 | + |
| 151 | +No incremental scaffolding, no delegation shims, no compat. Ordered for reviewable commits, but each |
| 152 | +lands real end-state code: |
| 153 | + |
| 154 | +1. Add the behavior methods (`dataType`/`coerce`/`validateValue`/`desugar`) to `MetaAttr`/`MetaField` |
| 155 | + bases + the `FilterAttr`/`StringArrayAttr`/`PropertiesAttr` subclasses; register classes in the |
| 156 | + factory. |
| 157 | +2. Convert `MetaData` storage to `MetaAttr` instances; reimplement `setAttr`/`ownAttr`/`attrs`/etc. |
| 158 | + over instances; add `ownMetaAttr`; delete `_attrs` raw map and legacy `_effectiveAttrs`. |
| 159 | +3. Rewrite the parser to materialize all attrs into instances; delete the central |
| 160 | + coercion/normalization calls. |
| 161 | +4. Rewrite the attr-schema pass to dispatch `validateValue`; delete `valueMatchesType` + subtype-sets. |
| 162 | +5. Update the serializer to the always-inline canonical rule; delete central datatype maps. |
| 163 | +6. Regenerate `attr-properties-basic` expected output (TS + C#); fix any internal monorepo consumers; |
| 164 | + fix downstream-consumer. |
| 165 | + |
| 166 | +**Verification (not optional):** full TS suite + whole-monorepo typecheck green; conformance |
| 167 | +byte-identical except the one regenerated fixture; C# `dotnet test` + Java metadata tests green; the |
| 168 | +Open-Closed proof test passes. |
| 169 | + |
| 170 | +## Out of scope |
| 171 | + |
| 172 | +- An instance-first public API migration beyond keeping the value accessors (they are the intended |
| 173 | + interface). `ownMetaAttr` is added but consumers aren't forced onto it. |
| 174 | +- Codegen/runtime behavior changes — unaffected via the value accessors. |
| 175 | +- Python. |
| 176 | +- Promoting the per-subtype-class pattern to other node kinds beyond attr/field (identity, relationship, |
| 177 | + origin, layout) — those don't carry coerced scalar values; revisit only if needed. |
| 178 | + |
| 179 | +## Risks |
| 180 | + |
| 181 | +- **Blast radius in core `MetaData`.** Storage + accessor rewrite is central. Mitigation: the value |
| 182 | + accessors keep their signatures; the test suite + byte-identical conformance are the guardrail. |
| 183 | +- **Effective-attr / merge semantics drift.** Instance-based super-chain + overlay merge must match |
| 184 | + the current last-writer-wins behavior exactly. Mitigation: existing extends/overlay tests + the |
| 185 | + conformance corpus. |
| 186 | +- **Canonical output drift.** Only `attr-properties-basic` should change. Mitigation: diff the full |
| 187 | + corpus canonical output; any other change is a bug. |
0 commit comments