Files
lmxopcua/docs/plans/2026-06-18-script-editor-ctx-completions.md
T

264 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 213229): 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 268272): 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.