<!-- task=m-node-provider-execution-liveness-recovery/13+10_recovery_observability plan=5 tag=REVIEW_REFACTOR milestone-task=ops-evidence -->

# Code Review Reference - REVIEW_REFACTOR

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> The task is NOT complete until every implementation-owned section below is filled in.
> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving.
> Fill implementation-owned sections, then stop with active files in place and report ready for review.
> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt.
> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields.
> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state.
> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume.
> Follow the ownership table at the bottom of this file for which sections you own.

## Overview

date=2026-08-06
task=m-node-provider-execution-liveness-recovery/13+10_recovery_observability, plan=5, tag=REVIEW_REFACTOR

## Archive Evidence Snapshot

- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/plan_cloud_G05_4.log` and `agent-task/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/code_review_cloud_G05_4.log`; official verdict `FAIL` with Required R2, no Suggested or Nit findings.
- R1 is closed: `newOpenAIProviderErrorEventFromFailure` again admits only `recovery_handoff=confirmed`, and the explicit unconfirmed handler row stays a single 502 terminal with one submit and zero liveness metric families.
- R2 remains: the gathered-family helper follows production name constants and ignores other families, while the safe-log helper checks only common fields, one eligibility anchor, and the final row instead of the exact row sequence.
- Fresh review verification passed focused count loops, selected package tests, race tests, vet, fake-provider smoke, the two-process reconnect diagnostic, predecessor evidence, formatting, and `git diff --check`; command success does not close the source-level oracle gap.
- Roadmap carryover: preserve approved SDD S06 and `milestone-task=ops-evidence`; this packet contributes only exact bounded Edge eligibility/result metric and raw-free safe-log evidence.

## For the Review Agent

> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section.

Compare implementation of each item against source files and verify that output in `Verification Results` matches code.
Review completion means the following steps are finished:

1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals.
2. Archive `CODE_REVIEW-cloud-G04.md` -> `code_review_cloud_G04_5.log` and `PLAN-cloud-G04.md` -> `plan_cloud_G04_5.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS and task group is `m-<milestone-slug>`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| REVIEW_REFACTOR-1 | [x] |
| REVIEW_REFACTOR-2 | [x] |

## Implementation Checklist

- [x] REVIEW_REFACTOR-1 anchors the two documented liveness metric family names as literal test expectations and rejects every missing, renamed, or extra gathered family, label key, label value, series, and count across positive and negative handler rows.
- [x] REVIEW_REFACTOR-2 compares the exact ordered six-field safe-log context sequence for each outcome, rejecting extra, missing, duplicated, reordered, or incorrectly valued intermediate/final rows while retaining unsafe-key/sentinel and generic-log suppression checks.
- [x] Run every focused, package, race, vet, fake-provider auxiliary smoke, two-process Edge/Node diagnostic, predecessor, formatting, and diff command in Final Verification with fresh output.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent.
> Implementing agents must not modify or check this section.

- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G04_5.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G04_5.log`.
- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
- [x] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [x] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/` and update this checklist at the final archive path.
- [x] If PASS and task group is `m-<milestone-slug>`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
- [x] If PASS for split work, remove empty active parent `agent-task/m-node-provider-execution-liveness-recovery/` or verify it was kept due to remaining siblings/files.
- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

None.

## Key Design Decisions

- `REVIEW_REFACTOR-1`: Defined test-owned literal family-name expectations `wantLivenessEligibilityFamily = "iop_edge_liveness_recovery_eligibility_total"` and `wantLivenessResultsFamily = "iop_edge_liveness_recovery_results_total"`. In `assertGatheredLivenessMetrics`, required exact match with sorted gathered family names for positive rows and 0 families for negative unconfirmed terminal rows.
- `REVIEW_REFACTOR-2`: Created `expectedSafeLogSequence` helper to construct explicit ordered 6-field map sequences for each handler outcome variant (`terminal`, `plan_rejected`, `redispatched`, `dispatch_failed`). Updated `assertSafeLogSchemaAndValues` to require exact slice length and match every row index against expected maps while retaining unsafe-key, string-type, and sentinel value checks.

## Reviewer Checkpoints

- Verify the expected metric family names are literal test-owned strings and the complete gathered family-name set is compared before series inspection.
- Verify positive rows retain exact one-series/one-count and static label-key/value assertions, while the unconfirmed negative row gathers neither family.
- Verify each outcome supplies an explicit ordered expected safe-log sequence with exact row count and exact six-field maps.
- Verify redispatch and dispatch-failure sequences cover selected, aborted, rebuilt, and final rows; plan rejection and ignored-unconfirmed sequences remain distinct.
- Verify every row retains unsafe-key, string-type, sentinel, and generic-log suppression protection.
- Verify no production source, contract, spec, shared StreamGate package, smoke script, or unrelated dirty-worktree file changed.

## Verification Results

Fill each output block with actual stdout/stderr. If a command changes, record the replacement and reason in `Deviations from Plan`.

