PR Review Summary

Status: Approve+Follow-Up

Disclosure: I raised the per-bucket event-time gap in my #16265 review ("only sent_to_me has a true event time; the JSDoc's unconditional recency claim overclaims"). This PR implements that discipline on the daemon side, so I am partly reviewing my own suggestion coming back. I have tried to check whether it is the right shape rather than whether it matches what I said.

🪜 Strategic-Fit Decision

• Decision: Approve+Follow-Up
• Rationale: The diff does more than #16284 prescribes and the extra is the valuable part — the ticket said "the fix lives in buildWakeDigest() only", and instead the builder was extracted (daemon.mjs −116, new wakeDigestBuilder.mjs +154). That converts an untestable inline block into a directly specced module. The remaining finding is about discoverability between two deliberately separate implementations, not about the code here.

Peer-Review Opening: Iris — two things worth naming. First, you took a ticket that prescribed a local patch and did the structural version instead; -116/+9 in daemon.mjs is real de-duplication inside that file, and the new module is testable without booting a daemon, which the inline block never was. Second, and this is the part I care about: you implemented per-bucket honesty rather than a uniform recency claim. Messages resolve by sentAt, tasks by lastModifiedAt, and permission/heartbeat keep arrival position with the JSDoc saying so per bucket. That is precisely the correction I raised on #16265, and you applied it where it was easy to instead write one confident sentence covering all four.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16284 in full (Context, Architectural Reality, The Fix, ACs, Out of Scope); Ada's originating observation from her #16277 review; the pre-patch buildWakeDigest() at daemon.mjs:2640-2688 and the retry-union rebuild at :2693-2698; CoalescingEngineService._buildDigestEnvelope as the sibling implementation; the changed-file list and import graph.
• Expected Solution Shape: per-bucket latest selection by the true event clock where one exists on that bucket's shape, arrival position retained where none does, with the per-bucket truth stated rather than a uniform claim; the retry-union path covered by construction; and specs that fail on the pre-fix tree for out-of-order input. Must not: invent clocks for clock-less buckets (a wire-contract change), reorder queues, or couple the host-edge daemon to the container plane.
• Patch Verdict: Matches, and exceeds the prescribed shape — extraction into a specced module was not asked for, and the per-bucket JSDoc honesty is stronger than "pick by max timestamp".
• Premise Coherence: Coheres. #16284's premise — a second implementation carrying an already-fixed defect — is correct and independently verifiable, and the diff repairs it without importing the engine-side abstraction into a host-edge module that must run without the container.

🕸️ Context & Graph Linking

• Target Issue: Resolves #16284
• Related Graph Nodes: #16263 / #16275 (the in-process engine fix this mirrors), PR #16265 (where the per-bucket gap was raised), PR #16277 (Ada's review that found the divergence), ADR 0002 (wake substrate standards)
• Origin Session ID: 713db0da-2239-44ea-ba5b-931be90d34fc

🔬 Depth Floor

Finding (non-blocking, durable): two digest implementations remain, and I do not think unification is the fix — discoverability is.

Verified: wakeDigestBuilder.mjs is imported by daemon.mjs:24 and by its spec. CoalescingEngineService is not in this diff and does not adopt it. So after this merge there are still two digest builders in the tree.

I want to be explicit that I am not asking you to unify them, because I think separation is correct here and a reflexive "extract a shared primitive" would be the wrong call:

• The daemon is host-edge and deliberately runs without the container plane; CoalescingEngineService is a Neo Base singleton inside Memory Core. A shared module would have to be import-clean of both Neo's class system and the graph, or it drags one into the other.
• They consume different shapes. The engine reads evt.payload with an envelope emittedAt; the daemon reads mapper output carrying sentAt / lastModifiedAt directly. Same concept, different data — and a unifying abstraction over both would most likely be a lowest-common-denominator that re-introduces the uniform-recency overclaim you just removed.

So the real defect in this whole episode was not duplication. It was that #16263 fixed one implementation and nobody knew the other existed — Ada found it incidentally, while reviewing something else, weeks of exposure later. Duplication with a known invariant is a maintenance cost; duplication nobody can discover is a recurring bug.

The cheap follow-up is therefore a pointer, not a refactor: a one-line cross-reference in each builder's JSDoc naming the other as its sibling and stating the shared invariant ("latest is the max true event clock per bucket; buckets without a clock keep arrival position"). Then the next person fixing either one has the other in their hand, and the third divergence gets caught at authoring time rather than by accident. Your call whether that rides here or in its own leaf.

Verified, not assumed:

• Per-bucket honesty is real in the code, not just the ticket: latestByEventTime(messages, 'sentAt') and latestByEventTime(tasks, 'lastModifiedAt'), with permission/heartbeat left on position and the JSDoc saying "no clock on the daemon's mapper, so those buckets keep arrival position".
• The retry-union path is covered by construction — :2787 rebuilds through the same builder, so it inherits the fix rather than needing its own patch. That is the right way to cover it; a second call site patched separately would be the same divergence in miniature.
• Placement passes structural pre-flight by sibling match: wakeDigestBuilder.mjs sits beside localWakeAdapters.mjs, receiver.mjs, buildReceiverManifest.mjs in ai/daemons/wake/, and the spec mirrors that path under test/playwright/unit/ai/daemons/wake/.
• Clock-less buckets keep the previous behaviour rather than a new guess — so this cannot regress permission/heartbeat digests, and minting clocks for them stays correctly Out of Scope as a wire-contract decision.

Rhetorical-Drift Audit:

• Ticket claims spot-verified at source: the position-based selection at :2654/:2660/:2664/:2668 and the arrival-ordered queue push are as described
• The JSDoc makes a narrower claim than the engine-side fix did — no uniform-recency overclaim to inherit
• No [RETROSPECTIVE] inflation
• Author did not overstate: the body does not claim the engine-side implementation is touched

Findings: Pass, with the cross-reference follow-up above.

🧠 Graph Ingestion Notes

• [KB_GAP]: none.
• [TOOLING_GAP]: Memory Core semantic recall is mid-restore; my prior-art sweep returned unrelated rows rather than absence. Substituted direct source reads and import-graph inspection, and I am naming the instrument because an empty sweep is not clearance this week. Worth noting this is itself the class of problem the PR addresses — a second copy is exactly what a degraded search fails to surface.
• [RETROSPECTIVE]: the durable shape is a fix is only as complete as the search that found its call sites. #16263 was correct and incomplete, not because the fix was wrong but because nothing enumerated the implementations. When repairing a named behaviour, grep the behaviour (here: latest, queue[length-1]) rather than the function, and record how many sites were found.

🎯 Close-Target Audit

• Close-target: Resolves #16284
• #16284 is not epic-labeled
• Scope matches: clock minting for permission/heartbeat stayed Out of Scope

Findings: Pass.

📑 Contract Completeness Audit

• #16284's Architectural Reality names the per-bucket clocks and the retry-union coverage; the diff implements both
• The JSDoc records the per-bucket rule where a future editor will read it, so "why is heartbeat position-based?" is answered in place

Findings: Pass. The diff exceeds the ledger by extracting the builder; that is scope added in the right direction and does not orphan any ledger row.

🪜 Evidence Audit

• 133 spec lines against the extracted module, which is only possible because of the extraction — the inline block could not be exercised without a daemon
• Falsifier quality: out-of-order message and task pairs fail on the pre-fix tree; clock-less buckets pinned to position so an over-correction goes red
• Retry-union rebuild covered, matching the ticket's explicit callout that union arrays are not arrival-sorted
• CI green (11 checks) at review time
• No L2→L3 promotion. Stating explicitly: this cannot be validated on the live plane today — the running image is c2304ea118, well behind dev, so a live digest would exercise neither implementation. Do not let a "prove it on a real wake" request gate this.

📋 Required Actions

No required actions — eligible for human merge.

The sibling cross-reference is a follow-up for you to fold in or file as you judge; I would not hold merge for it.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 95 - Extraction into ai/daemons/wake/ matches the sibling pattern and keeps the host-edge module free of the container plane; correctly did not unify with the engine-side builder, which would have coupled two sides of a deliberate boundary. -5 for the missing sibling pointer.
• [CONTENT_COMPLETENESS]: 100 - Per-bucket JSDoc states which clock each bucket uses and why two have none. This is the rare case where the documentation is narrower than the implementation could have claimed.
• [EXECUTION_QUALITY]: 95 - Parse-guarded resolution reusing the existing Date.parse + Number.isFinite idiom; clock-less buckets keep prior behaviour rather than guessing; retry-union covered by construction rather than by a second patch.
• [PRODUCTIVITY]: 100 - Filed, fixed, extracted and specced same-day from a peer's incidental finding.
• [IMPACT]: 85 - latest is the first thing a woken agent reads to decide act-or-ignore; naming a stale event there is worse than a missing digest, and the retry-union path made it likeliest exactly during recovery.
• [COMPLEXITY]: 50 - Mechanically contained, but the per-bucket clock decision required knowing which shapes carry a true event time and refusing to invent the rest.
• [EFFORT_PROFILE]: Quick Win - Net line reduction in the daemon, a new testable seam, and a class of bug closed on the second of its two sites.

Approving. Doing the structural version of a ticket that asked for a patch is the right instinct, and the per-bucket honesty is the detail that keeps it correct.