From e84f7a6e68cd0a22e936381d5e69333a7e5e627c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 02:30:58 -0400 Subject: [PATCH] docs(plan): script-editor ctx tag-completion correctness + truthfulness design --- ...18-script-editor-ctx-completions-design.md | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 docs/plans/2026-06-18-script-editor-ctx-completions-design.md diff --git a/docs/plans/2026-06-18-script-editor-ctx-completions-design.md b/docs/plans/2026-06-18-script-editor-ctx-completions-design.md new file mode 100644 index 00000000..3ac83096 --- /dev/null +++ b/docs/plans/2026-06-18-script-editor-ctx-completions-design.md @@ -0,0 +1,105 @@ +# Script-editor `ctx` tag-completion correctness + truthfulness — Design + +> Brainstormed 2026-06-18. Backlog item: stillpending.md §4 "Monaco `ctx.SetVirtualTag(...)` +> write-target completions/hover are best-effort" (`AdminUI/ScriptAnalysis/IScriptTagCatalog.cs`, +> `ScriptAnalysisService.cs`). Off master `651018f9`. + +## Problem (verified in code) + +The Monaco script editor's tag-path completion + hover (ScriptEdit page and the inline virtual-tag +modal) route through one private gate, `ScriptAnalysisService.TryGetTagPathLiteral(token, …)`. Two +defects: + +1. **Receiver-blind gate.** `TryGetTagPathLiteral` accepts ANY `.GetTag("…")` / + `.SetVirtualTag("…")` — it matches the member-access *method name* only + (`method is not ("GetTag" or "SetVirtualTag")`) and never inspects the receiver. So a stray + `someOtherObject.GetTag("…")` wrongly triggers tag-path completion/hover. This makes the editor + **more permissive than the runtime**, whose dependency harvest is syntactically `ctx`-anchored + (`Commons/Types/EquipmentScriptPaths.GetTagRefRegex` = `ctx\s*\.\s*(?:GetTag|SetVirtualTag)\s*\(`, + consumed by `Phase7Composer` to build `EquipmentVirtualTagPlan.DependencyRefs`). + +2. **`SetVirtualTag` advertises a no-op.** `IScriptTagCatalog`/`ScriptTagCatalog` emits a virtual + tag's leaf `Name` as a `SetVirtualTag` write-target completion, documented as "best-effort … the + live resolution of virtual-tag cascade/write targets is unconfirmed." It is now CONFIRMED: + **`ctx.SetVirtualTag(...)` is a no-op in production.** The active evaluator binding + `Host/Engines/RoslynVirtualTagEvaluator.cs` runs in **single-tag mode** and *drops* every + cross-tag write (logged: "cross-tag write to {Path} dropped (single-tag adapter)"). The cross-tag + cascade engine that DOES honor `SetVirtualTag` (`Core.VirtualTags.VirtualTagEngine`, + `OnScriptSetVirtualTag` validating the target against registered vtags) is **dormant** — not wired + into the host. So there is **no production write-target resolution** to be "authoritative" about. + +## Scope decision + +The original audit framed a fix sub-goal as "return *authoritative* `SetVirtualTag` write-target +paths." Given finding (2), that goal is **moot and explicitly dropped** — the cascade feature is +dormant; no authoritative production key exists. This phase ships the genuinely-fixable, fully +live-verifiable core: + +- **Component 1 — receiver-type guard** (correctness) +- **Component 2 — truthful `SetVirtualTag` hover** (honesty) + +Both AdminUI-only. **No runtime, Commons/wire/proto, or EF change. No bUnit.** + +## Component 1 — receiver-type guard + +In `TryGetTagPathLiteral`, after the member-access (`ma`) and method-name checks, require the +receiver to be the identifier `ctx`: + +```csharp +if (ma.Expression is not IdentifierNameSyntax { Identifier.ValueText: "ctx" }) return false; +``` + +This deliberately mirrors the runtime regex (syntactic `ctx` anchor) so the editor accepts EXACTLY +what `Phase7Composer` harvests — not a semantic "is the receiver a `ScriptContext`" check, which +could accept a differently-named local that the runtime regex would never harvest. The guard lives +in the single shared gate, so it fixes both `Complete` and `Hover` at once. Effect: `foo.GetTag("…")` +no longer offers tag completions/hover; only `ctx.GetTag(…)` / `ctx.SetVirtualTag(…)` do. + +## Component 2 — truthful `SetVirtualTag` + +`TryGetTagPathLiteral` surfaces WHICH method matched (a small out-param / enum, e.g. +`ScriptTagMethod { GetTag, SetVirtualTag }`). The `Hover` path, when the matched method is +`SetVirtualTag`, appends a clear note to the hover markdown: + +> ⚠ 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. + +So an author is no longer silently misled into relying on a no-op. The completion list still helps +type a path (the truthfulness signal is the hover). Restricting `SetVirtualTag` *suggestions* to +virtual-tag targets only is a possible refinement left to the implementation plan — NOT a hard +requirement — because the dormant-engine write-target key is itself unverified (and the whole write +is dropped), so over-engineering the suggestion set adds risk without runtime payoff. + +## Error handling + +Unchanged. `TryGetTagPathLiteral` already returns `false` cleanly on every non-match; the receiver +guard is one more early `return false`. The method-match out-param is only read when the gate returns +`true`. + +## Testing + +xUnit + Shouldly in the existing AdminUI test project, exercising `ScriptAnalysisService.Complete` / +`Hover` directly (NO bUnit — these are service-level, not Razor): + +- `ctx.GetTag("` → tag-path completions offered. +- `foo.GetTag("` / `something.SetVirtualTag("` → tag-path completions NOT offered (receiver guard). +- Hover on a `ctx.SetVirtualTag("…")` literal → markdown carries the single-tag-drop note. +- Hover on a `ctx.GetTag("…")` literal → unchanged (no spurious note). + +**Live `/run`** on the docker-dev rig (login-disabled; Chrome-driven against `http://localhost:9200` +ScriptEdit page + inline vtag modal Monaco): confirm `ctx.GetTag("` shows completions, a non-`ctx` +receiver does not, and the `SetVirtualTag` hover shows the truthful note. + +## Deferred / out of scope + +- "Authoritative `SetVirtualTag` write-target key" — moot (cascade engine dormant; production write + is a no-op). +- UNS browse-path as a non-inserted completion *detail* (the catalog's own documented discoverability + follow-up) — not in this phase. +- Wiring the cross-tag cascade engine into the host (the real fix for making `SetVirtualTag` + functional) — a much larger, separate effort. + +## Done = + +Build clean + `dotnet test` (AdminUI) green + live `/run` confirms the three behaviors → merge to +master + push.