### Verification 1

Command: `test -f agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/complete.log`

Expected: archived predecessor PASS evidence exists.

Output:

```text
EXISTS
```

### Verification 2

Command: `gofmt -d apps/edge/internal/openai/liveness_recovery_observability_test.go`

Expected: exits 0 with no output.

Output:

```text
(exited 0 with no output)
```

### Verification 3

Command: `go test -count=20 ./apps/edge/internal/openai -run '^TestOpenAILivenessObservationSink$'`

Expected: explicit-sink ownership and synthetic lifecycle regressions pass repeatedly.

Output:

```text
ok  	iop/apps/edge/internal/openai	0.164s
```

### Verification 4

Command: `go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAILivenessRecoveryObservability$'`

Expected: every endpoint/path/outcome row passes with literal metric-family and exact safe-log sequence oracles.

Output:

```text
ok  	iop/apps/edge/internal/openai	0.077s
```

### Verification 5

Command: `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane`

Expected: selected Edge/platform-common packages pass.

Output:

```text
ok  	iop/packages/go/streamgate	0.933s
ok  	iop/apps/edge/internal/openai	7.551s
ok  	iop/apps/edge/internal/service	6.038s
ok  	iop/apps/edge/internal/controlplane	6.657s
```

### Verification 6

Command: `go test -race -count=3 ./packages/go/streamgate ./apps/edge/internal/openai -run 'LivenessObservation|LivenessRecovery|Observation'`

Expected: exits 0 with no race report.

Output:

```text
ok  	iop/packages/go/streamgate	1.240s
ok  	iop/apps/edge/internal/openai	1.292s
```

### Verification 7

Command: `go vet ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane`

Expected: exits 0 with no diagnostics.

Output:

```text
(exited 0 with no output)
```

### Verification 8

Command: `IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh`

Expected: credential-free Chat streaming/non-streaming Edge -> Node -> fake-provider smoke passes.

Output:

```text
[openai-vllm] OpenAI-compatible vLLM serving test PASSED (mode=fake).
```

### Verification 9

Command: `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`

Expected: separate Edge/Node registration, two same-session messages, reconnect, post-reconnect message, payload equality, status commands, and exactly-once terminal ordering pass.

Output:

```text
[diagnostic] PASS: 3 mock-provider runs verified — registration, ordered Node==Edge payloads, one terminal after the last payload, /nodes, /capabilities, /transport, reconnect, and absent session/status ownership commands.
```

### Verification 10

Command: `git diff --check`

Expected: exits 0 with no whitespace errors.

Output:

```text
(exited 0 with no output)
```

---

> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?**
> If anything is blank, go back and fill it in before saving this file.
> Leave review-agent-only sections unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these (archive, complete.log, and task-directory archive move are review-agent only) |
| Archive Evidence Snapshot | Fixed at stub creation from plan when present | Implementing agent uses it as default prior-loop context; read only the specific archive files cited there when more detail is required |
| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` -> `[x]` only |
| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` -> `[x]` only |
| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section |
| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content |
| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan |
| Verification Results (section headings + commands) | Fixed at stub creation | Implementing agent fills in command output only; command changes require a `Deviations from Plan` entry |
| Code Review Result | Review agent appends | Not included in stub |

## Code Review Result

- **Overall Verdict:** PASS
- **Dimension Assessment:**
  - Correctness: Pass — the test-owned literal metric family names and exact ordered safe-log expectations match the production handler lifecycle across every endpoint, execution path, and outcome row.
  - Completeness: Pass — REVIEW_REFACTOR-1 and REVIEW_REFACTOR-2 are both implemented, the implementation evidence is complete, and every planned verification command was rerun successfully.
  - Test coverage: Pass — the matrix rejects missing, renamed, or extra metric families and series, and rejects every missing, duplicated, reordered, extra, or incorrectly valued safe-log row.
  - API contract: Pass — the exact metric families, closed labels, raw-free six-field log projection, confirmed-only recovery boundary, and public handler outcomes conform to the OpenAI-compatible contract.
  - Code quality: Pass — the scoped test change is formatted, deterministic, free of debug output and stale TODOs, and preserves the test-only write boundary.
  - Implementation deviation: Pass — the implementation follows the active plan with no deviations or unrelated source changes in this follow-up.
  - Verification trust: Pass — fresh focused, package, race, vet, fake-provider, two-process reconnect, predecessor, formatting, and diff checks all passed and agree with source inspection.
  - Spec conformance: Pass — the literal label guard and exact bounded structured-log lifecycle provide the Edge observability evidence required by SDD S06 for `milestone-task=ops-evidence`.
- **Findings:** None.
- **Routing Signals:** `review_rework_count=3`, `evidence_integrity_failure=false`
- **Next Step:** Finalize PASS by archiving the active pair, writing `complete.log`, and moving the task artifacts to the dated archive without modifying the roadmap.
