[tahodev]

Custom reactions were parsed from English summary text, losing emoji when summaries were localized or absent. Read associated_message_emoji across history, bulk snapshots, events, and message streams, with summary parsing retained for older schemas and null/empty column values.

Refs #392. Synthetic SQLite fixtures fail on main and pass with this change, including localized additions/removals, missing text, old schemas, null/empty values, and streaming. Independent Codex review through P2: scoped-clean. Exact-head CI is required before merge. No real Messages database or sending was used.

Thanks @tahodev for the implementation and @privatenumber for the report.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge.

What this changes

Prefer the stored custom reaction emoji across snapshots and streaming reads, retaining text fallback for missing or empty values.

Example: A type-2006 reaction stores 🔥 in associated_message_emoji while its Korean summary contains 🙂.

• Before: History and watch can report 🙂 from the summary.
• After: Both report the stored 🔥.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The focused repair resolves the earlier finding, but the required behavioral evidence gate remains unmet.
Proof confidence	🧂 unranked krab (1/6)	Real behavior proof is necessary before merge. See Before merge.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Custom reactions can disappear or display the wrong emoji when summary text is missing or conflicts with the stored reaction.
Reason: The reported database case and current source establish a concrete defect in existing reaction support, and the revised patch covers its known consumers.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep open: this PR fixes a current custom-reaction decoding bug, and the earlier watch-pipeline finding is resolved; no new actionable code defect was found.

Priority: P2
Reviewed head: 9c32a7a1b1c16080d4d78fe0a82ffd35824499bf

Before merge

• Add real behavior proof - The supplied evidence exercises IMsgCore through synthetic in-memory SQLite fixtures and fork CI; the body explicitly confirms no shipped CLI run against a real Messages database. The contributor should provide terminal screenshots or redacted output from history and watch --reactions showing database-only and conflicting-summary emoji cases, then request re-review; redact phone numbers and other private data. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.

Findings

None.

Tests

• Missing end-to-end proof: No supplied shipped-CLI history/watch run demonstrates stored-emoji precedence, and no base-failing/head-passing regression run is shown.

Agent review details

How this fits together

IMsgCore reads the Messages SQLite database and supplies decoded messages and reaction snapshots to CLI history, watch, and JSON-RPC consumers.

flowchart TD
  A[Messages database] --> B[Optional column detection]
  B --> C[Reaction snapshot queries]
  B --> D[Shared message row selection]
  C --> E[Reaction decoding]
  D --> E
  E --> F[History snapshots]
  E --> G[Watch and RPC events]

Loading

Technical review

Best possible solution:

Land consistent database-first reaction decoding through the existing query and message owners while preserving legacy fallback and removal semantics.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically omits a custom reaction when its emoji exists only in associated_message_emoji and no usable summary remains; this read-only review ran no target code.

Is this the best way to solve the issue?

Preferring the stored value addresses the cause, and the revised patch now covers snapshots, standalone events, and the shared streaming pipeline without adding a parallel subsystem.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 1bc0679663bf.

Provenance checked

• Custom emoji extraction changes intended behavior with a stated reason (70bf43f: The original implementation added custom reaction support by extracting the Reacted summary pattern; the PR explains why the stored emoji should take precedence while retaining that parser as fallback.)
• Reaction identity, ordering, and removal keeps the original intent (fix: unify poll and reaction state resolution #268: The merged repair unified reaction state resolution while preserving custom identity and raw timestamp ordering; this patch retains the reducer and changes only its emoji input.)
• Shared row decoding and message construction keeps the original intent (5a9f544: History records shared message decoding and RPC payload mapping, followed by query-layer and construction splits; the added field follows those existing owners.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Current main still has the bug: The default branch remains at the supplied release SHA; decodeReaction derives custom emoji solely from summary text and does not consume associated_message_emoji. (Sources/IMsgCore/MessageStore+Helpers.swift:106, 1bc0679663bf)
• Earlier streaming finding resolved: The introduced patch now selects, decodes, and carries associatedEmoji through the shared message pipeline into message construction; MessageWatcher consumes that pipeline through messagesAfterBatch. (Sources/IMsgCore/MessageStore+MessageConstruction.swift:20, 9c32a7a1b1c1)
• Concrete reported database case: The reporter at Read emoji reactions from associated_message_emoji instead of English summary text #392 describes 93 custom-reaction rows with populated emoji columns, including two whose emoji could not be recovered from text or attributedBody.
• Legacy behavior retained: Optional-column detection substitutes SQL NULL on older schemas; stringValue maps NULL to an empty string, preserving text fallback, while the existing reaction reducer retains ordering and removal mechanics. (Sources/IMsgCore/MessageStore+Reactions.swift:107, 9c32a7a1b1c1)
• Reaction feature history: Add Tapback and Emoji Reaction Support #8 introduced text-based custom emoji extraction; feat: reaction events in watch + react command #31 established watch reaction metadata; fix: unify poll and reaction state resolution #268 preserved reaction identity, timestamp ordering, and removals. (Sources/IMsgCore/MessageStore+Reactions.swift:107, 045234bc1105)
• Fork validation scope verified: https://github.com/tahodev/imsg/actions/runs/42519430207 completed lint and test steps successfully at this fork SHA; comparison confirms additional workflow changes and a TypedStreamParserTests expression split, so it is supporting validation rather than an exact-head target CI run. (.github/workflows/ci.yml, 69c01fbefd98)

Likely related people:

• Peter Steinberger: Raw commit 045234b adds Sources/IMsgCore/MessageStore+Reactions.swift:91 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 045234bc1105; files: Sources/IMsgCore/MessageStore+Reactions.swift)
• pangu25: Raw commit a03e81c adds Sources/IMsgCore/MessageStore+ReactionEvents.swift:54 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: a03e81c8b4ad; files: Sources/IMsgCore/MessageStore+ReactionEvents.swift)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Review metrics

Metric	Value	Why it matters
Patch scope	Production +34/-8 lines; tests +148; changelog +2	The production growth threads one optional value through existing owners, and fixtures exercise distinct read surfaces and fallback cases.

Root-cause cluster

Relationship: same_root_cause
Canonical: #392
Summary: This PR repairs the emoji-column omission documented by the canonical issue.

Members:

• canonical: Read emoji reactions from associated_message_emoji instead of English summary text #392 - The report identifies the same stored column, custom reaction types, and required fallback behavior.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Labels

Label changes:

No label changes.

Label justifications:

• P2: Custom reaction metadata can be omitted or decoded incorrectly, with limited scope and no established urgent regression.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)

• reviewed 2026-11-08T17:41:29.421Z sha b3d2d34 :: needs real behavior proof before merge. :: [P2] Pass the stored emoji through the message pipeline used by watch

Reviewed November 8, 2026, 3:27 PM ET / 20:27 UTC (Revision 2).

[tahodev]

@clawsweeper re-review

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/42520294490
• Updated: 2026-11-08T20:28:52.509Z

[tahodev]

I can't provide a real-device run from this environment. The evidence remains synthetic in-memory SQLite tests and fork CI; I have not run the shipped CLI against a real Messages database, so I can't claim real-device proof.

[steipete]

Reviewed code and fixture proof; exact-head CI is green.

[tahodev]

Thanks for reviewing and merging this!