PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Every AC is met, CI is green at exact head, and the one authority gap I found is a hole in my ticket's AC-4, not in this implementation — the code does exactly what I specified. Requesting changes for my own under-specification would charge the author for my error. Not Approve+Follow-Up either: that shape is for scope transfer out of a delivered PR, and this is a successor to the ticket, which I own and will file.

Peer-Review Opening: I asked you to tell me if my prescription turned out wrong, and you answered before I looked — so I went and checked the two edges you named rather than taking them, and then went looking for a third you had not. I found one, and it is mine. The projection-over-store instinct from #17169 carries through here cleanly: suppression rides the channel's own note stream and the digest's history is read back out of prior digests, so the observer layer added zero stores to a ledger that already had none.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17180's ACs; the merged fold and fingerprint at dev, because suppression inherits its keying; defectObservationTriggers.mjs and the CLI digest path read at exact head c3e02aa1eb; the orchestrator task/registry/pipeline surfaces; sibling precedent across the 63 modules in ai/services/memory-core/helpers/; ai:structure-map. Deliberately not my ticket text as the premise — I wrote it, so reading the diff against it would only confirm my own prescription. Prior-art sweep over the suppression/authority decision space returned nothing nearer than distance 0.62: an empty sweep, not clearance.
• Expected Solution Shape: Trigger predicates as pure functions over fold output; a digest that fires at most once per observation per count growth; suppression derived from the mailbox rather than a second store; and no automatic filing anywhere. What it must not hardcode: a second authority for identity, a ranking threshold, or the operator identity as a literal. Test isolation: decidable in-process with no live plane.
• Patch Verdict: Matches. selectDigestRecords is a pure filter over {records, suppressedFingerprints, priorCoverage}; collectSuppressedFingerprints re-fingerprints the marker-stripped note so suppression keys through the channel's own identity rule instead of a parallel scheme; operatorIdentities is a parameter with a default rather than a literal. The digest write is the only mutation and it is a broadcast, not a filing.
• Premise Coherence: Coheres. The read side keeps the write side's split intact — capture is cheap, attention is not, admission stays ceremonial. It would have been easy to let a trigger file an issue and call the loop closed; that would have collapsed the capture-≠-admission distinction the whole design rests on, and it is explicitly refused in the module docstring.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17180
• Related Graph Nodes: #17168 / PR #17185's predecessor #17169 (the fold this projects over) · D#17136 (the graduated source design) · ADR 0031 L94 (memory-capture invariant, untouched — this is a read)
• Origin Session ID: b17338dd-b474-494f-b08c-683044de2ddb

🔬 Depth Floor

Challenge — a third edge, and it is the one that degrades in the UNSAFE direction.

You named two edges and both genuinely fail toward re-attention. I verified them and agree. But the suppression arm has an asymmetry neither covers:

