Repository navigation
Conversation
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused cross-platform UI fix that surfaces existing pending merge-back state without changing the send pipeline, data model, defaults, or infrastructure. An unresolved High finding does identify scenarios where the notice may precede a rejected send, so that correctness risk remains important to address separately. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (7)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe client runtime identifies pending merge-back transfers and builds notice copy. The web composer and mobile thread view display notices that include the source thread title when available. ChangesMerge-back notice
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Low Sequence Diagram(s)sequenceDiagram
participant ThreadRouteContent
participant resolvePendingMergeBack
participant useThreadShell
participant ThreadDetailScreen
participant ComposerMergeBackNotice
ThreadRouteContent->>resolvePendingMergeBack: selected thread projection
resolvePendingMergeBack-->>ThreadRouteContent: pending transfer and fork state
ThreadRouteContent->>useThreadShell: scoped source thread reference
useThreadShell-->>ThreadRouteContent: source thread title
ThreadRouteContent->>ThreadDetailScreen: pending merge-back notice data
ThreadDetailScreen->>ComposerMergeBackNotice: source title, fork count, idle-wait state
Suggested reviewers:
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | The description explains the problem, change, and verification in detail. However, the required scope approval is missing: it states that issue #15063 has no maintainer triage and does not establish a… |
Add a link to maintainer approval of the direction and scope, or explain why this qualifies as a very small, focused fix of an obvious bug without prior approval. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | #15063 requires a visible confirmation on the source thread that a fork was merged and its context will be included in a later message. The shared selector finds the newest pending merge-back, and web… |
| Out of Scope Changes check | ✅ Passed | The shared selector, web/desktop and mobile notices, and related tests all implement or verify the confirmation required by #15063. The added active-run and multiple-fork states clarify when that conf… |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: showing a pending-context notice after merging a fork back. |
Full details: Description check
Explanation
The description explains the problem, change, and verification in detail. However, the required scope approval is missing: it states that issue #15063 has no maintainer triage and does not establish an exemption for this change as a very small, focused fix of an obvious bug.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
The notice promised the merged context would ride on the next message even when the server would not use it: a send while a run is active is rejected or steered without the transfer, and pending merge-backs from more than one fork reject every send. The notice now says to send once the thread is idle, and warns when more than one fork is pending.
|
Note 🤖 Claude Fable 5.1 on behalf of Mnigos Closing to keep my open queue focused; happy to reopen if this is still wanted. |
Fixes #15063.
Problem
Merging a fork back switches to the source thread, which looks exactly as it did before.
thread.merge_backrecords amerge_backcontext transfer withstatus: "pending"on the source thread, but nothing renders a pending transfer: the timeline draws "Context handoff" only forhandoffitems, which exist once the next run consumes the transfer. Until the next message is sent there is no sign the merge worked.Change
Shared.
resolvePendingMergeBackTransferinclient-runtime(threadRelationships.ts, next toresolveMergeBackTargetThreadId) returns the newest pendingmerge_backtransfer targeting the thread, using the same rule the server uses when it picks the transfer for the next send (newestupdatedAt, last entry on a tie).resolvePendingMergeBackwraps it and returns{ transfer, forkCount, waitsForIdle }: how many distinct forks have a pending merge-back here, and whether a run is active (the same run statuses the server treats as blocking).pendingMergeBackNoticeturns that into the copy both clients use, in three states:The fork title falls back to "a fork" when the fork's shell is not loaded (an archived fork, for example).
Web / desktop. While a merge-back is pending, the composer notice stack shows an info notice with the same merge glyph as the Lineage merge button (a warning notice when more than one fork is pending). It follows the rule that a composer notice exists only if it changes what pressing Enter does: this one changes what context the next send carries. It has no dismiss and no state of its own. It clears by itself when the next run consumes the transfer (the existing "Context handoff" divider takes over) or when a newer merge-back supersedes it; merging the same fork again keeps a single notice. The item is built in a small
ComposerMergeBackNotice.tsxbeside the other banner helpers;ChatView.tsxonly memoizes the selector, the source thread lookup and the item, keyed on primitives so the banner stays stable while the projection streams.Mobile. The same selector and copy feed a one-line notice above the composer in the thread screen, in the style of the queued-edit banner, with the same lifetime.
The server and contracts are unchanged. One rare case stays as the server behaves today: a transfer the server cannot resolve on the next send (for example a merge-back recorded while the fork's run had no provider thread) stays
pending, so the notice stays until a newer merge-back replaces it. Two forks pending is a dead end on the server that this PR does not change: a new merge-back only supersedes pending ones from the same fork, nothing else clears a pending transfer, so sends on that thread keep failing. The notice warns about it instead of promising the context.Scope and approval
Filed bug #15063; no maintainer triage yet. The notice fits the composer-notice rule in #11999 ("a composer notice exists only if it changes what pressing Enter does"), which is still open. Both PRs edit the banner list in
ChatView.tsx, so whichever lands second gets a small textual conflict there; keep both sides when resolving.Verification
Observed result in a real web client. Headless Chromium, 1400×900, light mode, against
vp run dev. Comparedmainatfed41fa88bwith this branch on the same commit, using isolated homes, synthetic Git projects and a stand-in Claude provider. Both builds completed a source turn and a fork turn. After Lineage → merge back, both projections contained a pendingmerge_backtransfer, but only the branch immediately showed "Merged back from Merge demo" above the composer. Repeating the merge superseded the old transfer and left one notice. Sending the next source prompt consumed the transfer, removed the notice and displayed the existing "Context handoff" divider. That continuation remained running in the stand-in setup; its completion is not claimed.mainafter merge: no confirmationTests.
threadRelationships.test.tscovers the selector with 14 pending-transfer cases: newest wins, ties pick the last entry, and consumed, superseded, failed, resolved, other-thread and other-type transfers are ignored. 7 more cases cover the pending state and notice: an idle thread, an active run in each blocking status, distinct-fork counting (superseded and consumed transfers and repeat merges of one fork do not count), and the warning only when more than one fork is pending. The file passes 38 tests; 165 pass acrossthreadRelationships.test.tsandChatView.logic.test.ts. Client-runtime, web and mobile typecheck with no errors or warnings; targeted lint has no errors and no new warnings; format and knip are clean.Not checked. Mobile was typechecked but not visually verified. Only the idle single-fork flow was captured in a real client; the active-run and more-than-one-fork notices were not seen there and are covered by the unit tests only. Also not checked: native Electron, remote/relay/tunnel connections, other providers, real-model use of the merged context, dark mode, long fork titles alongside several other notices, and completion of the stand-in continuation.
Implemented with Claude Code (Claude Opus 5.5, coordinated by Claude Fable 5.1); tests, independent review and the observed result by GPT-6 Astra via Codex.