Greptile Summary

This PR introduces WhatsApp live-commentary delivery by wiring onCommentaryReply through the embedded runner and extracting shared reply-delivery infrastructure. The implementation is thorough: a new assistant-output.ts module provides phase-aware segment extraction and two-path reconciliation (live stream and finalized snapshot), the WebSocket stream now carries phase/textSignature metadata on partial deltas, the payload builder correctly filters already-delivered commentary from the terminal reply, and the Telegram regression test enforces the scope boundary.

Key changes and observations:

• New assistant-output.ts: extractAssistantOutputCandidates correctly handles signed vs. unsigned segments, boundary splitting on non-text blocks, and isTerminal gating to avoid sending still-growing commentary. Compaction cursor recovery via resolveCompactedPromptBoundaryIndex is a well-reasoned solution for the message-history compaction edge case.
• liveAssistantFallbackMessageIdsByFingerprint (module-level Map): Never cleared. Grows for every unique content fingerprint encountered during the server's lifetime. The WeakMap counterpart is safe, but this strong-reference Map accumulates indefinitely. For long-running servers with diverse message content, a size cap or eviction strategy should be considered.
• Polling loop in attempt.ts: The 250 ms commentary reconciler loop starts unconditionally for every run, even when onCommentaryReply is undefined. No delivery occurs without a callback, but the loop still scans messages every tick. Guarding loop startup on params.onCommentaryReply would avoid unnecessary work when commentary delivery is disabled.
• Return value of processMessage: When !queuedFinal, the function now returns didSendReply instead of the previous hard-coded false. This is semantically correct (a reply was sent via commentary), and current callers discard the return value, so there is no live regression. The implicit contract change is worth documenting.
• reply-delivery.ts extraction: Clean abstraction. Directive normalization, silent suppression, and per-kind bookkeeping hooks are all reusable and well-tested.
• Test coverage is extensive: 731-line assistant-output.test.ts, new reply-delivery.test.ts, updated openai-ws-stream.test.ts, updated payloads.test.ts, and the Telegram regression guard.

Confidence Score: 4/5

• Safe to merge with minor style concerns; no correctness regressions found and scope is well-bounded.
• The core reconciliation logic is correct, the dual-set deduplication design prevents double delivery, and the fallback/filter path in payload construction is sound. The main open items are: (1) the unbounded liveAssistantFallbackMessageIdsByFingerprint Map which could grow over a server's lifetime, and (2) the unconditional polling loop when commentary delivery is disabled. Neither is a correctness bug for the current usage — the Map entries are small strings and the polling is inexpensive — but both warrant attention before this pattern is reused in higher-throughput contexts.
• src/agents/assistant-output.ts (module-level Map growth) and src/agents/pi-embedded-runner/run/attempt.ts (unconditional polling loop).

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run/attempt.ts
Line: 1757-1766

Comment:
**Reconciler loop runs unconditionally regardless of commentary delivery setting**

The `commentaryReconciler` polling loop starts for every run, even when `params.onCommentaryReply` is `undefined` (i.e., when `commentaryDelivery` is `"off"` or the channel doesn't wire it). The reconcilers themselves are guarded by `params.onCommentaryReply` for actual delivery, but the 250 ms polling loop still executes `reconcileLiveStreamCommentary()` and `reconcileFinalizedAssistantOutputs()` on every tick, doing work — fingerprint hashing, message scanning, set lookups — that has no observable effect when delivery is disabled.

Consider skipping the loop entirely when there is no consumer:

```ts
const commentaryReconciler = params.onCommentaryReply
  ? (async () => {
      while (!stopCommentaryReconciler) {
        await reconcileLiveStreamCommentary();
        await reconcileFinalizedAssistantOutputs(activeSession.messages);
        if (stopCommentaryReconciler) break;
        await sleep(250);
      }
    })()
  : Promise.resolve();
```

This also slightly reduces the surface area for the loop-related races if live commentary is never used for the current channel.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/web/auto-reply/monitor/process-message.ts
Line: 507-514

Comment:
**Return value semantics changed when no final reply was queued**

Previously, the `if (!queuedFinal)` branch always returned `false`. It now returns `didSendReply`, which will be `true` if at least one live commentary message was successfully delivered — even if no final reply was ever produced by the agent (e.g., because the run ended silently or with an error after commentary was already sent).

Callers in `broadcast.ts` and `on-message.ts` currently discard the return value, so this is not a live regression. But the semantics shift is worth documenting: `processMessage` previously meant "a _final_ reply was sent to the user" and now means "any reply (commentary or final) was sent." If future callers distinguish these cases, the implicit contract change could cause subtle bugs.

A comment on the return type clarifying the new semantics would help:

```ts
// Returns true if any reply (including live commentary) was delivered to the user.
export async function processMessage(...)
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/assistant-output.ts
Line: 25-26

Comment:
**Unbounded module-level fingerprint Map**

`liveAssistantFallbackMessageIdsByFingerprint` is a module-level `Map` that is never cleared. Its purpose is correct — enabling stable fallback ID reuse when the same message content appears under different JS object identities across the live-stream path and the finalized-snapshot path. However, on a long-running server handling many diverse conversations, each unique content fingerprint produces a permanent entry that is never evicted.

The `liveAssistantFallbackMessageIds` WeakMap is fine since entries are GC'd once the message object is released. But the fingerprint `Map` holds strong string keys indefinitely. For a server processing thousands of structurally distinct messages over its lifetime, this accumulates without bound.

Consider adding a size cap with LRU-style eviction (e.g., delete the oldest entry once the map exceeds a threshold like 10,000 entries), or add a comment explaining the expected growth bound if it is genuinely small in practice.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 8e32ebf}

Addressed the two summary-only review notes in 94f3e9505ddec09135ba5d6021a094f18a7545d8:

• run/attempt.ts now skips the live-commentary polling loop entirely unless onCommentaryReply is wired for the current channel.
• process-message.ts now documents that processMessage() returns true when any user-visible reply was delivered, including live commentary.

The same commit also replaces the old module-global fallback-id cache with per-run reconciliation state and adds a regression for repeated identical unsigned assistant messages.

Closing this in favor of #57279.

The feature is still wanted, but this branch is too stale relative to current review expectations. #57279 is the clean replacement PR with the WhatsApp live commentary implementation on a fresh branch and verified WhatsApp behavior.