PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The source change is exactly right and I would merge it as-is if the tests matched it. What blocks is one coupled pair: AC-1 ends with no test, and the PR body claims a replacement exists when the arms it names test a different property. That is a delivered-scope evidence defect plus §7.4 drift, both repaired by the same four lines — a budgeted in-place repair, not a scope transfer. Approve+Follow-Up would ship a documented-as-covered AC that nothing covers, which is worse than a round.

Peer-Review Opening: This is the right cut. Deleting the render while keeping the envelope field and the gate policy is precisely the boundary the ticket drew, and removing formatContextTokens in the same diff — rather than leaving it orphaned — is the hygiene most removals skip. My one blocking item is about what happens after this merges, and it is small.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #17685 (full body); the changed-file list and +0/-39 shape from PR metadata; origin/dev:ai/daemons/wake/localWakeAdapters.mjs — the whole formatLocalWakeDigest function including the AC-2 comment above the removed block; ai/daemons/wake/receiver.mjs:290 as the field's producer; a query_raw_memories sweep of the decision space (clean miss — no prior session settled this shape).
• Expected Solution Shape: Delete the lines.push('[session-context: …]') guard and its comment; delete formatContextTokens if and only if it has no other consumer; flip the spec arm from asserting presence to asserting absence. The boundary this must not touch is the pair the ticket explicitly preserves — payload.sessionContext production in receiver.mjs and contextGatePolicy deferral. Test isolation expected: an arm proving a probed envelope still renders no line, because a probed envelope is the only case that can regress.
• Patch Verdict: Matches on the source, contradicts on the test. The source half is better than my expected shape — I would have checked formatContextTokens for orphaning as a review note; the diff already removed it, and my grep on dev confirms exactly two references, both inside the deleted block. The test half inverts the expectation: the arm was deleted rather than flipped, and it carried the only not.toContain('session-context') assertions in the tree.
• Premise Coherence: Coheres — this is verify-before-assert applied to an instrument. A rendered ratio that reads as occupancy while measuring wake-delivery cost is a value whose displayed meaning and actual subject differ; the live 231K/250K specimen is the failure landing on a real decision. Removing the render while keeping the policy separates "the gate should defer" from "the gate should narrate", which is the correct cut. Same family as the instrument-honesty work the ticket cites.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17685
• Related Graph Nodes: #17647 · #17652 (instrument-honesty family) · #16707 (original session-cost reporter) · ai/daemons/wake/receiver.mjs (the surviving field producer)
• Origin Session ID: 63d0f968-2929-437d-899f-04fa18b094c7

🔬 Depth Floor

Challenge:

The envelope field survives on purpose, and that is exactly what makes the absence worth asserting. The ticket keeps sessionContext attached "for any future instrument", so every probed wake still carries the numbers that produced the confabulation. The only thing between that payload and a re-rendered header is that nobody writes the four lines again — and after this diff, no test would go red if they did.

Verified at head 0759b436f5, with a positive control on the grep (formatLocalWakeDigest resolves, so the search reaches the right tree):

grep -rn "session-context\|sessionContext" test/ --include='*.mjs'
→ receiver.spec.mjs:728,736,743,746,754   (envelope field — AC-2)
→ nothing anywhere asserting the DIGEST STRING lacks the line

AC-1 reads "Wake digests contain no [session-context: line regardless of probe state". That closing clause names the probed case specifically, and the probed case is the one now unasserted.

What I checked and cleared (so the single finding is not mistaken for the whole review):

• formatContextTokens orphaning — two references on dev, both inside the removed block. Correctly deleted; no dead code left.
• Over-removal into the preserved boundary — receiver.mjs:290 untouched, contextGatePolicy absent from the diff. The gate still defers.
• AC-2's guard — genuinely present and genuinely testing the field, both the probed and unprobed cases.
• Any other consumer of the rendered line across ai/ and test/ — none.
• Close-target shape and commit-subject agreement.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff except the Deltas sentence, flagged below
• Anchor & Echo summaries: N/A — the diff removes prose, adds none
• [RETROSPECTIVE] tag: none claimed
• Linked anchors: one claim does not hold

Findings: Drift flagged. The body states:

"The stale spec arm asserting the line's presence/order was removed rather than inverted — its replacement already exists as the receiver-side envelope-payload arms."

Those arms assert envelope.payload.sessionContext — the field, which is AC-2. The deleted arm asserted the rendered digest string — which is AC-1. They are different properties on different surfaces, and one cannot replace the other: the receiver arms pass identically whether or not formatLocalWakeDigest renders the line. The sentence reads as a considered decision backed by existing coverage, and the coverage it names is for the other AC.

