Files
ScadaBridge/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md
T
Joseph Doherty 1f261263b2 fix(centralui): surface inherited compositions in the templates tree (followup #9)
The templates tree rendered a derived/composed member (e.g. LeftReactorSide,
derived from ReactorSide) as a flat leaf, omitting compositions it inherits
from its base (e.g. LeakTest composed onto ReactorSide). BuildCompositionLeavesFor
recursed only over a template's OWN composition rows; an inherited composition
row lives on the ancestor, and TemplateComposition has no IsInherited placeholder
(unlike attributes/alarms/scripts/native-sources), so the child's own Compositions
was empty. Same 'derived templates don't surface inherited members' family as
followups #1/#2, but for compositions. Deploy/flatten was always correct
(TemplateResolver.ResolveAllMembers walks the chain) — display-only.

Fix:
- BuildCompositionLeavesFor now renders the EFFECTIVE composition set (own +
  inherited) via EffectiveCompositionsFor, which walks the inheritance chain
  (leaf->root, child wins on InstanceName), mirroring the resolver.
- Inherited slots are flagged (TemplateTreeNode.IsInherited), badged 'inherited'
  in the label, and their context menu offers only 'Open composed template'
  (Rename/Delete edit the ancestor's slot, so suppressed on inherited nodes).
- The same inherited row can appear under several derived members (LeakTest under
  both LeftReactorSide and RightReactorSide), so composition nodes use a
  path-qualified KeyOverride to keep TreeView selection/expansion keys unique;
  recursion is cycle-guarded.

Tests: +1 bUnit (TemplatesPageTests.Renders_InheritedComposition_UnderDerivedComposedMember);
CentralUI suite 867 green; full solution builds 0/0.

Docs: Component-CentralUI.md (effective composition set in tree); known-issues
tracker #9 recorded + resolved.

Note: CentralUI change — shows on wonder-app-vd03 only after that host is redeployed.
2026-06-24 19:29:48 -04:00

19 KiB
Raw Blame History

Follow-up tracker — template-inheritance UI gaps + CLI/validation footguns (2026-06-24 session)

Status: RESOLVED · Found: 2026-06-24 · Context: live ops session on wonder-app-vd03 (CvdReactor / Z28061 / Z28061Sim) — renaming the template, adding the LeakTest module, and adding MoveInType to the MESReceiver children. Components: Central UI (#9), Template Engine (#1), CLI (#19), Configuration Database (#17), Deployment Manager (#2)

Resolved: #3 (collision detector) and #7 (sandbox compile surface) on branch fix/followups-3-7; #1 + #2 (inherited-member propagation & resync) on branch fix/followups-1-2; #4 + #5 + #6 + #8 (CLI ergonomics + structured deploy validation error) and #9 (inherited compositions in the templates tree) on branch fix/followups-4-5-6-8 (all 2026-06-24). All items resolved.

Issues are listed worst-first. Severities are author estimates. None caused data loss; the runtime/flattened config and deployed instances are correct.


1. Template editor omits inherited-but-unmaterialized base attributes (user-reported)

Severity: Medium · Components: Central UI (#9), Template Engine (#1) · RESOLVED 2026-06-24 (branch fix/followups-1-2)

Fix: shared root cause with #2 — see #2's fix. Once a template's inherited rows are materialized (auto-propagation going forward, or the Resync action for already-stale templates), the editor's editable Attributes/Alarms/Scripts tabs list them. The "base changed" banner is now actionable: it carries a Resync inherited members button (TemplateEdit.razor) that calls TemplateService.ResyncInheritedMembersAsync and reloads. The read-only "Effective inherited set" preview is retained.

Symptom: On /design/templatesLeftMESReceiver, the Attributes tab does not list MoveInType. Same for RightMESReceiver. (Also missing from the list: all MoveOut* and ScanStateCmd.)

Root cause: MoveInType (String), the 12 MoveOut*, and ScanStateCmd live on the base MESReceiver template (id 3, 26 attrs). The derived children LeftMESReceiver/RightMESReceiver (ids 5/6) only have 12 materialized IsInherited rows (the MoveIn* basics) — the base attributes added after these children were derived were never materialized as child rows. The editor's main Attributes tab lists only the template's stored rows (TemplateEdit.razor:187GetAttributesByTemplateIdAsync(Id); count badge at :415), so the unmaterialized inherited members are invisible there.

Not a runtime bug: the flattener resolves the full chain, so LeftMESReceiver.MoveInType is in the flattened deploy config and is bound + live on the instances ('' / 'na', Good). The page also has an "Effective inherited set" read-only preview (M9-T26b, TemplateEdit.razor:465-491) + a staleness banner (:292-301) that do surface these — but the main, editable Attributes list does not.

Suggested fix: in the Attributes tab, render the resolved inherited set (with inherited badges) rather than only materialized rows — or auto-materialize missing inherited rows on load. See #2 (shared root cause).


2. Derived templates carry incomplete/stale IsInherited row sets

Severity: Medium · Components: Template Engine (#1), Configuration Database (#17) · RESOLVED 2026-06-24 (branch fix/followups-1-2)

Fix: root-cause fix — derived templates' stored inherited rows are now kept in sync two ways. (1) Auto-propagation: adding/updating/removing a member on a template now reconciles its entire derived subtree (TemplateService.ReconcileDescendantsAsync, called from every member-mutating path incl. native-alarm-source CRUD in the ManagementActor). (2) Resync: ResyncInheritedMembersAsync (CLI template resync-members, management ResyncInheritedMembersCommand, Designer-gated, audited; UI banner button) repairs a template + its subtree on demand — materializing missing placeholders, re-syncing drifted ones, removing orphans, across attributes/alarms/scripts/native sources. BuildDerivedTemplate also now materializes native-source placeholders at compose time (previously omitted, which made any inherited native source perpetually stale). Authored overrides are never touched. Covered by TemplateServiceTests (materialize / drift-update / orphan-remove / override-untouched / base-cascade / multi-type / propagation / end-to-end add). Documented in Component-TemplateEngine.md → "Inherited-Member Propagation & Resync".

Symptom: LeftMESReceiver/RightMESReceiver (parent=3) have 12 stored attribute rows vs the base's 26. By contrast LeftReactorSide/RightReactorSide (parent=7) mirror the full 61. So derived row-sets are inconsistent.

Root cause: when a base attribute is added after a child is derived, the child's stored IsInherited placeholder rows are not auto-resynced. There is a staleness banner but no surfaced one-click "resync inherited members" action (or it exists and was never run for these children). This is the data root cause of #1.

Suggested fix: a "resync inherited members" command (CLI + UI) that materializes missing base members onto derived children; consider running it automatically when a base attribute is added.


3. Collision detector blocks adding attributes/compositions to ANY derived template

Severity: Medium-High · Components: Template Engine (#1) · RESOLVED 2026-06-24 (branch fix/followups-3-7)

Fix: CollectDirectMembers now takes a skipInherited flag and skips IsInherited placeholder rows for the direct-template and inherited-parent walks (where the inheritance walk already re-adds those members under the parent's origin), while keeping them for the composed-module walk (the sole representation of a derived module's inherited members). Covered by CollisionDetectorTests (DerivedTemplateWithInheritedPlaceholders_NoFalseCollision, MultiLevelInheritedPlaceholders_NoFalseCollision, DerivedTemplate_GenuineCollisionStillDetected_DespiteInheritedPlaceholder) and the end-to-end TemplateServiceTests.AddAttribute_ToDerivedTemplateWithInheritedPlaceholders_Succeeds. Documented in Component-TemplateEngine.md → Naming Collision Detection.

Symptom: template attribute add --template-id 5 --name MoveInType ... fails with 13 "Naming collision" errors — the new attribute plus all 12 pre-existing inherited rows. Same class of failure when adding a composition to a derived template (hit earlier when trying to add LeakTest to LeftReactorSide).

Root cause: CollisionDetector.CollectDirectMembers (CollisionDetector.cs) emits every template.Attributes row — including IsInherited placeholders — with origin = child name, while CollectInheritedMembers emits the same names with origin = parent '…'. Two distinct origins for the same canonical name ⇒ reported as a collision. AddAttributeAsync (TemplateService.cs:294) and AddCompositionAsync (:869) both run DetectCollisions, so there is effectively no supported API path to extend a derived template — and the error message is misleading (it blames pre-existing inherited attrs, not the member you're adding).

Workaround used this session: add the feature module to the base template instead (LeakTest → base ReactorSide (7)); the flattener propagates it to all derivations.

Suggested fix: CollectDirectMembers should skip IsInherited rows (or the grouping should treat an inherited row and its parent source as the same origin), so only genuine cross-origin duplicates are flagged.


4. CLI instance set-bindings cannot set DataSourceReferenceOverride

Severity: Medium · Components: CLI (#19) · RESOLVED 2026-06-24 (branch fix/followups-4-5-6-8)

Fix: --bindings now accepts an optional third element per entry — [attributeName, dataConnectionId, dataSourceReferenceOverride] — so the CLI can set the per-instance reference override that the wire contract (ConnectionBinding) already carried. A string sets it; a JSON null or an omitted third element leaves it unset (template default). TryParseBindings accepts 2- or 3-element entries and rejects a non-string/non-null third element or 4+ elements with a clean validation error. The --bindings help and CLI README now document the full-replace behaviour (omitting the override on a re-bind clears any previously-set one). Covered by InstanceArgumentParsingTests (three-element / explicit-null / wrong-type / four-element).

Symptom: instance set-bindings --bindings only accepts [attributeName, dataConnectionId] pairs (InstanceCommands.csConnectionBinding(name, connId) 2-arg). The override is sent as null, and because SetConnectionBindingsAsync upserts DataSourceReferenceOverride = b.DataSourceReferenceOverride (InstanceService.cs:340), using the CLI on an attribute that already has an override would wipe it.

Workaround used this session: raw POST /management with {"command":"SetConnectionBindings","payload":{...,"dataSourceReferenceOverride":"…"}} — the wire contract ConnectionBinding(AttributeName, DataConnectionId, DataSourceReferenceOverride?) does carry the field; only the CLI omits it.

Suggested fix: add an optional 3rd element / --ref-override to the CLI bindings input.


5. CLI template update is full-replace, not partial

Severity: Low · Components: CLI (#19), Template Engine (#1) · RESOLVED 2026-06-24 (branch fix/followups-4-5-6-8)

Fix: TemplateService.UpdateTemplateAsync now uses leave-unchanged semantics for optional fields (fixed server-side, so every client benefits): a null description keeps the stored value (pass "" to explicitly clear it), and a null parentTemplateId keeps the existing parent. The parent remains immutable — a non-null value that differs from the current parent is still rejected — but omitting it (the CLI default) is now a no-op instead of tripping the immutability guard, which previously made template update fail on any derived template unless --parent-id was re-passed. CLI --description/--parent-id help, the UpdateTemplateCommand doc, and the CLI README document the semantics. Tests: UpdateTemplate_OmittedParentAndDescription_LeavesUnchanged, UpdateTemplate_EmptyDescription_ClearsIt (the prior UpdateTemplate_ClearParent_Fails was repurposed, since a null parent now means leave-unchanged rather than clear-and-fail).

Symptom: omitting --description on template update overwrites the stored description to NULL (TemplateService.cs:124-125 assigns Name+Description unconditionally). Renaming a template silently drops its description unless you re-pass it.

Suggested fix: treat omitted optional fields as "leave unchanged" (nullable-not-provided vs explicit-null), or warn when a non-empty description would be cleared.


6. (Minor) CLI template list/get table output dumps every attribute

Severity: Low · Components: CLI (#19) · RESOLVED 2026-06-24 (branch fix/followups-4-5-6-8)

Fix: template list/get table output is now a compact projection — id / name / description / parentTemplateId / isDerived plus member counts (#attrs, #alarms, #scripts, #comps, #nativeAlarms) — via a new TemplateTableProjection.ProjectSummary fed through an optional tableProjector seam on CommandHelpers.ExecuteCommandAsync/ HandleResponse. A --detail flag restores the full table dump. JSON output is deliberately left untouched (always the full payload) so machine consumers are unaffected — the projector only runs on the table path. Covered by TemplateTableProjectionTests (array/object projection, counts, non-JSON passthrough, size-shrink sanity check).

Symptom: --format table template list emitted ~171 KB (the full attribute set per template inline), unusable in a terminal. --format json is fine.

Suggested fix: a compact table projection (id/name/desc/#attrs/#comps) for list/get; reserve full attribute dumps for an explicit --verbose/--detail flag.


7. Central UI script editor false-flags batch/wait helpers (sandbox compile surface out of sync)

Severity: Medium · Components: Central UI (#9), Script Analysis (#25) · RESOLVED 2026-06-24 (branch fix/followups-3-7)

Fix: SandboxAttributeAccessor now mirrors the runtime AttributeAccessor — added WriteBatchAndWaitAsync, both WaitAsync overloads, and both WaitForAsync overloads with matching signatures. They throw a clearly-labelled ScriptSandboxException if exercised in Test Run (the central sandbox has no device-batch/event-waiter transport), but they now resolve at compile time so the editor stops false-flagging valid scripts. A reflection parity test isn't feasible across the CentralUI→SiteRuntime boundary (Central UI does not reference Site Runtime by design), so the guard is representative-script "diagnose clean" tests in ScriptAnalysisServiceTests (InstanceScript_BatchAndWaitHelpers_DiagnoseClean, ChildInstanceScript_WriteBatchAndWait_DiagnoseClean), consistent with how the inbound Database/WaitForAttribute and Notify surfaces are guarded. Documented in Component-ScriptAnalysis.md → Parity guard.

Symptom: In the template script editor (/design/templates/{id} → Scripts → Edit → Code), a script that calls Attributes.WriteBatchAndWaitAsync(...) (or on a child, Children["X"].Attributes.WriteBatchAndWaitAsync(...)) shows a red compile error: 'SandboxAttributeAccessor' does not contain a definition for 'WriteBatchAndWaitAsync' ... (CS1061). Confirmed on CvdReactor.MesMoveIn; the same false error hits the base MESReceiver.MoveIn/MoveOut, which also use the helper.

Root cause: the editor validates against the Central UI's own sandbox surface (CentralUI/ScriptAnalysis/SandboxScriptHost.cs). Its SandboxAttributeAccessor only defines this[string], GetAsync, SetAsync, Resolve — it is missing WriteBatchAndWaitAsync, WaitAsync, and WaitForAsync (none are defined anywhere in the CentralUI sandbox surface). The real runtime AttributeAccessor (SiteRuntime/Scripts/ScopeAccessors.cs) and the deploy-gate ScriptCompileSurface (ScriptAnalysis) both define them — so template validate reports the script clean and it deploys/runs fine. The error is purely the in-editor validator.

Impact: misleads authors and can block saving from the UI for any script using the batch-write/wait helpers, even though the script is valid. Authoring such scripts currently has to go through the management API (as was done for MesMoveIn).

Suggested fix: bring SandboxAttributeAccessor (and the sandbox composition/children accessors) to parity with the runtime AttributeAccessor — add WriteBatchAndWaitAsync / WaitAsync / WaitForAsync with matching signatures. Ideally enforce surface parity with a test, like the RoslynScriptCompiler "representative real script" corpus already does for ScriptCompileSurface. Three surfaces (runtime, deploy-gate ScriptCompileSurface, UI SandboxScriptHost) currently drift independently — consider collapsing to one source of truth.


8. Deploy-time unbound-binding validation returns one giant semicolon-joined error string

Severity: Low · Components: Template Engine (#1), Deployment Manager (#2) · RESOLVED 2026-06-24 (branch fix/followups-4-5-6-8)

Fix: new ValidationResult.SummarizeErrors() (Commons) returns a grouped, capped summary: a leading total count, one line per ValidationCategory, and within a category a per-module rollup (canonical name up to its last dot) with counts and a … and N more module(s) cap. DeploymentService now uses it for the Pre-deployment validation failed: message and logs the full per-entry list via LogWarning so nothing is lost. Entity-less findings (e.g. script-compile errors) fall back to a capped message list. Documented in Component-DeploymentManager.md → "Validation Error Reporting". Covered by ValidationResultSummaryTests (count header, module rollup, breadth cap, root grouping, message fallback, mixed categories).

Symptom: Deploying an instance whose data-sourced attributes aren't all bound fails with a single error that concatenates one clause per attribute: Pre-deployment validation failed: Attribute 'LeftReactorSide.LeakTest.DeltaVac' has a data source reference but no connection binding; Attribute 'LeftReactorSide.LeakTest.ResultType' has …; …. For 50194 unbound attrs (e.g. Z28062's unbound LeakTest members) it's a wall of text that's hard to scan in a CLI/UI toast.

Root cause: ValidateConnectionBindingCompleteness emits one clause per unbound attribute and joins them into a flat string; there is no grouping or count.

Suggested fix: return a structured/summarized error — leading count (52 attributes are unbound) + grouped-by-module breakdown (or a capped list with "…and N more") — instead of the flat semicolon-joined dump. Keep the full list available in a detail/expandable view or the deploy log.


9. Templates tree omits inherited compositions under derived composed-members (user-reported)

Severity: Low-Medium · Components: Central UI (#9) · RESOLVED 2026-06-24 (branch fix/followups-4-5-6-8)

Fix: Templates.razorBuildCompositionLeavesFor now renders the effective composition set (own + inherited) via a new EffectiveCompositionsFor that walks the inheritance chain (leaf→root, child wins on InstanceName), mirroring TemplateResolver.ResolveAllMembers. Inherited slots are flagged (TemplateTreeNode.IsInherited), badged "inherited" in the label, and their context menu offers only Open composed template (Rename/Delete edit the ancestor's slot, so they're suppressed on inherited nodes). Because the same inherited row can now appear under several derived members (LeakTest under both LeftReactorSide and RightReactorSide), composition nodes use a path-qualified KeyOverride (t:{owner}/c:{id}/…) so TreeView selection/expansion keys stay unique; the recursion is cycle-guarded. Covered by TemplatesPageTests.Renders_InheritedComposition_UnderDerivedComposedMember.

Symptom: On /design/templates, the base ReactorSide node expands to show its composed ↳ LeakTest. But under CvdReactor, the composed members LeftReactorSide / RightReactorSide (derived from ReactorSide) render as flat leaves with no LeakTest — even though they inherit it. Observed live on wonder-app-vd03.

Root cause: BuildCompositionLeavesFor(owner) recursed only over owner.Compositions (the template's own rows). A derived template's inherited composition row lives on its ancestor, and TemplateComposition has no IsInherited placeholder (unlike attributes/alarms/scripts/native-sources, which BuildDerivedTemplate/the #1/#2 reconcile materialize) — so the derived child's own Compositions is empty and the recursion found nothing. Same "derived templates don't surface inherited members" family as #1/#2, but for compositions, which the #1/#2 fix did not cover. Deploy/flatten was always correct (TemplateResolver.ResolveAllMembers walks the chain), so this was display-only.

Note: this is a Central UI change — it shows on wonder-app-vd03 only after that host is redeployed with the new build.