docs(plan): script-editor ctx tag-completion implementation plan + tasks

2026-06-18 02:34:45 -04:00
parent e84f7a6e68
commit 74aad3bc87
2 changed files with 277 additions and 0 deletions
@@ -0,0 +1,263 @@
+# Script-editor `ctx` tag-completion correctness + truthfulness — Implementation Plan
+
+> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task.
+
+**Goal:** Make the Monaco script-editor's tag-path completion/hover fire only on `ctx.` receivers (matching the runtime harvest) and stop advertising `ctx.SetVirtualTag(...)` cross-tag writes as if they work (they are a no-op in the live single-tag engine).
+
+**Architecture:** Both `ScriptAnalysisService.CompleteAsync` and `.Hover` route through one private syntactic gate, `TryGetTagPathLiteral`. Add a receiver guard (`ctx` identifier) there, and surface which method matched so `Hover` can append a truthful note for `SetVirtualTag`. AdminUI-only; no runtime/Commons/proto/EF change; no bUnit.
+
+**Tech Stack:** C# / .NET 10, Roslyn syntax API (`Microsoft.CodeAnalysis.CSharp`), xUnit + Shouldly. Design: `docs/plans/2026-06-18-script-editor-ctx-completions-design.md` (committed `e84f7a6e`).
+
+---
+
+### Task 1: Receiver-type guard + truthful `SetVirtualTag` hover
+
+**Classification:** small
+**Estimated implement time:** ~5 min
+**Parallelizable with:** none
+
+**Files:**
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs`
+  - `TryGetTagPathLiteral` (currently ~line 213–229): add the `ctx` receiver guard + a new `out bool isSetVirtualTag`.
+  - `CompleteAsync` call site (~line 164): pass `out _` for the new param.
+  - `Hover` call site (~line 255) + the `tagMd` build (~line 268–272): pass `out var isSetVirtualTag`; append the single-tag-drop note when it is true.
+- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs` (create)
+
+(The `Files:` block IS the contract — do not touch the runtime, Commons, or the catalog. `TryGetTagPathLiteral` is purely syntactic; keep it that way — do NOT introduce a semantic/type check on the receiver. The `ctx` identifier match is intentional: it mirrors `Commons/Types/EquipmentScriptPaths.GetTagRefRegex` = `ctx\s*\.\s*(?:GetTag|SetVirtualTag)`, so the editor accepts exactly what `Phase7Composer` harvests.)
+
+**Context — current gate (verbatim):**
+```csharp
+private static bool TryGetTagPathLiteral(SyntaxToken token, out string prefix)
+{
+    prefix = "";
+    var literal = token.Parent as LiteralExpressionSyntax
+        ?? token.GetPreviousToken().Parent as LiteralExpressionSyntax;
+    if (literal is null || !literal.IsKind(SyntaxKind.StringLiteralExpression)) return false;
+    if (literal.Parent is not ArgumentSyntax arg) return false;
+    if (arg.Parent is not ArgumentListSyntax argList) return false;
+    if (argList.Parent is not InvocationExpressionSyntax inv) return false;
+    if (inv.Expression is not MemberAccessExpressionSyntax ma) return false;
+    var method = ma.Name.Identifier.ValueText;
+    if (method is not ("GetTag" or "SetVirtualTag")) return false;
+    // only the FIRST argument is the path
+    if (argList.Arguments.Count > 0 && argList.Arguments[0] != arg) return false;
+    prefix = literal.Token.ValueText ?? "";
+    return true;
+}
+```
+
+The two callers today:
+- `CompleteAsync`: `if (_catalog != null && TryGetTagPathLiteral(token, out var pathPrefix))`
+- `Hover`: `if (_catalog is not null && TryGetTagPathLiteral(token, out var tagPath) && !string.IsNullOrEmpty(tagPath))`, which then builds:
+```csharp
+var info = await _catalog.GetTagInfoAsync(tagPath, CancellationToken.None);
+var tagMd = info is null
+    ? $"**Tag path** `{Code(tagPath)}`\n\n⚠ Not a known configured tag path."
+    : $"**Tag path** `{Code(info.Path)}`\n\n{info.Kind} · Type **{info.DataType}**"
+        + (info.DriverInstanceId is null ? "" : $" · Driver `{Code(info.DriverInstanceId)}`");
+return new HoverResponse(tagMd);
+```
+
+**Step 1: Write the failing tests**
+
+Create `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs`:
+
+```csharp
+using System.Threading;
+using System.Threading.Tasks;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+public sealed class CtxCompletionGuardTests
+{
+    // Catalog that always offers one distinctive path + resolves a vtag, so the tag-path branch is
+    // observable without a DB. The path "Line1.Speed" never appears in C# scoped/dot completions,
+    // so its presence/absence cleanly distinguishes the tag-path branch from the fallback.
+    private sealed class FakeCatalog : IScriptTagCatalog
+    {
+        public Task<IReadOnlyList<string>> GetPathsAsync(string? f, CancellationToken ct)
+            => Task.FromResult<IReadOnlyList<string>>(new[] { "Line1.Speed" });
+        public Task<ScriptTagInfo?> GetTagInfoAsync(string path, CancellationToken ct)
+            => Task.FromResult<ScriptTagInfo?>(new ScriptTagInfo(path, "Virtual tag", "Double", null));
+        public Task<IReadOnlyList<string>> GetEquipmentRelativeLeavesAsync(string? f, CancellationToken ct)
+            => Task.FromResult<IReadOnlyList<string>>(System.Array.Empty<string>());
+    }
+
+    private static readonly ScriptAnalysisService Svc = new(new FakeCatalog());
+
+    // Caret column lands INSIDE the string literal (just after the opening quote). If a test does not
+    // hit the tag-path branch, nudge the column to sit on a literal char and re-run — the gate keys off
+    // the token under the caret. Columns are 1-based.
+
+    [Fact] public async Task Completion_on_ctx_GetTag_literal_offers_catalog_paths()
+    {
+        // return ctx.GetTag("|   — caret just after the opening quote
+        var res = await Svc.CompleteAsync(new CompletionsRequest("""return ctx.GetTag("""" + "\"", 1, 19));
+        res.Items.ShouldContain(i => i.InsertText == "Line1.Speed");
+    }
+
+    [Fact] public async Task Completion_on_non_ctx_receiver_does_NOT_offer_catalog_paths()
+    {
+        // foo.GetTag("|  — receiver is not ctx, so the tag-path branch must NOT fire.
+        var res = await Svc.CompleteAsync(new CompletionsRequest("""return foo.GetTag("""" + "\"", 1, 19));
+        res.Items.ShouldNotContain(i => i.InsertText == "Line1.Speed");
+    }
+
+    [Fact] public async Task Hover_on_ctx_SetVirtualTag_literal_warns_write_is_dropped()
+    {
+        // hover the path literal inside ctx.SetVirtualTag("V")
+        var md = (await Svc.Hover(new HoverRequest("""return ctx.SetVirtualTag("V", 1);""", 1, 27))).Markdown;
+        md.ShouldNotBeNull();
+        md!.ShouldContain("single-tag mode");
+    }
+
+    [Fact] public async Task Hover_on_ctx_GetTag_literal_has_no_dropped_write_note()
+    {
+        var md = (await Svc.Hover(new HoverRequest("""return ctx.GetTag("V").Value;""", 1, 20))).Markdown;
+        md.ShouldNotBeNull();
+        md!.ShouldNotContain("single-tag mode");
+    }
+
+    [Fact] public async Task Hover_on_non_ctx_receiver_literal_is_not_treated_as_a_tag_path()
+    {
+        // bar.GetTag("V") — receiver not ctx; must not produce the "Tag path" hover.
+        var md = (await Svc.Hover(new HoverRequest("""return bar.GetTag("V");""", 1, 20))).Markdown;
+        (md is null || !md.Contains("Tag path")).ShouldBeTrue();
+    }
+}
+```
+
+**Step 2: Run the tests; verify they FAIL**
+
+Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests --filter "FullyQualifiedName~CtxCompletionGuardTests"` (use `dangerouslyDisableSandbox: true`).
+Expected: the non-`ctx` tests FAIL (today the gate fires on any receiver) and the SetVirtualTag-note test FAILS (no note yet). The two positive `ctx.` tests may already pass. If a caret-column does not hit the literal branch, adjust the column and re-run until the positive cases hit the tag-path branch, THEN confirm the negatives still fail for the right reason.
+
+**Step 3: Implement Component 1 (receiver guard) + Component 2 (method-discriminated note)**
+
+In `TryGetTagPathLiteral`, change the signature and body:
+```csharp
+private static bool TryGetTagPathLiteral(SyntaxToken token, out string prefix, out bool isSetVirtualTag)
+{
+    prefix = "";
+    isSetVirtualTag = false;
+    var literal = token.Parent as LiteralExpressionSyntax
+        ?? token.GetPreviousToken().Parent as LiteralExpressionSyntax;
+    if (literal is null || !literal.IsKind(SyntaxKind.StringLiteralExpression)) return false;
+    if (literal.Parent is not ArgumentSyntax arg) return false;
+    if (arg.Parent is not ArgumentListSyntax argList) return false;
+    if (argList.Parent is not InvocationExpressionSyntax inv) return false;
+    if (inv.Expression is not MemberAccessExpressionSyntax ma) return false;
+    // Receiver guard: only ctx.GetTag(...) / ctx.SetVirtualTag(...) are real tag-path calls. Mirrors the
+    // runtime harvest (EquipmentScriptPaths.GetTagRefRegex is syntactically `ctx`-anchored), so the editor
+    // offers tag completions/hover for exactly what Phase7Composer harvests — not an unrelated x.GetTag(...).
+    if (ma.Expression is not IdentifierNameSyntax { Identifier.ValueText: "ctx" }) return false;
+    var method = ma.Name.Identifier.ValueText;
+    if (method is not ("GetTag" or "SetVirtualTag")) return false;
+    // only the FIRST argument is the path
+    if (argList.Arguments.Count > 0 && argList.Arguments[0] != arg) return false;
+    prefix = literal.Token.ValueText ?? "";
+    isSetVirtualTag = method == "SetVirtualTag";
+    return true;
+}
+```
+
+Update `CompleteAsync` (the tag-path branch condition) to discard the new param:
+```csharp
+if (_catalog != null && TryGetTagPathLiteral(token, out var pathPrefix, out _))
+```
+
+Update `Hover`'s condition + `tagMd` build:
+```csharp
+if (_catalog is not null && TryGetTagPathLiteral(token, out var tagPath, out var isSetVirtualTag) && !string.IsNullOrEmpty(tagPath))
+{
+    ...
+    var info = await _catalog.GetTagInfoAsync(tagPath, CancellationToken.None);
+    var tagMd = info is null
+        ? $"**Tag path** `{Code(tagPath)}`\n\n⚠ Not a known configured tag path."
+        : $"**Tag path** `{Code(info.Path)}`\n\n{info.Kind} · Type **{info.DataType}**"
+            + (info.DriverInstanceId is null ? "" : $" · Driver `{Code(info.DriverInstanceId)}`");
+    // Truthfulness: ctx.SetVirtualTag(...) is a no-op in the live single-tag evaluator
+    // (RoslynVirtualTagEvaluator drops cross-tag writes; the cascade VirtualTagEngine is dormant).
+    // Surface that so an author is not misled into relying on it.
+    if (isSetVirtualTag)
+        tagMd += "\n\n⚠ Cross-tag `SetVirtualTag` writes are currently dropped in single-tag mode " +
+                 "(the cascade engine is not wired into the host); this write will not take effect at runtime.";
+    return new HoverResponse(tagMd);
+}
+```
+(Leave the `{{equip}}` short-circuit branch above this untouched — `SetVirtualTag("{{equip}}.x")` is a rare combination and out of scope.)
+
+**Step 4: Run the tests; verify they PASS**
+
+Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests --filter "FullyQualifiedName~CtxCompletionGuardTests"`
+Expected: all 5 PASS.
+
+**Step 5: Run the full AdminUI suite to confirm no regression**
+
+Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests` (use `dangerouslyDisableSandbox: true`).
+Expected: green. Existing tag-path tests (e.g. `HoverSignatureTests`) use `ctx.` receivers, so the guard does not affect them. If any pre-existing test used a non-`ctx` receiver on a tag-path literal, that is a real behavior change — surface it, do not weaken the guard.
+
+**Step 6: Commit**
+```bash
+git add src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs \
+        tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs
+git commit -m "fix(adminui): ctx-receiver guard + truthful SetVirtualTag hover in script-editor completions"
+```
+
+---
+
+### Task 2: Docs + full build/test + live `/run` + finish
+
+**Classification:** small
+**Estimated implement time:** ~4 min (excluding the live-rig step, which the controller drives)
+**Parallelizable with:** none (blocked by Task 1)
+
+**Files:**
+- Modify: `docs/ScriptEditor.md` (the completions/hover section — note the `ctx`-receiver scoping and that `SetVirtualTag` cross-tag writes are dropped in single-tag mode, hover surfaces this).
+
+**Step 1: Update docs**
+
+In `docs/ScriptEditor.md`, find the section describing tag-path completions/hover and add a short, accurate note:
+- Tag-path completions/hover fire only on `ctx.GetTag(...)` / `ctx.SetVirtualTag(...)` (a non-`ctx` receiver is not treated as a tag call), matching the deploy-time dependency harvest.
+- `ctx.SetVirtualTag(...)` cross-tag writes are currently dropped by the live single-tag evaluator (the cascade engine is not wired into the host); the editor's hover surfaces this so it is not relied upon. Do NOT overclaim a fix to the cross-tag write path itself.
+
+**Step 2: Full build**
+
+Run: `dotnet build ZB.MOM.WW.OtOpcUa.slnx` (use `dangerouslyDisableSandbox: true`).
+Expected: 0 errors.
+
+**Step 3: AdminUI tests**
+
+Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests` (use `dangerouslyDisableSandbox: true`).
+Expected: green.
+
+**Step 4: Commit docs**
+```bash
+git add docs/ScriptEditor.md
+git commit -m "docs(scripteditor): ctx-receiver scoping + SetVirtualTag single-tag-drop note"
+```
+
+**Step 5: Live `/run` verification (controller-driven, docker-dev)**
+
+The docker-dev rig is LOCAL + login-disabled (no sign-in needed). Rebuild the AdminUI from the branch and drive the script editor via the Chrome tools against `http://localhost:9200`:
+- Open a virtual-tag's ScriptEdit page (or the inline vtag-modal Monaco).
+- Type `ctx.GetTag("` → tag-path completions appear.
+- Type a non-`ctx` receiver `foo.GetTag("` → tag-path completions do NOT appear (C# completions only).
+- Hover a `ctx.SetVirtualTag("…")` literal → the single-tag-drop note shows.
+
+If the rig cannot exercise a step (e.g. Monaco completion timing), record it honestly as unit-proven + smoke-checked rather than overclaiming.
+
+**Step 6: Finish — merge to master + push**
+
+Use superpowers-extended-cc:finishing-a-development-branch: verify tests green, then merge `feat/script-editor-ctx-completions` to master (ff) and push. Stage by explicit path only; never `git add .`; never stage `stillpending.md` / `pending.md` / `docker-dev/docker-compose.yml` / `sql_login.txt` / `pki/`.
+
+---
+
+## Notes for the executor
+
+- **Two tasks, sequential** (Task 2 blocked by Task 1) — both touch a tightly-coupled surface; no parallelism.
+- **Hard rules:** stage by path (never `git add .`); never stage the never-stage files; NO runtime/Commons/proto/EF change; NO bUnit; `dangerouslyDisableSandbox: true` for all build/test/rig commands.
+- **Done =** build 0 errors + AdminUI tests green + live `/run` confirms the three behaviors → merged to master + pushed.
@@ -0,0 +1,14 @@
+{
+  "planPath": "docs/plans/2026-06-18-script-editor-ctx-completions.md",
+  "designPath": "docs/plans/2026-06-18-script-editor-ctx-completions-design.md",
+  "designCommit": "e84f7a6e",
+  "baseMaster": "651018f9",
+  "branch": "feat/script-editor-ctx-completions",
+  "scope": "AdminUI-only ScriptAnalysis fix. Component 1: ctx-receiver guard in ScriptAnalysisService.TryGetTagPathLiteral (mirror the runtime EquipmentScriptPaths.GetTagRefRegex `ctx`-anchor) so tag-path completion/hover fires only on ctx.GetTag/ctx.SetVirtualTag, not an unrelated x.GetTag(...). Component 2: surface which method matched; Hover appends a truthful note that ctx.SetVirtualTag cross-tag writes are dropped in single-tag mode (RoslynVirtualTagEvaluator drops them; cascade VirtualTagEngine dormant). The 'authoritative write-key' goal is dropped as moot. NO runtime/Commons/proto/EF change; NO bUnit.",
+  "dependencyGraph": "T1 -> T2",
+  "tasks": [
+    {"id": 1, "nativeId": 532, "subject": "ctx-receiver guard + truthful SetVirtualTag hover", "classification": "small", "status": "pending"},
+    {"id": 2, "nativeId": 533, "subject": "Docs + build/test + live /run + finish", "classification": "small", "status": "pending", "blockedBy": [1]}
+  ],
+  "lastUpdated": "2026-06-18"
+}