DriverHostActorNativeAlarmAckRoutingTests: Applied does not mean the child DriverInstanceActor is
Connected, and an ack reaching a still-Connecting child is dropped by design — the single
positive-control ack was itself dropped (4/20 under DOTNET_PROCESSOR_COUNT=1 x8, still failing
with the presence budget dilated to 45 s, so not a budget problem). SpawnHostAndApply now waits
for SubscribeAlarmsAsync, which only the Connected state calls. ScriptedAlarmEngine
OnEvent_emission_happens_outside_evalGate: 2 s blocking poll -> 15 s (5/20 under starvation).
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
- v2-ci.yml: upload-artifact@v4 -> @v3 + continue-on-error (act_runner reports as GHES and
v4 refuses GHES, failing every run); runner-capacity diagnostic step; AKKA_TEST_TIMEFACTOR=3
on the test step.
- Runtime/ControlPlane harnesses: akka.test.timefactor from AKKA_TEST_TIMEFACTOR
(TestTimeFactor), single-expect-default 15s; ControlPlane gains PresenceBudget.
- Leaked spinner threads: the infinite-loop timeout tests (Calculation, Host.IntegrationTests
RoslynVirtualTagEvaluator, 3x Core.Scripting) pinned a core for the rest of the test process
(abandoned, never stopped). Now a bounded 5 s wall-clock spin; outer compile-inclusive bound 30 s.
- Real test races fixed: ScriptedAlarmEngine push-during-drain (Inactive is also the start
state + persist-before-update => wait for the Cleared save); DriverReconnectE2e (both nodes
host the driver; Created[] is last-writer-wins and ForceReconnect is a no-op while
Connecting); ControlPlaneStarterOrder (lost-race heal waited for a 60 s refresh tick).
- Fixed sleeps -> poll-for-condition: ReconnectSupervisor LastError, Modbus deadband steps
(wait for observed reads), Modbus dispose hygiene precondition, FOCAS probe-loop log.
- Short presence budgets -> PresenceBudget in the Runtime/ControlPlane tests seen failing on
Gitea; Calculation Settle 10 -> 30 s. No assertion removed, nothing skipped.
- infra/README.md §10 CI; stale TreatWarningsAsErrors comment in Directory.Build.props.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
Layer 4 of the condition-Quality chain (#477 -> #478). #478 gave a scripted
condition the worst quality of its input tags, but only for inputs whose driver
PUBLISHES a Bad data change. A poll driver whose device goes unreachable
publishes nothing — it raises ConnectivityChanged(false) and goes silent — so
the engine held its last Good value, WorstInputStatus stayed Good, and worse,
AreInputsReady stayed true, so the predicate went on evaluating a stale value
after the source had died.
`DriverHostActor.FanCommsLossToDependencyMux` pushes a synthetic
BadNoCommunication with a null value into the dependency mux for every value ref
the disconnected driver owns.
The issue asked for a driver -> all-tag-refs map to be built for this. It
already exists: `_nodeIdByDriverRef`, keyed (DriverInstanceId, RawPath) and
rebuilt from the composition on every apply. Alarm-bearing tags live in the
separate `_alarmNodeIdByDriverRef` and are deliberately excluded — a native
condition is not a mux value and takes the Part 9 quality path instead.
Nothing is pushed on reconnect, deliberately: publishing Good would assert a
value quality this actor has not observed. Recovery rides the driver's own
republish, and the mechanism was verified rather than assumed — the
Connected -> Reconnecting edge calls DetachSubscription, which nulls the
subscription handle, so the reconnect's ResubscribeDesired builds a FRESH
subscription whose PollGroupEngine initial push runs with forceRaise: true.
Without that detach the poll engine would compare against its retained
LastValues, see an unchanged value, raise nothing, and strand these refs Bad
forever — the value-dedup trap that bit the v3 Batch 4 virtual-tag reassert.
Closes#478 Finding 3, which the above depends on. WorstInputStatus skipped
every input whose VALUE was null as "no data yet" — and Bad-with-no-value is
exactly the shape a comms-lost driver produces, so the skip was hiding the case
that matters most; without this half the fan-out above would be silently inert.
The upstream source now stamps a never-pushed path BadWaitingForInitialData —
the honest Part 4 code for that state — and the engine skips that specific code
rather than every null value.
Deleting the null-skip outright is the obvious fix and is wrong in the other
direction: it makes every not-yet-published input drag its condition Bad at
deploy, flooding the quality path with load-time annotations, which is what the
skip existed to prevent. Both mistakes are pinned by a test pair that goes red
under the opposite error, and both were falsified by reverting the production
code:
old null-skip restored -> A_pushed_Bad_carrying_no_value RED
null-skip deleted outright -> An_input_that_was_never_pushed RED
fan-out call removed -> the three DriverHostActor tests RED (1 of 7)
The never-pushed test was vacuous on first writing and is worth recording: the
quality bucket only emits on CHANGE, so the load-time evaluation is the one that
flashes Bad, and subscribing to OnEvent after LoadAsync made the assertion
unfalsifiable — it passed against the broken behaviour. It now subscribes first.
`FakeUpstream.ReadTag` returned 0x80340000 (BadNotConnected) for an unset path,
a different Bad code from production's placeholder. Harmless while any Bad
behaved alike; load-bearing now, so it is aligned and pinned by a reflection
guard asserting the engine's private constant agrees with what the production
source stamps.
Behaviour change to weigh, flagged in the issue and confirmed: virtual tags
computed from a comms-lost input now go Bad too. That follows from using the
shared mux and reads as correct, but it is broader than scripted alarms.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
Layer 3 of #477: a scripted alarm's condition Quality now reflects the WORST
quality across its input tags, mirroring the native OT semantic (#477 L2).
Plumbing (quality was silently discarded twice on the live path):
- VirtualTagActor.DependencyValueChanged gains Quality (defaulted Good); the
DependencyMuxActor forwards the published AttributeValuePublished.Quality it
already carried; ScriptedAlarmHostActor.OnDependencyChanged pushes the real
quality into the engine (was hardcoded 0u/Good).
Engine (Core.ScriptedAlarms):
- ScriptedAlarmEngine computes worst-of-input quality each eval (skipping
not-yet-published inputs, which are a readiness concern, not a quality signal)
and carries it on ScriptedAlarmEvent.WorstInputStatusCode.
- A real transition carries the current worst quality so ToSnapshot's full
snapshot doesn't clobber quality back to Good (e.g. transition while Uncertain).
- A Bad input freezes the condition (no transition), like a comms-lost native
driver; a quality-bucket change with no transition emits the new
EmissionKind.QualityChanged, routed to the existing #477-L2
AlarmQualityUpdate -> WriteAlarmQuality node path (quality only, no /alerts
row, no historian write). ScriptedAlarmSource skips QualityChanged so it never
fabricates a phantom IAlarmSource event.
Host: ToSnapshot maps WorstInputStatusCode -> OpcUaQuality; OnEngineEmission
routes QualityChanged out of band.
Tests (TDD, RED-first): engine worst-carry + Bad/restore QualityChanged +
unchanged-bucket-no-emit; source swallows QualityChanged; mux forwards quality;
host Bad-dep -> AlarmQualityUpdate(no alerts) + transition snapshot carries worst.
Docs: AlarmTracking.md Layer-3 section + design doc.
Closes#478
Re-review at 7286d320. -015: dispose shelving timer at top of LoadAsync so a failed
reload doesn't leave it firing against partially-cleared state + test. -014: make
pendingEmissions required (removes unreachable fire-under-gate branch that could
reintroduce the -003 deadlock).
Adds <summary>, <param>, <typeparam>, and <inheritdoc/> tags to public
members surfaced by commentchecker — resolves 5,847 of 5,869 issues
(99.6%) across three /fixdocs passes.
Both VirtualTagEngine.Load and ScriptedAlarmEngine.LoadAsync were calling
ScriptEvaluator.Compile directly, bypassing CompiledScriptCache. The
Core.Scripting-008 collectible-ALC fix wired Dispose only through the cache's
Clear()/Dispose(), so the per-publish accretion the -008 fix was meant to
eliminate was still in effect on the actual production path — the headline
'no more restarts needed' guarantee wasn't delivered.
Resolution:
- VirtualTagEngine + ScriptedAlarmEngine each gained a private
CompiledScriptCache<TContext, TResult> instance.
- Both Load methods now call _compileCache.GetOrCompile(source).
- Publish-replace path: _compileCache.Clear() runs alongside the existing
_tags / _alarms clears so the prior generation's ALCs are disposed
before recompile.
- Engine Dispose now calls _compileCache.Dispose() so shutdown actually
releases the emitted assemblies.
Side-fix in CompiledScriptCache: Dispose() set _disposed=true then called
Clear(), but Clear() had a pre-existing 'if (_disposed) return' guard that
aborted the drain unconditionally — making the Dispose-triggered cleanup a
silent no-op. Removed the disposed-guard on Clear() (clearing an empty/
cleared cache is idempotent).
Side-fix in ScriptedAlarmEngine.Dispose: cleared _alarms AFTER the
Task.WhenAll drain. The drain guarantees no background callback is mid-
flight, so clearing is safe. Previously _alarms was deliberately NOT
cleared on Dispose (per Core.ScriptedAlarms-005), but that left the
AlarmState records holding TimedScriptEvaluator → ScriptEvaluator → delegate
references that rooted the emitted assemblies, defeating the cache's
Dispose work on the engine side.
Regression tests:
- VirtualTagEngineTests.Dispose_unloads_compiled_script_assembly
- ScriptedAlarmEngineTests.Dispose_unloads_compiled_predicate_assembly
Both use WeakReference + bounded GC.Collect() to prove the emitted
assembly is reclaimable after engine.Dispose(). The alarms test had to
be synchronous (not 'async Task<WeakReference>') because async state
machines capture locals as state-struct fields, keeping them alive past
the method's apparent end and defeating GC.
Verification:
- Core.Scripting.Tests: 104/104 (unchanged).
- VirtualTags.Tests: 57/57 (was 56 — +1 unload test).
- ScriptedAlarms.Tests: 67/67 (was 66 — +1 unload test).
- All other consumer suites still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Core.ScriptedAlarms-009 resolution: replace the per-call Dictionary +
AlarmPredicateContext allocation with a per-alarm reusable AlarmScratch
held in _scratchByAlarmId, refilled in place under _evalGate on each
evaluation. The hot path no longer allocates per upstream tag change.
Why this matters:
On a busy line where many tags feeding many alarms change frequently,
the old BuildReadCache allocated a fresh dictionary + context on every
predicate evaluation — a steady stream of short-lived allocations the
GC eventually has to reclaim. With the reuse, the dictionary and
context are allocated once per alarm (on first evaluation) and refilled
in place across every subsequent re-eval.
Implementation:
- New private AlarmScratch class holds the reusable
Dictionary<string, DataValueSnapshot> read cache (pre-sized to the
alarm's Inputs.Count) and the AlarmPredicateContext that wraps it by
reference. The context observes refilled values without being
re-created.
- ConcurrentDictionary<string, AlarmScratch> _scratchByAlarmId on the
engine, cleared in LoadAsync alongside _alarms so a config-publish
drops the prior generation's scratch (Inputs / Logger may change).
- EvaluatePredicateToStateAsync looks up scratch via GetOrAdd, calls
the new RefillReadCache(Dictionary, IReadOnlySet) helper to clear +
repopulate the dictionary in place, then runs the predicate against
the reused context.
- BuildReadCache removed.
Safety:
Reuse is serialised under _evalGate which guarantees no two threads
ever observe the same scratch in a half-refilled state. The
AlarmPredicateContext is bound to the scratch dictionary by reference,
so the predicate's ctx.GetTag(path) sees the freshly-refilled values
rather than a stale snapshot.
Verification:
- All 66 ScriptedAlarms tests pass (was 63 — three new regression tests
locking the reuse contract).
- All 56 VirtualTags tests still pass (unchanged).
- All 104 Core.Scripting tests still pass (unchanged).
New tests in ScriptedAlarmEngineTests:
- Reevaluation_reuses_the_same_read_cache_dictionary — asserts
ReferenceEquals(scratch_before, scratch_after) across two
evaluations of the same alarm.
- Reevaluation_reuses_the_same_predicate_context — same, for the
context.
- LoadAsync_drops_the_prior_generations_scratch — asserts a config
publish wipes the prior scratch (so a stale Logger / Inputs can't
leak into the new generation).
Internal test hooks TryGetScratchReadCacheForTest /
TryGetScratchContextForTest added via the existing
InternalsVisibleTo for the tests project. Kept internal — not part of
the public engine surface.
Docs:
- docs/v2/Galaxy.Performance.md "Scripted-alarm engine" section
rewritten as "hot-path allocation reuse" documenting the new
contract + reuse safety reasoning + the three regression tests.
- code-reviews/Core.ScriptedAlarms/findings.md -009 flipped
Won't Fix → Resolved.
- code-reviews/README.md regenerated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Core.ScriptedAlarms-003: emit OnEvent OUTSIDE _evalGate by collecting
pending emissions during the gate-held section and flushing them after
release; eliminates re-entrancy deadlock the docs already promised.
- Core.ScriptedAlarms-006: track every fire-and-forget Reevaluate /
ShelvingCheck task in _inFlight; Dispose drains the set so the engine
no longer races store writes against teardown.
- Core.ScriptedAlarms-008: store comments as ImmutableList<AlarmComment>
so AppendComment is O(log n) instead of O(n).
- Core.ScriptedAlarms-010: document the deliberate input-quality
asymmetry (Uncertain drives the predicate, renders {?} in the message)
in docs/ScriptedAlarms.md and on MessageTemplate.Resolve remarks.
- Core.ScriptedAlarms-011: propagate the no-op reason through
TransitionResult.NoOp(state, reason) and log it from
ScriptedAlarmEngine.ApplyAsync.
- Core.ScriptedAlarms-009 (Won't Fix per recommendation): documented the
per-evaluation dictionary allocation in docs/v2/Galaxy.Performance.md
with a mitigation path if a future soak surfaces pressure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add engine-level tests covering the six gaps identified in the finding:
(1) timed-shelve auto-expiry driven via injectable clock + RunShelvingCheckForTest
hook so timer tests are deterministic;
(2) ConfirmAsync, TimedShelveAsync/UnshelveAsync round-trip, EnableAsync engine
methods exercised end-to-end;
(3) OnEvent subscriber-throws isolation — engine state advances and stays
operational after a subscriber throws;
(4) IAlarmStateStore.SaveAsync failure leaves in-memory state unchanged (locks in
the persist-before-update invariant from finding-007);
(5) second LoadAsync does not leak the old timer (regression for finding-002);
(6) AreInputsReady cold-start guard correctly blocks on Bad/missing inputs and
allows Uncertain-quality inputs through.
Expose RunShelvingCheckForTest() internal method on ScriptedAlarmEngine to
support deterministic timer tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_alarms was a plain Dictionary<string, AlarmState> mutated under the
_evalGate semaphore, but four read paths (GetState, GetAllStates, the
LoadedAlarmIds property, and RunShelvingCheck) touched it from arbitrary
threads with no synchronisation. A Dictionary read concurrent with a
writer's entry reassignment can throw InvalidOperationException or return
torn state.
Switched _alarms to ConcurrentDictionary<string, AlarmState>. The only
write shapes are indexer-set and Clear, both atomic on ConcurrentDictionary,
so all mutations stay correct without further change; reads now get safe
snapshot semantics. LoadedAlarmIds materialises the key snapshot to keep
its IReadOnlyCollection<string> return type. This matches _valueCache,
which is already a ConcurrentDictionary.
Added a regression test (Concurrent_reads_during_mutation_do_not_throw)
that hammers the engine with state mutations while four reader threads
continuously call the three unguarded read paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Group all 69 projects into category subfolders under src/ and tests/ so the
Rider Solution Explorer mirrors the module structure. Folders: Core, Server,
Drivers (with a nested Driver CLIs subfolder), Client, Tooling.
- Move every project folder on disk with git mv (history preserved as renames).
- Recompute relative paths in 57 .csproj files: cross-category ProjectReferences,
the lib/ HintPath+None refs in Driver.Historian.Wonderware, and the external
mxaccessgw refs in Driver.Galaxy and its test project.
- Rebuild ZB.MOM.WW.OtOpcUa.slnx with nested solution folders.
- Re-prefix project paths in functional scripts (e2e, compliance, smoke SQL,
integration, install).
Build green (0 errors); unit tests pass. Docs left for a separate pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Joseph Doherty