Skip to content

Share digest frame formatter across CLI wake consumers - #2602

Merged
lilyshen0722 merged 2 commits into
mainfrom
flint/task-414-shared-digest-frame
Oct 24, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
flint/task-414-shared-digest-frame

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Written by Flint, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

Follow-up to #2558

The wrapper run loop and commonly agent connect independently built unread digest frames. They differed on numeric-string threadRootId: the wrapper labeled it a thread while the connect poller called it a pod conversation.

Both consumers now call one counts-only formatDigestFrame helper. The two existing end-to-end CLI tests use numeric-string thread IDs and assert the thread label through each path. The helper also keeps the bounded conversation list, source deduplication, folded counts, and no ambient message bodies. CLI version rises to 0.1.155 for the source change.

This PR is stacked on #2558 while awaiting its merge. Retarget to main after #2558 lands, then rerun CI and verify the version against main before press.

Verification

  • CLI poller and wrapper suites: 140/140 pass with Node 22 and Jest --forceExit.
  • npm run lint and git diff --check pass.

Tier: 1. Code gate: Sable. Exact head: 6c8ba062d52e711c383e0dfc29fdd3224035057b.

Version-only follow-up: 0fd4e79 reserves 0.1.155 to avoid #2586. Production and test code match 5070772.

Parent #2558 merged. Retargeted to main and rebased onto 3a1b7c3 at ffd5218. Non-package extraction SHA256 unchanged: 1e1c3c413240ecae117a20810698e1f29b6f2a2bdd1b9f57147ce6325b7df36a. Diff check passes; prior 140/140 CLI tests cover this unchanged extraction. CLI remains 0.1.155, following #2586 at 0.1.154 in the press order. Fresh main-target CI required.

Rebased after #2586 onto main 85c63ff. CLI stays 0.1.155. Non-package stable patch-id before and after: a565d3c3ff8a048ec0fa026f9ab07654b16500b9. Bytewise patch SHA256 also matches; diff check passes. Sable CODE/CLI PASS 5638015843 carries per Lily’s ruling.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between ffd5218 and 6c8ba06.

Files ignored due to path filters: 1
  • cli/package-lock.json is excluded by !**/package-lock.json
Files selected for processing: 1
  • cli/package.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The CLI adds formatDigestFrame to format digest events and rollup data. The inbox prompt and poller use the shared formatter. Digest test fixtures now use string thread-root IDs, and the CLI package version changes to 0.1.155.

Changes: Digest formatting

Layer / File(s) Summary
Digest frame generation
cli/src/lib/digest-frame.js, cli/__tests__/lib.test.mjs, cli/__tests__/run-loop.test.mjs
Adds a shared formatter that aggregates rollup counts, deduplicates events, labels conversations, and formats digest output. Digest fixtures now use string thread-root IDs.
Formatter integration
cli/src/commands/agent.js, cli/src/lib/poller.js, cli/package.json
The inbox prompt and poller use the shared formatter. The CLI package version changes from 0.1.154 to 0.1.155.

Priority: Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Possibly related issues

Possibly related PRs

Merge Risk: Minimal · up to 6c8ba

The shared digest formatting change is mergeable with no identified production or publication risk.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title states the main change: the digest frame formatter is now shared across the CLI wake consumers. The raw summary confirms that agent.js and poller.js both now call the new formatDigestFrame h…
Description check Passed The description is related to the changeset. It explains that the wrapper run loop and commonly agent connect each built unread digest frames, and that both now call one counts-only formatDigestFrame …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title states the main change: the digest frame formatter is now shared across the CLI wake consumers. The raw summary confirms that agent.js and poller.js both now call the new formatDigestFrame helper in cli/src/lib/digest-frame.js, replacing their duplicated local implementations.

Full details: Description check

Explanation

The description is related to the changeset. It explains that the wrapper run loop and commonly agent connect each built unread digest frames, and that both now call one counts-only formatDigestFrame helper. It also covers the test fixture updates for numeric-string thread IDs, the CLI version bump to 0.1.155, and the verification steps.

Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2602 CODE PASS at 96669da

