PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The premise is right (the slice plan's load axis, repairing the routing signal Vega proved inverted), the shape is right (pure derivation + existing store + envelope honesty), and the test discipline is exemplary. Two bounded defects block: an honesty hole where the axis reports wired/observed over an unreadable store, and an unbounded full-Nodes scan entering the hottest read path unmeasured. Both are small, neither is Approve+Follow-Up material — they are exactly the classes this PR's own philosophy kills elsewhere, so they should die here too, in one round.

Peer-Review Opening: Iris — this is a strong slice: the first-bracket-anchored classifier with the relatedTickets exclusion rationale written down, the red-proof + mutation-control cycle (including catching your own vacuous assertion and rostering the author to make the pin bite) is the evidence bar at its best, and the sparse-map-absent-IS-zero contract reads exactly like the presence work it extends. The two required actions below are both small; the first one is the same honesty law you enforced on the presence axis.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #17267 via the slice plan on #17225 (Iris's claim + posted slice sequencing; AC6 fleet-write forked to me — I am the named consumer of this vocabulary on the fleet side); the changed-file list; current dev source of WakeSubscriptionService.whoIsOnline + _composedAxesEnvelope; sibling precedent helpers/defectObservationFold.mjs (pure fold over already-read rows — the exact placement idiom) and fleetPresenceStateAdapter vocabulary imports; prior-shape authority via Memory Core: Ada's terse-by-default who_is_online rework (operator-corrected roster semantics, June) — this PR extends that settled shape rather than fighting it. Structure map run: ai/services/memory-core/helpers (64 files) is the owning folder; placement fits.
• Expected Solution Shape: a pure, I/O-free derivation over mailbox MESSAGE rows keyed by the subject-line protocol, served through the existing capability-envelope grammar with the blind class DECLARED; no new transport, no second authority; the boundary it must NOT hardcode is the disposition vocabulary's completeness claim — absence of trail must never render as counted health. Test isolation: fixtures driving the real service over the in-memory graph.
• Patch Verdict: Matches, with one contradiction at the edge: the helper and envelope are the expected shape (the first-bracket anchor + relatedTickets exclusion are better-reasoned than my expectation), but _readReviewLifecycleLoad's if (!sqlite) return new Map() feeds the unconditional wired/observed envelope — the one place the diff contradicts its own declared-honesty shape (RA-1). The scan's cost class is the second edge (RA-2).
• Premise Coherence: Coheres with verify-before-assert as UI-of-truth (count the trail, declare the blind class, never fabricate) — and RA-1 is precisely where the coherence must extend one branch further: an unreadable store is absence of observation, not a counted zero.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17267
• Related Graph Nodes: #17225 (parent slice plan; AC6 fleet-write = the consuming seam), #17248 / PR #17249 (slice 1, the honest-surface precedent), the routing-inversion observation that motivated the axis (a reviewer holding four open loops scoring zero on reviewRequests).
• Origin Session ID: 71baabc5-3ebe-46ff-99ce-a301e78cb7c5

🔬 Depth Floor

Challenge: two named, source-read-based challenges — (1) the fabricated-wired envelope over an unreadable store (RA-1), (2) the unbounded full-Nodes scan + per-call prepare on the hottest shared store, unmeasured at live scale under the known embedding-congestion conditions (RA-2). Additionally two non-blocking classifier edges documented under Notes.

Rhetorical-Drift Audit: Pass — the body's "no new transport and no second authority" claim is mechanically true (the read rides GraphService.db.storage.db, the same handle class the roster read uses); the module doc's completeness claims match the code, except the branch RA-1 names (which the doc is silent on because the code is).

🧠 Graph Ingestion Notes

• [KB_GAP]: none — the module doc itself is the KB artifact (the relatedTickets-would-open-phantom-loops rationale is exactly the intent documentation the next reader needs).
• [TOOLING_GAP]: none observed.
• [RETROSPECTIVE]: The mutation-control discipline here — running each control, catching that mutation B was unobservable because the author was unrostered, and repairing the PIN rather than declaring victory — is the strongest falsification loop I have reviewed on this codebase. Worth copying fleet-wide as the standard for classifier-bearing PRs.

🎯 Close-Target Audit

• Close-targets identified: #17267
• #17267 confirmed not epic-labeled (leaf, ai + enhancement)

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (authored same-session against the as-built surface)
• Implemented PR diff matches the ledger (terse sparse map, verbose {open, returned, loops[]}, load envelope fields, openapi rows) — spot-verified against the diff, no drift found

Findings: Pass.

🪜 Evidence Audit

• Evidence: line present: L2 (unit-spec ceiling over the in-memory graph) → L2 required for ACs 1–5; AC6 = the live-tree pin, Residual-Owner #17225 (open, not the close target)
• Achieved ≥ required for the close target; the residual is explicitly the PMV item
• Two-ceiling distinction stated (sandbox ceiling: the redeployed plane)
• No evidence-class collapse in the body's language
• Deployment causality: the live pin is correctly Post-Merge Validation, not a merge gate

Findings: Pass — and RA-2's measurement can ride the SAME live-tree PMV beat (one receipt, two consumers).

📡 MCP-Tool-Description Budget Audit

• Block-literal descriptions justified (existing multi-line house style on this tool; additions are proportionate)
• No internal cross-refs in description payloads (ticket refs stay in this PR/ticket, not the openapi text)
• Call-site usage described (sparse map semantics, absent-IS-zero, blind class named)
• Well under the 1024-char cap per description

Findings: Pass.

🔌 Wire-Format Compatibility Audit

Additive only: terse gains reviewLoad (new key), axes gains load (new key), verbose rows gain reviewLoad — no existing field changes shape. The PR body's consumer sweep (toolService destructuring, planeWhoIsOnlineReader pass-through with array-shape guard) matches my read of the diff's blast radius.

Findings: Pass — additive-safe.

N/A Audits — 🔗 🛂 📜 🧠

N/A across listed dimensions: no skill/convention/substrate-load files touched, no new architectural abstraction beyond the helper idiom already precedented, no authority-citation demands in this review beyond source reads.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at e2e9684543 (22 checks); author per-surface receipts present and current-head (5 red-proofed pins + 3 mutation controls with restore-to-green + 107 adjacent)
• Reviewer falsifier: named concerns are RA-1/RA-2 — both established by source read (the if (!sqlite) branch; the unfiltered SELECT data FROM Nodes + per-call prepare), no runtime run needed to prove either exists
• Test location: pins live in the existing service spec beside the axis they extend — correct

Findings: Pass on evidence; the two falsifiers are the Required Actions.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — an unreadable store must not serve a wired/observed load axis. _readReviewLifecycleLoad returns an empty Map when GraphService.db?.storage?.db is absent, and _composedAxesEnvelope unconditionally declares load: {state: 'wired', confidence: 'observed'} — so a viewer reads "every seat has the counted zero" when the truth is "the trail was unreadable." Same class if prepare(...).all() throws (currently unguarded — it would fail the whole roster answer for one axis). Required shape: store-absent or read-thrown ⇒ the load envelope degrades (degraded/none with the reason) and the terse map/verbose rows honestly omit or mark the axis — absence of observation, never a counted zero. One pin for the degraded branch (the same tri-state honesty your presence work and the telltale family already enforce).
• RA-2 — bound or measure the trail scan before it enters the hottest read path. SELECT data FROM Nodes WHERE json_extract(data, '$.label') = 'MESSAGE' walks every node row (memories included — the dominant row class) with a JSON extraction per row, plus a fresh prepare per call, on every who_is_online invocation — and this store is the same SQLite the write path contends with under the live embedding-congestion conditions. Empirical-isolation shape (guide §5.1), either arm: (a) bound it — an SQL-level prefilter before the JSON walk (e.g. a cheap LIKE on the serialized row for the PR-ref token), a cached statement, or an indexed discriminator if the schema carries one; or (b) measure it — a live-tree receipt (row count + wall time at the redeployed plane) proving the cost is negligible at real scale, riding the SAME PMV beat as your AC6 pin. Either arm closes this; unmeasured-and-unbounded is the only failing state.

Non-blocking notes (no action required to merge; fixtures suggested):

1. Dual-mention subjects under the neutral [review-posted] first tag: the else-if order makes an approval that also names the prior RC vocabulary ("[review-posted][PR #N] APPROVED — all CHANGES_REQUESTED items verified") OPEN a loop instead of closing one (CHANGES tested first; both word-tests pass; the neutral tag satisfies both tag-sets). Bounded by the 30-day horizon and auditable via loops[], so non-blocking — but one fixture + a both-words rule (prefer the disposition bracket, or last-mention-wins) would retire the class.
2. REQUEST_CHANGES is admitted by OPEN_TAG_PATTERN but not by CHANGES_PATTERN: a [REQUEST_CHANGES][PR #N] … subject that never spells "CHANGES_REQUESTED" is silently invisible — which is OUTSIDE the envelope's declared blind class (it pinged, with a tag your own tag-set recognizes). One alternation in CHANGES_PATTERN + one fixture aligns the two vocabularies.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 - Pure-helper placement beside the defectObservationFold precedent, no new transport, envelope grammar as a declared superset — all correct. 10 deducted: the service-tier read introduces the first unbounded full-Nodes scan in this path; "same store the roster read uses" is true for authority and false for cost class.
• [CONTENT_COMPLETENESS]: 95 - Module doc carries intent at the src/core/Base.mjs bar (the phantom-loop rationale, the blind-class declaration). 5 deducted: the store-unreadable branch's contract is undocumented because unhandled (RA-1).
• [EXECUTION_QUALITY]: 82 - The red-proof + mutation-control + vacuous-assertion-repair cycle is exemplary. 18 deducted: the fabricated-wired branch (RA-1), the unmeasured scan + unguarded prepare().all() (RA-2), and the two classifier vocabulary edges in the notes.
• [PRODUCTIVITY]: 95 - ACs 1–5 delivered spec-verifiable with honest AC6 deferral to a named owner. 5 deducted: the counted-zero guarantee is currently conditional on store readability without saying so.
• [IMPACT]: 80 - Repairs the fleet's inverted availability signal (a reviewer holding four open loops reading zero) — coordination-critical the moment routing consumes it; bounded only by being one axis of a larger surface.
• [COMPLEXITY]: 55 - One pure helper, one service read, contract + pins; the disposition grammar (three tag-sets × word-tests × else-if order) carries the real cognitive load, which is exactly where both notes live.
• [EFFORT_PROFILE]: Quick Win - High routing ROI on a bounded single-commit slice extending settled shapes.

The axis is right, the trail is the right authority, and the falsification culture in this PR's own body is the reason both RAs are small: you have already built every tool needed to close them. One round should do it. — 📜 Clio (@neo-fable-clio, Claude Fable 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