ScadaBridge/archreview/04-data-audit-backbone.md

Architecture Review 04 — Central Data Layer & Audit/KPI Backbone

Scope reviewed:

• src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase — EF Core mappings, repositories, migrations, IAuditService, partition maintenance
• src/ZB.MOM.WW.ScadaBridge.Commons — as the data-shape layer (entities, repository interfaces, audit enums/types, KPI types)
• src/ZB.MOM.WW.ScadaBridge.AuditLog — central ingest/purge/reconciliation actors, CentralAuditWriter, site SQLite writer + telemetry
• src/ZB.MOM.WW.ScadaBridge.NotificationOutbox — NotificationOutboxActor, repository, delivery adapters, KPI source
• src/ZB.MOM.WW.ScadaBridge.SiteCallAudit — SiteCallAuditActor, monotonic upsert repository, reconciliation, relay
• src/ZB.MOM.WW.ScadaBridge.KpiHistory — KpiHistoryRecorderActor, KpiHistoryRepository, bucketer
• Design docs: docs/requirements/Component-ConfigurationDatabase.md, Component-AuditLog.md, Component-NotificationOutbox.md, Component-SiteCallAudit.md, Component-KpiHistory.md, Component-Commons.md
• Test projects: AuditLog.Tests, NotificationOutbox.Tests, SiteCallAudit.Tests, KpiHistory.Tests, ConfigurationDatabase.Tests

Maturity Verdict

This is an unusually mature data layer for a system at this stage: idempotent ingest is implemented correctly and defensively everywhere (duplicate-key race handling in all three insert-if-not-exists paths), the combined cached-telemetry dual-write really is transactional (verified: AuditLogIngestActor.OnCachedTelemetryAsync shares one scoped DbContext across both repositories under BeginTransactionAsync), UTC discipline is enforced with converters and SpecifyKind re-tagging at every ADO.NET boundary, actor/DB interaction follows a consistent scope-per-message + PipeTo pattern with in-flight guards, and MSSQL-backed integration tests exist for the hard parts (partition purge, execution-tree walks, outage reconciliation). The weaknesses are concentrated in scale-dependent operational behavior rather than logic: the partition purge's full-table unique-index rebuild will start failing on default command timeouts as the table grows, several KPI predicates have no supporting index and full-scan year-sized tables every 60 seconds, the execution-tree query scans the entire AuditLog per invocation, the spec'd site SQLite 7-day retention purge simply does not exist, the append-only DB-role story is DDL-only (one connection principal runs both writer and maintenance paths), and the persisted backlogTotal KPI trend is hardwired to zero. None of these bites at demo scale; several will bite within months of production volume.

---

Findings — Stability

[High] Site SQLite 7-day retention purge is specified but not implemented — unbounded site DB growth

Component-AuditLog.md:412-414 specifies: "Sites: daily site job; default 7-day retention (configurable, min 1, max 90). Respects the hard ForwardState invariant." No such job exists. There is no DELETE against audit_event / audit_forward_state anywhere in src/ — the only destructive statement is the one-time legacy-table DROP TABLE IF EXISTS AuditLog at startup (src/ZB.MOM.WW.ScadaBridge.AuditLog/Site/SqliteAuditWriter.cs:198). The writer's own comments treat the purge as future work: SqliteAuditWriter.cs:145 ("a future PRAGMA incremental_vacuum shrink the file after the 7-day retention purge") and :33 claims the store is "recreated per deployment", which is not a retention mechanism.

Failure scenario: a long-lived site node accumulates every Forwarded/Reconciled audit row forever. At even modest script activity (10 events/s ≈ 860k rows/day) the SQLite file grows by GBs per month; GetBacklogStatsAsync's COUNT(*) stays cheap (it filters Pending), so nothing surfaces on the health dashboard — the disk just fills. The auto_vacuum = INCREMENTAL pragma (SqliteAuditWriter.cs:149) is set up for a purge that never runs.

[High] SwitchOutPartitionAsync — full-table unique-index rebuild inside one transaction, no command timeout