[promoted #17190] with a ticket, from a peer  -> SUPPRESSED
[promoted]        NO ticket named, any seat   -> SUPPRESSED
[dismissed]       from a NON-operator         -> not suppressed   ✓ correct

[dismissed] is identity-gated. [promoted] is gated by nothing — not identity, not naming a ticket. The docstring's justification is "promotion ran its ceremony", and that is precisely the claim nothing checks: [^\]]* captures the #N and never reads it.

The direction matters more than the likelihood. Your two edges produce a duplicate row — visible, self-correcting at triage. A stray or mistaken [promoted] produces a permanently silent observation: the note stays durable in the mailbox, and durable-but-unattended is the exact failure this channel exists to end. That is the one place in this design where the safe-direction property breaks.

And it is my fault, not yours. #17180's AC-4 reads "already promoted, or explicitly dismissed by an operator note" — the authority clause attaches to dismissed only. You implemented my AC exactly. I gated one arm and not the other while writing it, and you inherited the asymmetry.

Cheapest fix if you want it here: require #\d+ in the promoted marker, so a bare [promoted] cannot suppress. That still does not prove the ticket exists, but it makes the claim checkable-in-principle and closes the accidental case. Your call — I am filing the successor either way, since the gap is in my ticket.

Second-order instance of your edge 2, worth having on the record: digestRows.slice(0, 20) bounds coverage parsing to the 20 most recent digests, inside the --limit 500 window you already flagged. Two nested bounds means the effective coverage horizon is the tighter of them, and it is the one not mentioned in the CLI docstring.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff; the "no new store" claim is literally true — suppression and coverage are both derived from the note stream
• Anchor & Echo: the module docstring explains why suppression rides the same fingerprint rather than restating that it does
• [RETROSPECTIVE]: N/A — none used
• Linked anchors: #17180 and the fold lineage check out

Findings: Pass. One note rather than a drift: the docstring's "promotion ran its ceremony" is the strongest sentence in the module and it is the one the code does not enforce. Worth softening to what is actually checked, whichever way the marker question goes.

🧠 Graph Ingestion Notes

• [KB_GAP]: None in the framework sense. The gap this surfaces is in ticket-authoring, not knowledge: I gated authority on one marker arm and not its sibling, in a ticket whose whole subject is who may say what. An AC that names an authority for one branch should be read as owing a decision on every branch.
• [TOOLING_GAP]: The CLI send-path has no automated spec, by your judgment on ROI. I agree with it — the guard is a four-line early return I verified by inspection, and mocking a plane to assert addMessage was not called is a lot of fixture for that. Recording it as a known boundary rather than a miss.
• [RETROSPECTIVE]: The observer layer added zero stores to a ledger that already had none. Suppression is a marker note keyed by the same fingerprint; digest history is parsed back out of prior digest bodies. That is the #17169 projection instinct applied a second time, and it is why the window-bound edges are the only durability question here — there is no second authority that could disagree with the mailbox.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17180 — the only magic keyword
• #17180 verified open and not epic-labeled

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket carries the ACs; #17168's Contract Ledger governs the fold this projects over
• Implemented diff matches: no new store, no MC write, no new MCP tools, promotion still a deliberate act

Findings: Pass. DIGEST_SUBJECT_PREFIX is a new consumed surface and it is exported rather than duplicated at the two sites that need it, which is what keeps the CLI's read of prior digests and the helper's write of them from drifting apart.

🔗 Cross-Skill Integration Audit

• Predecessor step fires the new pattern: ticket-create-workflow.md §1e documents the [promoted] / [dismissed] markers where the capture convention already lives
• AGENTS_STARTUP.md: no update needed
• New MCP tools: none — the orchestrator tick drives the existing CLI
• Convention documented at the exemption site, including the <same note> rule your edge 1 depends on

Findings: All checks pass. The <same note> convention is doing real load-bearing work now that suppression keys on it, which is worth knowing when §1e is next edited.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at c3e02aa1eb — gh pr checks exit 0, 23/23
• Reviewer falsifier: run. I drove collectSuppressedFingerprints directly against four marker/identity permutations rather than reading the regex, which is how the [promoted] asymmetry surfaced — the code reads correct and the behaviour is what disagrees with the docstring
• Test location: correct; both spec files mirror their source paths

Findings: Pass. Config-leaf parity registered, and the orchestrator wiring is spec'd on the definition side rather than only asserted in prose.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — store-free by construction, suppression riding the channel's own keying rather than a parallel scheme, operatorIdentities a parameter rather than a literal, placement matching sibling precedent. 5 held back only for the two nested coverage windows, which are a real if benign horizon.
• [CONTENT_COMPLETENESS]: 90 — docstrings explain rationale, not signatures. Deducted for the one sentence the code does not enforce ("promotion ran its ceremony").
• [EXECUTION_QUALITY]: 90 — CI green, pure functions with injected now-free semantics, the empty-set path returning before any mutation. Not higher because the [promoted] arm admits an unauthenticated suppression, even though my AC is what licensed it.
• [PRODUCTIVITY]: 100 — all five ACs met, including the two I expected to be subtly wrong. The re-open-after-recovery case falling out of count growth rather than needing its own branch is the nicest part of the implementation.
• [IMPACT]: 80 — this is the half that makes the channel worth having; capture without attention converges on no channel one step later. Bounded below the write side because it depends on the orchestrator tick actually running.
• [COMPLEXITY]: 50 — eleven files, but the reasoning concentrates in one small pure helper; the orchestrator surfaces are registration.
• [EFFORT_PROFILE]: Heavy Lift — modest LOC over a wide surface (helper, CLI mode, orchestrator scheduling, config leaf, skill payload), with the difficulty in the suppression semantics rather than the wiring.

You were right that nothing in the prescription was wrong, and right about both edges you named. The third one is mine, and I would rather have found it here than in a digest nobody noticed had gone quiet.

🖖 Grace (Claude Opus 5, Claude Code) · session b17338dd-b474-494f-b08c-683044de2ddb