perf(deployment): staleness/comparison paths no longer Roslyn-compile every script — deploy gate remains the authoritative compile

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj

docs/requirements/Component-DeploymentManager.md

@@ -142,6 +142,20 @@ The system maintains two views per instance:
 
 These are compared to determine staleness and generate diffs.
 
+The comparison path is **hash-only by design**. When the Template-Derived
+Configuration is re-flattened for a comparison (the Deployments comparison page)
+or for the Transport stale-instance probe
+(`IStaleInstanceProbe.GetCurrentRevisionHashAsync`, used per-instance across a
+bundle import), the pipeline is invoked with `validateScripts: false`, which
+skips **only** the expensive Roslyn script-compilation stage
+(`ValidationService.ValidateScriptCompilation` — a non-collectible assembly load
+per script). Structural and semantic validation still run. These read-only paths
+need only the flattened config and its revision hash, not a compile, so a script
+that does not currently compile still yields a comparison. The **deploy gate
+remains the authoritative compile**: instance deployment re-flattens with
+`validateScripts: true` (the default), and a script that fails to compile blocks
+the deployment (see Site-Side Apply Atomicity).
+
 ## Deployable Artifacts
 
 A deployment to a site includes the flattened instance configuration plus any system-wide artifacts that have changed:

src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs

@@ -648,8 +648,12 @@ public class DeploymentService
         if (snapshot == null)
             return Result<DeploymentComparisonResult>.Failure("No deployed snapshot found for this instance.");
 
-        // Compute current template-derived config
+        // Compute current template-derived config. This read-only comparison only needs
-        var currentResult = await _flatteningPipeline.FlattenAndValidateAsync(instanceId, cancellationToken);
+        // the revision hash (staleness) + the flattened config (structured diff) — NOT a
+        // script compile — so skip the expensive Roslyn ValidateScriptCompilation stage.
+        // The deploy gate remains the authoritative compile.
+        var currentResult = await _flatteningPipeline.FlattenAndValidateAsync(
+            instanceId, cancellationToken, validateScripts: false);
         if (currentResult.IsFailure)
             return Result<DeploymentComparisonResult>.Failure($"Cannot compute current config: {currentResult.Error}");
 

src/ZB.MOM.WW.ScadaBridge.DeploymentManager/FlatteningPipeline.cs

@@ -54,7 +54,8 @@ public class FlatteningPipeline : IFlatteningPipeline
     /// <inheritdoc />
     public async Task<Result<FlatteningPipelineResult>> FlattenAndValidateAsync(
         int instanceId,
-        CancellationToken cancellationToken = default)
+        CancellationToken cancellationToken = default,
+        bool validateScripts = true)
     {
         // Load instance with full graph
         var instance = await _templateRepo.GetInstanceByIdAsync(instanceId, cancellationToken);
@@ -158,13 +159,19 @@ public class FlatteningPipeline : IFlatteningPipeline
         // on the site — blocks the deployment. (The template DESIGN-TIME validate path in
         // ManagementActor leaves this non-blocking by NOT enforcing, since bindings are
         // set later at instance/deploy time.)
+        // validateScripts == false is the read-only staleness/comparison path: skip
+        // only the expensive Roslyn ValidateScriptCompilation (which loads a
+        // non-collectible InteractiveAssemblyLoader assembly per script). Structural
+        // and semantic validation still run so a genuinely broken flatten is surfaced;
+        // the deploy path (validateScripts: true) remains the authoritative compile gate.
         var validation = _validationService.Validate(
             config,
             resolvedSharedScripts,
             alarmCapableConnectionNames,
             enforceConnectionBindings: true,
             siteConnectionNames: siteConnectionNames,
-            resolveSchemaRef: resolveSchemaRef);
+            resolveSchemaRef: resolveSchemaRef,
+            validateScriptCompilation: validateScripts);
 
         // Compute revision hash
         var hash = _revisionHashService.ComputeHash(config);

src/ZB.MOM.WW.ScadaBridge.DeploymentManager/IFlatteningPipeline.cs

@@ -15,10 +15,19 @@ public interface IFlatteningPipeline
     /// </summary>
     /// <param name="instanceId">Id of the instance to flatten and validate.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
+    /// <param name="validateScripts">
+    /// When <c>true</c> (the deploy default), the authoritative Roslyn script-compilation
+    /// gate runs. When <c>false</c>, that compile step is skipped — used by the read-only
+    /// staleness/comparison paths (Deployments comparison page,
+    /// <see cref="ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport.IStaleInstanceProbe"/>)
+    /// which need only the flattened config + revision hash, not a compile. Structural and
+    /// semantic validation still run in both cases.
+    /// </param>
     /// <returns>A task that resolves to the flattened configuration, revision hash, and validation result; or a failure result if flattening could not complete.</returns>
     Task<Result<FlatteningPipelineResult>> FlattenAndValidateAsync(
         int instanceId,
-        CancellationToken cancellationToken = default);
+        CancellationToken cancellationToken = default,
+        bool validateScripts = true);
 }
 
 /// <summary>

