feat: add message reactions#98
Conversation
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs changes before merge. Reviewed July 18, 2026, 9:53 PM ET / July 19, 2026, 01:53 UTC. Summary Reproducibility: not applicable. as a bug report: this PR proposes a new capability. The contributor provides a high-confidence real-app execution path for the proposed behavior, but that does not replace the missing thread-path coverage. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Render and exercise reactions in both the main timeline and thread panel, add focused upgrade-path evidence for persisted reaction data, then land only if a maintainer sponsors reactions as a core ClickClack interaction. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug report: this PR proposes a new capability. The contributor provides a high-confidence real-app execution path for the proposed behavior, but that does not replace the missing thread-path coverage. Is this the best way to solve the issue? No. The architecture and proof are substantial, but rendering controls solely through MessageRow leaves thread roots and replies without the proposed capability; the narrower maintainable fix is to share the reaction surface with ThreadPanel and prove upgrade behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 146f76255bcc. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61a1869d66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <ReactionsBar | ||
| messageId={message.id} | ||
| reactions={message.reactions ?? []} | ||
| {currentUserID} | ||
| /> |
There was a problem hiding this comment.
This only wires ReactionsBar into the main timeline rows; the thread pane renders its root and replies directly in ThreadPanel.svelte and never includes the bar. As a result, reactions that are now hydrated and realtime-updated for selectedThread/replies are invisible and not toggleable for thread replies, which do not have a separate main-timeline row where the user could see them.
Useful? React with 👍 / 👎.
| body: JSON.stringify({ emoji }), | ||
| }); | ||
| } | ||
| const data = await api<{ message: { reactions?: Reaction[] } }>(`/api/messages/${messageId}`); |
There was a problem hiding this comment.
When the POST/DELETE has already succeeded but this follow-up GET /api/messages/:id fails transiently, execution falls into the same catch as a failed mutation and may roll back the optimistic add/remove even though the server committed it. This leaves the current row showing an error and the opposite reaction state until a later realtime/reload corrects it; separate mutation failures from reconciliation-refresh failures or keep the committed optimistic state when only the refresh fails.
Useful? React with 👍 / 👎.
| messages.some((message) => message.id === messageID) || | ||
| replies.some((reply) => reply.id === messageID) || | ||
| selectedThread?.id === messageID; | ||
| if (!isLoaded) { |
There was a problem hiding this comment.
When the reaction is for a message that is already loaded, this skips recording a pending snapshot and only mutates the current arrays. If a message-page or thread refresh that read data before the reaction resolves afterward, its stale response can overwrite the updated reactions because applyPendingReactionSnapshots has nothing to apply. Store the snapshot for loaded messages too, or version those reloads, so in-flight refreshes cannot revert the realtime reaction state.
Useful? React with 👍 / 👎.
61a1869 to
abfeb87
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
1 participant
Summary
Real behavior proof
tests/e2e/message-reactions.spec.tsruns against ClickClack's locally built production web bundle served by the real Go API. In one isolated run it:The captured real-app state is committed at
docs/proof/message-reactions-real-app.png, with reproduction details indocs/proof/message-reactions-proof.md.Focused browser run:
Validation
pnpm check— passed onabfeb871fc3065103f8ae8b4e53afd2bfaf91368pnpm exec playwright test tests/e2e/message-reactions.spec.ts— 10 passedgo test ./apps/api/internal/store/sqlite ./apps/api/internal/store/postgres -run '^TestGetThreadHydratesReactions$' -count=1— passedpnpm generate:sqlc— generated SQLite and PostgreSQL store code from the checked-in query sources