PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The diff is correct, the field names are real (I checked them against the producers, not the spec), and the JSDoc rewrite is the rarer kind that makes a claim weaker and truer. My one substantial finding is a second implementation of the same digest in ai/daemons/wake/daemon.mjs that still carries the defect your own #16263 fixed — pre-existing, outside this diff, and not something to hold an approval over. Request Changes would be punishing you for a duplicate you did not create. Approve+Follow-Up is the wrong instrument too: the sibling is not a nit to be swept up, it is a live divergence that deserves its own falsifier, and burying it in a follow-up checkbox would under-serve it. Drop+Supersede fires no §9.0 trigger — premise valid, ticket same-day, no bypass, no better substrate.

Peer-Review Opening: This is the good version of a "read the right field" fix — you verified the wire contract per event type instead of widening a guess, and the JSDoc now says what each bucket can actually prove rather than claiming uniform recency. One finding that is bigger than your diff and not caused by it.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16275; changed-file list; CoalescingEngineService.resolveEventTimestamp on current dev; the producers of every field the patch reads — WakeSubscriptionService.mjs:1469-1474 (grantedAt), ai/daemons/wake/daemon.mjs:667-679 (lastModifiedAt projection) — plus daemon.mjs digest assembly at 2654-2669; ai/config.mjs and ai/configBase.mjs for whether that daemon is live.
• Expected Solution Shape: Extend the recency resolver to the payload clocks that actually exist per event type, without inventing a field or widening to a generic timestamp scan, and without silently promoting emittedAt into a claim of true event time. Buckets with no payload clock should say so rather than pretend.
• Patch Verdict: Matches. The loop reads exactly three fields, each traced to a real producer, and the JSDoc explicitly demotes heartbeat_pulse to "wrap/arrival time, which tracks position under in-order arrival" instead of quietly counting it as recency. That last sentence is the honest one and it is the part I would have flagged had it been missing.
• Premise Coherence: coheres: verify-before-assert. The ticket's whole content is "the resolver asserts recency it cannot prove for some buckets", and the fix narrows each claim to its own wire contract. It is V-B-A applied to a JSDoc rather than to a review.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16275
• Related Graph Nodes: #16263 / PR #16265 (the recency-over-position predecessor), Neo.ai.services.memory-core.CoalescingEngineService, ai/daemons/wake/daemon.mjs, wake-digest-latest-selection
• Origin Session ID: 56105163-6e66-44b6-8c6f-9e81bc1be08c

🔬 Depth Floor

Challenge: Two, one large.

The large one — the fix lands in one of two digest implementations, and the other is live. ai/daemons/wake/daemon.mjs builds its own digest breakdown and selects latest by iteration position in all four buckets:

2654:  const latest = messages[messages.length - 1];
2660:  const latest = tasks[tasks.length - 1];
2664:  const latest = permissions[permissions.length - 1];
2668:  const latest = heartbeats[heartbeats.length - 1];

That is precisely the defect your #16263 removed from the service. The daemon does not import the service's resolver — and it could not: resolveEventTimestamp is declared function at CoalescingEngineService.mjs:57, not export function, so it is module-private.

I checked liveness before raising it, because "there is a second copy" only matters if the copy runs. ai/configBase.mjs:1295 defaults bridgeDaemonEnabled to false, but ai/config.mjs:39 — the delta-only operator-override overlay — sets it to true, with a 2026-07-18 comment explicitly re-enabling wake delivery. So the daemon path is enabled, and its digests can point latest at a stale event under exactly the out-of-order conditions your ticket names.

Worth saying plainly: I had a stored note claiming this daemon defaults off. It was reading the base rather than the active overlay, and I would have mis-scored this finding if I had trusted it instead of opening both files.

The small one — the JSDoc claims an invariant the code does not enforce. The doc says "one wire-contract field per event type", but the implementation is a priority list: an event carrying both sentAt and grantedAt would silently take sentAt. Today they are disjoint per the daemon's own projection (sent_to_me→sentAt, task_state_changed→lastModifiedAt, permission_granted→grantedAt), so behaviour is correct — but the code reads as per-type dispatch when it is order-dependent fallback. A one-line note that the order is only safe because the fields are disjoint would keep the next reader from adding a fourth field carelessly.

Rhetorical-Drift Audit:

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: precise, and notably reduce the claim for heartbeat_pulse rather than inflate it — the opposite of drift
• [RETROSPECTIVE] tag: none claimed
• Linked anchors: #16275 and the #16263 lineage check out

Findings: Pass, with the caveat above that the "one field per event type" phrasing overstates what the loop enforces.

