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

# Architecture Review 04 — Central Data Layer & Audit/KPI Backbone (Round 2)

**Date:** 2026-07-12 (round 2; round-1 baseline commit `b910f5eb`, current HEAD `8c888f13`)

**Scope reviewed:**

- `src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — EF Core mappings, repositories, migrations (5 new since baseline), `IAuditService`, partition maintenance
- `src/ZB.MOM.WW.ScadaBridge.AuditLog` — central ingest/purge/reconciliation actors, pull clients, site SQLite writer + telemetry + new retention job
- `src/ZB.MOM.WW.ScadaBridge.NotificationOutbox` — dispatcher (now bounded-parallel), repository (single-query KPIs), delivery adapters
- `src/ZB.MOM.WW.ScadaBridge.SiteCallAudit` — actor (composite keyset reconciliation cursor, pinned event, relay identity), repository (same-rank tiebreaker, RECOMPILE)
- `src/ZB.MOM.WW.ScadaBridge.KpiHistory` — recorder actor **including the post-initiative hourly-rollup subsystem shipped 2026-07-10** (`KpiRollupHourly`, `kpi-rollup` tick, backfill, `KpiMetricAggregationCatalog`, raw-vs-rollup routing) — brand-new code no round-1 review saw
- Design docs: `Component-ConfigurationDatabase.md`, `Component-AuditLog.md`, `Component-NotificationOutbox.md`, `Component-SiteCallAudit.md`, `Component-KpiHistory.md`, `Component-Commons.md`

**Method:** read the round-1 report and `archreview/plans/PLAN-04-data-audit-backbone.md` in full; verified every round-1 finding's true disposition in current source (file:line, not trusting the plan's claims); fresh static sweep of `git diff b910f5eb..HEAD` scoped to the five projects + new migrations, with the rollup subsystem reviewed line-by-line. No builds/tests run (suites verified green 2026-07-10).

**Round-1 vs round-2 verdict:** every round-1 time bomb was genuinely defused — all 12 Fixed dispositions verified in code, deferrals are honestly documented in the design docs rather than silently dropped — and the fix quality is uniformly high (the NodeB-failover pull clients and the keyset cursor are exemplary); the one new material risk is concentrated in the brand-new rollup **backfill** path, which re-introduces at startup exactly the "unbounded single-pass over a year-scale table" shape the plan spent five tasks eliminating elsewhere.

---

## Round-1 Finding Disposition

Every disposition below was verified against current source, not the plan's coverage table.

| # | Round-1 finding | R1 sev | Disposition | Evidence (file:line) |
|---|-----------------|--------|-------------|----------------------|
| S1/U2 | Site SQLite 7-day retention purge missing | High | **Fixed (verified)** | `SqliteAuditWriter.PurgeExpiredAsync` — temp-table purge honoring the ForwardState invariant (`AuditLog/Site/SqliteAuditWriter.cs:858-966`), post-purge `incremental_vacuum` (:949-961); `SiteAuditRetentionService` hosted loop (`Site/SiteAuditRetentionService.cs:62-117`); options bound (`AuditLog/ServiceCollectionExtensions.cs:107`) and service registered (:335); clamped options (`Site/SiteAuditRetentionOptions.cs`); tests `SqliteAuditWriterRetentionTests.cs`, `SiteAuditRetentionServiceTests.cs`, `SiteAuditRetentionOptionsTests.cs` |
| S2 | `SwitchOutPartitionAsync` no CommandTimeout; purge failure log-only | High | **Fixed (verified)** | `commandTimeout` parameter + `SetCommandTimeout` with finally-restore (`ConfigurationDatabase/Repositories/AuditLogRepository.cs:230, 368, 389-402`), same on per-channel `DELETE TOP` (:472); actor passes `ResolvedMaintenanceCommandTimeout` (default 30 min, 1-min floor — `AuditLogPurgeOptions.cs:73-81`) at `AuditLogPurgeActor.cs:200, 295`; failure now publishes `AuditLogPurgeFailedEvent` + increments the `IAuditPurgeFailureCounter` health counter (`AuditLogPurgeActor.cs:235-247`, `Central/AuditLogPurgeFailedEvent.cs`, `Central/IAuditPurgeFailureCounter.cs`) |
| S3 | Append-only role DDL-only; purger grants insufficient | High | **Fixed (verified)** | Migration `20260709105112_FixAuditPurgerRoleGrants.cs:25-33` grants `CREATE TABLE` + scoped `DELETE ON dbo.AuditLog` (idempotent, reversible Down); docs now state honestly that default deployments enforce append-only via the CI grep guard, with the roles as optional DBA hardening (`Component-AuditLog.md:489-495`; `Component-ConfigurationDatabase.md:342` retracts the stale "no row DELETE even for purge" claim) |
| S4 | SiteCalls upsert freezes RetryCount/LastError at rank | Medium | **Fixed (verified)** | Same-rank freshness tiebreaker on `UpdatedAtUtc`, with terminal rows deliberately immutable so a later terminal never overwrites an earlier one (`SiteCallAuditRepository.cs:106-165`) |
| S5 | No EF `EnableRetryOnFailure` | Medium | **Fixed (verified)** | `EnableRetryOnFailure(5, 30s)` fleet-wide (`ConfigurationDatabase/ServiceCollectionExtensions.cs:33-36`); the combined-telemetry dual-write transaction is wrapped in `CreateExecutionStrategy().ExecuteAsync` per entry (`AuditLogIngestActor.cs:263-298`) |
| S6 | Operator Retry emits no audit row; relay lacks operator identity | Medium | **Fixed (verified)** | `RetryAsync` emits a `Submitted` NotifyDeliver row with `RequestedBy` as actor (`NotificationOutboxActor.cs:1078-1126`); Discard passes it too (:1183); `SiteCallAuditActor` takes an optional `ICentralAuditWriter` and emits relay-identity rows (`SiteCallAuditActor.cs:104, 199, 288-291`) |
| S7 | Reconciliation dead-ends (cursor pin; permanent abandonment) | Medium | **Fixed (verified)** | Composite `(UpdatedAtUtc, TrackedOperationId)` keyset cursor per site (`SiteCallAuditActor.cs:135-159, 635-713`); legacy-site pin now publishes `SiteCallReconciliationPinnedChanged` on transitions (:737-746, `SiteCallReconciliationPinnedChanged.cs`); site store honors strict keyset with legacy `>=` fallback (`SiteRuntime/Tracking/OperationTrackingStore.cs:365-395`; proto `sitestream.proto:187-192`; server maps empty→null `SiteStreamGrpcServer.cs:607-613`); abandonment writes a durable synthetic `AuditKind.ReconciliationAbandoned` row (`SiteAuditReconciliationActor.cs:333-385`, `AuditKind.cs:43`) |
| S8 | Outbox repository dual SQLite/T-SQL dialect | Medium | **Deferred (accepted, documented)** | Convention recorded: new repositories are MSSQL-only with MSSQL fixtures; `NotificationOutboxRepository` is the named legacy exception, "do not copy" (`Component-ConfigurationDatabase.md:69`) |
| S9 | Purge timers' first tick waits a full interval | Low | **Fixed (verified)** | Shared `PurgeTimerSchedule.InitialDelay` (`Commons/Types/PurgeTimerSchedule.cs`) used by all three: `AuditLogPurgeActor.cs:100`, `SiteCallAuditActor.cs:422`, `KpiHistoryRecorderActor.cs:191` |
| S10 | `SqliteAuditWriter.Dispose` sync-over-async | Low | **Won't-fix (accepted)** | Per plan; round-1 itself judged the thread-pool-hop mitigation correct and documented |
| P1 | SiteCalls KPI predicates full-scan (no `TerminalAtUtc` index) | High | **Fixed (verified)** | Filtered `IX_SiteCalls_NonTerminal` on `CreatedAtUtc WHERE [TerminalAtUtc] IS NULL` with `SourceSite`/`SourceNode`/`Status` includes (`SiteCallEntityTypeConfiguration.cs:90-99`; migration `20260709110614_AddSiteCallsNonTerminalIndex.cs`) |
| P2 | `GetExecutionTreeAsync` scans entire AuditLog | High | **Fixed (verified)** | Edges anchor bounded to `[rootFirst − 1h, rootFirst + 7d)` (`AuditLogRepository.cs:682-683, 756, 846-852`); row-less-root degenerate case deliberately keeps the unbounded form |
| P3 | AuditLog clustered key leads with random GUID | Medium | **Deferred (accepted, tracked)** | Recorded as a tracked benchmark follow-up with the partition-scheme rationale (`Component-ConfigurationDatabase.md:70`) |
| P4 | KpiSample volume + unbatched purge | Medium | **Fixed (purge) / Deferred (cadence, tracked)** | Time-sliced (≤1h/DELETE) batched purge (`KpiHistoryRepository.cs:65-84`), backed by the standalone `IX_KpiSample_Captured` index (`KpiSampleEntityTypeConfiguration.cs:54-55`); per-node sampling cadence recorded as a tracked follow-up (`Component-ConfigurationDatabase.md:71`) |
| P5 | Catch-all `(@p IS NULL OR col=@p)` predicates | Medium | **Fixed (verified)** | `OPTION (RECOMPILE)` on the SiteCalls query page (`SiteCallAuditRepository.cs:236`) |
| P6 | Outbox KPI 5-7 round trips; unbounded oldest materialization; offset paging | Medium | **Fixed (KPIs) / Deferred (paging, tracked)** | Global/per-site/per-node snapshots are each one grouped conditional-aggregation query with server-side `MIN`-over-CASE — no entity materialization (`NotificationOutboxRepository.cs:244-270, 286-315, 330-374`); offset→keyset page conversion deferred per plan, recorded in `Component-ConfigurationDatabase.md:71` |
| P7 | Non-persisted `IngestedAtUtc` computed column | Low | **Fixed (guard)** | "NEVER use in a WHERE predicate" comment guard (`AuditLogEntityTypeConfiguration.cs:159-164`) |
| P8 | `MarkForwarded/ReconciledAsync` near SQLite param limit | Low | **Fixed (verified)** | ≤500-id chunks inside one transaction, both methods (`SqliteAuditWriter.cs:61-63, 657-673, 759-775`) |
| C1 | Design docs drifted (enums, Teams, per-channel, SiteCalls schema) | Medium | **Fixed (verified)** | `AuditChannel` 5 values / `AuditKind` 15 incl. `ReconciliationAbandoned` (`Component-Commons.md:46-47`); `NotificationType` Email+Sms with Teams-dropped rationale (:90); per-channel retention correctly described (`Component-ConfigurationDatabase.md:110, 408`); SiteCalls schema rewritten to shipped Channel/AuditStatus shape with explicit "no Kind/TargetSummary/provenance columns" (`Component-SiteCallAudit.md:41-72`); no load-bearing "Teams" references remain |
| C2 | `AuditLogRow` lives in ConfigurationDatabase, not Commons | Medium | **Deferred (accepted, documented)** | Deviation documented as intentional persistence-only projection of the external `ZB.MOM.WW.Audit.AuditEvent` contract, "do not lift it into Commons" (`Component-Commons.md:51`) |
| C3 | Mixed timestamp CLR types | Low | **Deferred (accepted, documented)** | Convention recorded: new tables use UTC `DateTime` + converter; `Notification`'s `DateTimeOffset` is the documented legacy exception (`Component-ConfigurationDatabase.md:68`) |
| C4 | Uncharted KPI metric names as string literals | Low | **Fixed (verified)** | `KpiMetrics.NotificationOutbox.StuckCount`/`OldestPendingAgeSeconds` constants consumed by the source (`NotificationOutboxKpiSampleSource.cs:39-43`) — but see new finding R4: the pattern partially recurs inside `KpiMetricAggregationCatalog` |
| U1 | `backlogTotal` KPI history hardwired to zero | High (underdev.) | **Fixed (verified)** | `IAuditBacklogProvider` seam consumed with snapshot fallback (`AuditLogKpiSampleSource.cs:47, 66, 95`); `CentralHealthAuditBacklogProvider` registered centrally (`HealthMonitoring/ServiceCollectionExtensions.cs:72`); test `AuditLogKpiSampleSourceTests.cs` |
| U3 | Hash-chain / Parquet archival | — | **Deferred (accepted)** | Unchanged v1.x deferral; no drift |
| U4 | SecuredWrite rows leave `SourceNode` NULL | — | **Fixed (elsewhere, verified seam)** | `CentralAuditWriter` stamps `SourceNode` via optional `INodeIdentityProvider` (`Central/CentralAuditWriter.cs:47, 80`) — landed under plan 07 as predicted; doc under-promise on per-channel keys fixed in C1 pass |
| U5 | Reconciliation keyset upgrade untracked | — | **Fixed (SiteCalls) / Deferred (AuditLog pull, tracked)** | SiteCalls keyset shipped (see S7); `PullAuditEvents` remains inclusive-`>=` (idempotent on `EventId`) with the deferral recorded (`Component-ConfigurationDatabase.md:71`) |
| U6 | Dispatcher throughput ceiling (sequential delivery) | — | **Fixed (verified)** | `SemaphoreSlim`-gated `Task.WhenAll` with per-notification DI scope, `MaxParallelDeliveries` default 4 clamped ≥1 (`NotificationOutboxActor.cs:288-408`; `NotificationOutboxOptions.cs:62-69`); fault isolation per delivery so `WhenAll` never throws |
| U7 | Test gaps mirror findings | — | **Fixed (verified)** | Retention tests exist (S1 row); `PartitionPurgeTests` extended for the timeout path per plan; backlog-provider test exists |
| X1 | Reconciliation dials site NodeA only (owned from report 08) | — | **Fixed (verified)** | `SiteEntry.FallbackGrpcEndpoint` populated from NodeB (`Central/SiteEnumerator.cs:34-41`); both pull clients retry once against the fallback on transport faults only, never on mapping faults, skipping when cancelled (`GrpcPullSiteCallsClient.cs:110-127, 172-211`; `GrpcPullAuditEventsClient.cs:194`) |

**Disposition counts:** 19 Fixed (verified), 2 Fixed-with-documented-deferral remainder (P4, P6, U5 counted once each as Fixed), 6 Deferred (accepted, all now documented/tracked: S8, P3, C2, C3, U3), 1 Won't-fix (accepted: S10), 0 Not fixed, 0 Regressed. No plan claim was found to be false.

---

## New Findings — KPI Rollup Subsystem (post-initiative, 2026-07-10)

The rollup design is sound where it was thought through — the gauge-vs-rate catalog with its safe-Gauge default and the corrected `*LastHour` classification (`KpiMetricAggregation.cs:85-96`) is genuinely careful work, the `HasFilter(null)` suppression of EF's default filtered unique index (`KpiRollupHourlyEntityTypeConfiguration.cs:57-63`) dodges a real null-ScopeKey uniqueness trap, and the exclusive-upper-bound / in-progress-hour exclusion is correct. The problems are concentrated where the periodic-fold assumptions meet the backfill path.

### [High] R1 — The startup backfill folds the entire raw-retention window in one unbounded pass, on every failover

`FoldHourlyRollupsAsync` loads its whole window with a single **tracked** `ToListAsync` and then issues one `FirstOrDefaultAsync` per (series, hour) group plus one giant `SaveChanges` (`ConfigurationDatabase/Repositories/KpiHistoryRepository.cs:104-106, 133-139, 167`). Its own comment says "The caller passes a small trailing lookback (e.g. 3 h), so the window is bounded" — but its second caller is the one-shot backfill, which passes `[TruncateToHour(now) − RetentionDays, TruncateToHour(now))` — **90 days by default** (`KpiHistory/KpiHistoryRecorderActor.cs:510-511`).

At round-1's own fleet estimate (~500 samples/min ≈ 700k rows/day for 10 sites), the backfill materializes tens of millions of tracked `KpiSample` entities into one `List<>` — a multi-GB allocation on the active central node that no `catch` can contain if it OOMs — then runs ~(series-count × 2,160 hours) single-row upsert probes and commits everything in one `SaveChanges`. Even a modest single-site deployment (≈26 samples/min) materializes ~3.4M tracked rows. And because `_backfillDone` is per actor lifetime (`KpiHistoryRecorderActor.cs:129-136`), **every singleton failover/restart re-runs the full-window fetch** — the pre-existing rollup rows shrink the writes but not the raw read. While it grinds, `_backfillInFlight` also blocks every periodic fold (`:435-441`), so fresh hours stall behind the historical pass. `Component-KpiHistory.md:93` calls the backfill "cheap and safe to re-run" — the "safe" is true (idempotent), the "cheap" is not.

**Fix:** slice the backfill in the actor into bounded windows (e.g. one day per `FoldHourlyRollupsAsync` call, oldest-first, yielding between slices) — the repository method then never sees more than 24 hours; this also releases the fold path between slices. Alternatively (or additionally) skip the backfill when the newest `KpiRollupHourly.HourStartUtc` is already near `now` (the failover case, which is the common one). This is the same "bound the maintenance pass" discipline Tasks 4/19 applied to every other year-scale sweep in this domain.

### [Medium] R2 — Fold fetch is change-tracked (`AsNoTracking` missing)

The sample fetch in `FoldHourlyRollupsAsync` (`KpiHistoryRepository.cs:104-106`) reads `KpiSample` rows it will never modify, without `AsNoTracking()`. Even on the healthy periodic path (3-hour lookback), that is ~5k-90k read-only entities registered in the change tracker every hour, all re-scanned by `DetectChanges` on the final `SaveChanges` alongside the rollup entities that actually changed. One-word fix; it also cuts the R1 backfill's memory overhead roughly in half (tracking snapshots) even before R1 is properly sliced.

### [Medium] R3 — Rate metrics change units ~60× across the raw/rollup routing boundary, and the bucketer re-introduces the fold error the catalog warns about

`KpiHistoryQueryService.FetchSeriesAsync` routes windows ≤168h to raw per-minute samples and wider windows to hourly rollups (`CentralUI/Services/KpiHistoryQueryService.cs:107-116`). For Gauge metrics both paths agree (last-value either way). For **Rate** metrics they do not: the raw path plots per-minute deltas, the rollup path plots **sum-per-hour** values — so the same `deliveredLastInterval` chart jumps by up to ~60× in magnitude when the operator widens the range from 7d to 30d, with no normalization in the bucketer or `KpiTrendChart`. Compounding it, `KpiSeriesBucketer.Bucket` takes **last-value-per-bucket** on top of the rollup series (`Component-KpiHistory.md:151`): a 90d/200-point chart keeps 1 of ~11 hourly sums per bucket and discards the rest — precisely the "keep one delta, discard the others" mistake `KpiMetricAggregationCatalog`'s doc warns silently corrupts rate trends (`KpiMetricAggregation.cs:26-28`), re-introduced one layer up. **Fix:** teach the bucketer (or the query service) a per-metric reduction — sum-per-bucket for Rate series — and normalize both paths to a common unit (e.g. per-hour rate) so the boundary is visually seamless; the catalog needed to drive this already exists in Commons.

### [Low] R4 — `KpiMetricAggregationCatalog` re-creates the private-metric-literal drift hazard C4 just fixed

The catalog hardcodes `"deliveredLastInterval"` (SiteCallAudit), `"alarmEvalErrors"`, and `"eventLogWriteFailures"` as its own private literals because the emitting sources keep those names private (`Commons/Types/Kpi/KpiMetricAggregation.cs:60-64`). If an emitter's literal ever drifts, the pair silently resolves to Gauge and the hourly sum quietly becomes a last-value — no error, just an under-counted long-range trend. The Gauge default makes this degradation safe-ish, but silent. A lock-in test asserting every `RatePairs` entry matches a metric actually emitted by the corresponding `IKpiSampleSource` (or promoting the three literals to `KpiMetrics`) closes it.

### [Low] R5 — Overlapping-singleton fold race faults an entire fold batch

All groups in a fold commit through one `SaveChanges` (`KpiHistoryRepository.cs:167`). During a failover overlap window (old incarnation's in-flight fold vs. the new node's first fold), both may `Add` the same `(series, hour)` row; the unique `IX_KpiRollupHourly_Series` then faults the loser's **whole** `SaveChanges`, discarding every group in that pass, not just the contested row. Self-heals on the next tick (idempotent re-fold), and the in-actor guards prevent same-node races, so this is noise rather than loss — but worth knowing the failure grain is the pass, not the row, when reading fold-failure logs after a failover.

---

## New Findings — Other post-baseline code

### [Medium] R6 — `PurgeTerminalAsync` on SiteCalls is the one remaining unbatched, unindexed year-scale maintenance DELETE

`SiteCallAuditRepository.PurgeTerminalAsync` is still a single unbatched `DELETE FROM dbo.SiteCalls WHERE TerminalAtUtc IS NOT NULL AND TerminalAtUtc < cutoff` (`SiteCallAuditRepository.cs:247-252`). Nothing indexes non-null `TerminalAtUtc` — the new `IX_SiteCalls_NonTerminal` is filtered to `IS NULL` and unusable here — so the daily purge full-scans the 365-day table, and its steady-state day-of-terminal-rows (plus any catch-up after an outage) deletes in one transaction under the default command timeout. This is exactly the shape Task 19 batched for `KpiSample` and Task 4 time-boxed for `AuditLog`; SiteCalls was left out. Volume is lower than either sibling, so Medium not High — but it inherits both prior failure modes (lock/log spike; a 30s-timeout loop after an outage backlog). Fix: `TOP (n)` loop like `PurgeChannelOlderThanAsync` plus either an index on `TerminalAtUtc` or a `(Status, UpdatedAtUtc)`-driven predicate.

### [Low] R7 — `SiteAuditRetentionService.StopAsync` surfaces the loop's cancellation to the host

`SafePurgeAsync` deliberately rethrows `OperationCanceledException` (`Site/SiteAuditRetentionService.cs:106-110`), but the two `await SafePurgeAsync(ct)` call sites in `RunLoopAsync` (:76, :89) are outside any try/catch, so a shutdown that lands mid-purge cancels the `_loop` task — and `StopAsync` returns that canceled task directly (:122-126), making the host's `StopAsync` await throw `TaskCanceledException`. The generic host logs and continues, so this is shutdown-log noise, not a hang — but the sibling `SiteAuditBacklogReporter` pattern it mirrors is worth aligning: catch OCE at the loop level (or `Task.WhenAny`-guard in `StopAsync`).

---

## What's genuinely good

- **The fix execution matches the fix claims.** All 19 verified-Fixed items are real, tested, and in several cases exceed the plan: the purge-failure path got a dedicated no-op-defaulted counter seam (`IAuditPurgeFailureCounter` + `NoOpAuditPurgeFailureCounter`), and the S1 purge encodes the ForwardState invariant *in the SQL itself* with a param-limit-proof temp table (`SqliteAuditWriter.cs:877-897`).
- **The NodeB-failover pull clients are exemplary defensive code**: transport-fault-only failover (a mapping fault correctly does not retry on the other node, "since a second node would hit the same non-transport fault"), per-row DTO fault isolation, race-safe channel cache with loser-dispose, and the `DateTime.MinValue`/`ToUniversalTime` underflow trap explicitly dodged and documented (`GrpcPullSiteCallsClient.cs:110-127, 236-242, 309-325`).
- **The keyset cursor landed end-to-end with honest legacy behavior**: proto additive field, empty-string-as-null mapping at the server, strict-keyset vs. legacy-inclusive branches in the site store, and a latched pinned-state event that fires only on transitions (`OperationTrackingStore.cs:384-395`, `SiteCallAuditActor.cs:737-746`).
- **Deferral hygiene**: every accepted deferral (S8, P3, P4-cadence, P6-paging, C2, C3, U5) now has a written home in a design doc — the round-1 complaint that dead-ends were "deferred without a tracking artifact" is fully answered.
- **The rollup catalog's semantics section** (`KpiMetricAggregation.cs:85-96`) is a model of the kind of reasoning that prevents silent data corruption: it overrides the plan's tentative Rate classification for the `*LastHour` metrics after checking the actual source semantics, and documents why in place.
- **Execution-strategy adoption was done deliberately**, exactly as round-1 cautioned: the dual-write's user-initiated transaction is wrapped per entry with the idempotency argument stated (`AuditLogIngestActor.cs:263-298`), rather than a blind `EnableRetryOnFailure` toggle.

## Underdeveloped areas

1. **Backfill scale testing** — no test exercises `FoldHourlyRollupsAsync` at a window larger than a few hours; the R1 failure mode (90-day materialization) is invisible to the current suites, mirroring how the round-1 partition-switch timeout was invisible until sized. A `SkippableFact` MSSQL test at representative volume, or a unit test asserting the actor slices the backfill, would lock in the R1 fix.
2. **Rate-metric presentation across the routing boundary** (R3) — the storage layer has full fidelity (`MinValue`/`MaxValue`/`SampleCount` are stored but uncharted); the presentation semantics are the unfinished half of the rollup feature.
3. **AuditLog pull keyset** (U5 residue) — tracked, additive, and lower-urgency; fine as-is, noting it here so the tracker's entry doesn't age out.

## New-Findings Severity Tally (round 2 only)

| Severity | Count | Findings |
|----------|-------|----------|
| Critical | 0 | — |
| High | 1 | R1 (unbounded rollup backfill, re-run per failover) |
| Medium | 3 | R2 (tracked fold fetch), R3 (rate-unit discontinuity + bucketer re-fold error), R6 (SiteCalls terminal purge unbatched/unindexed) |
| Low | 3 | R4 (catalog literal drift), R5 (fold-batch failover race grain), R7 (retention-service shutdown OCE) |

**Round-2 bottom line:** the round-1 debt is paid — verified, not just claimed. The new debt is small and localized: bound the rollup backfill (R1) and add `AsNoTracking` (R2) before any production deployment accumulates months of `KpiSample` rows, decide the rate-metric presentation (R3) before operators learn to distrust the 30d charts, and give `PurgeTerminalAsync` the same batching its two siblings received (R6).