merge: Driver.Historian.Gateway code review — severity round-trip fix (High), provisioner dispose, value-writer/serializer offline coverage

code-reviews/Driver.Historian.Gateway/findings.md

@@ -0,0 +1,239 @@
+# Code Review — Driver.Historian.Gateway
+
+| Field | Value |
+|---|---|
+| Module | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway` |
+| Reviewer | Claude Code |
+| Review date | 2026-08-10 |
+| Commit reviewed | `b990adae` |
+| Status | Reviewed |
+| Open findings | 0 |
+
+## Scope
+
+The sole historian backend: a thin gRPC seam (`IHistorianGatewayClient` /
+`HistorianGatewayClientAdapter`) over the `ZB.MOM.WW.HistorianGateway.Client` package, plus
+`GatewayHistorianDataSource` (HistoryRead), `GatewayAlarmHistorianWriter` (alarm `SendEvent`),
+`GatewayTagProvisioner` (`EnsureTags` upsert), the `Mapping/` pure mappers, and the
+`Recorder/` continuous-historization write path (`FasterLogHistorizationOutbox`,
+`GatewayHistorianValueWriter`, `HistorizationOutboxEntrySerializer`). 17 source files, ~1,850 LOC
+(excluding generated/obj); test suite `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests`
+(130 tests: 124 offline unit + 6 env-gated `Category=LiveIntegration`, skip cleanly offline).
+
+Read against `CLAUDE.md`'s "Historian / HistoryRead" section in full before reviewing — several
+things that look like defects on first read are deliberate, documented decisions (Boolean→Int2,
+the dead `Quality` field on `WriteLiveValues`, the per-path-own-channel design, the two `EnsureTags`
+gotchas, the #491/#537/#538 live-gate history). None of those were re-litigated here.
+
+## Checklist coverage
+
+| # | Category | Result |
+|---|---|---|
+| 1 | Correctness & logic bugs | 1 finding (Driver.Historian.Gateway-001, High) — Resolved |
+| 2 | OtOpcUa conventions | No issues found |
+| 3 | Concurrency & thread safety | No issues found |
+| 4 | Error handling & resilience | No issues found |
+| 5 | Security | No issues found |
+| 6 | Performance & resource management | 1 finding (Driver.Historian.Gateway-002, Low) — Resolved |
+| 7 | Design-document adherence | No issues found |
+| 8 | Code organization & conventions | No issues found |
+| 9 | Testing coverage | 1 finding (Driver.Historian.Gateway-003, Medium) — Resolved |
+| 10 | Documentation & comments | No issues found |
+
+### Category notes
+
+**1. Correctness & logic bugs.** See Driver.Historian.Gateway-001 (Severity write/read mismatch).
+Everything else checked out: `SampleMapper`/`GatewayQualityMapper` quality-byte bucketing is exact
+and tested per-byte; `AggregateModeMapper`/`HistorianTypeMapper` are exhaustive-matrix-guarded
+(`Every_..._member_is_classified` tests fail on a future unmapped enum member rather than silently
+defaulting); `GatewayReadEventsTests` confirms the truncation/continuation-point boundary
+(`collected.Count == maxEvents` checked *before* `Add`, so an exact-cap read has no continuation but
+one more matching event sets it); `AlignAtTimeSnapshots` correctly keeps the first of a duplicate
+timestamp and Bad-fills gaps at the *requested* time, not the nearest returned one.
+
+**2. OtOpcUa conventions.** Driver-level code contains no OPC UA SDK reference (`uint` status codes
+via the named `HistorianStatusCodes`, not the SDK's `Opc.Ua.StatusCodes` — matches the repo's stated
+pattern of keeping the driver layer SDK-free so `StatusCodeParityTests` elsewhere can reflect-guard
+it). No authorization decisions are made in this driver — HistoryRead access-level gating happens at
+materialization in the server layer per CLAUDE.md, and this module is a pure translation seam.
+Serilog-style `ILogger<T>` used throughout via constructor injection.
+
+**3. Concurrency & thread safety.** `GatewayHistorianDataSource`'s health counters
+(`_totalQueries`/`_totalSuccesses`/`_totalFailures`/etc.) are all mutated and read under one
+`_healthLock`, preserving the `TotalQueries == TotalSuccesses + TotalFailures` invariant at every
+observed snapshot (per its own XML remarks and `GatewayHealthSnapshotTests`).
+`FasterLogHistorizationOutbox` documents an explicit single-writer assumption (the recorder actor
+processes messages sequentially) and its `_state` lock protects only the in-memory FIFO index, never
+held across an `await` — correct, since FasterLog itself tolerates concurrent enqueue/scan. No shared
+mutable state was found that lacked a lock or a documented ownership model.
+
+**4. Error handling & resilience.** `GatewayAlarmHistorianWriter` and `GatewayHistorianValueWriter`
+both honor their "never throws" contracts (verified by dedicated exception-classification tests
+covering the typed client hierarchy, a defensive raw `RpcException` path, and an unclassifiable
+exception default). Cancellation is handled as a first-class, non-poisoning case in both the alarm
+writer (mid-drain cancellation yields `RetryPlease`, never `PermanentFail`) and the tag provisioner
+(a shutdown `OperationCanceledException` is logged quietly and counted `Failed`, never thrown). The
+fire-and-forget `EnsureTagsAsync` hook is genuinely non-blocking end to end — no `await` in its
+caller's synchronous path can escape as an exception (verified by tracing `AddressSpaceApplier`'s
+fire-and-forget dispatch, outside this module's scope but confirmed non-blocking here).
+
+**5. Security.** No API key, tag value, hostname, or credential is ever passed to a logger call
+anywhere in the module (audited every `_logger.Log*` call site) — failures are logged by exception
+*type name* only. `HistorianGatewayClientAdapter.Create` echoes the `Endpoint` in its
+`InvalidOperationException` (deliberately, documented as non-secret) but never the `ApiKey`. TLS /
+certificate-validation options (`UseTls`, `AllowUntrustedServerCertificate`, `CaCertificatePath`) map
+onto the package client with the documented inverted `RequireCertificateValidation` semantics and are
+correctly *not* forwarded on a plaintext h2c connection (pinned by
+`Adapter_ignores_tls_only_settings_on_a_plaintext_endpoint`). No SQL or other injectable surface in
+this module.
+
+**6. Performance & resource management.** See Driver.Historian.Gateway-002 (missing dispose on
+`GatewayTagProvisioner`). Everything else: `GatewayHistorianDataSource` and
+`GatewayAlarmHistorianWriter` both correctly dispose their owned `IHistorianGatewayClient` (verified
+by dedicated tests); the three `GatewayHistorian.Create*` factories deliberately open independent
+gRPC channels per path rather than sharing one, a documented and reasonable trade (each channel's
+lifetime stays simple; the gateway pools/amortizes historian sessions server-side). Channel
+construction is lazy (no network I/O at construction, confirmed by the adapter's own tests) so a
+misconfigured/disabled path never dials. Per-call deadlines exist via
+`ServerHistorianOptions.CallTimeout` → `HistorianGatewayClientOptions.DefaultCallTimeout`, and every
+seam method threads a `CancellationToken` through to the package client — no unbounded call was found.
+
+**7. Design-document adherence.** Matches `CLAUDE.md`'s "Historian / HistoryRead" section point for
+point: gateway is the sole backend; Boolean→Int2 is implemented exactly as documented (with an
+explicit-presence pin test guarding the proto3-optional contract); the continuous-historization
+outbox/writer/serializer shape matches the FasterLog-outbox design; the `Quality` field is
+deliberately populated despite being a documented-dead input on the SQL write path, with the
+reasoning captured in the class's own remarks (proto3 default-0 would be a worse lie than the
+correct-but-ignored 192). No stale documentation was found in this module.
+
+**8. Code organization & conventions.** Clean separation: `Mapping/` holds pure, stateless,
+internal-visibility mappers (each independently unit-tested); `Recorder/` holds the
+continuous-historization write path; the top-level files are the three public seams
+(`IHistorianGatewayClient`, the adapter, and the `GatewayHistorian` factory facade the Host consumes).
+`InternalsVisibleTo` is scoped to the test project only. The `ServerHistorianOptions` type living in
+`Runtime` rather than this driver (creating the documented Host→{Runtime, Gateway}, Gateway→Runtime
+diamond) is explained inline in the `.csproj` and is a reasonable ownership call, not a layering
+violation.
+
+**9. Testing coverage.** See Driver.Historian.Gateway-003 (two offline-coverage gaps, now closed).
+Otherwise the suite is thorough: every mapper has a matrix-guard test for unmapped enum members,
+every writer's outcome-classification switch is covered by every named branch (including the
+FIFO-truncation semantics of the FasterLog outbox — append/peek/remove/restart-durability/capacity
+drop-oldest/periodic-commit-recovery all have dedicated tests), and the six env-gated
+`Category=LiveIntegration` tests (skip cleanly without `HISTGW_*` env vars, confirmed offline) cover
+the write/read/alarm/EnsureTags round trips against a real gateway per `docs/Historian.md`'s
+re-run recipe.
+
+**10. Documentation & comments.** XML docs are extensive, accurate against the code they describe,
+and several carry load-bearing historical context (e.g. why each `GatewayHistorian.Create*` factory
+opens its own channel, why `Quality` is populated despite being server-ignored, why
+`HistorianEvent.Id` is deliberately left unset). No misleading or stale comment was found.
+
+## Findings
+
+### Driver.Historian.Gateway-001
+
+| Field | Value |
+|---|---|
+| Severity | High |
+| Category | Correctness & logic bugs |
+| Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs:39` (write side) and `Mapping/EventMapper.cs:62` (read side) |
+| Status | Resolved |
+
+**Description:** `AlarmEventMapper.ToHistorianEvent` wrote the alarm's `Severity` wire property as
+`alarm.Severity.ToString()` — the .NET enum's *name* (`"Low"`/`"Medium"`/`"High"`/`"Critical"`).
+`AlarmHistorianEvent.Severity`'s own XML doc states the contract: "Mapped to Historian's numeric
+priority on the sink side." The read side, `EventMapper.ParseSeverity`, `int.TryParse`s that exact
+property and falls back to the minimum severity (1) on any unparseable value — which an enum name
+always is. So round-tripping any alarm event's severity through this module's own write mapper then
+its own read mapper silently collapsed every severity to the minimum, regardless of the real
+severity: a `Critical` alarm and a `Low` alarm would read back identically as severity 1. This is a
+self-contained defect (both mappers live in this module) independent of whether the gateway's own
+storage happens to echo arbitrary custom event properties back on `ReadEvents` — the write and read
+mappers disagreed with each other and with their own documented contract, and the existing test
+(`AlarmEventMapperTests.Maps_source_time_type_and_rich_properties`) asserted the *wrong* value
+(`"High"`) as correct, so no test caught it.
+
+**Recommendation:** Map `AlarmSeverity` to a numeric OPC UA Part 9 priority string before writing it
+to the wire property, using the same severity convention already established elsewhere in the
+codebase for this exact enum (`ZB.MOM.WW.OtOpcUa.Runtime.Drivers.NativeAlarmProjector.MapSeverity`:
+Low=200, Medium=500, High=700, Critical=900).
+
+**Resolution:** `55b12dc2` (branch `review/historian-gateway`), 2026-08-10. Added a private
+`AlarmEventMapper.ToNumericSeverity` mirroring `NativeAlarmProjector.MapSeverity`'s convention and
+routed the `Severity` property through it instead of `alarm.Severity.ToString()`. TDD: added
+`AlarmEventMapperTests.Severity_round_trips_through_EventMapper_as_numeric_priority` (a
+`[Theory]` over all four `AlarmSeverity` values, round-tripping through both mappers) and corrected
+the existing assertion to `"700"`; both were RED against the pre-fix code (confirmed by running the
+suite before the source change) and are GREEN after it. All 130 module tests pass
+(124 run + 6 skipped `LiveIntegration`).
+
+### Driver.Historian.Gateway-002
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Performance & resource management |
+| Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs` |
+| Status | Resolved |
+
+**Description:** Unlike its two siblings — `GatewayHistorianDataSource` (`IAsyncDisposable`,
+disposes its `IHistorianGatewayClient`) and `GatewayAlarmHistorianWriter` (`IDisposable`, same) —
+`GatewayTagProvisioner` implemented neither `IDisposable` nor `IAsyncDisposable`, so the gRPC channel
+`GatewayHistorian.CreateProvisioner` opens for it was never closed. `IHistorianProvisioning` (the
+interface it's registered as) does not declare disposal either, so nothing in the codebase had any
+way to close that channel. Impact is bounded — `AddHistorianProvisioning` registers it as a single
+process-lifetime DI singleton (built once at startup, not per-deploy), so this was not an
+accumulating leak — but it is a real gap in graceful-shutdown hygiene and an inconsistency with the
+established pattern in this same module.
+
+**Recommendation:** Implement `IAsyncDisposable` on `GatewayTagProvisioner`, disposing the
+underlying `IHistorianGatewayClient` — the built-in `Microsoft.Extensions.DependencyInjection`
+container tracks and disposes any singleton instance it constructs that implements
+`IDisposable`/`IAsyncDisposable`, regardless of the declared service-interface type, so no other
+wiring change is required.
+
+**Resolution:** `55b12dc2` (branch `review/historian-gateway`), 2026-08-10. Added
+`GatewayTagProvisioner : IHistorianProvisioning, IAsyncDisposable` with a `DisposeAsync()` delegating
+to `_client.DisposeAsync()`. TDD: added
+`GatewayTagProvisionerTests.DisposeAsync_disposes_underlying_client`, confirmed RED (compile error —
+`GatewayTagProvisioner` did not implement `IAsyncDisposable`) before the fix, GREEN after.
+
+### Driver.Historian.Gateway-003
+
+| Field | Value |
+|---|---|
+| Severity | Medium |
+| Category | Testing coverage |
+| Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/GatewayHistorianValueWriter.cs`, `Recorder/HistorizationOutboxEntrySerializer.cs` |
+| Status | Resolved |
+
+**Description:** Two production-critical pieces of the continuous-historization write path had no
+offline unit coverage:
+
+- `GatewayHistorianValueWriter` — the recorder's `WriteLiveValues` adapter, and per CLAUDE.md's
+  "KNOWN LIMITATION 2" history the exact class where the #491 `ConfigureAwait(false)`-destroys-`Self`
+  defect and the #537/#538 gate findings were discovered live — was exercised only by the env-gated
+  `Category=LiveIntegration` suite (`tests/.../Live/GatewayLiveIntegrationTests.cs`), which skips
+  cleanly offline. Its ack-to-bool mapping (`Success || Queued`), timestamp-presence handling (a null
+  timestamp must leave the proto field unset, not send a default), and non-throwing exception
+  contract had zero offline verification.
+- `HistorizationOutboxEntrySerializer` — a hand-rolled fixed-layout little-endian binary
+  (de)serializer with manual span-offset arithmetic — was only exercised *indirectly* through
+  `FasterLogHistorizationOutboxTests`, and only ever with single-character ASCII tag names ("A"/"B"/
+  "C"), never testing the one part of the layout arithmetic (the UTF-8 *byte*-length prefix vs. the
+  tag's *char* length) that a multi-byte tag name would exercise.
+
+**Recommendation:** Add direct offline unit tests for `GatewayHistorianValueWriter` (reusing the
+existing `FakeHistorianGatewayClient` test double already used by every other seam consumer in this
+module) and for `HistorizationOutboxEntrySerializer` (direct Serialize/Deserialize round trips,
+including a multi-byte UTF-8 tag name, an empty tag, and boundary `ushort`/`double` values).
+
+**Resolution:** `a86a0976` (branch `review/historian-gateway`), 2026-08-10. Added
+`tests/.../Recorder/GatewayHistorianValueWriterTests.cs` (10 tests: success/queued/soft-failure ack
+mapping, exception swallowing, empty-batch short-circuit, timestamp presence/absence, quality/value
+forwarding, null-argument guards, dispose delegation) and
+`tests/.../Recorder/HistorizationOutboxEntrySerializerTests.cs` (4 tests: full round trip incl.
+`DateTimeKind` preservation, a multi-byte UTF-8 tag `"Pumpe_Ω_日本語_Temp"`, an empty tag, and
+`double.MinValue`/`ushort.MaxValue` boundaries). All 14 new tests pass; full module suite is
+124 passed / 6 skipped (LiveIntegration) / 0 failed.

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs

@@ -17,7 +17,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway;
 ///     as <see cref="HistorianProvisionResult.Failed"/> and returns. The method never throws and never
 ///     logs tag values, hostnames, or credentials.
 /// </remarks>
-public sealed class GatewayTagProvisioner : IHistorianProvisioning
+public sealed class GatewayTagProvisioner : IHistorianProvisioning, IAsyncDisposable
 {
     /// <summary>
     ///     Storage rate (ms) stamped on every provisioned analog tag. The gateway's <c>EnsureTags</c> path
@@ -104,4 +104,13 @@ public sealed class GatewayTagProvisioner : IHistorianProvisioning
             return new HistorianProvisionResult(requests.Count, Ensured: 0, Skipped: skipped, Failed: definitions.Count);
         }
     }
+
+    /// <summary>
+    ///     Disposes the underlying gateway client (and its gRPC channel). The DI container owns this
+    ///     provisioner as a singleton (<c>AddHistorianProvisioning</c>), so this fires once at host
+    ///     shutdown — closing the channel gracefully, matching the read (<see cref="GatewayHistorianDataSource"/>)
+    ///     and alarm-write (<see cref="GatewayAlarmHistorianWriter"/>) paths' dispose ownership.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    public ValueTask DisposeAsync() => _client.DisposeAsync();
 }

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs

@@ -1,5 +1,6 @@
 using Google.Protobuf.WellKnownTypes;
 using ZB.MOM.WW.HistorianGateway.Contracts.Grpc;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian;
 
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Mapping;
@@ -10,6 +11,25 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Mapping;
 /// </summary>
 internal static class AlarmEventMapper
 {
+    /// <summary>
+    /// Maps <see cref="AlarmSeverity"/> to the numeric OPC UA Part 9 priority (1-1000) carried in the
+    /// wire event's <c>Severity</c> property, per the <see cref="AlarmHistorianEvent.Severity"/>
+    /// contract ("Mapped to Historian's numeric priority on the sink side"). The read side
+    /// (<see cref="EventMapper.ToHistoricalEvent"/>) <c>int.TryParse</c>s this same property, so it
+    /// must stay a numeric string — the enum's <c>.ToString()</c> name (e.g. "High") is NOT
+    /// parseable and used to silently collapse every read-back severity to the minimum (1). The
+    /// values mirror the existing native-alarm convention
+    /// (<c>ZB.MOM.WW.OtOpcUa.Runtime.Drivers.NativeAlarmProjector.MapSeverity</c>).
+    /// </summary>
+    private static string ToNumericSeverity(AlarmSeverity severity) => severity switch
+    {
+        AlarmSeverity.Low => "200",
+        AlarmSeverity.Medium => "500",
+        AlarmSeverity.High => "700",
+        AlarmSeverity.Critical => "900",
+        _ => "500",
+    };
+
     /// <summary>Maps an alarm historian event to a gateway event.</summary>
     /// <param name="alarm">The driver-agnostic alarm event.</param>
     /// <returns>The gateway wire event ready for <c>SendEvent</c>.</returns>
@@ -36,7 +56,7 @@ internal static class AlarmEventMapper
             historianEvent.Properties["AlarmId"] = alarm.AlarmId;
         historianEvent.Properties["AlarmName"] = alarm.AlarmName;
         historianEvent.Properties["EventKind"] = alarm.EventKind;
-        historianEvent.Properties["Severity"] = alarm.Severity.ToString();
+        historianEvent.Properties["Severity"] = ToNumericSeverity(alarm.Severity);
         historianEvent.Properties["User"] = alarm.User;
         historianEvent.Properties["Message"] = alarm.Message;
         if (alarm.Comment is not null)

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayTagProvisionerTests.cs

@@ -149,6 +149,27 @@ public sealed class GatewayTagProvisionerTests
         Assert.Equal(1, result.Failed); // non-blocking: no throw
     }
 
+    /// <summary>
+    /// Driver.Historian.Gateway-002: unlike its siblings (<c>GatewayHistorianDataSource</c>,
+    /// <c>GatewayAlarmHistorianWriter</c>), <see cref="GatewayTagProvisioner"/> had no dispose path at
+    /// all — the gRPC channel <c>GatewayHistorian.CreateProvisioner</c> opens was never closed. The
+    /// provisioner is registered as a process-lifetime DI singleton
+    /// (<c>AddHistorianProvisioning</c>), and the built-in container disposes any singleton instance
+    /// it creates that implements <see cref="IAsyncDisposable"/>/<see cref="IDisposable"/> — regardless
+    /// of the registered service-interface type — so implementing it here is sufficient for a clean
+    /// shutdown with no other wiring change.
+    /// </summary>
+    [Fact]
+    public async Task DisposeAsync_disposes_underlying_client()
+    {
+        var fake = new FakeHistorianGatewayClient();
+        var p = Provisioner(fake);
+
+        await ((IAsyncDisposable)p).DisposeAsync();
+
+        Assert.Equal(1, fake.DisposeCallCount);
+    }
+
     [Fact]
     public async Task Ensured_count_reflects_successful_results()
     {

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AlarmEventMapperTests.cs

@@ -23,12 +23,41 @@ public sealed class AlarmEventMapperTests
         Assert.Equal("A1", e.Properties["AlarmId"]);
         Assert.Equal("HiHi", e.Properties["AlarmName"]);
         Assert.Equal("Activated", e.Properties["EventKind"]);
-        Assert.Equal("High", e.Properties["Severity"]);
+        // Severity is the AlarmHistorianEvent.Severity XML doc contract: "Mapped to Historian's
+        // numeric priority on the sink side" — NOT the .NET enum name. EventMapper.ParseSeverity on
+        // the read side int.TryParse()s this same property, so a non-numeric string (the old bug:
+        // alarm.Severity.ToString() == "High") silently always read back as the minimum severity (1).
+        Assert.Equal("700", e.Properties["Severity"]);
         Assert.Equal("operator1", e.Properties["User"]);
         Assert.Equal("ack note", e.Properties["Comment"]);
         Assert.Equal("Temp high", e.Properties["Message"]);
     }
 
+    /// <summary>
+    /// Driver.Historian.Gateway-001 regression: the write side (<see cref="AlarmEventMapper"/>) and
+    /// the read side (<see cref="EventMapper"/>) must agree on the wire representation of severity —
+    /// a numeric OPC UA Part 9 priority string, matching the convention already used elsewhere in the
+    /// codebase for the same enum (<c>NativeAlarmProjector.MapSeverity</c>: Low=200/Medium=500/
+    /// High=700/Critical=900). Round-tripping through both mappers must reproduce the numeric severity,
+    /// not collapse to the minimum every time.
+    /// </summary>
+    [Theory]
+    [InlineData(AlarmSeverity.Low, (ushort)200)]
+    [InlineData(AlarmSeverity.Medium, (ushort)500)]
+    [InlineData(AlarmSeverity.High, (ushort)700)]
+    [InlineData(AlarmSeverity.Critical, (ushort)900)]
+    public void Severity_round_trips_through_EventMapper_as_numeric_priority(AlarmSeverity severity, ushort expected)
+    {
+        var a = new AlarmHistorianEvent("A1", "Area/Line/Pump1", "HiHi", "LimitAlarm",
+            severity, "Activated", "m", "operator1", null,
+            new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc));
+
+        var wire = AlarmEventMapper.ToHistorianEvent(a);
+        var readBack = EventMapper.ToHistoricalEvent(wire);
+
+        Assert.Equal(expected, readBack.Severity);
+    }
+
     [Fact]
     public void Null_comment_is_omitted_not_null()
     {

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Recorder/GatewayHistorianValueWriterTests.cs

@@ -0,0 +1,149 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using Xunit;
+using ZB.MOM.WW.HistorianGateway.Contracts.Grpc;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests.Recorder;
+
+/// <summary>
+/// Driver.Historian.Gateway-003 (offline unit coverage for the continuous-historization write path):
+/// before this file, <see cref="GatewayHistorianValueWriter"/> — the recorder's <c>WriteLiveValues</c>
+/// adapter, in production the write path CLAUDE.md's #491/#537/#538 live-gate history bit hard — was
+/// exercised only by the env-gated <c>Category=LiveIntegration</c> suite, which skips cleanly offline.
+/// These tests use the same <see cref="FakeHistorianGatewayClient"/> the rest of the module's offline
+/// tests share.
+/// </summary>
+public sealed class GatewayHistorianValueWriterTests
+{
+    private static GatewayHistorianValueWriter Writer(FakeHistorianGatewayClient fake) =>
+        new(fake, NullLogger<GatewayHistorianValueWriter>.Instance);
+
+    [Fact]
+    public async Task Success_ack_returns_true()
+    {
+        var fake = new FakeHistorianGatewayClient { WriteLiveValuesResult = new WriteAck { Success = true } };
+        var values = new[] { new HistorizationValue(DateTime.UtcNow, 42.0, 192) };
+
+        var ok = await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        Assert.True(ok);
+        Assert.Equal(1, fake.WriteLiveValuesCallCount);
+        Assert.Equal("T", fake.LastWriteLiveTag);
+    }
+
+    [Fact]
+    public async Task Queued_ack_is_treated_as_success()
+    {
+        // A value the gateway durably store-forward-queued must not be re-drained by the recorder.
+        var fake = new FakeHistorianGatewayClient
+        {
+            WriteLiveValuesResult = new WriteAck { Success = false, Queued = true },
+        };
+        var values = new[] { new HistorizationValue(DateTime.UtcNow, 1.0, 192) };
+
+        var ok = await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        Assert.True(ok);
+    }
+
+    [Fact]
+    public async Task Soft_failure_ack_returns_false_for_retry()
+    {
+        var fake = new FakeHistorianGatewayClient
+        {
+            WriteLiveValuesResult = new WriteAck { Success = false, Queued = false },
+        };
+        var values = new[] { new HistorizationValue(DateTime.UtcNow, 1.0, 192) };
+
+        var ok = await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        Assert.False(ok);
+    }
+
+    [Fact]
+    public async Task Gateway_exception_is_swallowed_and_returns_false()
+    {
+        // NEVER throw out of the writer — the recorder's drain loop expects a plain bool.
+        var fake = new FakeHistorianGatewayClient { WriteLiveValuesThrows = new InvalidOperationException("boom") };
+        var values = new[] { new HistorizationValue(DateTime.UtcNow, 1.0, 192) };
+
+        var ok = await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        Assert.False(ok);
+    }
+
+    [Fact]
+    public async Task Empty_batch_is_trivially_acked_without_calling_the_gateway()
+    {
+        var fake = new FakeHistorianGatewayClient();
+
+        var ok = await Writer(fake).WriteLiveValuesAsync(
+            "T", Array.Empty<HistorizationValue>(), TestContext.Current.CancellationToken);
+
+        Assert.True(ok);
+        Assert.Equal(0, fake.WriteLiveValuesCallCount);
+    }
+
+    [Fact]
+    public async Task Present_timestamp_is_sent_and_absent_timestamp_leaves_the_proto_field_unset()
+    {
+        var fake = new FakeHistorianGatewayClient();
+        var stamped = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        var values = new[]
+        {
+            new HistorizationValue(stamped, 1.0, 192),
+            new HistorizationValue(null, 2.0, 192),
+        };
+
+        await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        var sent = fake.LastWriteLiveValues!;
+        Assert.Equal(2, sent.Count);
+        Assert.Equal(stamped, sent[0].Timestamp!.ToDateTime());
+        Assert.Null(sent[1].Timestamp); // absent -> unset proto field -> gateway server-stamps now
+    }
+
+    [Fact]
+    public async Task Quality_and_numeric_value_are_forwarded_unchanged()
+    {
+        var fake = new FakeHistorianGatewayClient();
+        var values = new[] { new HistorizationValue(null, 123.5, 192) };
+
+        await Writer(fake).WriteLiveValuesAsync("T", values, TestContext.Current.CancellationToken);
+
+        var sent = fake.LastWriteLiveValues!;
+        Assert.Equal(123.5, sent[0].NumericValue);
+        Assert.Equal(192u, sent[0].Quality);
+    }
+
+    [Fact]
+    public async Task Null_tag_throws()
+    {
+        var fake = new FakeHistorianGatewayClient();
+        var values = new[] { new HistorizationValue(null, 1.0, 192) };
+
+        await Assert.ThrowsAsync<ArgumentNullException>(() =>
+            Writer(fake).WriteLiveValuesAsync(null!, values, TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task Null_values_throws()
+    {
+        var fake = new FakeHistorianGatewayClient();
+
+        await Assert.ThrowsAsync<ArgumentNullException>(() =>
+            Writer(fake).WriteLiveValuesAsync("T", null!, TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task DisposeAsync_disposes_underlying_client()
+    {
+        var fake = new FakeHistorianGatewayClient();
+        var writer = Writer(fake);
+
+        await writer.DisposeAsync();
+
+        Assert.Equal(1, fake.DisposeCallCount);
+    }
+}

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Recorder/HistorizationOutboxEntrySerializerTests.cs

@@ -0,0 +1,73 @@
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests.Recorder;
+
+/// <summary>
+/// Driver.Historian.Gateway-003 (offline unit coverage): <see cref="FasterLogHistorizationOutboxTests"/>
+/// exercises <see cref="HistorizationOutboxEntrySerializer"/> only indirectly, and only with short
+/// ASCII tag names ("A"/"B"/"C"). The hand-rolled little-endian layout computes its tag byte-length
+/// from <c>Encoding.UTF8.GetByteCount</c> before writing, so a multi-byte tag exercises the one part
+/// of the layout arithmetic the indirect coverage never reaches. This asserts the (de)serializer
+/// directly, in isolation from FasterLog.
+/// </summary>
+public sealed class HistorizationOutboxEntrySerializerTests
+{
+    [Fact]
+    public void Round_trips_all_fields()
+    {
+        var entry = new HistorizationOutboxEntry(
+            Guid.NewGuid(), "Pump1.Temp", 123.456, 192,
+            new DateTime(2026, 1, 1, 12, 34, 56, DateTimeKind.Utc));
+
+        var bytes = HistorizationOutboxEntrySerializer.Serialize(entry);
+        var round = HistorizationOutboxEntrySerializer.Deserialize(bytes);
+
+        Assert.Equal(entry.Id, round.Id);
+        Assert.Equal(entry.Tag, round.Tag);
+        Assert.Equal(entry.NumericValue, round.NumericValue);
+        Assert.Equal(entry.Quality, round.Quality);
+        Assert.Equal(entry.TimestampUtc, round.TimestampUtc);
+        Assert.Equal(entry.TimestampUtc.Kind, round.TimestampUtc.Kind); // ToBinary/FromBinary round-trips Kind
+    }
+
+    [Fact]
+    public void Round_trips_a_multi_byte_utf8_tag()
+    {
+        // The tag length prefix is a UTF-8 BYTE count, not a char count — a tag whose char count and
+        // byte count diverge (e.g. Ω is 2 bytes, 日 is 3) is the one case that would desync the layout
+        // if GetByteCount/GetBytes ever disagreed with what Deserialize slices out.
+        var entry = new HistorizationOutboxEntry(
+            Guid.NewGuid(), "Pumpe_Ω_日本語_Temp", -1.5, 0, DateTime.UtcNow);
+
+        var round = HistorizationOutboxEntrySerializer.Deserialize(
+            HistorizationOutboxEntrySerializer.Serialize(entry));
+
+        Assert.Equal(entry.Tag, round.Tag);
+        Assert.Equal(entry.NumericValue, round.NumericValue);
+    }
+
+    [Fact]
+    public void Round_trips_an_empty_tag()
+    {
+        var entry = new HistorizationOutboxEntry(Guid.NewGuid(), string.Empty, 0.0, 0, DateTime.UtcNow);
+
+        var round = HistorizationOutboxEntrySerializer.Deserialize(
+            HistorizationOutboxEntrySerializer.Serialize(entry));
+
+        Assert.Equal(string.Empty, round.Tag);
+    }
+
+    [Fact]
+    public void Round_trips_boundary_quality_and_negative_value()
+    {
+        var entry = new HistorizationOutboxEntry(Guid.NewGuid(), "T", double.MinValue, ushort.MaxValue, DateTime.UtcNow);
+
+        var round = HistorizationOutboxEntrySerializer.Deserialize(
+            HistorizationOutboxEntrySerializer.Serialize(entry));
+
+        Assert.Equal(ushort.MaxValue, round.Quality);
+        Assert.Equal(double.MinValue, round.NumericValue);
+    }
+}