docs: arch-review remediation — component docs sweep, execution log, residuals register
Final consistency sweep per plan §6: verified component docs against shipped WP1-WP3 + adversarial-review-fix state, corrected drift found in SiteRuntime (recursion-exempt run cap, stale ScriptExecutionActor/AlarmExecutionActor references), TemplateEngine (BundleImporter watermark path), DeploymentManager (phase-2 PendingDeployment staging), CentralUI (shared KPI cache, dedup'd alarm poll, render coalescing), StoreAndForward (rate-limited drop logging), and ConfigurationDatabase (documented DbContext-pooling non-adoption). Updated the docs/components/ developer-reference set (SiteRuntime, SiteEventLogging, InboundAPI) to drop the deleted per-run actor classes. Amended one known-issue for the superseding MaxBatchSize:64 read-page pin. Added CLAUDE.md bullets for stream graceful-completion reconnect, the required site audit DB path, honest CLI HTTP timeouts, bulk DeploySiteAsync, and LocalDb 0.2.1. New execution log records the phase→commit map, gate results, adversarial-review tally, the three test-flake root causes, and the nine-item residuals register.
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,135 @@
|
||||
# Arch-review remediation — execution log
|
||||
|
||||
**Ran:** 2026-08-14/15 · **Plan:** `docs/plans/2026-08-14-arch-review-remediation-plan.md` ·
|
||||
**Branch:** `arch-review-remediation` (off `main`) · **Status:** code-complete, docs-propagated,
|
||||
not yet pushed/merged.
|
||||
|
||||
Source: the 2026-08-14 eight-pass performance/architecture review (all 27 components +
|
||||
`ZB.MOM.WW.LocalDb`). This log is the compact record of what shipped, how it was gated, and
|
||||
what's left. Full package specs live in the plan; this is the after-action summary.
|
||||
|
||||
## Phases and package → commit map
|
||||
|
||||
Every package ran as an isolated-worktree subagent, merged `--no-ff` onto
|
||||
`arch-review-remediation`; commit hashes below are the substantive commit on the branch (merge
|
||||
commits omitted).
|
||||
|
||||
**Phase 0 — Preflight.** `0b201e41` docs(plans): the remediation plan itself.
|
||||
|
||||
**Phase 1 — Quick wins (7 packages, parallel).**
|
||||
| WP | Commit | What |
|
||||
|---|---|---|
|
||||
| 1.1 | `34a3f4bb` | Reconnect on graceful (OK-status) stream completion — closes the 4h silent stream death |
|
||||
| 1.2 | `2e4e41a8` | Site audit DB onto the mounted data volume; required path + soft flush |
|
||||
| 1.3 | `7ebdcd37` | CDC capture installed only when replication is configured |
|
||||
| 1.4 | `600659d5` | Sweep/KPI covering indexes + sliced notification terminal purge |
|
||||
| 1.5 | `125055d9` | O(1) attribute resolution, precomputed types, coalesced static writes, shared JSON options |
|
||||
| 1.6 | `c5e66ed4` | Fail known-dead sends immediately instead of burning Ask timeouts |
|
||||
| 1.7 | `2cfcd890` | Batched event-log commits + sliced retention purge |
|
||||
|
||||
**Phase 2 — Seam rework (6 packages, parallel after the WP2.1a design doc `1040dc0f`).**
|
||||
| WP | Commit | What |
|
||||
|---|---|---|
|
||||
| 2.1 | `d15c5f02` | DCL batch subscribe/read/write seam, bounded reconnect, sharded subscriptions |
|
||||
| 2.2 | `5db2a810` | Central set-based ingest, aligned partition purge, KPI query shapes, EF hygiene |
|
||||
| 2.3 | `2ce0ad7e` | Alarms-only stream seed, capped buffers, at-least-once audit pull |
|
||||
| 2.4 | `8c0b36b2` | Shared KPI cache, live-cache-backed alarm summary, coalesced Debug View renders |
|
||||
| 2.5 | `48b3c40a` | Flatten-session caching, bulk `DeploySiteAsync`, paged management queries |
|
||||
| 2.6 | `a2122831` | Cached hot-path lookups, bounded observer queue, alarm-priority stream path |
|
||||
|
||||
Plus `a5882753` closing Phase-2-gate residuals (direct ingest path, monotonic timeouts, synthetic
|
||||
probe, not-reporting set, cursor-exact audit pull) found while gating.
|
||||
|
||||
**Phase 3 — Structural (design-first).**
|
||||
| WP | Design | Commit | What |
|
||||
|---|---|---|---|
|
||||
| 3.1 | `312216ff` | `c4fc1f8e` | Script execution pool split — trigger evals off the blocking pool, bounded/deadline-aware execution |
|
||||
| 3.2 | `6cfb2dd8` | `c254d074` | `site_events` volume policy — sampled per-run events, interval summaries, replication pinned |
|
||||
| 3.3 | (scadaproj) | `cca7f178` + scadaproj `9377fa1` | LocalDb 0.2.0 — dereg cleanup, late-opt-in baselining, byte-budget replication |
|
||||
|
||||
**Phase 4 — Verification, adversarial review, docs.** Six parallel `code-reviewer` passes over
|
||||
the full diff, one per area (site runtime, DCL, comms, site persistence, central SQL, UI/deploy),
|
||||
each instructed to try to refute the fixes. Confirmed findings landed as targeted follow-ups:
|
||||
|
||||
| Area | Commit | What |
|
||||
|---|---|---|
|
||||
| Site runtime | `950c54c5` | Recursion-exempt run cap (nested `CallScript` no longer double-gated), atomic detach counter, summary edge cases, per-row event-log fallback |
|
||||
| DCL | `37f13e2e` | Discard in-flight subscribe results for tags unsubscribed mid-flight; release the orphaned handle |
|
||||
| Central SQL | `5d075f13` | No client-side audit truncation, insert-first upsert (Site Call Audit), QI-safe filtered-index scripts, honest operator-not-found replies |
|
||||
| Comms | `fd5e023d` | Consumer-based debug-stream orphan net, foreign-cancel triad, honest `onConnected`, served-row-exact retirement, full-rate reconcile |
|
||||
| UI/deploy | `e0e4b246` | Honest CLI HTTP timeouts, watermark-complete staleness (3 missed bump sites), phase-2 `PendingDeployment` staging, lock-safe cancellation |
|
||||
| Site persistence | `56c99c92` + `f689f495` | Required audit DB path on wonder; explicit `MaxBatchSize:64` LocalDb read-page cap; rate-limited observer drop logging; LocalDb 0.2.1 (HLC anchor flush on dereg, bounded 64-message sync inbox) |
|
||||
|
||||
## Gate results
|
||||
|
||||
- **Baseline:** 7587 tests green at Phase 0 entry; test count never reduced across any phase gate.
|
||||
- **Live probes (rig):**
|
||||
- Stream lifetime forced to 2 minutes — the alarm stream reconnected in **8.5s**, within one
|
||||
reconcile tick of the OK completion, `IsLive` correctly reflected the gap (WP1.1).
|
||||
- CDC conditional-registration: site-b booted clean on LocalDb 0.2.0 with **30 stale triggers
|
||||
dropped** at startup (WP1.3/WP3.3) — confirms the self-heal path fires, not just the steady
|
||||
state.
|
||||
- S&F due-sweep: `EXPLAIN QUERY PLAN` confirmed index-terminated (WP1.4).
|
||||
- Failover drill (`docker/failover-drill.sh`) unaffected by the actor/timeout changes.
|
||||
- **Adversarial review tally:** ~25 confirmed findings across the six areas, **4 High**, all
|
||||
fixed in the Phase 4 commits above. Zero findings deferred as won't-fix.
|
||||
- **Test-flake root causes (3, all test-side, not production bugs):**
|
||||
1. `cfa6acbf` — an assertion on `MarkForwarded` ran before the push it depended on was
|
||||
guaranteed to have landed; reordered behind the push.
|
||||
2. `c4caebe9` — two dispatcher audit-safety tests asserted an attempt count without
|
||||
synchronizing on the async write that produced it; the unsynchronized assertion was removed.
|
||||
3. `950c54c5` (embedded) — `ScriptDeadlineAtEnqueueTests`' "no started event" assertion went
|
||||
vacuous once WP3.2 flipped `PerRunScriptEvents` to off-by-default (fixed by opting the test
|
||||
back in); `ScriptRunLauncherParityTests` widened an `ExpectMsg` window that would have passed
|
||||
for any deadline from 1s to 300s, not just the intended one (sharpened to assert the reported
|
||||
timeout value AND a tight wall-clock range).
|
||||
- **Follow-up recommended, not done here:** a suite-wide sweep for the same
|
||||
`AwaitAssert(...)`-then-bare-`Assert` pattern — an `AwaitAssert` that only proves "eventually
|
||||
true," followed by a plain assertion that silently inherits its timing slack, is the shape
|
||||
behind all three; worth a grep-and-review pass rather than fixing on-demand as flakes surface.
|
||||
|
||||
## Residuals register
|
||||
|
||||
Deliberately not fixed in this program — each has a stated reason, not an oversight:
|
||||
|
||||
1. **DCL unsubscribe-during-reconnect count staleness.** The `37f13e2e` fix discards orphaned
|
||||
in-flight results but a per-connection counter can still drift under rapid
|
||||
subscribe/unsubscribe churn during a reconnect; needs a per-tag counted set. Low severity,
|
||||
cosmetic (a health-report number), deferred.
|
||||
2. **Per-table `needs_snapshot` in LocalDb.** Baselining one table currently re-streams every
|
||||
registered table in both directions. Narrowing it needs an on-disk schema change LocalDb 0.2.1
|
||||
deliberately avoided (wire/schema compatibility). Documented as a follow-up in the library's
|
||||
own README and `RegisterReplicated` remarks.
|
||||
3. **Event batching per proto message.** Individual `AttributeValueChanged`/`AlarmStateChanged`
|
||||
events still ride one gRPC message each; batching them is a new wire shape (proto + both
|
||||
client/server), deferred rather than folded into this program's additive-only changes.
|
||||
4. **Deployments page server-side paging + status counts.** Still client-materializes the full
|
||||
list; out of scope for this pass (WP2.5 touched the deploy pipeline, not this specific UI
|
||||
surface).
|
||||
5. **OtOpcUa still pins LocalDb 0.1.3.** A supported skew — 0.1.x peers sync with 0.2.x under the
|
||||
library's wire-compatibility guarantee — not a blocker for this program.
|
||||
6. **Fragile `SandboxTests` timing pin.** Pre-existing, unrelated to this remediation's changes;
|
||||
noted so it isn't mistaken for a regression if it flakes later.
|
||||
7. **Target-scale load test (deferred-work register #25).** This program's exit criterion is the
|
||||
live probes above, not #25 — #25 remains the follow-on validation that the moved ceilings hold
|
||||
under real load; schedule separately.
|
||||
8. **Playwright 14 pre-existing env failures.** Present on `main` too, rig-state related, not
|
||||
introduced by this branch.
|
||||
9. **`site_events` retention purge still oplog-visible.** WP3.2's sliced retention DELETE is a row
|
||||
change like any other and is captured by CDC on a replicated site (site-a) — correct per the
|
||||
"CDC does all three jobs" design (no separate resync path to gate), but means a purge burst is
|
||||
visible in the oplog/backlog metrics; not a correctness issue, just a metrics-reading note for
|
||||
operators watching `LocalDbOplogBacklog` during a purge window.
|
||||
|
||||
## Docs propagated
|
||||
|
||||
Component docs (`DataConnectionLayer`, `SiteRuntime`, `Communication`, `AuditLog`,
|
||||
`SiteEventLogging`, `StoreAndForward`, `NotificationOutbox`, `SiteCallAudit`, `TemplateEngine`,
|
||||
`DeploymentManager`, `CentralUI`, `ConfigurationDatabase`), CLAUDE.md Key Design Decisions
|
||||
(stream-completion reconnect, required site audit DB path, CLI HTTP timeout honesty, bulk
|
||||
`DeploySiteAsync`, LocalDb 0.2.1), and `docs/known-issues/2026-06-26-deploy-config-exceeds-akka-frame-size.md`
|
||||
(amended for the `MaxBatchSize:64` read-page pin superseding the "retired, left at defaults" note).
|
||||
No components added/removed; README component table unchanged. Cross-reference sweep found no
|
||||
stale live references to `ScriptExecutionActor`/`AlarmExecutionActor` or `UX_AuditLog_EventId`
|
||||
outside historical plan/known-issue records, after correcting four residual mentions in
|
||||
`docs/requirements/Component-SiteRuntime.md` left over from the WP3.1 doc pass.
|
||||
Reference in New Issue
Block a user