From 91b786eb1cf72f5ab1008a32af422c58561a5bef Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 12 May 2026 08:18:43 -0400 Subject: [PATCH] docs(templates): derive-on-compose phase status + resume plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Companion to the design doc — captures current state, the four decisions already made, what's done (phase 1, commit 5615f3d), and a full play-by-play for phases 2 through 9 with exact files, methods, and tests to touch. Written so a future session after context compaction can pick up cleanly. --- .../2026-05-12-derive-on-compose-status.md | 270 ++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 docs/plans/2026-05-12-derive-on-compose-status.md diff --git a/docs/plans/2026-05-12-derive-on-compose-status.md b/docs/plans/2026-05-12-derive-on-compose-status.md new file mode 100644 index 0000000..cb73570 --- /dev/null +++ b/docs/plans/2026-05-12-derive-on-compose-status.md @@ -0,0 +1,270 @@ +# Derive-on-compose: implementation status + remaining plan + +> **For Claude resuming after compaction:** This is the single starting point. +> Read it in full. Then read the companion design doc +> `2026-05-12-derive-on-compose-design.md` for the architectural rationale. +> Do NOT make code changes until you've also confirmed which phase the user +> wants next. + +## Where we are + +**Branch**: `feature/templates-folder-hierarchy`. Already pushed. + +**Last commit on this feature**: `5615f3d` — *Phase 1 complete, additive +schema only*. + +**All test suites currently green**: +- `tests/ScadaLink.CentralUI.Tests` — 159 passing +- `tests/ScadaLink.SiteRuntime.Tests` — 129 passing +- `tests/ScadaLink.TemplateEngine.Tests` — 199 passing + +## Design decisions already made (from the brainstorm) + +User picked the **full Aveva model** with all four customization scopes: + +- **Naming**: dot-separated → `Pump.TempSensor` +- **Delete base with derivatives**: block with a list of the dependents +- **Migration of existing compositions**: auto-migrate all on the EF Core + migration step in Phase 3 +- **Tree UX**: derived templates hidden by default; toggle to reveal +- **Customization scope**: override attribute values, override script bodies, + add new attrs/scripts per slot, lock fields against override + +## Done — Phase 1: Additive schema + +Commits: `6854843` (design doc) + `a968cef` (decisions recorded) + `5615f3d`. + +Files touched in `5615f3d`: + +- `src/ScadaLink.Commons/Entities/Templates/Template.cs` + - Added `IsDerived: bool` + - Added `OwnerCompositionId: int?` (plain int — not an EF nav prop) +- `src/ScadaLink.Commons/Entities/Templates/TemplateAttribute.cs` + - Added `IsInherited: bool` + - Added `LockedInDerived: bool` +- `src/ScadaLink.Commons/Entities/Templates/TemplateScript.cs` + - Same two fields +- `src/ScadaLink.ConfigurationDatabase/Migrations/20260512121446_AddDerivedTemplateFields.cs` + - EF Core migration. Six new columns, all NOT NULL DEFAULT 0 (or nullable + int). No data transform — existing rows get defaults. +- `ScadaLinkDbContextModelSnapshot.cs` regenerated. + +**No behavior changes**. New fields are never read or written yet. + +## Remaining — phases 2 through 9 + +Each phase ships independently. The cumulative behavior change fully lands at +phase 3. + +### Phase 2: Compose flow change + +`TemplateService.AddCompositionAsync` currently creates a `TemplateComposition` +that references a base template directly. Change it so NEW compositions: + +1. Create a new `Template` entity: + - `Name = $"{parentTemplate.Name}.{instanceName}"` + - `ParentTemplateId = composedTemplateId` (the base) + - `IsDerived = true` + - `FolderId = null` (or inherit from parent? — pick null; derived + templates aren't visible in the tree) +2. Copy the base's `Attributes` into the new template marked + `IsInherited = true`. Same for `Scripts` and `Alarms`. +3. Create the `TemplateComposition` with `ComposedTemplateId = newTemplate.Id` + (the derived, not the base). +4. Set the derived template's `OwnerCompositionId = composition.Id` (back-ref). + +Files to touch: + +- `src/ScadaLink.TemplateEngine/TemplateService.cs` — extend + `AddCompositionAsync` (currently around line 290+). +- `src/ScadaLink.TemplateEngine/TemplateService.cs` — also extend + `DeleteCompositionAsync` to cascade-delete the derived template. +- `src/ScadaLink.TemplateEngine/TemplateService.cs` — extend `DeleteTemplateAsync` + to block when the template is composed (already happens) AND when + derivatives exist; reword the error. +- Validation: derived template names must remain unique (the base existing + uniqueness index on `Template.Name` covers this — but if `Pump.TempSensor` + collides with a user-created template of that name, the add fails. Add a + guard with a clear error message.) + +Tests to add (`tests/ScadaLink.TemplateEngine.Tests/`): + +- AddCompositionAsync creates a derived template. +- The derived template inherits attributes / scripts with `IsInherited = true`. +- DeleteCompositionAsync cascades to remove the derived template. +- Deleting a base with derivatives is blocked with a message that lists them. + +### Phase 3: Migration of existing compositions + +EF Core data migration that runs on next startup. Two paths: + +**Option A (simpler) — write the migration as a custom SQL/code step** + +In `src/ScadaLink.ConfigurationDatabase/Migrations/`, generate a new migration +called e.g. `MigrateCompositionsToDerived`. In its `Up` method, use raw +`migrationBuilder.Sql(...)` or a delegate to: + +``` +FOREACH composition IN TemplateComposition WHERE ComposedTemplate.IsDerived = false: + derived := INSERT INTO Templates ( + Name = parent.Name || '.' || composition.InstanceName, + ParentTemplateId = composition.ComposedTemplateId, + IsDerived = true, + OwnerCompositionId = composition.Id) + INSERT INTO TemplateAttributes (Name, Value, DataType, IsLocked, Description, + DataSourceReference, IsInherited=true, + LockedInDerived=false, TemplateId=derived.Id) + FROM TemplateAttributes WHERE TemplateId = composition.ComposedTemplateId + same for TemplateScripts and TemplateAlarms + UPDATE TemplateComposition SET ComposedTemplateId = derived.Id WHERE Id = composition.Id +``` + +Run as raw SQL to keep the migration deterministic across providers (SQL +Server vs SQLite if used in dev). + +**Option B — service-layer one-time migration** + +Add a startup hook that runs once (controlled by a flag column) and uses +`TemplateService` directly. Cleaner for complex logic but more code. + +**Pick Option A** unless the logic doesn't fit comfortably in SQL. + +Validation post-migration: every `TemplateComposition.ComposedTemplateId` +should now reference a template with `IsDerived = true`. + +Tests: integration test that seeds pre-migration data, runs the migration, +asserts the new shape. + +### Phase 4: Inherit/override resolution in flattening + +`src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs`: + +- The service currently walks `Template.ParentTemplateId` chains in + `ResolveInheritedScripts`. That same chain now naturally includes the + base→derived link. No special handling needed for the chain itself. +- BUT when a derived template's attribute is `IsInherited = true`, the + effective value comes from the base (resolved through the chain). If + `IsInherited = false`, the derived's own value is the override and wins. +- Treat `LockedInDerived` as an enforcement check: if a derived row has + `IsInherited = false` (override) AND the base row has `LockedInDerived = + true`, emit a flattening validation error. + +Tests: round-trip flattening with overridden + inherited combinations. + +### Phase 5: Lock semantics enforcement + +In `TemplateService.UpdateAttributeAsync` and `UpdateScriptAsync`: + +- If the target template has `IsDerived = true`, look up the base via + `ParentTemplateId`. +- If the same-named attribute on the base has `LockedInDerived = true`, + reject the update with a clear error: *"This attribute is locked by the + base template '$Sensor' and cannot be overridden."* +- If the update is being applied on a base template AND there are + derivatives that have `IsInherited = false` for this field (overrides), the + caller may want to know — surface a warning result. Out of scope for now; + flag as TODO. + +Tests: lock-blocked updates fail with the expected message. + +### Phase 6: Template tree UI — hide derived + +`src/ScadaLink.CentralUI/Components/Pages/Design/Templates.razor`: + +- Filter `_templates` to exclude `t.IsDerived` by default. +- Add a "Show derived templates" checkbox at the top. +- When shown, render derived templates indented under their base + (`t.ParentTemplateId == base.Id && t.IsDerived`). +- Compositions tab on `TemplateEdit` should link each composition row to the + derived template's edit page (the existing link likely points to the base + — needs to follow `composition.ComposedTemplateId` which after phase 3 is + the derived). + +### Phase 7: Derived TemplateEdit UI + +`src/ScadaLink.CentralUI/Components/Pages/Design/TemplateEdit.razor`: + +- Top banner when `_selectedTemplate.IsDerived`: + *"Derived from `[base.Name]` — composed inside `[parent.Name]` as + `[composition.InstanceName]`."* +- Attributes table: new column showing **Inherited** or **Overridden** badge. +- Locked-from-base attributes (base has `LockedInDerived = true`) render + readonly with a 🔒 icon and tooltip *"Locked by base — cannot override."* +- Editing an inherited row flips it to override (`IsInherited = false`). +- "Revert to base" button per row when `IsInherited = false` — clears the + override; row reverts to the base value (`IsInherited = true`, value + cleared). +- Adding a new attribute/script: creates a row with `IsInherited = false` + (it's not from the base). +- Removing an inherited row should be blocked (the row exists because it's + on the base; can't remove it on the derived). Removing an own-added row + deletes normally. + +### Phase 8: Base TemplateEdit UI — lock toggle + +Same `TemplateEdit.razor`, when editing a base template: + +- New column on Attributes table: a 🔒 toggle representing `LockedInDerived`. +- Same on Scripts table. +- Tooltip on the toggle: *"Lock this against per-slot override in derived + templates."* + +When a base template is open and the user toggles `LockedInDerived = true` +while derived templates exist with overrides, surface a warning toast: +*"N derived templates currently override this. Existing overrides won't be +silently reverted but deploy validation will flag them."* + +### Phase 9: Editor metadata simplification + +Now that derived templates have a single owner, `BuildParentContextsAsync` +in `TemplateEdit.razor` simplifies: + +- **If the open template `IsDerived`**: parent = the single template that + owns the `OwnerCompositionId`. Always one — no picker. +- **If the open template is a base** (`!IsDerived`): suppress `Parent.*` + assistance entirely. Add a SCADA008 hint diagnostic on `Parent.*` use: + *"Parent access on a base template is ambiguous — override this script in + a derived template instead."* + +Remove the multi-parent `select` dropdown that was added in commit `0139c9c` +(or keep it but guard with a check that should never fire after phase 3). + +`GetTemplatesComposingAsync` (added in `0139c9c`) is still useful elsewhere — +keep it. + +## How to resume + +After compaction, the future session should: + +1. Read this file. Read the design doc + `2026-05-12-derive-on-compose-design.md` for context. +2. Run `git log --oneline -15` to confirm the branch is at `5615f3d` or + later. +3. Check test status: `dotnet test` across the three suites named above. +4. Ask the user which phase to tackle next (or proceed from phase 2 if the + user has explicitly said "continue"). +5. Each phase is its own commit. Prefer phase 2 + phase 3 together because + phase 2 alone leaves new and existing compositions in different shapes + until phase 3's migration runs. + +## Pre-existing capability worth knowing + +The script-scope editor work (commits `3ed05f0` → `0139c9c`) is already in +place. Scripts on derived templates will automatically benefit from the +single-parent context simplification in phase 9. The runtime accessors +(`Attributes` / `Children` / `Parent` / `Scope`) defined there continue to +work unchanged — the canonical-name paths they resolve are stable across +the derivation change. + +The multi-parent picker introduced in `0139c9c` becomes mostly dormant +after phase 3 (no template should be composed by multiple parents anymore +through the new flow). Plan to remove it in phase 9. + +## Quick sanity script (run before phase 2) + +```bash +git status --short # should be clean +git log --oneline -5 # top should include 5615f3d +dotnet build # 0 errors, 0 warnings +dotnet test tests/ScadaLink.TemplateEngine.Tests/ScadaLink.TemplateEngine.Tests.csproj +```