Show "Fork from here" button on the mobile chat#4413
Open
kevin-dp wants to merge 11 commits into
Open
Conversation
5a459e3 to
56c6823
Compare
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## kevin/fork-at-message #4413 +/- ##
==========================================================
- Coverage 47.36% 35.47% -11.90%
==========================================================
Files 253 184 -69
Lines 19797 13822 -5975
Branches 6230 4674 -1556
==========================================================
- Hits 9377 4903 -4474
+ Misses 10401 8904 -1497
+ Partials 19 15 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
msfstef
approved these changes
May 26, 2026
Contributor
msfstef
left a comment
There was a problem hiding this comment.
can probably have smaller comments
First step of fork-at-message. Introduces the { offset, subOffset }
pair that addresses one event on a Durable Stream - maps 1:1 to
PR #347's Stream-Fork-Offset + Stream-Fork-Sub-Offset header pair
when forking.
- EventPointer.offset = null represents "anchor at stream start"
(translates to omitting Stream-Fork-Offset on the wire).
- formatPointerOrderToken produces lex-sortable
stream:<padded-offset>:<padded-subOffset> tokens, kept format-
compatible with the existing single-offset _timeline_order
prefix so existing LIKE 'stream:%' query matchers keep working.
- comparePointers for explicit ordering.
- STREAM_START_POINTER sentinel for the no-batches-yet state.
The module's doc comment records why sub-offsets are computed
locally (PR #347 ships fork-side only; the read-side spec is
explicitly reserved for a future revision) and the limitation that
follows (local counter is correct for fresh reads from offset 0,
which is the only thing we use today).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second step of fork-at-message. The per-row side-table on each
EntityStreamDB collection (__electricRowOffsets) now stores the
{ offset, subOffset } pair instead of a single offset string. The
pointer is computed in onBeforeBatch by pairing each item's
position-in-batch with the END offset of the *previous* batch — the
shape PR #347's Stream-Fork-Sub-Offset header expects when forking.
- entity-stream-db: track previousBatchOffset across batches;
per-item pointer = { offset: previousBatchOffset, subOffset: i+1 }.
Replaced legacy formatStreamTimelineOrder with the shared
formatPointerOrderToken helper. applyEvent synthesizes a single-
item pointer for in-process events (subOffset=1, offset from
header or a monotonic local: token).
- entity-timeline: order-token derivation uses
formatPointerOrderToken. orderTokenToHistoryOffset is now a no-op
since the order token IS already a stable string representation
of the pointer.
- setup-context: row comparison via comparePointers.
- context-factory: readContextHistoryOffset formats the pointer
through the same formatter so loadContextHistory round-trips
lookups.
- Tests updated to construct pointer objects from the existing
offset(index) helpers; one superseded_at_offset assertion updated
to match the new stream:<padded-offset>:<padded-subOffset> format.
All 519 existing tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #347 (Stream-Fork-Sub-Offset, the dependency for fork-at-message) has been rebased onto PR #350. The rebase needed one extra fix: PR size (payload + 5 bytes overhead), so the sub-offset prefix write in PR #347 had to be updated to match or chained sub-offset forks fail. PR #347 is now retargeted to base on PR #350's branch and its CI is green. pkg.pr.new has been re-published with the combined PR #350 + PR #347 + fork-frame fix under the @347 URL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements Phase 3 of docs/fork-at-message.md. A fork PUT can now
carry an optional pointer `{ offset, sub_offset }` (snake_case on the
wire) addressing an event on the source root's `main` stream:
- `streamClient.fork(path, source, { forkPointer })` emits
`Stream-Fork-Offset` + `Stream-Fork-Sub-Offset` per PR #347.
- `streamClient.readJsonWithPointers(path)` reads a JSON stream and
yields each item with a `{ offset, subOffset }` pointer minted by
counting items locally within each `JsonBatch`, mirroring how
`entity-stream-db.ts`'s `onBeforeBatch` already mints them on the
runtime side. Pointers are stable across the runtime and the server.
- `entity-manager.forkSubtree({ forkPointer })`:
- validates the pointer against the root's `main` stream
(`HTTP 400` on offset/sub-offset mismatch),
- filters the root's manifest to entries with pointer ≤ X,
- drops descendants whose manifest entry on the root was filtered
out (their subtrees go with them — Q3 "strict on root"),
- threads the pointer only to the root's `main` fork; `error` and
shared-state streams clone at HEAD (Q2).
- `forkBodySchema` gains `fork_pointer`; the route handler translates
snake_case → camelCase at the body-parse boundary.
- `createForkEntity(url, { pointer })` extends the UI client helper.
Phase 4 ("Fork from here" affordance on user-message rows) builds on
this in a follow-up commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements Phase 4 of docs/fork-at-message.md. `UserMessage` gets a hover-revealed icon button (top-right of the bubble) that re-rolls the conversation starting at the clicked message. The conversation up to and including the previous completed response is forked into a new entity; the new entity boots idle. `ChatView` computes the fork anchor map by walking the timeline rows in order, tracking the latest preceding `runs` row with `status === 'completed'`, and looking up that run's pointer via `db.collections.runs.__electricRowOffsets`. Inbox rows with no preceding completed run (first message, in-flight run) get no entry in the map, which suppresses the affordance — see Q4 "Affordance is disabled when…". UX matches the existing SearchPalette and SplitMenu fork flows: silently swallow errors, navigate to the new entity URL on success. Note: this hasn't been exercised in a browser yet — the agents-server backend needs Docker, which isn't running locally. Type-checks clean across the affected packages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous implementation compared each event's runtime-minted
pointer to the fork pointer via `comparePointers`. That breaks because
the two pointers live in different coordinate systems:
- The runtime mints pointers from LIVE delivery-batch boundaries
(`previousBatchOffset` = end of the previous HTTP response).
- The agents-server's pre-read (`readJsonWithPointers`) groups the
same events into HISTORICAL batches, which can be arbitrarily
chunked by the server — often into a single big batch on a
one-shot `live:false` read.
A pointer minted live as `{X, N}` won't match any historical batch's
anchor on the server side, so the previous validation rejected every
real pointer with "fork_pointer.offset does not match any chunk
anchor", and the manifest filter let all events through (no event
pointer ever matched).
Switch both validation and filtering to canonical CUMULATIVE position
in the source's flattened history: walk events, group by their TRUE
`headers.offset` (the per-event Stream-Next-Offset, stored at write
time and stable across reads). Translate the fork pointer to "count
of events with headers.offset ≤ anchor, plus subOffset" — this matches
the PR #347 server's "N flattened messages past anchor" interpretation
regardless of how delivery was chunked.
Verified end-to-end with a running stack: two completed Horton runs,
fork at pointer `{offset: <run-0-completion-batch-end>, sub_offset:
3}` produced exactly the expected 10-event truncated history; bad
offset and out-of-range sub_offset both surface 400s; no-pointer
fork still produces a HEAD clone unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #347's `Stream-Fork-Sub-Offset` addresses items WITHIN A SINGLE LOG ENTRY (the first one past the anchor), not items globally past the anchor. The previous pointer minting used the live-delivery batch's anchor + position-in-batch, which is only equivalent to the server's semantic when each delivery batch contains exactly one log entry. During catch-up reads, the server can combine multiple log entries into one HTTP response. The runtime's `onBeforeBatch` then minted sub-offsets that span entry boundaries; when the UI later tried to fork at one of those pointers, the durable-streams server interpreted the sub-offset against only the FIRST entry past the anchor and either picked the wrong event or returned 400 "overshoots". Fix: group items in a batch by their `headers.offset` (the per-event Stream-Next-Offset, stamped at write time). Items sharing an offset belong to the same log entry; sub-offsets reset to 1 at each entry boundary; the anchor for an entry is the END of the preceding entry (`previousBatchOffset` for the first entry in a batch). Also drops the now-unneeded `comparePointers` import in entity-manager.ts (the manifest filter switched to canonical position in the previous commit, and the position lookup no longer compares pointers directly). Also tightens that filter to handle `null`-anchor fork pointers correctly (stream-start: no events precede the anchor). Verified end-to-end on a running stack with two completed Horton runs, where the live read happened to chunk text/text_delta across two separate batches. The runtime mints the run-completion pointer exactly as the server's first-log-entry-past-anchor interpretation expects, and the fork produces the right truncated history. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`forkSubtreeInner` previously called `waitForIdleSubtree` for every fork, which polls until *every* entity in the subtree — including the root — flips to `status: 'idle'`. The runtime keeps a worker warm for five minutes after the last `handler returned` (so a follow-up message hits a hot process instead of paying cold-start), so an entity that's visibly "done" in the UI still reports `running` for that whole window. The default fork wait is 120 s, which means clicking the fork button right after the response lands hangs for two minutes and then 409s — exactly the case the affordance is designed for. The full-subtree-idle constraint is necessary for HEAD-clones: those read the source's state at "now," and concurrent writes during the read produce a torn snapshot. For pointer-forks it's strictly unnecessary: we read the source's `main` historically up to the chosen offset, and concurrent writes past that offset can't affect anything we capture. The Q3 manifest filter only keeps events with `headers.offset ≤ pointer`, also frozen-in-time. Skip the root-idle wait when `opts.forkPointer` is set. Kept descendants (Q3 "loose on descendants") are still HEAD-cloned, so they still need to be idle before we read their snapshots — wait+ lock only those once `computeEffectiveSubtree` has identified them. The HEAD-clone path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove pointers to the unmerged durable-streams PR (#347) and to the local-only RFC doc (`docs/fork-at-message.md`, including its Q2/Q3/Q4 section callouts), and replace one special ellipsis character with three dots. Comments now stand on their own — they describe the behaviour, not where the spec lives. No behaviour change.
f8313f9 to
0be3760
Compare
56c6823 to
1e0c3f1
Compare
Two pieces:
1. `ChatLogView` (the view the mobile DOM embed mounts under
`view='chat-log'`) now computes the same `forkFromHereByInboxKey`
map that `ChatView` has on the web. Without this the embed's
`EntityTimeline` received no per-row callbacks and `UserMessage`
stayed without the button regardless of any CSS.
2. `UserMessage.module.css` gains a scoped reveal —
`:global(html[data-electric-mobile-dom='true']) .forkButton { opacity: 1 }`
— so the button is visible by default on touch (the web's
`.bubble:hover` rule never fires there). Placing the rule inside
the CSS module (rather than in a sibling stylesheet that targets
the hashed class by substring) means whichever bundler is in play
— Vite for the web/desktop bundles, Metro for the Expo DOM embed —
hashes `.forkButton` consistently with the JSX.
The mobile package itself doesn't change. The embed already mounts
`ChatView` → `EntityTimeline` → `UserMessage`, the fork POST already
routes through `serverFetch()` in the embed's JS context, and
post-fork navigation already routes through `onRequestOpenEntity` →
`openSession(target)`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1e0c3f1 to
5e2d693
Compare
dfe647d to
d465213
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #4410: extends the per-message "Fork from here" affordance to the mobile chat (and any other surface that mounts agents-server-ui via an Expo "use dom" embed).
What changes
Two source files, both in
packages/agents-server-ui:src/components/views/ChatView.tsx—ChatLogView(the view the mobile embed mounts underview='chat-log') now computes the sameforkFromHereByInboxKeymap thatChatViewalready had on the web side. It pullsdbfromuseEntityTimeline,forkEntityfromuseElectricAgents, walksvisibleRowsto find the latest preceding completedrunsrow's pointer, and threads the per-row callback toEntityTimeline. Without this,UserMessagerendered in the mobile embed never received anonForkFromHereprop and the button stayed off the DOM regardless of CSS.src/components/UserMessage.module.css— adds a touch-targeted reveal rule scoped via:global(html[data-electric-mobile-dom='true']) .forkButton { opacity: 1 }. Touch devices don't fire:hover, and.bubble:focus-withinonly kicks in after a tap, so without the override the button stayed invisible until interaction. The reveal lives inside the CSS module (rather than a sibling stylesheet that targets the hashed class by substring) so whichever pipeline is in play — Vite for the web/desktop bundles, Metro for the Expo DOM embed — hashes.forkButtonconsistently with the JSX.The mobile package itself doesn't change. The embed already mounts
ChatView→EntityTimeline→UserMessage, the fork POST already routes throughserverFetch()running in the embed's JS context, and post-fork navigation already routes throughonRequestOpenEntity→openSession(target). OnceChatLogViewproduces the callback map and CSS makes the button visible, the whole flow lights up.Why split from #4410
#4410 was framed as the web/desktop release. The mobile surface depends on the same wiring but ships separately so the web work can land without being held by mobile-specific UX polish. Once both merge, the affordance is uniform across web, desktop, and mobile.
Test plan
pnpm -C packages/agents-mobile ios.-fork-<hash>; the new session contains only the first exchange.✓ done.data-electric-mobile-domattribute the embed sets).🤖 Generated with Claude Code