src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/AuditLogRepository.cs:257-332: the monthly partition switch drops UX_AuditLog_EventId, switches, then CREATE UNIQUE NONCLUSTERED INDEX UX_AuditLog_EventId ON dbo.AuditLog (EventId) — a scan+sort over every remaining row of the table (up to 365 days of audit events), all inside a single transaction executed via ExecuteSqlRawAsync.

Three compounding problems:

1. Default 30s command timeout. No CommandTimeout is configured anywhere (ServiceCollectionExtensions.cs:29-34 sets only UseSqlServer(connectionString); repo-wide grep confirms no CommandTimeout on the data layer). Once the table is large enough that the index rebuild exceeds 30 seconds, the batch times out, the CATCH block rolls back and re-THROWs, AuditLogPurgeActor.OnTickAsync logs and moves on (AuditLogPurgeActor.cs:198-206) — and the same failure repeats every daily tick forever. Retention silently stops; the table grows past 365 days; the rebuild gets slower each day. This is a self-reinforcing failure with no alerting beyond an error log line.
2. Write blockage. The transaction holds Sch-M from the DROP INDEX through the CREATE INDEX; all ingest (InsertIfNotExistsAsync, dual-writes, direct writes) blocks for the full rebuild duration. The actors tolerate this (rows stay Pending at sites), but central direct-write paths (Inbound API middleware, dispatcher audit emission) will eat the latency or the swallow-and-count path.
3. Uniqueness window. While UX_AuditLog_EventId is absent, the IF NOT EXISTS probe becomes an unindexed scan and single-column EventId uniqueness is enforced only via the composite PK. In practice a retried event carries the same OccurredAtUtc so the PK still dedupes; the residual risk is small but the per-insert scan cost during the window is not.

Recommendation: set a long CommandTimeout for the maintenance path (or run the rebuild with ONLINE = ON where edition allows, outside the switch transaction), and monitor purge success as a health metric, not just a log line. Long term, consider making UX_AuditLog_EventId unnecessary (see the clustered-key finding under Performance).

[High] Append-only DB-role enforcement is DDL-only — one principal runs writer and maintenance paths

The migrations create scadabridge_audit_writer (INSERT+SELECT, DENY UPDATE/DELETE) and scadabridge_audit_purger (SELECT + ALTER ON SCHEMA::dbo) — 20260520142214_AddAuditLogTable.cs:146-158, re-granted in 20260602174346_CollapseAuditLogToCanonical.cs:212-223. But the application registers one ScadaBridgeDbContext with one connection string (ServiceCollectionExtensions.cs:29-45); the same principal executes InsertIfNotExistsAsync (writer path), PurgeChannelOlderThanAsync's DELETE TOP (AuditLogRepository.cs:409-410), BackfillSourceNodeAsync's UPDATE TOP (AuditLogRepository.cs:851-852), and the switch-out DDL. Consequences:

• If the runtime login were actually placed in scadabridge_audit_writer, the DENY would break the purge/backfill on the same connection. So in any real deployment the login must hold INSERT+UPDATE+DELETE+DDL — meaning the append-only guarantee is enforced only by the CI grep guard (tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/AuditLogAppendOnlyGuardTests.cs), not by the database.
• The purger role as granted cannot even perform the purge: CREATE TABLE (staging) requires the database-level CREATE TABLE permission, which ALTER ON SCHEMA::dbo alone does not confer.

Component-AuditLog.md:417-424 presents the role separation as the enforcement mechanism ("The application accesses AuditLog via a dedicated DB role"). Spec and code have drifted: either wire a second connection string/principal for the maintenance path (and fix the purger grants), or re-document enforcement honestly as "CI guard + code review, roles provided for DBAs who choose to segregate principals."

[Medium] SiteCalls monotonic upsert drops mid-lifecycle progress (RetryCount / LastError frozen at rank)

