fix(adminui): script analysis reuses one compilation, honours cancellation, indexes the catalog (D2)

R04:P2 - every analysis derives from one process-wide bound base compilation via ReplaceSyntaxTree (the scoped service keeps its current compilation); every entry point takes the request token, the endpoints bind RequestAborted and answer 499 on abort, and the Monaco providers abort their fetch when Monaco cancels the request. R04:P3 - the catalog snapshot carries a sorted, de-duplicated string[] (binary-search prefix completion) and an Ordinal path->info dictionary, built once per snapshot. R04:S1 - Diagnose emits an OTSCRIPT_SETVTAG_INERT warning for each ctx.SetVirtualTag call. It is a warning, so editor-accepts == publish-accepts still holds. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 17:31:54 -04:00
parent 82380d2bee
commit 8e3bbf91fb
10 changed files with 612 additions and 97 deletions
@@ -0,0 +1,134 @@
+using System.Text;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+/// <summary>
+///     R04:P2 — an abandoned editor request (Monaco cancelled it, or the browser aborted the fetch) used to run
+///     its full compile to completion. The request's token now reaches the service, the Roslyn calls and the
+///     catalog, and the endpoints bind <c>HttpContext.RequestAborted</c>.
+/// </summary>
+public sealed class AnalysisCancellationTests
+{
+    internal sealed class CapturingCatalog : IScriptTagCatalog
+    {
+        public readonly List<CancellationToken> Seen = new();
+
+        public Task<IReadOnlyList<string>> GetPathsAsync(string? filter, CancellationToken ct)
+        {
+            Seen.Add(ct);
+            return Task.FromResult<IReadOnlyList<string>>(new[] { "Plant/Dev/Speed" });
+        }
+
+        public Task<ScriptTagInfo?> GetTagInfoAsync(string path, CancellationToken ct)
+        {
+            Seen.Add(ct);
+            return Task.FromResult<ScriptTagInfo?>(new ScriptTagInfo(path, "Tag", "Float", "DRV-1"));
+        }
+
+        public Task<IReadOnlyList<string>> GetEquipmentReferenceNamesAsync(string equipmentId, string? filter, CancellationToken ct)
+        {
+            Seen.Add(ct);
+            return Task.FromResult<IReadOnlyList<string>>(new[] { "Speed" });
+        }
+    }
+
+    private static CancellationToken Cancelled() => new(canceled: true);
+
+    [Fact]
+    public async Task Diagnose_with_a_cancelled_token_throws_instead_of_returning_empty_markers()
+    {
+        var svc = new ScriptAnalysisService();
+        // Positive control: the same request with a live token produces the error marker.
+        (await svc.DiagnoseAsync(new DiagnoseRequest("return ctx.Nope();"), CancellationToken.None))
+            .Markers.ShouldContain(m => m.Severity == 8);
+
+        await Should.ThrowAsync<OperationCanceledException>(
+            svc.DiagnoseAsync(new DiagnoseRequest("return ctx.Nope();"), Cancelled()));
+    }
+
+    [Fact]
+    public async Task Completions_hover_and_signature_help_honour_a_cancelled_token()
+    {
+        var svc = new ScriptAnalysisService(new CapturingCatalog());
+        const string code = "return ctx.GetTag(\"\");";
+
+        // Positive controls.
+        (await svc.CompleteAsync(new CompletionsRequest(code, 1, 20), CancellationToken.None)).Items.ShouldNotBeEmpty();
+        (await svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), CancellationToken.None))
+            .Markdown.ShouldNotBeNull();
+        svc.SignatureHelp(new SignatureHelpRequest("ctx.GetTag(", 1, 12), CancellationToken.None).Label.ShouldNotBeNull();
+
+        await Should.ThrowAsync<OperationCanceledException>(svc.CompleteAsync(new CompletionsRequest(code, 1, 20), Cancelled()));
+        await Should.ThrowAsync<OperationCanceledException>(
+            svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), Cancelled()));
+        Should.Throw<OperationCanceledException>(() => svc.SignatureHelp(new SignatureHelpRequest("ctx.GetTag(", 1, 12), Cancelled()));
+    }
+
+    [Fact]
+    public async Task The_request_token_reaches_the_catalog()
+    {
+        var catalog = new CapturingCatalog();
+        var svc = new ScriptAnalysisService(catalog);
+        using var cts = new CancellationTokenSource();
+
+        await svc.CompleteAsync(new CompletionsRequest("return ctx.GetTag(\"\");", 1, 20), cts.Token);
+        await svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), cts.Token);
+        await svc.DiagnoseAsync(new DiagnoseRequest("return ctx.GetTag(\"{{equip}}/Nope\").Value;", "EQ-1"), cts.Token);
+
+        catalog.Seen.Count.ShouldBe(3);
+        catalog.Seen.ShouldAllBe(t => t == cts.Token);
+    }
+
+    // ---------------------------------------------------------------- endpoints
+
+    // A bare DefaultHttpContext reports "no body"; Kestrel sets this feature for a real POST.
+    private sealed class HasBody : Microsoft.AspNetCore.Http.Features.IHttpRequestBodyDetectionFeature
+    {
+        public bool CanHaveBody => true;
+    }
+
+    internal static (WebApplication App, CapturingCatalog Catalog) BuildApp()
+    {
+        var builder = WebApplication.CreateBuilder();
+        var catalog = new CapturingCatalog();
+        builder.Services.AddSingleton<IScriptTagCatalog>(catalog);
+        builder.Services.AddScoped(sp => new ScriptAnalysisService(sp.GetRequiredService<IScriptTagCatalog>()));
+        builder.Services.AddAuthorization();
+        var app = builder.Build();
+        app.MapScriptAnalysisEndpoints();
+        return (app, catalog);
+    }
+
+    internal static IReadOnlyList<RouteEndpoint> Endpoints(WebApplication app)
+        => ((IEndpointRouteBuilder)app).DataSources.SelectMany(d => d.Endpoints).OfType<RouteEndpoint>().ToList();
+
+    [Fact]
+    public async Task The_endpoint_binds_RequestAborted_through_to_the_catalog()
+    {
+        var (app, catalog) = BuildApp();
+        var endpoint = Endpoints(app).Single(e => e.RoutePattern.RawText!.EndsWith("/completions"));
+
+        using var scope = app.Services.CreateScope();
+        using var aborted = new CancellationTokenSource();
+        var ctx = new DefaultHttpContext { RequestServices = scope.ServiceProvider, RequestAborted = aborted.Token };
+        ctx.Request.Method = "POST";
+        ctx.Request.ContentType = "application/json";
+        var body = Encoding.UTF8.GetBytes("""{"codeText":"return ctx.GetTag(\"\");","line":1,"column":20}""");
+        ctx.Request.Body = new MemoryStream(body);
+        ctx.Request.ContentLength = body.Length;
+        ctx.Response.Body = new MemoryStream();
+        ctx.Features.Set<Microsoft.AspNetCore.Http.Features.IHttpRequestBodyDetectionFeature>(new HasBody());
+
+        await endpoint.RequestDelegate!(ctx);
+
+        ctx.Response.StatusCode.ShouldBe(200);
+        catalog.Seen.ShouldHaveSingleItem().ShouldBe(aborted.Token);
+    }
+}
@@ -0,0 +1,58 @@
+using Microsoft.CodeAnalysis;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+/// <summary>
+///     R04:P2 — the editor used to build a brand-new <c>CSharpCompilation</c> (and re-bind the whole sandbox
+///     reference set) on every completion / hover / signature-help / diagnostics request. Each analysis now
+///     derives from ONE process-wide base compilation via <c>ReplaceSyntaxTree</c>, so the referenced-assembly
+///     binding is done once and shared.
+/// </summary>
+public sealed class AnalysisCompilationReuseTests
+{
+    [Fact]
+    public void Two_services_share_the_bound_reference_set()
+    {
+        // Two separate scopes (two HTTP requests) analysing different sources.
+        var c1 = new ScriptAnalysisService().CompilationFor("return 1;");
+        var c2 = new ScriptAnalysisService().CompilationFor("return 2;");
+
+        // Positive control: the two compilations really carry different user sources.
+        c1.SyntaxTrees.Single().ToString().ShouldContain("return 1;");
+        c2.SyntaxTrees.Single().ToString().ShouldContain("return 2;");
+        c1.ShouldNotBeSameAs(c2);
+
+        // Derived from one bound base: the corlib symbol is the SAME instance, not a re-bound copy.
+        c1.GetSpecialType(SpecialType.System_Object)
+            .ShouldBeSameAs(c2.GetSpecialType(SpecialType.System_Object));
+        c1.GetSpecialType(SpecialType.System_Object)
+            .ShouldBeSameAs(ScriptAnalysisService.BaseCompilation.GetSpecialType(SpecialType.System_Object));
+    }
+
+    [Fact]
+    public void Repeated_analysis_in_one_scope_replaces_the_tree_rather_than_adding_one()
+    {
+        var svc = new ScriptAnalysisService();
+        svc.CompilationFor("return 1;");
+        var second = svc.CompilationFor("return 2;");
+
+        second.SyntaxTrees.Count().ShouldBe(1);
+        second.SyntaxTrees.Single().ToString().ShouldContain("return 2;");
+        second.SyntaxTrees.Single().ToString().ShouldNotContain("return 1;");
+    }
+
+    [Fact]
+    public void Diagnostics_are_unchanged_by_reuse()
+    {
+        // A clean script stays clean and an error stays an error across interleaved analyses on one service
+        // — a stale tree leaking from the previous call would surface here.
+        var svc = new ScriptAnalysisService();
+        svc.Diagnose(new DiagnoseRequest("return ctx.Nope();")).Markers.ShouldContain(m => m.Severity == 8);
+        svc.Diagnose(new DiagnoseRequest("""return (double)ctx.GetTag("A").Value * 2.0;""")).Markers.ShouldBeEmpty();
+        svc.Diagnose(new DiagnoseRequest("var a = 1;\nreturn ctx.Nope();")).Markers
+            .ShouldContain(m => m.Severity == 8 && m.StartLineNumber == 2);
+    }
+}
@@ -0,0 +1,79 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+/// <summary>
+///     R04:P3 — the catalog used to build a HashSet of every fleet path and sort it on every completion keystroke,
+///     and hover did a linear scan. The snapshot now carries a sorted, de-duplicated <c>string[]</c> (prefix-searched
+///     with a binary search) and an Ordinal path→info dictionary, built once per snapshot.
+/// </summary>
+public sealed class ScriptTagCatalogIndexTests
+{
+    private const int Max = 200;
+
+    // The previous per-keystroke implementation, kept here as the oracle the index must match exactly.
+    private static IReadOnlyList<string> Naive(IReadOnlyList<ScriptTagInfo> entries, string? filter)
+    {
+        IEnumerable<string> q = new HashSet<string>(entries.Select(e => e.Path), StringComparer.OrdinalIgnoreCase);
+        if (!string.IsNullOrWhiteSpace(filter))
+            q = q.Where(p => p.StartsWith(filter, StringComparison.OrdinalIgnoreCase));
+        return q.OrderBy(p => p, StringComparer.OrdinalIgnoreCase).Take(Max).ToList();
+    }
+
+    private static ScriptTagInfo Tag(string path) => new(path, "Tag", "Float", "DRV-1");
+
+    [Fact]
+    public void Prefix_search_is_case_insensitive_sorted_and_bounded()
+    {
+        var index = ScriptTagCatalogIndex.Build(new[]
+        {
+            Tag("Plant/b/Speed"), Tag("plant/A/Temp"), Tag("Other/X"), Tag("Plant/a/Flow"),
+        });
+
+        index.FindByPrefix("PLANT/", Max).ShouldBe(new[] { "Plant/a/Flow", "plant/A/Temp", "Plant/b/Speed" });
+        index.FindByPrefix("other", Max).ShouldBe(new[] { "Other/X" });
+        index.FindByPrefix("zzz", Max).ShouldBeEmpty();
+        index.FindByPrefix(null, 2).Count.ShouldBe(2);
+        index.FindByPrefix("  ", Max).Count.ShouldBe(4);   // whitespace filter = unfiltered, as before
+    }
+
+    [Fact]
+    public void Case_only_duplicates_collapse_to_the_first_seen_casing()
+    {
+        // HashSet(OrdinalIgnoreCase) kept the FIRST-added casing; the index must too.
+        var index = ScriptTagCatalogIndex.Build(new[] { Tag("A/speed"), Tag("A/Speed"), Tag("A/SPEED") });
+        index.SortedPaths.ShouldBe(new[] { "A/speed" });
+    }
+
+    [Fact]
+    public void Exact_lookup_is_ordinal_and_keeps_the_first_entry()
+    {
+        var first = new ScriptTagInfo("A/B", "Tag", "Float", "DRV-1");
+        var index = ScriptTagCatalogIndex.Build(new[] { first, new ScriptTagInfo("A/B", "Virtual tag", "Int32", null) });
+
+        index.TryGet("A/B").ShouldBeSameAs(first);
+        index.TryGet("a/b").ShouldBeNull();      // case-mismatched path does not resolve at runtime
+        index.TryGet("A/C").ShouldBeNull();
+    }
+
+    [Fact]
+    public void Matches_the_previous_implementation_on_a_generated_fleet()
+    {
+        // Deterministic seed — a mixed-case fleet with case-only duplicates and shared prefixes.
+        var rng = new Random(745);
+        string Seg() => new string(Enumerable.Range(0, rng.Next(1, 4)).Select(_ => "abAB_/1"[rng.Next(7)]).ToArray());
+        var entries = Enumerable.Range(0, 3000).Select(_ => Tag(Seg() + "/" + Seg() + "/" + Seg())).ToList();
+        var index = ScriptTagCatalogIndex.Build(entries);
+
+        var filters = new List<string?> { null, "", "a", "A", "ab", "aB/", "b/", "_", "1", "/", "zz" };
+        filters.AddRange(entries.Take(50).Select(e => e.Path[..Math.Min(e.Path.Length, rng.Next(1, 6))]));
+
+        foreach (var f in filters)
+            index.FindByPrefix(f, Max).ShouldBe(Naive(entries, f), $"filter '{f}'");
+        // Positive control: the generated fleet really exercises truncation and filtering.
+        Naive(entries, null).Count.ShouldBe(Max);
+        Naive(entries, "ab").Count.ShouldBeGreaterThan(0);
+    }
+}
@@ -0,0 +1,58 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+using ZB.MOM.WW.OtOpcUa.Core.Scripting;
+using ZB.MOM.WW.OtOpcUa.Core.VirtualTags;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+/// <summary>
+///     R04:S1 — <c>ctx.SetVirtualTag(...)</c> compiles and publishes but is dropped at runtime (single-tag mode).
+///     Only a hover used to say so; <c>Diagnose</c> now emits an <c>OTSCRIPT_SETVTAG_INERT</c> WARNING marker so the
+///     Problems list shows it. A warning, never an error: publish accepts the script, so the editor must too.
+/// </summary>
+public sealed class SetVirtualTagInertDiagnosticTests
+{
+    private const string InertCode = "OTSCRIPT_SETVTAG_INERT";
+    private static readonly ScriptAnalysisService Svc = new();
+    private static IReadOnlyList<DiagnosticMarker> Diag(string code) => Svc.Diagnose(new DiagnoseRequest(code)).Markers;
+
+    [Fact]
+    public void SetVirtualTag_call_gets_a_warning_marker_on_its_line()
+    {
+        var markers = Diag("var a = 1;\nvar b = 2;\nctx.SetVirtualTag(\"Other\", a + b);\nreturn 0;");
+
+        var m = markers.ShouldHaveSingleItem();
+        m.Code.ShouldBe(InertCode);
+        m.Severity.ShouldBe(4);               // Monaco warning
+        m.StartLineNumber.ShouldBe(3);
+        m.StartColumn.ShouldBe(1);
+        m.Message.ShouldContain("SetVirtualTag");
+    }
+
+    [Fact]
+    public void Every_SetVirtualTag_call_is_marked()
+        => Diag("ctx.SetVirtualTag(\"A\", 1);\nctx.SetVirtualTag(\"B\", 2);\nreturn 0;")
+            .Count(m => m.Code == InertCode).ShouldBe(2);
+
+    [Theory]
+    [InlineData("return ctx.GetTag(\"A\").Value;")]                                 // a different ctx member
+    [InlineData("return 0; // ctx.SetVirtualTag(\"A\", 1)")]                         // a comment
+    [InlineData("var s = \"ctx.SetVirtualTag(\\\"A\\\", 1)\"; return s;")]           // a string literal
+    public void Non_calls_are_not_marked(string code)
+    {
+        var markers = Diag(code);
+        markers.ShouldNotContain(m => m.Code == InertCode);
+        markers.ShouldNotContain(m => m.Severity == 8);   // the probe scripts themselves are valid
+    }
+
+    [Fact]
+    public void The_warning_does_not_block_and_publish_accepts_the_script()
+    {
+        // Editor accepts ⇔ publish accepts: no error-severity marker, and the real publish compiler accepts it.
+        const string source = "ctx.SetVirtualTag(\"Other\", 1);\nreturn 0;";
+        Diag(source).ShouldNotContain(m => m.Severity == 8);
+        using var evaluator = ScriptEvaluator<VirtualTagContext, object>.Compile(source);
+        evaluator.ShouldNotBeNull();
+    }
+}