src/ZB.MOM.WW.ScadaBridge.DeploymentManager/StaleInstanceProbe.cs

@@ -30,8 +30,11 @@ public sealed class StaleInstanceProbe : IStaleInstanceProbe
         // instance as "staleness indeterminate" and skips it. Flattening reuses
         // the scoped ITemplateEngineRepository, so it observes template rows the
         // in-flight import has staged on the shared change tracker.
+        // Staleness only needs the revision hash — skip the expensive Roslyn
+        // script-compilation stage (this probe is called per-instance across a whole
+        // bundle import in BundleImporter.ComputeStaleInstanceIdsAsync).
         var result = await _flatteningPipeline
-            .FlattenAndValidateAsync(instanceId, cancellationToken)
+            .FlattenAndValidateAsync(instanceId, cancellationToken, validateScripts: false)
             .ConfigureAwait(false);
         return result.IsSuccess ? result.Value.RevisionHash : null;
     }

src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs

@@ -95,6 +95,14 @@ public class ValidationService
     /// <c>$ref</c> are unaffected (behavior unchanged), and a <c>$ref</c> with no
     /// resolver is treated as dangling (the safe option).
     /// </param>
+    /// <param name="validateScriptCompilation">
+    /// <c>true</c> (default) runs the authoritative Roslyn <see cref="ValidateScriptCompilation"/>
+    /// stage — the single most expensive check (a non-collectible assembly load per script).
+    /// <c>false</c> skips ONLY that stage; every other structural/semantic check still runs.
+    /// Read-only staleness/comparison callers (DeploymentManager's FlatteningPipeline on its
+    /// comparison/probe paths) pass <c>false</c> because they need only the flattened config and
+    /// revision hash, not a compile. The deploy gate keeps the default (<c>true</c>).
+    /// </param>
     /// <returns>A merged <see cref="ValidationResult"/> aggregating all pipeline stage outcomes.</returns>
     public ValidationResult Validate(
         FlattenedConfiguration configuration,
@@ -102,7 +110,8 @@ public class ValidationService
         IReadOnlySet<string>? alarmCapableConnectionNames = null,
         bool enforceConnectionBindings = false,
         IReadOnlySet<string>? siteConnectionNames = null,
-        Func<string, string?>? resolveSchemaRef = null)
+        Func<string, string?>? resolveSchemaRef = null,
+        bool validateScriptCompilation = true)
     {
         ArgumentNullException.ThrowIfNull(configuration);
 
@@ -110,7 +119,13 @@ public class ValidationService
         {
             ValidateFlatteningSuccess(configuration),
             ValidateNamingCollisions(configuration),
-            ValidateScriptCompilation(configuration),
+            // The Roslyn compile is the single most expensive validation stage (a
+            // non-collectible InteractiveAssemblyLoader assembly load per script).
+            // Read-only staleness/comparison callers pass validateScriptCompilation:
+            // false to skip ONLY this stage; every other structural/semantic check
+            // still runs. The deploy gate keeps the default (true) — it is the
+            // authoritative compile.
+            validateScriptCompilation ? ValidateScriptCompilation(configuration) : ValidationResult.Success(),
             ValidateAlarmTriggerReferences(configuration),
             ValidateScriptTriggerReferences(configuration),
             ValidateExpressionTriggers(configuration),

tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyPageTests.cs

@@ -360,7 +360,7 @@ public class TopologyPageTests : BunitContext
                 }
             }
         };
-        _pipeline.FlattenAndValidateAsync(100, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(100, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Task.FromResult(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, "hash-new", ValidationResult.Success()))));
 
@@ -410,7 +410,7 @@ public class TopologyPageTests : BunitContext
                     JsonSerializer.Serialize(deployedConfig))));
 
         var currentConfig = new FlattenedConfiguration { InstanceUniqueName = "Pump-001" };
-        _pipeline.FlattenAndValidateAsync(100, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(100, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Task.FromResult(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, "hash-new", ValidationResult.Success()))));
     }

tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploymentComparisonTests.cs

@@ -1,7 +1,23 @@
+using NSubstitute;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
+using ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening;
+using ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation;
+
 namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests;
 
 /// <summary>
 /// WP-8: Tests for deployed vs template-derived state comparison.