SiteCallAuditRepository.cs:30-40 ranks Attempted == Skipped == 2 and the UPDATE fires only when incomingRank > storedRank (:128-138). A cached call retrying 20 times emits 20 Attempted telemetry packets with increasing RetryCount and fresher LastError/HttpStatus — every one after the first is a no-op ("first-write-wins at each rank" per the comment). The Central UI Site Calls page therefore shows RetryCount = 1 and the first error for the entire retry phase, until the terminal packet lands. The spec's intent (Component-SiteCallAudit.md:76-78, "upsert-on-newer-status... at-least-once and out-of-order telemetry are harmless") is about status regression, not about freezing progress fields. A safe refinement: within equal rank, apply the update when incoming.UpdatedAtUtc > stored.UpdatedAtUtc (still idempotent and regression-proof).

[Medium] No EF Core connection resiliency (EnableRetryOnFailure)

ServiceCollectionExtensions.cs:31 configures UseSqlServer(connectionString) with no execution strategy. The audit/dispatcher actors compensate with their own retry loops (rows stay Pending, next tick retries), so the backbone self-heals — but every read-side path (KPI requests, outbox queries, Notify.Status round-trips, execution-tree queries) surfaces transient faults (failover of the SQL instance, connection pool churn) directly to the caller as a failed response. One line of sqlOptions.EnableRetryOnFailure(...) would cover the fleet. (Note: retry strategies and user-initiated transactions interact — OnCachedTelemetryAsync's BeginTransactionAsync would need CreateExecutionStrategy wrapping — so adopt deliberately.)

[Medium] Operator Retry on a parked notification emits no audit row

Component-NotificationOutbox.md:128: "Operator Retry/Discard still mutates only the Notifications row, and each transition emits the corresponding Notification.Attempt / Notification.Terminal audit row." DiscardAsync does emit (NotificationOutboxActor.cs:1021), but RetryAsync (NotificationOutboxActor.cs:951-977) resets Parked→Pending with no audit emission. Forensically, a parked notification that an operator retried and that then delivered shows an unexplained Parked → Attempted → Delivered sequence with no record of who un-parked it or when. Same gap for SiteCallAudit's retry/discard relay (site-side telemetry covers the state change but not the operator identity).

[Medium] Reconciliation edge cases are detected but dead-ended

• Single-timestamp cursor pin (SiteCallAuditActor.cs:636-654): when > ReconciliationBatchSize rows share one UpdatedAtUtc the cursor cannot advance; the code correctly detects and logs it, but the backlog tail then never reconciles ("will not reconcile until those rows' timestamps differ") and nothing surfaces beyond a log Warning — no health metric, no stalled event (unlike the sibling SiteAuditReconciliationActor, which publishes SiteAuditTelemetryStalledChanged). The fix the log message itself names — a composite (timestamp, id) keyset in the pull contract — is deferred without a tracking artifact.
• Permanent row abandonment (SiteAuditReconciliationActor.cs:283-293): after 5 failed inserts a row is dropped with LogCritical and the cursor advances. For a compliance-oriented audit log, "we permanently lost EventId X" deserves a durable record (e.g., a synthetic Skipped audit row or a health counter), not only a log line that rotates away.

[Medium] NotificationOutboxActor ingest ack path can ack before the row is committed under a SQLite test provider mismatch — verify provider parity

NotificationOutboxRepository.InsertIfNotExistsAsync maintains two dialects (NotificationOutboxRepository.cs:70-95 SQLite INSERT OR IGNORE, :99-117 T-SQL IF NOT EXISTS). This is correct today but is production code carrying a test-only branch, and the two branches have subtly different concurrency semantics (the SQLite path can never observe the duplicate-key race the SQL Server path handles). The AuditLog and SiteCall repositories chose the opposite strategy (MSSQL-only SQL, MSSQL-backed test fixtures). Pick one convention; the divergence invites the next repository to guess.

[Low] Purge actors' first tick waits a full interval

AuditLogPurgeActor.PreStart (:95-100), SiteCallAuditActor.StartPurgeTimer (:380-386), and KpiHistoryRecorderActor purge timer (KpiHistoryRecorderActor.cs:124-127) all use initialDelay = interval (24h). A central node that restarts daily (crash loop, scheduled recycling) would never purge. Low likelihood, cheap fix (short initial delay; purges are idempotent).

[Low] SqliteAuditWriter.Dispose sync-over-async

SqliteAuditWriter.cs:890-894 blocks on DisposeAsync via Task.Run(...).GetAwaiter().GetResult(). The mitigation (thread-pool hop) is correct and documented; flagged only because it remains a blocking dispose on a hot-path component.

---

Findings — Performance

[High] SiteCalls KPI predicates full-scan the table — no index on TerminalAtUtc

The "schema-honest" non-terminal predicate is TerminalAtUtc IS NULL (SiteCallAuditRepository.cs:224-232), used by ComputeKpisAsync (buffered :243-244, stuck :259-260, oldest :262-268), the per-site and per-node variants (:286-307, :336-362), and PurgeTerminalAsync (:213-218). But the table's only indexes are (SourceSite, CreatedAtUtc) and (Status, UpdatedAtUtc) (SiteCallEntityTypeConfiguration.cs:79-87) — nothing covers TerminalAtUtc. Every such count is a clustered scan of a table retained for 365 days (SiteCallAuditOptions.RetentionDays = 365, potentially millions of rows for busy sites). These queries run:

• every 60 s from SiteCallAuditKpiSampleSource (global + per-site + per-node = ~9 scanning queries per recorder tick),
• on every Health-dashboard poll (10 s cadence per the UI design) via SiteCallKpiRequest.

Failure scenario: at ~2M rows, each scan is hundreds of ms of CPU/IO; the KPI backbone alone drives a near-continuous scan load on the central DB, and the KPI Ask timeouts start failing intermittently. Fix: a filtered index CREATE INDEX IX_SiteCalls_NonTerminal ON SiteCalls (CreatedAtUtc) WHERE TerminalAtUtc IS NULL (plus SourceSite/SourceNode includes) makes every one of these predicates a small seek, since the non-terminal population is the live queue, not the archive.

[High] GetExecutionTreeAsync scans the entire AuditLog per invocation

AuditLogRepository.cs:704-756: the recursive CTE's Edges anchor is SELECT DISTINCT ExecutionId, ParentExecutionId FROM dbo.AuditLog WHERE ExecutionId IS NOT NULL. Neither IX_AuditLog_Execution nor IX_AuditLog_ParentExecution covers the other column, so satisfying the DISTINCT requires scanning an entire index (or the clustered index) across all partitions, all 365 days — on every audit tree CLI call and every UI execution-tree drilldown. The upward walk (:652-677) is fine (seeks on IX_AuditLog_Execution), and the STRING_AGG subqueries seek per node; it is the edge materialization that is O(table).

Fix options: (a) add ParentExecutionId as an INCLUDE on IX_AuditLog_Execution (index-only DISTINCT scan — better but still O(table)); (b) constrain the Edges CTE to a time window derived from the root's FirstOccurredAtUtc (execution trees span minutes, not years — a OccurredAtUtc >= root - slack predicate gets partition elimination); (c) seed the recursion by seeking children per level (WHERE ParentExecutionId IN (…)) instead of pre-materializing a global edge set.

[Medium] AuditLog clustered key leads with a random GUID

AuditLogEntityTypeConfiguration.cs:172 / migration CollapseAuditLogToCanonical.cs:75: clustered PK (EventId, OccurredAtUtc) with EventId = Guid.NewGuid(). Inserts land randomly across the current month's partition → page splits, fragmentation, and larger-than-necessary buffer pool footprint on the system's highest-volume table. The composite order also means the PK is useless for time-range queries (all served by secondary indexes). A clustered key of (OccurredAtUtc, EventId) would give append-locality and make IX_AuditLog_OccurredAtUtc redundant; EventId uniqueness would still need the non-aligned unique index (which exists anyway). This interacts with the switch-out dance either way; worth a deliberate benchmark before the table gets big — changing it later is a full rebuild.

[Medium] KpiSample volume and unbatched purge

Per recorder tick (60 s), sample count ≈ NotificationOutbox(5 × (1 global + N sites + M nodes)) + SiteCallAudit(6 × (1 + N + M)) + AuditLog(3) + SiteHealth(12 × N). For 10 sites / 20 site-nodes this is ~500 rows/min ≈ 700k rows/day ≈ 60M+ rows at the 90-day default retention — with a 5-column composite index (IX_KpiSample_Series) maintained on every insert. Meanwhile PurgeOlderThanAsync is a single unbatched ExecuteDeleteAsync (KpiHistoryRepository.cs:65-71): in steady state it deletes ~700k rows in one transaction daily — a lock/log spike on the same DB serving the operational tables; after any purge outage the catch-up delete is worse (and subject to the same default 30s command timeout as the partition switch). Batch the delete (TOP (n) loop, mirroring PurgeChannelOlderThanAsync), and consider whether per-node scope sampling every 60 s is worth 40% of the row volume.

[Medium] Catch-all (@p IS NULL OR col = @p) predicates in SiteCalls query

SiteCallAuditRepository.QueryAsync (:186-202) builds one static SQL shape where every filter is ({param} IS NULL OR col = {param}). This is the classic optional-parameter anti-pattern: SQL Server compiles one plan for all filter combinations, generally a scan ordered by CreatedAtUtc DESC. Fine while the table is small; on a year of rows, a filtered query (e.g. Status = 'Parked') will scan rather than use IX_SiteCalls_Status_Updated. OPTION (RECOMPILE) or dynamic predicate composition (as AuditLogRepository.QueryAsync does with LINQ, :117-199) fixes it.

[Medium] Notification outbox query page uses offset paging + double query + unsargable LIKE

NotificationOutboxRepository.QueryAsync (:165-228): COUNT + Skip((page-1)*size) — deep pages re-scan; Subject.Contains(keyword) translates to LIKE '%kw%' (unsargable). The sibling AuditLog and SiteCalls pages both use keyset paging. For a 365-day table this page will degrade first in the UI. Also ComputeKpisAsync/ComputePerSiteKpisAsync/ComputePerNodeKpisAsync issue 5-7 sequential round trips each (:239-283, :286-334, :337-391), and the per-site/per-node "oldest" computations materialize every non-terminal row into memory (:312-318, :368-375) — bounded by live-queue size in normal operation, unbounded during a delivery outage (exactly when the KPI page gets watched). A single GROUP BY with conditional aggregation would serve each snapshot in one query.

[Low] Non-persisted IngestedAtUtc computed column re-parses DetailsJson per row read

AuditLogEntityTypeConfiguration.cs:158-164: every materialization of AuditLogRow (e.g. QueryAsync pages) evaluates SWITCHOFFSET(CAST(JSON_VALUE(DetailsJson …))) over an nvarchar(max) per row. Page-sized reads make this tolerable; just avoid ever using IngestedAtUtc in a predicate (it would force a full-table JSON parse).

[Low] MarkForwardedAsync / MarkReconciledAsync build per-id IN lists

SqliteAuditWriter.cs:644-664, 735-753: parameter-per-id is safe at the default telemetry batch of 256 (SiteAuditTelemetryOptions.BatchSize = 256) but silently approaches SQLite's 999-parameter limit if an operator raises the batch size ~4×. Worth a guard or chunking.

---

Findings — Conventions

[Medium] Design docs have drifted from the code in several load-bearing places

• Component-Commons.md:47 lists AuditKind with 10 values; the code has 14 (Commons/Types/Enums/AuditKind.cs:10-26, adds the four SecuredWrite* kinds). :46 lists AuditChannel with 4 values; the code has 5 (AuditChannel.cs:8-15, adds SecuredWrite).
• Component-Commons.md:89 still says NotificationType: "Email — currently the only value; Teams and other channels are planned but not yet present" — Sms shipped 2026-06-19 and the Teams plan was dropped (per CLAUDE.md).
• Component-ConfigurationDatabase.md:58 describes the NotificationList Type discriminator as "Email / Teams / …".
• Component-ConfigurationDatabase.md:399 states AuditLog retention is a "single global value in v1, no per-channel overrides", directly contradicting the shipped PerChannelRetentionDays feature (and Component-AuditLog.md:396-411, which documents it).
• Component-SiteCallAudit.md:44-51 describes the SiteCalls schema with Kind/TargetSummary/Provenance columns and a Pending/Retrying/... status set; the shipped schema (Component-ConfigurationDatabase.md:64 and SiteCallEntityTypeConfiguration.cs) uses Channel/Target and AuditStatus-derived strings with no instance/script provenance columns. The ConfigDb doc is current; the SiteCallAudit doc's own table section is stale.

Per the repo's own rule ("design doc and code travel together"), these should be reconciled in one pass.

[Medium] AuditLogRow entity lives in ConfigurationDatabase, not Commons

src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Entities/AuditLogRow.cs breaks the "POCO entities in Commons / mappings in ConfigurationDatabase" convention that every sibling table follows (SiteCall, Notification, KpiSample, AuditLogEntry are all Commons POCOs). There is a defensible rationale — the contract type is the external ZB.MOM.WW.Audit.AuditEvent and AuditLogRow is a persistence-only projection — but the deviation is undocumented in Component-Commons.md/Component-ConfigurationDatabase.md, and IAuditLogRepository (in Commons) consequently traffics in AuditEvent while ISiteCallAuditRepository traffics in a Commons entity. Document the deviation or move the row shape.

[Low] Mixed timestamp CLR types across sibling tables

Notification uses DateTimeOffset (Commons/Entities/Notifications/Notification.cs:68-77) while SiteCall, KpiSample, and AuditLogRow use UTC DateTime. Both satisfy the UTC convention, but the DateTimeOffset choice is what forced the awkward in-memory Min reductions and "converter makes SQL Min awkward" workarounds in NotificationOutboxRepository (:262-274, :308-318) that its DateTime-based siblings didn't need. New tables should standardize (the DateTime+Utc-converter pattern in AuditLogEntityTypeConfiguration.cs:32-40 is the cleaner precedent).

[Low] Uncharted KPI metric names are string literals while charted ones use the catalog

NotificationOutboxKpiSampleSource.cs:40-43: "stuckCount" / "oldestPendingAgeSeconds" are inline literals with a comment explaining the split, while charted metrics use KpiMetrics.* constants. Consistent cataloging (all persisted names in KpiMetrics) would remove a rename hazard on values that are, per the code's own comment, "persisted; do not rename."

Positive observations (conventions done right)

• Options pattern with dedicated validators and clamped intervals everywhere (SiteCallAuditOptions.ResolvedReconciliationInterval guarding the Akka zero-interval spin footgun, AuditLogOptionsValidator, KpiHistoryOptionsValidator).
• The Sender-capture-before-await and Context.System.EventStream-capture-before-await disciplines are applied consistently across all five actors — a subtle Akka correctness point handled uniformly.
• Duplicate-key race hardening (2601/2627 swallow) is uniform across all three idempotent-insert repositories, with the check-then-act window explicitly documented at each site.
• AuditService stages within the caller's unit of work exactly as the spec's transactional-audit guarantee requires, with cycle-tolerant serialization that can't roll back the business operation (AuditService.cs:20-24, 71-85).
• The dual-write transaction (AuditLogIngestActor.cs:262-311) is genuinely atomic per entry with per-entry isolation, and IngestedAtUtc is stamped from one instant across both rows.

---

Underdeveloped Areas

1. backlogTotal KPI history series is permanently zero. AuditLogRepository.GetKpiSnapshotAsync hardcodes BacklogTotal: 0L (AuditLogRepository.cs:618) — documented in the interface as "left at zero" — and the Central UI's live tile compensates by summing site backlog reports (CentralUI/Services/AuditLogQueryService.cs:125-155). But AuditLogKpiSampleSource persists the repo snapshot directly (AuditLogKpiSampleSource.cs:69-85), so every recorded backlogTotal sample is 0, and the Audit Log page's backlog trend chart (AuditLogPage.razor.cs:297) renders a flat zero line regardless of actual backlog. The one KPI whose history matters most during an outage post-mortem (how big did the backlog get, how fast did it drain) records nothing. Fix: inject the same health-aggregator read the query service uses into the sample source.
2. Site SQLite retention purge — spec'd, commented as future, absent (see Stability [High] above).
3. Hash-chain tamper evidence (T1) and Parquet archival (T2) — explicitly deferred to v1.x per Component-AuditLog.md:433-437; the verify-chain CLI is a documented no-op placeholder. Consistent with plan; no drift.
4. SecuredWrite audit rows leave SourceNode NULL — documented as a logged follow-up (Component-AuditLog.md:154-157); also note PerChannelRetentionDays validation accepts SecuredWrite as a key (the enum has it) even though the design doc's config section (:396-399) enumerates only the four original channels — harmless, but the doc under-promises.
5. Reconciliation cursor keyset upgrade — the (timestamp, id) composite cursor that would eliminate the single-timestamp pin is acknowledged in a log message (SiteCallAuditActor.cs:640-654) but not tracked anywhere; the pull contracts (CachedCallReconcileRequest, PullAuditEvents) would need an additive field.
6. Dispatcher throughput ceiling — notification delivery is strictly sequential within a sweep (batch 100, 10 s interval, blocking SMTP/Twilio per row: NotificationOutboxActor.RunDispatchPass:321-347). A slow SMTP endpoint (5 s/attempt) caps throughput at ~1 notification/5 s regardless of batch size, and the in-flight guard means subsequent ticks drop. The spec's "dedicated blocking-I/O dispatcher" (Component-NotificationOutbox.md:23) is not literally implemented (delivery runs on the thread pool off the actor thread — functionally acceptable, doc drift), but there is no per-notification parallelism at all. Fine for alarm-notification volume; a burst (alarm storm fanning to hundreds of notifications) drains slowly.
7. Test coverage is strong overall (MSSQL-backed integration tests for partition purge, execution trees, outage reconciliation, dual-write; per-actor TestKit suites; options validators tested). Gaps mirror the findings: no test asserts a site-SQLite retention purge (it can't — the feature is missing), no test exercises SwitchOutPartitionAsync at a size that would expose the command-timeout failure, and no test asserts backlogTotal history is non-zero under backlog.

---

Prioritized Recommendations

1. Implement the site SQLite 7-day retention purge (daily timer on SiteAuditTelemetryActor or a dedicated job; DELETE joined on audit_forward_state.ForwardState IN ('Forwarded','Reconciled') + age, followed by PRAGMA incremental_vacuum) — closes the unbounded-growth hole and honors the spec'd ForwardState invariant.
2. Fix the backlogTotal KPI sample source to read the health aggregator like AuditLogQueryService does — one-file change, restores a shipped feature.
3. Harden SwitchOutPartitionAsync operationally: explicit long CommandTimeout on the maintenance commands, a health metric/event on purge failure (the success event exists; the failure path is log-only), and an integration test at representative volume.
4. Add the filtered non-terminal index on SiteCalls (WHERE TerminalAtUtc IS NULL) and turn each KPI snapshot into a single conditional-aggregation query — removes the dominant recurring scan load.
5. Bound the GetExecutionTreeAsync edge scan with a time-window predicate (partition elimination) or per-level child seeks.
6. Decide the DB-role story honestly: either a second maintenance connection string/principal (and grant CREATE TABLE to the purger), or update Component-AuditLog.md/Component-ConfigurationDatabase.md to state that append-only enforcement is CI-guard-based with roles offered as an optional DBA hardening.
7. Allow same-rank freshness updates in the SiteCalls upsert (UpdatedAtUtc tiebreaker) so retrying calls show live RetryCount/LastError.
8. Batch the KpiSample purge and reconsider per-node sampling cadence before fleet size multiplies row volume.
9. Emit an audit row for operator Retry on parked notifications (and consider operator identity on the SiteCalls relay), matching the spec's transition-audit promise.
10. Reconcile the stale design-doc passages (AuditKind/AuditChannel/NotificationType enum lists, Teams references, "no per-channel overrides", SiteCalls schema section) in one documentation pass, per the repo's docs-and-code-travel-together rule.