PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The source ticket’s premise is valid and the uptime-capped clock is repairable in place, so this is not a Drop+Supersede case. The current head cannot merge because its pipeline wiring breaks deterministic test isolation and its latch behavior does not implement the ticket’s post-downtime drain-verification AC.

Peer-Review Opening: Thanks for taking the false overnight STALLED alert seriously and for keeping the evaluator pure. The distinction between host-off time and live-process starvation is the right one; the remaining work is to carry that distinction through the stateful pipeline contract.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #15687; the three-file changed-surface list; current dev watchdog/pipeline precedent; ADR-0023’s requirement that consolidation liveness be observed rather than assumed; exact-head CI at e24e664b3648779f05577e2ee45c5771fd92249f.
• Expected Solution Shape: Exclude time when the host could not run the scheduler, while keeping live-process starvation observable. The runtime clock must remain injectable for pipeline tests, and recovery needs explicit cross-cycle state so backlog drain is observed before the alarm/recovery state clears.
• Patch Verdict: Partially matches. effectiveStalenessMs separates offline time from live uptime, but pipeline.mjs:839-849 hardcodes ambient process.uptime(), and remConsolidationLivenessWatchdog.mjs:137-139 clears the latch on the first non-stalled reading without proving backlog decrease.
• Premise Coherence: Cohesive with verify-before-assert in intent, but the current implementation conflicts with it mechanically: the PR claims drain verification and green witnesses that the exact head does not provide.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15687
• Related Graph Nodes: ADR-0023; #13839; REM consolidation liveness; host-down exclusion; backlog-drain recovery

🔬 Depth Floor

Challenge: A restart creates a recovery phase, not an immediately healthy state. Today the first uptime-capped non-stall both clears remConsolidationAlarm and emits the digest-resume note even when backlog has not decreased. That makes the third AC unobservable and lets the INFO note repeat on every check while the recorded cycle remains older than process boot.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the statement that the existing healthy-check latch clear “handles the drain verification” is not substantiated; no prior backlog baseline or decreasing comparison exists.
• Anchor & Echo summaries: terminology is otherwise scoped to the watchdog behavior.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: the close target and related watchdog lineage are real.

Findings: The PR body’s 23/23 and 871/871 green claims are stale/incorrect for the exact head, and AC3 is described as implemented although the diff only performs an unconditional healthy-state clear.

🧠 Graph Ingestion Notes

• [KB_GAP]: Downtime exclusion and recovery completion are separate state transitions; effective age alone cannot witness a draining backlog.
• [TOOLING_GAP]: Reading process.uptime() directly in the pipeline bypasses the runtime-injection boundary and makes existing pipeline fixtures depend on test-worker age.
• [RETROSPECTIVE]: A restart-aware liveness watchdog needs both an injectable monotonic clock and explicit resume/drain state.

🎯 Close-Target Audit

• Close-targets identified: #15687
• #15687 confirmed not epic-labeled (bug, ai).

Findings: Pass.

🪜 Evidence Audit

• PR body contains an Evidence: declaration.
• Achieved evidence meets the close-target ACs: exact-head required unit CI is red, with five failures in this touched watchdog spec, and there is no across-cycle decreasing-backlog witness for AC3.
• Residual handling is coherent: the PR says all ACs are L2-verifiable while moving the actual overnight/drain observation into Post-Merge Validation.
• No external deployment receipt is being used as a merge gate.

Findings: Evidence-to-AC mismatch. The declared L2 result is disproved by current-head CI and does not cover the drain-verification behavior.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: this internal watchdog behavior does not modify a public contract, MCP/OpenAPI surface, skill convention, or loaded instruction substrate.

🧪 Test-Evidence & Location Audit

• Execution evidence: required unit CI failed at e24e664b3648779f05577e2ee45c5771fd92249f (run 29907175744, job 88881309788). The five blocking failures are all existing pipeline-active-alarm cases in the touched spec: recent recognized deferral, unrecognized skip, one-shot latch, non-owned dream lane, and dispatcher failure.
• Reviewer falsifier: exact-head CI ran those pipeline witnesses in a fresh worker; because worker uptime is below the watchdog threshold, the new ambient uptime cap turns every expected stall into stalled: false.
• Test location: the added pure-evaluator witnesses are in the correct unit spec.

Findings: Incorrect current-head test claim plus missing pipeline-level recovery-state witnesses.

📋 Required Actions

To proceed with merging, please address the following:

• Route uptime/boot time through the existing injectable runtime/provider boundary instead of reading ambient process.uptime() inside the execution path, then add pipeline-level witnesses for both a recent boot and a process alive past the threshold. Restore exact-head required CI to green.
• Implement AC3 as durable cross-cycle recovery state: record the post-downtime backlog baseline, keep recovery pending until a later successful check proves the backlog decreased/drained, and do not clear the relevant latch/recovery marker merely because the uptime-capped verdict is currently non-stalled. Preserve the alive-but-starved alarm path.
• Define downTimeSuppressed/digest-resume as an actual transition (wall-clock would stall, uptime-adjusted clock does not) and deduplicate its INFO note. Add zero-backlog, below-threshold, repeated-check, and non-decreasing/decreasing-backlog witnesses.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 65 - The pure evaluator is well placed, but the pipeline bypasses its deterministic runtime boundary and recovery has no state model.
• [CONTENT_COMPLETENESS]: 55 - Offline exclusion is present; the explicit drain-verification AC is not.
• [EXECUTION_QUALITY]: 25 - Required exact-head CI fails five touched watchdog cases deterministically.
• [PRODUCTIVITY]: 45 - The core calculation is reusable, but the current state shape creates another repair cycle.
• [IMPACT]: 75 - Correctly preventing false swarm-wide alerts is valuable, while premature recovery can hide real starvation.
• [COMPLEXITY]: 55 - The calculation is small; correct cross-cycle recovery and deterministic clock injection require moderate care.
• [EFFORT_PROFILE]: Maintenance - A bounded repair to an existing liveness watchdog and its tests.

The direction is sound. Once uptime is injected and recovery means observed drain rather than a one-cycle inference, this should be approval-shaped.

🔁 Exact-Head Re-review — bce8f9cdd52c0ff6a66f1fb55d00319d35def27a

The uptime injection and the original five deterministic pipeline failures are addressed. One genuine release blocker remains under this existing review:

• Persist the recovery phase through the pipeline. evaluateConsolidationStallAlarm() now returns nextAlarmState.recoveryBaseline, but pipeline.mjs:859 writes only {alarmed, stalledSince}. The next check therefore receives no baseline, re-enters recoveryStarted, repeats the digest-resume INFO note, and never compares the backlog against the prior value. Persist the complete recovery state (including a coherent stalledSince) and add a two-run pipeline witness: unchanged backlog preserves the baseline with no second note; decreased backlog clears it.
• Use a real suppressed-stall predicate for phase onset. The current downTimeSuppressed = hasCycle && stalenessMs > uptimeMs opens recovery even with zero backlog or wall-clock staleness below the threshold. Gate onset on the wall-clock verdict actually being a stall while the uptime-adjusted verdict is not; add zero-backlog and below-threshold witnesses.

Current-head required unit CI is still in progress. This update does not create another formal review round; it records the one remaining release blocker on the existing review.

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z