+///
+/// PLAN-05 Task 16: adds behavioral coverage proving the read-only
+/// staleness/comparison path can flatten (and thus hash) an instance whose
+/// script does NOT compile — because <c>validateScripts: false</c> skips the
+/// expensive Roslyn <see cref="ValidationService.ValidateScriptCompilation"/>
+/// step while keeping structural/semantic validation. The default (deploy) path
+/// still compiles.
 /// </summary>
 public class DeploymentComparisonTests
 {
@@ -34,4 +50,81 @@ public class DeploymentComparisonTests
 
         Assert.Equal(deployedAt, result.DeployedAt);
     }
+
+    // ── PLAN-05 Task 16: read-only paths skip Roslyn script compilation ──
+
+    private const int InstanceId = 1;
+    private const int TemplateId = 10;
+    private const int SiteId = 100;
+
+    // A deliberately non-compiling script body. The deploy gate
+    // (validateScripts: true) flags this as a ScriptCompilation error; the
+    // read-only staleness/comparison path (validateScripts: false) must skip
+    // the compile entirely and still produce a flattened config + hash.
+    private const string NonCompilingScript = "int x = ;";
+
+    private static FlatteningPipeline BuildPipeline(
+        ITemplateEngineRepository templateRepo,
+        ISiteRepository siteRepo,
+        ISharedSchemaRepository sharedSchemaRepo)
+    {
+        sharedSchemaRepo.ListAsync(Arg.Any<CancellationToken>()).Returns([]);
+        return new FlatteningPipeline(
+            templateRepo,
+            siteRepo,
+            new FlatteningService(),
+            new ValidationService(),
+            new RevisionHashService(),
+            sharedSchemaRepo);
+    }
+
+    private static (ITemplateEngineRepository, ISiteRepository, ISharedSchemaRepository) ArrangeNonCompilingScript()
+    {
+        var templateRepo = Substitute.For<ITemplateEngineRepository>();
+        var siteRepo = Substitute.For<ISiteRepository>();
+        var sharedSchemaRepo = Substitute.For<ISharedSchemaRepository>();
+
+        var template = new Template("Tank") { Id = TemplateId };
+        template.Scripts.Add(new TemplateScript("BadScript", NonCompilingScript));
+
+        var instance = new Instance("Tank-01") { Id = InstanceId, TemplateId = TemplateId, SiteId = SiteId };
+
+        templateRepo.GetInstanceByIdAsync(InstanceId, Arg.Any<CancellationToken>()).Returns(instance);
+        templateRepo.GetTemplateWithChildrenAsync(TemplateId, Arg.Any<CancellationToken>()).Returns(template);
+        templateRepo.GetCompositionsByTemplateIdAsync(TemplateId, Arg.Any<CancellationToken>()).Returns([]);
+        templateRepo.GetAllSharedScriptsAsync(Arg.Any<CancellationToken>()).Returns([]);
+        siteRepo.GetDataConnectionsBySiteIdAsync(SiteId, Arg.Any<CancellationToken>()).Returns([]);
+
+        return (templateRepo, siteRepo, sharedSchemaRepo);
+    }
+
+    [Fact]
+    public async Task FlattenAndValidate_ValidateScriptsFalse_NonCompilingScript_SucceedsWithHashAndNoCompilationError()
+    {
+        var (templateRepo, siteRepo, sharedSchemaRepo) = ArrangeNonCompilingScript();
+        var sut = BuildPipeline(templateRepo, siteRepo, sharedSchemaRepo);
+
+        var result = await sut.FlattenAndValidateAsync(InstanceId, validateScripts: false);
+
+        Assert.True(result.IsSuccess);
+        Assert.False(string.IsNullOrEmpty(result.Value.RevisionHash));
+        // The Roslyn compile step was skipped, so no ScriptCompilation error is raised
+        // even though the script body does not compile.
+        Assert.DoesNotContain(result.Value.Validation.Errors,
+            e => e.Category == ValidationCategory.ScriptCompilation);
+    }
+
+    [Fact]
+    public async Task FlattenAndValidate_DefaultValidateScripts_NonCompilingScript_ReportsCompilationError()
+    {
+        var (templateRepo, siteRepo, sharedSchemaRepo) = ArrangeNonCompilingScript();
+        var sut = BuildPipeline(templateRepo, siteRepo, sharedSchemaRepo);
+
+        // Default (deploy) path — compilation is the authoritative gate.
+        var result = await sut.FlattenAndValidateAsync(InstanceId);
+
+        Assert.True(result.IsSuccess);
+        Assert.Contains(result.Value.Validation.Errors,
+            e => e.Category == ValidationCategory.ScriptCompilation);
+    }
 }

tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploymentServiceTests.cs

