Skip to content

feat(agents-mobile): session comments (desktop parity)#4587

Open
balegas wants to merge 12 commits into
mainfrom
worktree-mobile-comments
Open

feat(agents-mobile): session comments (desktop parity)#4587
balegas wants to merge 12 commits into
mainfrom
worktree-mobile-comments

Conversation

@balegas

@balegas balegas commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Brings session comments to agents-mobile, closing the parity gap with the desktop UI (comments shipped in #4551). Part of the agents-mobile ↔ agents-desktop parity track (follows #4564, #4553).

The mobile session timeline is a shared web component (SessionChatLogDomEmbed) rendered inside an Expo-DOM WebView, while the composer is native. Because the embed already merges comments via useEntityTimeline and renders CommentBubble, reading comment bubbles already worked on mobile — this PR adds the missing write, reply, and dedicated-view surface.

What's included

  • Comment mode in the native composer. A compact Message / Comment toggle (top of the composer), shown only when the entity type declares the comments collection and the user has write access. Comment mode reuses the desktop's createSendCommentAction verbatim (optimistic insert + authenticated POST /collections/comments), disables attachments/slash, and posts plain-text comments. Editing a queued message resets to prompt mode (mirrors desktop) so the comment path can't hijack an edit.
  • Tap-a-row-to-reply. A new optional Expo-DOM callback onRequestReplyToComment(target) is threaded SessionChatLogDomEmbed → EmbedApp → ChatLogView → EntityTimeline.onReplyToRow. Tapping "reply" on a timeline row forwards the SelectedCommentTarget (plain JSON) to the native shell, which switches the composer to comment mode, focuses the input, and shows a reply banner.
  • Optimistic-comment bridge. The composer (native) and timeline (embed) run in separate JS contexts with separate stream dbs, so the optimistic row inserted on the native side isn't visible in the embed on its own. Still-pending comments are forwarded across the boundary and projected into the embed timeline (deduped by key against synced rows), mirroring the existing inlineQueuedMessages bridge — so a posted comment renders immediately. Their ~pending orders keep them in the same bottom band the shared query uses, so ordering matches desktop.
  • Comments-only view. A new comments EmbedViewId renders buildCommentsTimeline (via a commentsOnly prop on the chat-log embed) with a comment-locked native composer, reachable from a new gated Comments entry in the session menu.

The shared agents-server-ui changes are additive and optional — desktop callers of ChatLogView (embed-only) are unaffected. No server / runtime changes.

Test plan

  • @electric-ax/agents-mobile + @electric-ax/agents-server-ui — typecheck clean
  • Full suites pass (server-ui 102, mobile 84), incl. the reused comments.test.ts (send action, buildCommentsTimeline, target encode/decode)
  • ESLint + Prettier clean

No new automated tests: the new logic lives in deeply-integrated screens/providers, and neither package has a render harness for them (mobile has no component-render setup; server-ui only renders self-contained components). The reused comment primitives stay covered by comments.test.ts.

Known limitation

  • Comments-only view target-click. Tapping a reply's timeline-row target snapshot doesn't scroll (that row isn't in the filtered comment list); tapping a comment target works. Target-click in the main chat view works fully. A "switch to full timeline + focus" bridge would close this.

🤖 Generated with Claude Code

balegas and others added 2 commits June 15, 2026 23:08
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add comments to the native app: a prompt/comment mode in the native
composer, tap-a-row-to-reply forwarded from the embed timeline over a
new Expo-DOM callback, and a dedicated comments-only view reachable
from the session menu. Reading comment bubbles already worked via the
shared chat-log embed; this adds the write + reply + dedicated-view
surface. No server/runtime changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Electric Agents Desktop Builds

Build artifacts for commit 7f02888.

Platform Status Artifact
macOS Apple Silicon Passed DMG
macOS Intel Passed DMG
Windows x64 Passed Installer
Linux x64 Passed AppImage / deb

Workflow run

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 13.33333% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.83%. Comparing base (c1310b9) to head (7f02888).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...agents-server-ui/src/components/views/ChatView.tsx 0.00% 21 Missing ⚠️
packages/agents-server-ui/src/embed/EmbedApp.tsx 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4587      +/-   ##
==========================================
+ Coverage   57.31%   58.83%   +1.52%     
==========================================
  Files         341      384      +43     
  Lines       39765    42510    +2745     
  Branches    11559    12183     +624     
==========================================
+ Hits        22791    25011    +2220     
- Misses      16934    17421     +487     
- Partials       40       78      +38     
Flag Coverage Δ
packages/agents 72.82% <ø> (ø)
packages/agents-mcp 77.70% <ø> (?)
packages/agents-mobile 78.81% <ø> (ø)
packages/agents-runtime 82.62% <ø> (-0.05%) ⬇️
packages/agents-server 75.26% <ø> (-0.03%) ⬇️
packages/agents-server-ui 7.40% <13.33%> (+0.02%) ⬆️
packages/electric-ax 46.42% <ø> (ø)
packages/experimental 87.73% <ø> (?)
packages/react-hooks 86.48% <ø> (?)
packages/start 82.83% <ø> (?)
packages/typescript-client 91.83% <ø> (?)
packages/y-electric 56.05% <ø> (?)
typescript 58.83% <13.33%> (+1.52%) ⬆️
unit-tests 58.83% <13.33%> (+1.52%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Electric Agents Mobile Build

Local mobile checks ran for commit 7f02888.

The EAS Android preview build was skipped because the mobile-eas-build label is not present.
Add the mobile-eas-build label to this PR to produce an installable preview build.

Workflow run

balegas and others added 6 commits June 16, 2026 12:20
- Compact, right-aligned Message/Comment toggle pill (was a full-width
  orphaned row).
- Reset to prompt mode when editing a queued message so the comment
  branch can't hijack an edit (mirrors desktop startEditing).
- Match desktop reply-banner wording ("Reply to ...").
- Document the native/embed cross-context gap: optimistic comments
  render after stream sync, not instantly (follow-up: comment bridge).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nner

- Bridge optimistic comments from the native composer's db into the
  embed timeline (mirrors the inlineQueuedMessages bridge): forward
  still-pending comments and project them, deduped by key against
  synced comments. A posted comment now renders immediately and
  reconciles in place instead of popping in only after stream sync —
  desktop parity (shared ordering untouched).
- Render the Message/Comment switch above the reply banner.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ity; move switch atop composer

- Revert the optimistic-comment bridge: it appended projected comments
  rather than letting the shared createEntityTimelineQuery sort them,
  diverging from desktop ordering. Mobile now uses the identical shared
  timeline path, so message/comment ordering matches the desktop app.
  (Trade-off, accepted: on a device a posted comment appears on stream
  sync rather than optimistically.)
- Move the Message/Comment switch to the top of the composer card so it
  no longer sits in the seam between the queued drawer and the input.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The reply banner sat between the queued drawer's open bottom and the
input, breaking that connection. Render it above the drawer so the
drawer connects directly to the input again: toggle, reply banner,
drawer, input.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@balegas balegas added the claude label Jun 16, 2026
Restore the optimistic-comment bridge so a posted comment renders
immediately instead of waiting for the embed's stream to sync. The
native composer's optimistic insert lands in its own (native) db; the
still-pending comments are forwarded across the Expo-DOM boundary and
projected into the embed timeline (deduped by key against synced rows),
mirroring the existing inlineQueuedMessages bridge. The `~pending`
orders keep them in the same bottom band the shared query uses, so
ordering still matches desktop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown

Claude Code Review

Summary

Incremental review of commit 7f02888d4 since iteration 2. This commit is a focused follow-up that resolves both remaining open items: showStop now excludes comment mode on mobile, and formatReplyBannerLabel is deduplicated into shared lib/comments.ts with a unit test. The change is small, correct, and well-scoped — nothing new to flag.

Review of the new commit (7f02888)

  • showStop fix is correct. SessionScreen.tsx:692 now prepends !commentMode && to the showStop predicate, mirroring desktop's MessageInput.tsx:223. commentMode is in scope (defined at :592), so in comment mode (including the dedicated Comments view) the Post button no longer silently becomes "Stop generating" while the agent runs. Resolves iteration-2 Important Working with Postgres WAL format from Elixir #1.
  • formatReplyBannerLabel deduped cleanly. The helper moved to comments.ts:86 as an exported function and is imported by both MessageInput.tsx:14 and SessionScreen.tsx:31; the two local copies are deleted. Resolves iteration-2 Suggestion Working with Postgres WAL format from Elixir #1 and prevents future drift.
  • Null-handling unified — a small robustness gain for mobile. The shared signature is SelectedCommentTarget | null (the desktop variant). Mobile's old copy took a non-null SelectedCommentTarget; the call site (SessionScreen.tsx:950) is already inside a commentMode && commentTarget && guard so it only ever passes non-null, but the shared null-safe version is strictly safer. Type-checks fine since widening a non-null arg to | null is sound.
  • Test added. comments.test.ts covers the lowercase-first-char behavior and both fallback paths (null and blank label). The target() helper's { label }-only snapshot is valid against CommentSnapshotValue (only label is required). Good — exactly the kind of pure helper worth pinning.

No new correctness, security, or convention concerns. Both @electric-ax/agents-mobile and @electric-ax/agents-server-ui are private ("private": true), so no changeset is required. No server/runtime contract changes.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None — both iteration-2 Important/Suggestion items are now addressed.

Suggestions (Nice to Have)

Carried over from earlier iterations, still low priority and unchanged by this commit:

  1. Silent no-op when comment send is unavailablesend() is a no-op when sendCommentAction is null. A dev-time warning or disabled affordance would make the failure mode visible.
  2. Bridge test coverage — the optimistic-comment bridge (projectedComments dedup, the ~pending filter, the key-signature gate) remains untested (Codecov 0% patch). The new formatReplyBannerLabel test shows the pattern works well for the pure helpers; extracting and pinning the bridge's pure logic would be worthwhile if it grows.

Issue Conformance

No linked issue (soft warning per convention). PR body is thorough and accurate; scope matches. The one documented known limitation (comments-only view target-click doesn't scroll to non-comment rows) is acknowledged as a follow-up.

Previous Review Status

With these resolved, the PR looks ready from a correctness and conventions standpoint.


Review iteration: 3 | 2026-06-16

…banner label

Address review feedback:
- showStop now excludes comment mode (mirrors desktop MessageInput), so
  the Post button on the comments surface never becomes a Stop-generating
  control while the agent is running.
- Move formatReplyBannerLabel into shared lib/comments.ts and import it on
  both desktop and mobile instead of duplicating; add a unit test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@balegas

balegas commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed in 7f02888.

Fixed

  • Important Working with Postgres WAL format from Elixir #1 (showStop in comment mode). Added !commentMode to the showStop condition so the Post button on the comments surface (and the comments-only view) never becomes a Stop generating control while the agent is running — mirrors desktop MessageInput.tsx.
  • Suggestion Working with Postgres WAL format from Elixir #1 (duplicated formatReplyBannerLabel). Moved it into shared lib/comments.ts (no React-Native dep) and import it on both desktop and mobile; removed both local copies. Added a unit test for it (lowercase-first-char + Reply fallback).

Deferred (reasoning)

Verified locally: both packages typecheck, ESLint + Prettier clean, full suites pass (agents-server-ui 104, agents-mobile 84).

The native shell always passes the reply callback to the chat-log embed,
so the timeline showed a reply button even for entity types that don't
declare the comments contract. Gate onReplyToRow/onCommentTargetClick on
commentsEnabled in ChatLogView, mirroring desktop GenericChatBody.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@netlify

netlify Bot commented Jun 16, 2026

Copy link
Copy Markdown

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit 2641835
🔍 Latest deploy log https://app.netlify.com/projects/electric-next/deploys/6a315d256706060008c4aba2
😎 Deploy Preview https://deploy-preview-4587--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

The native composer stops forwarding an optimistic comment as soon as
its own stream syncs it, but the embed renders off its own (separate)
stream — so if native synced first the projected row blinked out before
the embed's authoritative row arrived. ChatLogView now latches optimistic
comments and only drops one once THIS embed's stream delivers it (with a
timeout safety net for a failed POST). Render = (incoming union latched)
minus synced, so a posted comment shows on the first frame and survives
the hand-off without flicker.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@balegas balegas added shepherd and removed shepherd labels Jun 16, 2026
@balegas balegas added shepherd and removed shepherd labels Jun 16, 2026
@balegas balegas closed this Jun 16, 2026
@balegas balegas reopened this Jun 16, 2026
@balegas balegas removed the shepherd label Jun 16, 2026

@msfstef msfstef left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks reasonable but it needs a rebase and also I would appreciate screenshots/video for the feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants