Review 2026-10-21 G-5, G-6, G-8 ("wire them all").
- G-5: GatewayGalaxyHierarchySource now pages the DiscoverHierarchy RPC
itself (DiscoverHierarchyRawAsync) at Repository.DiscoverPageSize. The
mxaccessgw client's typed DiscoverHierarchyAsync overloads hard-code a
5000-row page and DiscoverHierarchyOptions carries no page size, so the
authored knob reached nothing; the raw request takes page_size on the
wire, so it is honoured here (non-positive → 5000). The loop keeps the
client's repeated-page-token guard.
- G-6: GalaxyDriverProbe reads ProbeTimeoutSeconds from the config and
runs under min(caller budget, configured cap) — the knob can shorten a
probe, never extend it past the host's budget. The TCP preflight now
shares that deadline instead of hanging on the caller's token alone.
- G-8: GatewayGalaxyDataWriter runs a batch as one lane per distinct tag,
concurrently and bounded at 8 (MaxConcurrentWriteLanes); same-tag
writes keep their authored order inside one lane and results land at
their request's index. Acks stay serial by design.
- GalaxyDriverForm: help text now says what the two knobs actually do.
Refs #735
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
Review 2026-10-21 G-4, G-7, G-11, G-10.
- G-4: PerPlatformProbeWatcher publishes its probe handles into the
driver's SubscriptionRegistry under the reserved ProbeSubscriptionId
(-1), so the EventPump's fan-out finally reaches the watcher — every
ScanState event used to be dropped as "stale after unsubscribe" and
per-platform host status never updated. ResubscribeAllAsync delegates
the reserved entry to the watcher's new ResubscribeAsync, so the probe
survives a reconnect; StopAsync removes the entry. Probe traffic is
routed by the reserved handle and never reaches the public OnDataChange.
The watcher is also built on the seam path (subscriber only), which is
what makes the wiring testable without a gateway.
- G-7: GalaxyDiscoverer.DiscoverAsync returns the hierarchy it fetched and
the driver selects its $WinPlatform/$AppEngine probe targets from that
same sweep instead of asking the gateway for the whole galaxy twice.
- G-11: OnPumpDataChange stamps LastSuccessfulRead from inbound stream
data, throttled (1 s default), so the comms-loss watchdog has honest
liveness evidence under a Degraded supervisor.
- G-10: an EventPump dispatch-loop exit that is not a cancellation now
reports a stream fault so the supervisor recreates the pump, instead of
leaving a deaf pump reporting Healthy.
Refs #735
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
GalaxyDriver.ShutdownAsync was a no-op: the MX session, both owned gRPC
channels, the EventPump, the alarm feed, the DeployWatcher, the probe
watcher and the reconnect supervisor were released only by DisposeAsync,
which the host never calls — one leaked gateway session per respawn,
removal and /raw browse, and a watchdog Shutdown→Initialize that neither
tore down nor rebuilt anything (review 2026-10-21 G-1, G-2, G-9, G-3).
- ShutdownAsync and DisposeAsync share one ordered TeardownRuntimeAsync
(supervisor first, then deploy/probe watchers, pump, alarm feed,
session, channels); owned seams are nulled so the next InitializeAsync
rebuilds; injected seams survive a shutdown and drop only on the final
dispose. Lifecycle calls are serialised behind a gate.
- A failed production connect releases its partial runtime (no orphaned
client per watchdog retry) and lands Faulted before and after.
- StartDeployWatcherAsync stops the previous watcher before starting a
replacement, and now starts on a production instance's FIRST
InitializeAsync (the old guard returned whenever discovery had not run
here, which on a runtime instance is always).
- ReconnectSupervisor releases each cycle's CancellationTokenSource when
its loop exits and gains an awaitable StopAsync; PerPlatformProbeWatcher
gains an awaitable StopAsync.
- EventPump treats any exception under cancellation as a clean stop, so a
teardown never wakes the supervisor.
- Internal seams for tests: an IGalaxyDeployWatchSource ctor parameter and
a session-open override forwarded to the owned GalaxyMxSession.
Refs #735
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
Keeps the root CLAUDE.md lean by lazy-loading per-area detail (drivers, topology,
data-plane auth, alarms, historian) only when matching files are touched.
Clears CommentChecker findings in src/ (3,280 -> 209, the remainder being false
positives like PLC-5/IEEE-754/M0.0) by adding missing summaries/params/returns,
normalising inheritdoc usage, and removing Gitea/review tracking IDs while keeping
the rationale they annotated. Comment-only: comment-stripped sources are identical
to the parent commit.
The #687/F4 crowding re-read doubled its room up to 8 re-reads with no cap, so a pathological single-ms
cluster on a streaming gateway was streamed several times over for one page. The room is now capped at
ServerHistorian:MaxTieClusterOverfetch (plumbed into GatewayHistorianDataSource as maxPreWindowRows,
default 65536); past it, or once a read has already seen more, the read is BadTooManyOperations with
PreWindowRows set.
Item 1: IHistorianDataSource.MaxRawValuesPerRead (Gateway: learned ceiling - 16 = L; 0 elsewhere, guard-tested).
RawPageLimit plans every raw page on top of #692's lookahead: with L > 0 the read (page P + boundary ties k
+ lookahead) is <= L, P = min(N, L - k - 1) (uncapped: L - k - 1); no room => straight to the tie-cluster
over-fetch (never an uncapped or underflowing request). One re-plan + re-read when the read taught a new
limit or was refused for crowding pre-window rows (HistoryReadLimitExceededException.PreWindowRows): the
page shrinks by them. Over-fetch bound min(MaxTieClusterOverfetch, L - 1); slice size never 0.
Review fixes: F1 a window starting at/before the epoch counts AVEVA's synthetic row as pre-window (a
ceiling-truncated read was reported complete); F2 the page path disables ReverseRawReader's own re-run
(cold end-only ceiling 20/N=3000 cost 3,023 reads, now <= 40); F3 crowded look-back re-read around, not
refused; F4 the #687 crowding re-read room at least doubles (up to 8 re-reads), so a cluster of hundreds
just before a resume point no longer fails on a default gateway; a crowding refusal that leaves no room
is BadTooManyOperations, never a tick-by-tick cursor.
Item 2: ServerHistorian:EventReadHorizon (default 00:00:00 = exact, unchanged). When set, every fresh
HistoryReadEvents window's earlier bound is raised to now - horizon (both directions, all backends).
Lossy by design; documented.
Item 3 (native TimeAverage/Total re-check) stays blocked on historiangw#27.
A time inside a gap of a bursty tag costs a 64-row jump read landing in the next
burst plus a bisection back across the gap (~6 reads), so the 64 + 4/time cap
refused a 1 h/day 1 Hz tag asked at noon on 30/100/300 consecutive days
(186/606/1,806 reads) that AVEVA's own AtTime served. The cap is a runaway guard
(HistoryReadDeadline bounds wall clock): now 64 + 16 per distinct time.
Regression tests for x30/x100/x300 (each <= 7 reads a time, values checked).
Docs: DateTime-hinted nodes are stepped but keep the gateway's Double (the
Gateway never historizes DateTime nodes; the SQL backends return DateTime).
H1: the walk-back reset its width after a full segment and discarded a full
whole-remainder read, so a dense tag that stopped reporting weeks ago gave up
after 65 reads (BadTooManyOperations). It now gallops (x4, whole-remainder read
after two empties) and then BISECTS once any stored row is known below — a full
segment's (incl. a full whole-remainder read's) last row, or an earlier window
read's last row: (mid, hi] empty => hi = mid; full => known = its last row;
complete with rows => done; one density-aimed segment after a full read until an
aimed segment comes back empty; below 2 ms the whole stretch from the known row
(never a 100 ns step). Stopped 1 h..365 d with 5 y history: 6-18 reads at
1/min, 21-25 at 1 Hz, 25-31 at 10 Hz.
M1: a jump read asks for 64 rows (1,000 times over 10 y: ~66k rows, was ~1M).
L1: per-request read cap 64 + 4 x distinct times with a clear message; a strict
ceiling below 4 rows is refused naming the ceiling.
L2: DateTime hints are stepped. L3: under an integer hint a NaN/inf or
out-of-Int64-range value is unusable, never saturated Good.
L5: conformance Int64 AtTime asserts the type. L4: docs name the backend
differences. Nit: the window read end is clamped to now + 1 day.
Reviewer harnesses (Scratch693Fuzz/Cost, not committed): 0 differential diffs
over 2,400 cases.
F1 (pre-existing): SliceTieCluster continued a forward read while next <= endUtc, but the
forward window is end-exclusive; an oversized cluster one step below EndTime resumed AT
EndTime, the resume became a start == end point read and returned the EndTime samples as
data (microsecond: EndTime 8.1 us truncated to 8.0). Continue only while next is strictly
before EndTime truncated to the backend resolution.
Tests: A_tie_cluster_just_below_the_end_time_never_resumes_onto_the_end_time (native +
microsecond; both fail on the old rule) and
A_cursor_inside_a_tie_cluster_over_fetches_without_a_normal_read_first (pins the
WithinTieCluster shortcut by per-page read count; fails with the shortcut disabled).
Docs: the oversized-cluster exception to 'every page full / no lookahead => last' (F2),
and the N + skip + 1 strict-ceiling edge note referencing #694 (F4).
Review follow-up: on the Gateway every value is a Double (AVEVA stores a Boolean
as a 0/1 Int2), so the raw-evaluated AtTime interpolated a Boolean node to 0.5.
- IHistorianDataSource gains a default-interface overload
ReadAtTimeAsync(fullReference, timestamps, TypeCode? valueType, ct) that
forwards to the existing one (SqlServer/QuestDB/Null/fakes unchanged).
- OtOpcUaNodeManager.HistoryReadAtTime passes the node's scalar DataType
(AtTimeValueType: built-in type -> TypeCode; arrays/other types => null);
Raw and UNS NodeIds alike.
- GatewayAtTimeReader converts each raw row to the hinted kind BEFORE the shared
AtTimeEvaluator runs (Boolean = value != 0 => stepped; integers => Int64 =>
interpolated, rounded half away from zero, returned as the node's integer
type; floats Double), so the evaluator's own type branch decides.
- AtTimeEvaluator boxed every interpolated Int64 as a Double (`cond ? (long)x :
v` widens to double); its "keeping its type" test passed because Shouldly
ShouldBe compares numbers across types. Fixed (affects SqlServer/QuestDB too)
and pinned with ShouldBeOfType.
Joseph Doherty
dohertj2