@@ -520,7 +520,7 @@ public class DeploymentServiceTests : TestKit
         _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns(snapshot);
 
         var currentConfig = deployedConfig with { };
-        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, sameHash, ValidationResult.Success())));
 
@@ -557,7 +557,7 @@ public class DeploymentServiceTests : TestKit
             Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "9", DataType = "Int32" }]
         };
         var currentHash = new RevisionHashService().ComputeHash(currentConfig);
-        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, currentHash, ValidationResult.Success())));
 
@@ -567,6 +567,72 @@ public class DeploymentServiceTests : TestKit
         Assert.True(result.Value.IsStale);
     }
 
+    // ── PLAN-05 Task 16: read-only paths skip Roslyn script compilation ──
+
+    [Fact]
+    public async Task GetDeploymentComparisonAsync_RequestsValidateScriptsFalse()
+    {
+        // The comparison page only needs the revision hash for staleness — the
+        // expensive Roslyn compile must be skipped (validateScripts: false).
+        var deployedConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "TestInst",
+            Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "5", DataType = "Int32" }]
+        };
+        var hash = new RevisionHashService().ComputeHash(deployedConfig);
+        var snapshot = new DeployedConfigSnapshot(
+            "dep1", hash, System.Text.Json.JsonSerializer.Serialize(deployedConfig))
+        {
+            InstanceId = 1,
+            DeployedAt = DateTimeOffset.UtcNow
+        };
+        _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns(snapshot);
+        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
+            .Returns(Result<FlatteningPipelineResult>.Success(
+                new FlatteningPipelineResult(deployedConfig with { }, hash, ValidationResult.Success())));
+
+        var result = await _service.GetDeploymentComparisonAsync(1);
+
+        Assert.True(result.IsSuccess);
+        await _pipeline.Received(1).FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), false);
+        await _pipeline.DidNotReceive().FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), true);
+    }
+
+    [Fact]
+    public async Task DeployInstanceAsync_RequestsValidateScriptsTrue()
+    {
+        // The deploy gate stays the authoritative compile — validateScripts defaults to true.
+        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
+        _repo.GetInstanceByIdAsync(1).Returns(instance);
+
+        var validationResult = new ValidationResult
+        {
+            Errors = [ValidationEntry.Error(ValidationCategory.ScriptCompilation, "Compile error")]
+        };
+        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
+            .Returns(Result<FlatteningPipelineResult>.Success(
+                new FlatteningPipelineResult(new FlattenedConfiguration(), "hash1", validationResult)));
+
+        await _service.DeployInstanceAsync(1, "admin");
+
+        await _pipeline.Received().FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), true);
+    }
+
+    [Fact]
+    public async Task StaleInstanceProbe_RequestsValidateScriptsFalse()
+    {
+        // The Transport stale-instance probe also only needs the hash — same skip.
+        _pipeline.FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), Arg.Any<bool>())
+            .Returns(Result<FlatteningPipelineResult>.Success(
+                new FlatteningPipelineResult(new FlattenedConfiguration(), "sha256:probe", ValidationResult.Success())));
+        var probe = new StaleInstanceProbe(_pipeline);
+
+        var hash = await probe.GetCurrentRevisionHashAsync(5);
+
+        Assert.Equal("sha256:probe", hash);
+        await _pipeline.Received(1).FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), false);
+    }
+
     // ── DeploymentManager-007: comparison must produce a structured diff ──
 
     [Fact]
@@ -594,7 +660,7 @@ public class DeploymentServiceTests : TestKit
             InstanceUniqueName = "DiffInst",
             Attributes = [new ResolvedAttribute { CanonicalName = "NewAttr", DataType = "Int" }]
         };
-        _pipeline.FlattenAndValidateAsync(40, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(40, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, "sha256:new", ValidationResult.Success())));
 
@@ -668,7 +734,7 @@ public class DeploymentServiceTests : TestKit
             ]
         };
         var nativeHash = hasher.ComputeHash(nativeConfig);
-        _pipeline.FlattenAndValidateAsync(50, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(50, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(nativeConfig, nativeHash, ValidationResult.Success())));
 
@@ -736,7 +802,7 @@ public class DeploymentServiceTests : TestKit
             ]
         };
         var currentHash = hasher.ComputeHash(currentConfig);
-        _pipeline.FlattenAndValidateAsync(51, Arg.Any<CancellationToken>())
+        _pipeline.FlattenAndValidateAsync(51, Arg.Any<CancellationToken>(), Arg.Any<bool>())
             .Returns(Result<FlatteningPipelineResult>.Success(
                 new FlatteningPipelineResult(currentConfig, currentHash, ValidationResult.Success())));