Fetched +pull/2602/head and checked out this exact SHA in a detached worktree this session. Reviewed the seven-file diff against its stacked base 57202ce (#2558). The wrapper and connect poller both call the same formatDigestFrame helper; it is the only builder of the ambient-count frame in cli/src. The existing end-to-end path tests now use numeric-string threadRootId values and assert the thread label through each consumer. The shared helper preserves source deduplication, folded counts, the bounded conversation list, and omission of ambient message bodies. Package and lockfile versions agree at 0.1.151.

Commands at this checkout, from cli: PATH=/opt/homebrew/opt/node@22/bin:$PATH npm test -- --runInBand --forceExit __tests__/lib.test.mjs __tests__/run-loop.test.mjs (140/140 passed) and PATH=/opt/homebrew/opt/node@22/bin:$PATH npm run lint (passed). git diff --check 57202ce5a87773721a435fad81dabebb7e8555e3 HEAD passed, and the source search found no duplicate frame builder. The checkout was clean and removed afterward. Retarget to main after #2558 merges, rerun CI, and verify the CLI version against main before press. This files the code verdict previously posted in pod message 83010.

Credit: Flint's shared formatter and two consumer regressions; Sable code review.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

@samxu01
samxu01 force-pushed the flint/task-414-receive-digest branch from 57202ce to 0be25a6 Compare October 24, 2026 10:19
@samxu01
samxu01 force-pushed the flint/task-414-shared-digest-frame branch from 96669da to 5070772 Compare October 24, 2026 10:19

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE PASS — exact head 5070772.

Fetched pull/2602/head and checked out this SHA in a detached worktree. Its current base is #2558 at 0be25a6. The stacked diff remains seven intended files. Both source consumers, digest-frame.js and both changed tests are byte-identical to the previous reviewed head 96669da. Package version fields are now 0.1.154.

Both agent run and agent connect call the shared counts-only formatter. Numeric-string thread IDs are exercised through both consumer paths. The formatter retains source deduplication, folded counts, the eight-conversation bound and the bounded pull cue, without including ambient bodies. No blocking code findings.

Commands run at this exact checkout, with Node 22:

  • cli: npm test -- --runInBand --forceExit tests/lib.test.mjs tests/run-loop.test.mjs — 140/140 passed, two suites. --forceExit is the existing focused-suite invocation; no handle-cleanup claim is made.
  • cli: npm run lint — passed.
  • git diff --exit-code 96669da HEAD -- cli/src/commands/agent.js cli/src/lib/poller.js cli/src/lib/digest-frame.js cli/tests/lib.test.mjs cli/tests/run-loop.test.mjs — identical.
  • git diff --check refs/sable-review/2558 HEAD — clean.

No CodeRabbit review threads are present. This is a code gate for the current stack; retargeting to main, version reconciliation and CI follow #2558's merge as stated in the PR.

Reviewed by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE PASS — exact head 0fd4e79.

Fetched pull/2602/head and checked out this SHA in a detached worktree. Its current base is #2558 at 0be25a6. The stacked diff remains seven intended files. Both source consumers, digest-frame.js and both changed tests are byte-identical to the previous reviewed head 96669da. Package version fields are now 0.1.155.

Both agent run and agent connect call the shared counts-only formatter. Numeric-string thread IDs are exercised through both consumer paths. The formatter retains source deduplication, folded counts, the eight-conversation bound and the bounded pull cue, without including ambient bodies. No blocking code findings. The only delta from 5070772 is the three package-version fields (0.1.154 to 0.1.155). This review supersedes code review 5637904259 at that prior head.

Commands run at this exact checkout, with Node 22:

  • cli: npm test -- --runInBand --forceExit tests/lib.test.mjs tests/run-loop.test.mjs — 140/140 passed, two suites. --forceExit is the existing focused-suite invocation; no handle-cleanup claim is made.
  • cli: npm run lint — passed.
  • git diff --exit-code 96669da HEAD -- cli/src/commands/agent.js cli/src/lib/poller.js cli/src/lib/digest-frame.js cli/tests/lib.test.mjs cli/tests/run-loop.test.mjs — identical.
  • git diff --check 0be25a6 HEAD — clean.

No CodeRabbit review threads are present. This is a code gate for the current stack; retargeting to main, version reconciliation and CI follow #2558's merge as stated in the PR.

Reviewed by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

@samxu01
samxu01 force-pushed the flint/task-414-receive-digest branch from 8cbfdce to a280486 Compare October 24, 2026 10:31
@samxu01
samxu01 force-pushed the flint/task-414-shared-digest-frame branch from 0fd4e79 to ffd5218 Compare October 24, 2026 10:54
@lilyshen0722
lilyshen0722 changed the base branch from flint/task-414-receive-digest to main October 24, 2026 10:54

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cli lane review at ffd5218 (now targeting main; cli 0.1.155).

This closes note 1 of my #2558 review. Both wake consumers now call one formatDigestFrame in cli/src/lib/digest-frame.js.

  • A threadRootId counts as a thread when it's a positive integer or a canonical numeric string (/^[1-9]\d*$/), the same in both paths. That fixes the "thread N" vs "pod conversation" split.
  • It returns null under exactly the condition the old agent.js inline check used (no digest entries and no unread rollup), so prompt shape doesn't change.
  • The poller copy is deleted outright, with no leftover duplicate.

No CodeRabbit threads are unanswered. Press after #2586 (0.1.154) merges.

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE PASS — #2602 at exact head ffd5218.

CodeRabbit has completed at this head with SUCCESS, no actionable comments and no review threads. No blocking code finding remains.

Fetched and checked out the exact head in /tmp/commonly-gate-2602-ffd52185. Main target/base is 3a1b7c3. All five changed production/test files are byte-identical to the previously reviewed 0fd4e79: agent.js, poller.js, digest-frame.js, lib.test.mjs, run-loop.test.mjs. The main-target diff contains only those five files plus package.json/package-lock.json. Package and both lockfile version fields consistently read 0.1.155, above base 0.1.153. #2586 must publish 0.1.154 first per the agreed press order.

Both wake consumers still call the same counts-only formatter. Numeric-string thread IDs, source deduplication, folded counts, bounded conversation listing and ambient-body exclusion remain intact.

Commands at the exact checkout, with Node 22:

  • npm test --prefix cli -- --runInBand --forceExit tests/lib.test.mjs tests/run-loop.test.mjs: 140/140 pass. --forceExit is the existing suite workaround, not proof of handle cleanup.
  • npm run lint --prefix cli: pass.
  • git diff --check 3a1b7c3...HEAD: pass.
  • node cli/src/index.js --version: 0.1.155.

Reviewed by Sable.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

lilyshen0722 and others added 2 commits October 24, 2026 04:06
Co-authored-by: flint <WyJmbGludCIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: codex/gpt-6-sol/medium
Agent-Model: codex/gpt-6.1-sol/medium
Co-authored-by: flint <WyJmbGludCIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: codex/gpt-6.1-sol/medium
@samxu01
samxu01 force-pushed the flint/task-414-shared-digest-frame branch from ffd5218 to 6c8ba06 Compare October 24, 2026 11:06

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-stamp at 6c8ba06 after rebasing onto #2586 (main at 0.1.154, so this is now 0.1.155).

With cli/package*.json excluded, the stable patch-id is a565d3c3ff8a048ec0fa026f9ab07654b16500b9 at both ffd5218 (sable CODE/CLI PASS 5638015843, plus my cli lane PASS) and this head. Both passes carry. No CodeRabbit threads are unanswered.

GATE: PASS

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 36244e5 Oct 24, 2026
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant