code-reviews: re-review Contracts at 42b0037

No new findings — the only Contracts commit in window (bd1d1f1) is a
comment-only proto edit; field numbering remains additive with no
reuse, renumbering, or type narrowing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-reviews/Contracts/findings.md

@@ -5,8 +5,8 @@
 | Module | `src/ZB.MOM.WW.MxGateway.Contracts` |
 | Reviewer | Claude Code |
 | Review date | 2026-05-24 |
-| Commit reviewed | `d692232` |
-| Status | Reviewed |
+| Commit reviewed | `42b0037` |
+| Status | Re-reviewed |
 | Open findings | 0 |
 
 ## Checklist coverage
@@ -306,3 +306,38 @@ Python and Go descriptors. No fields renumbered or repurposed.
 **Recommendation:** Extend the RPC comment with one line: "`QueryActiveAlarmsRequest.alarm_filter_prefix` optionally narrows the snapshot to alarms whose `alarm_full_reference` starts with the given prefix; an empty prefix returns the full set." Comment-only.
 
 **Resolution:** _(2026-05-24)_ Confirmed against `mxaccess_gateway.proto` at `d2d2e5f`: lines 29-34 of the `rpc QueryActiveAlarms` block describe the snapshot order and session-less origin but never mention `alarm_filter_prefix`, which is only documented on the request message itself two lines below — a client author reading the RPC comment alone cannot discover the filter capability. Extended the RPC comment with the finding's recommended one-line addition verbatim: "`QueryActiveAlarmsRequest.alarm_filter_prefix` optionally narrows the snapshot to alarms whose `alarm_full_reference` starts with the given prefix; an empty prefix returns the full set." Comment-only change; the RPC signature, request/reply types, and field numbers are unchanged. The same regression test added under Contracts-016 (`ProtobufContractRoundTripTests.QueryActiveAlarmsRequest_PinsFieldNumbersAndRoundTripsPrefixFilter`) also exercises a round-trip of `QueryActiveAlarmsRequest` with `alarm_filter_prefix` populated and pins `QueryActiveAlarms` on the public `MxAccessGateway` service surface, so the RPC + filter field combination the new comment documents is wire-stable. `dotnet build src/ZB.MOM.WW.MxGateway.slnx` is green (0 warnings, 0 errors).
+
+#### 2026-05-24 re-review (commit 42b0037)
+
+Re-review pass at `42b0037` scoped to the contract changes since `d692232`.
+The window contains exactly one Contracts commit (`bd1d1f1`, "Resolve
+Contracts-016..017"), which is a comment-only edit to
+`mxaccess_gateway.proto`: the `rpc QueryActiveAlarms` block gains the
+`alarm_filter_prefix` description sentence (Contracts-017 resolution) and
+the `QueryActiveAlarmsRequest` header comment replaces the misleading
+"reserved" wording with the unambiguous "Clients may leave `session_id`
+empty; the gateway currently ignores it…" phrasing (Contracts-016
+resolution). The two `Generated/*.cs` edits in the same window are pure
+regeneration of those comments into XML doc — no wire-format,
+field-number, or type changes. The `StreamAlarms` / `AcknowledgeAlarm`
+alarm surface mentioned in the re-review brief was last touched at
+`397d3c5` (already covered in the `d692232` pass) and is unchanged in this
+window; field numbering across all three protos remains
+additive-only with no reuse, renumbering, or type narrowing.
+
+| # | Category | Result |
+|---|---|---|
+| 1 | Correctness & logic bugs | No issues found. The two comment changes are factually correct against the source (`MxAccessGatewayService.QueryActiveAlarms` does ignore `session_id` and does apply the `alarm_filter_prefix` `StartsWith` match). |
+| 2 | mxaccessgw conventions | No issues found. The wire-compatibility policy comment block at the top of `mxaccess_gateway.proto` (Contracts-005 resolution) remains intact; the edits are additive comment-only changes consistent with the additive-only contract evolution rule. Generated code under `Generated/` regenerated cleanly from the source comments, not hand-edited. |
+| 3 | Concurrency & thread safety | N/A — pure contract definitions and a static constants class. |
+| 4 | Error handling & resilience | No issues found. |
+| 5 | Security | No issues found — no new credential-bearing fields in the window. |
+| 6 | Performance & resource management | No issues found. |
+| 7 | Design-document adherence | No drift. `docs/AlarmClientDiscovery.md` already documents the `alarm_filter_prefix` filter and the session-less central-monitor cache; the new RPC-comment line restates what the doc already says, in line with the CLAUDE.md "Update docs in the same change as the source" rule (docs were already current at `d692232`, source caught up at `bd1d1f1`). |
+| 8 | Code organization & conventions | No issues found. Field numbers and type names unchanged; additive-only invariant preserved. |
+| 9 | Testing coverage | No issues found. `ProtobufContractRoundTripTests.QueryActiveAlarmsRequest_PinsFieldNumbersAndRoundTripsPrefixFilter` (added under Contracts-016) pins the three `QueryActiveAlarmsRequest` field numbers and round-trips an `alarm_filter_prefix`-bearing payload; comment-only proto changes do not need additional test coverage. |
+| 10 | Documentation & comments | No issues found. The two edits are themselves documentation fixes that close prior Contracts-016 and Contracts-017 findings. |
+
+Re-review: no new findings. Open finding count remains 0. All seventeen
+recorded Contracts findings (Contracts-001..017) remain closed
+(Resolved / Won't Fix).