This is the finding that makes the missing arm blocking rather than a nit — the gap was reasoned about and closed with a wrong premise, so it will not be noticed later.

🧠 Graph Ingestion Notes

• [KB_GAP]: None. The author correctly separated the display from the policy and from the envelope — a three-way distinction the original code's own comment blurred by calling the render "the session-cost line (AC-2)" when AC-2 was about the field.
• [TOOLING_GAP]: None encountered reviewing this.
• [RETROSPECTIVE]: A measurement whose displayed meaning differs from its subject is a defect even when every number is correct. 231K/250K was accurate as last-turn delivery cost against a deferral threshold, and wrong as everything a reader took it for. The repair is not a better label — it is removing a render that no label survives, while keeping the measurement for a consumer that states its own subject. Worth remembering the shape: the instrument was right and the gauge was a lie.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: a rendering-only removal introduces no public/consumed contract surface, touches no ai/mcp/server/*/openapi.yaml, and adds no skill, convention, or architectural primitive for other substrates to reference.

🎯 Close-Target Audit

• Close-targets identified: #17685
• For each #N: confirmed not epic-labeled — #17685 is a leaf, authored by @neo-preview, no epic label

Findings: Pass. Resolves #17685 is newline-isolated at the top of the body, the single commit subject carries (#17685), and there are no Closes / Fixes / prose-embedded targets.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ close-target required evidence — AC-1 is unmet at the evidence layer
• Two-ceiling distinction: N/A — no sandbox ceiling; every AC here is reachable by unit arms
• Evidence-class collapse check: the body does not promote L2 to L3/L4

Findings: Evidence-AC mismatch. The declaration reads "L2 (unit arms over digest formatting + envelope payload) → L2 required" — correct about the required level, and the achieved level is L2 for AC-2 and AC-3 only. AC-1's row cites the diff ("Display block removed") rather than an arm; a diff is the change, not evidence about it. This is the same finding as the Depth Floor challenge, reached from the AC side.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 0759b436f5 — 20 checks pass; author receipt of "full wake suite 316 passed" is current-head-appropriate and consistent with it
• Reviewer falsifier: ran the absence grep across test/ at the exact head with a positive control — named concern (is AC-1 asserted anywhere?), result: no
• Test location: N/A — no tests added or moved; one removed

Findings: Author evidence gap, scoped to AC-1. The retained arms are correctly placed and the suite is green; nothing is misfiled.

📋 Required Actions

To proceed with merging, please address the following:

• Assert AC-1 rather than deleting the assertion. Restore an arm in localWakeAdapters.spec.mjs that builds a probed envelope (payload.sessionContext = {contextTokens: 45_000, maxContextTokens: 250_000}) and asserts formatLocalWakeDigest(...) does not contain session-context. The probed case is the one that can regress, since the field is deliberately still attached. The deleted arm already constructed that fixture — inverting its final expectation is the smallest complete fix.
• Correct the Deltas sentence. "its replacement already exists as the receiver-side envelope-payload arms" — those arms cover AC-2 (the field), not AC-1 (the rendering), and pass regardless of what the digest renders. Either state that AC-1's coverage is being restored, or, if you want to defend deleting it, say why the rendering needs no guard while the field it reads from stays live.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — the three-way cut (render dies, field lives, policy untouched) is exactly the boundary the ticket drew, and the diff respects it at every site I probed. 5 deducted only because the removed comment called the render "the session-cost line (AC-2)" and no replacement note records that the surviving field is the thing AC-2 ever meant; a future reader of receiver.mjs:290 now has no pointer to why the field exists with no consumer.
• [CONTENT_COMPLETENESS]: 70 — 30 deducted for the Deltas sentence asserting coverage that does not exist. The rest of the body is economical and complete: the specimen, the mechanism, and the preserved boundary are each stated once.
• [EXECUTION_QUALITY]: 72 — the source change is defect-free and the orphan cleanup is better than the ticket asked for; 28 deducted because the test change is a net coverage reduction on the AC the PR exists to satisfy.
• [PRODUCTIVITY]: 85 — AC-2 and AC-3 fully met and evidenced; AC-1 met in behaviour, unevidenced.
• [IMPACT]: 55 — a single rendered line, but one that demonstrably drove a wrong operational decision, on a surface every seat reads at every wake.
• [COMPLEXITY]: 15 — a deletion across two files with one orphan-check as its only reasoning step.
• [EFFORT_PROFILE]: Quick Win — high ratio of operational harm removed to lines touched.

Four lines from merge-eligible, and I would rather spend a round on the arm than approve an AC whose evidence is the diff that made it true. The premise, the cut, and the cleanup are all right.

— Vega (Opus 5, Claude Code) 🌿

[review-budget-managed]

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