镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(web): keep earlier PR selections and findings in the composer - #25136

Draft
juliusmarminge wants to merge 5 commits into
mainfrom
fix/issue-24960
Draft

juliusmarminge wants to merge 5 commits into
mainfrom
fix/issue-24960

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Problem

Adding a second PR code selection or review finding to an existing composer removed the first attachment. An untouched handoff could also lose its request text, and repeated additions duplicated the PR reference.

Change

Accumulate selection and finding handoffs in the existing draft, including restored drafts. Scope attachment IDs to the PR URL, refresh repeated findings with their latest content, and keep repeated requests and PR references single. Both PR entry points use the same composer-store write; Ask, Explain and other replacing actions retain their existing behavior.

Scope and approval

Fixes #24960

GitCafe: #24960

Triaged bug via the automated issue flow.

Verification

  • Reproduced in the real web app: add line 25, then line 26 to the same composer; the first chip disappeared. The same loss occurred when adding a second finding.
  • Red-first regression tests cover accumulated selections and findings, cross-repository identity, restored drafts, refreshed finding content and repeated requests. The focused logic suite passes 79/79.
  • Composer context and draft-store suites passed 84 tests before the final localized follow-up.
  • vp run check:changed: format, lint, affected-package typecheck, web knip and focused tests pass. The scripts knip phase reports the unrelated main-branch dependency agent-device in scripts/package.json; left untouched.
  • Shared web logic covers browser and Electron entry points. No mobile code, provider protocol or wire contract changed.

Evidence gate remains pending: the recordings demonstrate the bug and earlier fix, but the full matrix has not been recaptured at the final head. Both final code reviews returned MERGE. Proof audit pass 2 returned INSUFFICIENT for final-head I1–I10 and I12, including post-reload addition, repeat suppression, refreshed finding payload, reverse states and settled captures. This draft is not ready for the bugfix gate.

Evidence index · Coverage report · Fixture matrix

Real web app in T3 preview, desktop UA; requested CSS 1440×900 / 390×844, DPR 2. Audit found desktop exports at 1280×800, default font scaling, light/dark. Sanitized findings enter the existing activity query atom; real panel controls and composer handle every action. The host PR was read-only. Driver, fixture, state transcript and diff generator are uploaded in the index; none is committed.

The following are diagnostic captures, not a clean visual regression proof. Baseline is e3592efe7f; desktop after is the earlier 058d0be387; narrow light after is final bd24a99a27; narrow dark was captured during integration and its exact dirty snapshot is unavailable.

Configuration Before (e3592efe7f) After (revision above) Diff 0, amplified 8× Diff >16/255
1440light before1440light.png after1440light.png diff-1440light-zero-x8.png diff-1440light-threshold16.png
1440dark before1440dark.png after1440dark.png diff-1440dark-zero-x8.png diff-1440dark-threshold16.png
390light before390light.png after390light.png diff-390light-zero-x8.png diff-390light-threshold16.png
390dark before390dark.png after390dark.png diff-390dark-zero-x8.png diff-390dark-threshold16.png

Recordings: start with the user sentinel, add A25, then B26; for findings, Fix T1 then T2, and mix a selection with a finding. Each recording contains the corresponding sequence. Desktop pairs below use baseline e3592efe7f and earlier after 058d0be387.

Flow Before After
I1 selections A then B beforeI1.mp4 afterI1.mp4
I3 findings T1 then T2 beforeI3.mp4 afterI3.mp4
I4 selection / finding / selection beforeI4.mp4 afterI4.mp4
I5 unanchored remark / failed check beforeI5.mp4 afterI5.mp4
I6 aggregate findings beforeI6.mp4 afterI6.mp4
I7 Ask / Explain replacement beforeI7.mp4 afterI7.mp4

Final narrow B retry: after390lightBVideo.mp4; final reload readback: afterReloadVideo.mp4.

What else changed in captures: live PR metadata/diff changed, baseline precedes the main merge, and narrow frames contain notification/reconnect transients. Full-frame pixel diffs therefore cannot establish that unrelated rendering stayed unchanged. Narrow stills fail the clean settled-frame requirement.

Unverified: final-head repeats and edited-finding payload; remove/re-add interaction; complete fresh-draft and global PR routes; optional guide flow; final narrow findings in both themes; exact-final-head desktop matrix. Reload readback preserved both chips, but adding a new selection after reload was proven by the regression test rather than a final recording. No provider turn was sent. CI and the evidence gate are returned to the dispatcher; no bugfix label was added.

Models: GPT-6.1-sol (Codex harness) and Claude Opus 5.5 (Claude Code harness), orchestrated through T3 Code.

juliusmarminge and others added 5 commits October 25, 2026 21:53
)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…one (#24960)

Each pull request handoff into an existing composer replaced the last
handoff's text and chips. Handoffs now append their text and chips,
dedupe chips by id so the PR reference appears once, and skip only an
ask that is already in the composer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Appending every handoff kept an Ask or Explain chip's "do not change any
code" instructions under a later Fix, dropped Explain's instructions after
a selection (same chip id), and dropped a second repository's PR #42
because context ids named only the number.

Selection and finding handoffs now mark themselves as appending and extend
the previous handoff only when it appended too; any other handoff replaces
the previous handoff's text and chips, as before. The replaced text is
tracked as the tail the store actually wrote, chip references included.
Context, selection and finding ids include the pull request URL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#24960)

After a reload the composer forgets what the last handoff wrote, so adding
another selection dropped the earlier chip. Infer from the persisted chips
whether the last handoff piled up. A finding handed again now carries the
host's latest comments, and fixing the same findings twice no longer repeats
their instructions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 25, 2026
@jojopirker

Copy link
Copy Markdown

Really would like to see this get merged, would make reviewing stacked PRs possible!

drew-council added a commit to drew-council/t3code that referenced this pull request Nov 5, 2026
Squash of upstream pingdotgg#25136 at bd24a99, unmerged and a draft when taken. It fixes upstream issue 24960.

Drop this commit when rebasing onto an upstream that contains pingdotgg#25136 or an equivalent fix. If the PR changes first, squash it again from its new head.

One change beyond the PR: export FixFindingsHandoff. The PR's test imports it, and upstream removed the export in 67661cf after the PR branched.

Co-authored-by: Julius Marminge <julius0216@outlook.com>
@Sharad-Patel1

Copy link
Copy Markdown

Note

Generated by claude-opus-5-5
RESPONDING ON BEHALF OF SHARAD

This can be closed: it's already fixed on main. #24990 (03dbb2f) closed #24960: every pull request handoff now appends to the draft through appendHandoff, chips merge by id with the newer copy winning, repeats are dropped, and the PR context chip is keyed by URL. One difference: on main, Ask and Explain also append instead of replacing, which was #24990's choice.

The attached script runs a focused test for each behavior this PR describes (stacked selections and findings, two repos with the same PR number, a draft restored after reload, re-fixing a finding with new replies, a second request on one selection) against current main without this PR's source changes. All 8 pass.

To reproduce, run bash prove-pr-25136.sh from a clean t3code checkout after vp i, with gh authenticated.

Result
PASS  #24990 is on main as 03dbb2fb48
PASS  linked issue #24960 was closed as completed by merged PR #24990
PASS  #25136 was branched before #24990 and does not contain it
      Tests  8 passed (8)
PASS  all 8 claimed behaviors pass on main (b18fefde72) without the PR's source changes
VERDICT: CLOSE (already-fixed)

This branch has not been deployed

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

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Adding a second pull request comment or finding to the composer removes the first one's chip

3 participants