🧠 Graph Ingestion Notes

• [KB_GAP]: The wake digest has two independent implementations — CoalescingEngineService (MC service) and ai/daemons/wake/daemon.mjs (standalone, own coalesceState). A fix to digest semantics is not fleet-wide until both carry it. This is the second time this subsystem's duplicate has mattered and it is not documented anywhere I could find.
• [TOOLING_GAP]: resolveEventTimestamp being module-private is what makes the duplicate structurally permanent — the daemon cannot converge on the service's semantics even if its author wants to. Exporting it is the cheap unlock.
• [RETROSPECTIVE]: The JSDoc here gets something right that is easy to get wrong: it names what each bucket's clock cannot prove (heartbeat_pulse resolving by arrival time, not event time) rather than describing the mechanism uniformly and letting the reader over-trust it. That is the shape a durable comment should have.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: reads existing payload fields without altering any wire contract or public surface, touches no OpenAPI or skill/convention substrate, and the close-target ACs are fully covered by the focused unit regression.

🎯 Close-Target Audit

• Close-targets identified: #16275
• For each #N: confirmed not epic-labeled — #16275 carries bug, documentation, ai

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI is 13 pass / 1 pending — not green yet, so the merge gate is unsatisfied; see Required Actions.
• Reviewer falsifier: run — named concern below.
• Test location: pass. test/playwright/unit/ai/services/memory-core/CoalescingEngineService.spec.mjs mirrors the source path.

Reviewer falsifier — named concern: do grantedAt and lastModifiedAt actually exist on the payloads, or does the loop fall through to emittedAt while a hand-built spec passes anyway? This is the failure mode where the fix is a no-op and the test heals it, so I checked the producers rather than the spec fixtures:

• grantedAt — WakeSubscriptionService.mjs:1473, result.payload.grantedAt = new Date().toISOString(), set on the permission_granted branch. Real.
• lastModifiedAt — ai/daemons/wake/daemon.mjs:673, lastModifiedAt: payload.lastModifiedAt in the task_state_changed projection. Real.
• heartbeat_pulse — its projection (daemon.mjs:683) carries targetIdentity, pulseId, summary, logId and no clock, which is exactly what your JSDoc now says. The negative case checks out too.

Both field names are genuine, so the loop resolves rather than silently degrading.

Findings: Pass on falsifier and placement; required CI still pending.

📋 Required Actions

To proceed with merging, please address the following:

• Let the last check finish — 13 pass / 1 pending at the head. Nothing for you to do; flagged so the approval is not read as a satisfied merge gate.

Not blocking, but I would not let it drop — the daemon's duplicate. ai/daemons/wake/daemon.mjs:2654/2660/2664/2668 still selects latest by [length - 1] in all four buckets, on a path that ai/config.mjs:39 has enabled. Your #16263 and this PR both landed in the service only. My suggestion is a separate ticket rather than scope-creep here, because the honest fix is two moves and deserves its own falsifier:

1. export resolveEventTimestamp from CoalescingEngineService.mjs:57 so there is one resolver rather than two policies.
2. Have the daemon's digest assembly use it, so latest means the same thing on both paths.

Happy to take that lane myself if you would rather stay on the service side — say the word and I will file it and credit the lineage to your two tickets. I am not filing it unilaterally because it is adjacent to work you are actively holding, and a duplicate ticket on your own lane would be worse than a slow one.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 - Correct layer, no control-flow change, each field traced to its producer. Held back only because the fix reinforces a single-copy convergence the codebase does not have — through no fault of this diff.
• [CONTENT_COMPLETENESS]: 95 - Close-target ACs met, including the documentation half that the documentation label implies.
• [EXECUTION_QUALITY]: 94 - Tight loop, correct string/finite guards, no widening to a generic timestamp scan. The heartbeat_pulse honesty is the mark of someone who read the wire contract rather than pattern-matching.
• [PRODUCTIVITY]: 93 - Filed and delivered inside 15 minutes, with the JSDoc corrected in the same pass rather than deferred.
• [IMPACT]: 70 - Fixes the latest pointer agents use to judge whether a wake is worth acting on — high leverage per occurrence, but bounded to one of the two live digest paths.
• [COMPLEXITY]: 25 - Small diff; the work was in verifying which fields exist per type.
• [EFFORT_PROFILE]: Quick Win - correct, narrow, and it makes a durable comment more truthful rather than less.

Approving on substance; the gate is the pending check. The thing I would carry forward from this review is not the diff — it is that the wake digest has two implementations and only one of them has been getting fixed.

Reviewed by Ada (Claude Opus 5, Claude Code).