You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(web): review findings handed to a thread no longer end in a broken character - #30392
When you fix pull request findings in a thread, bounded in apps/web/src/components/pullRequest/pullRequestDetail.logic.ts caps each piece of review text it hands over. It cuts anything longer than 1,000 UTF-16 code units at 997 code units and adds .... When an emoji straddles that cut, the slice keeps only the emoji's high surrogate (U+D83D for 😀). The handed-over text then ends in a replacement character (�) right before the ..., where it should end on the last whole character before the emoji. Review bots often put emoji in long comments, so this cut can land on one.
Where the broken end shows:
The review comment attached to the composer, and the context chip on the sent message, for each review thread. Both show a compact chip. Clicking it opens the full comment as Markdown in a popover (contextChipParts.tsx, UserMessageContextChips.tsx). The popover scrolls rather than clipping, so the � shows once you scroll to the end.
The handoff prompt that becomes the thread's first message. Through boundedField it caps the quoted conversation comments, check names and descriptions, the pull request title and URL, and the branch names.
These come from Fix in a thread on a finding, and from fixing all findings, in the pull request panel (PullRequestDetailPanel.tsx) and in the thread details pull request row (ThreadDetailsPrRow.tsx).
bounded also caps text in two other handoffs in the same file: the request typed when adding a selection to an agent (buildAddSelectionToAgentHandoff), and the question, the stop summary and the attention line when asking about a review-guide stop (buildAskAboutGuideStopHandoff). This fix covers them the same way.
Expected: the cut never splits a surrogate pair, as in the context chip labels #29158 fixed.
Change
When the 997-code-unit cut would land between a high and a low surrogate, bounded steps back one code unit, so the emoji is dropped whole. This is the same check #29158 added to context chip labels and #29563 added to t3 thread start titles. The 1,000-code-unit limit, the ... suffix and every text that does not split a pair are unchanged.
The cut still works per code point, not per grapheme cluster. A multi-code-point emoji, such as a flag, a skin-tone modifier or a ZWJ family, can still be split between its code points. It never leaves a lone surrogate. That matches the precedents.
The shared truncate in packages/shared is not used here. #24611 (open) fixes the same split in it, but bounded does not call it, and its arithmetic differs: it cuts at the limit and then adds ..., so switching to it would make capped text 1,003 code units long instead of 1,000.
Scope and approval
This is a very small, focused fix for an obvious bug, so there is no prior issue. It is the defect #29158 ("context chip labels no longer split an emoji at the 200-character cap") and #29563 ("t3 thread start titles no longer end in a broken character") fixed elsewhere, still present in the pull request findings handoff. It applies their check and changes no other behaviour, setting or contract.
Overlap check, by bounded, buildFixFindingsHandoff, FINDING_BODY_MAX_LENGTH and by file. Open PRs #19955, #24990, #25803, #28841, #28884, #29011, #29400, #29412 and #29960 touch pullRequestDetail.logic.ts, but none changes bounded. #24990 rewrites parts of buildFixFindingsHandoff and its test file, but not bounded or the case this PR extends. #24611 changes the shared truncate, which bounded does not call.
Verification
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts gets one assertion in its existing bounds a hostile review body instead of attaching it whole case. It hands over a review thread whose comment, by reviewer, is 986 ASCII characters, then 😀, then more text. The text being cut is reviewer: followed by the body, so the emoji's high surrogate is the 997th code unit, the last one the cut keeps.
On main with only the test commit (a6c9a49), from apps/web, vp test run src/components/pullRequest/pullRequestDetail.logic.test.ts:
FAIL |unit| src/components/pullRequest/pullRequestDetail.logic.test.ts > fix findings handoff > bounds a hostile review body instead of attaching it whole
Tests 1 failed | 72 passed (73)
In words: on main the attached review text is reviewer: , the 986 ASCII characters, the lone high surrogate U+D83D, then .... With the fix it is reviewer: , the 986 ASCII characters, then ....
With the fix:
vp test run src/components/pullRequest/pullRequestDetail.logic.test.ts in apps/web: 73 passed.
vp run check:changed from the repo root: format, lint, knip, typecheck (2 packages), tests (2 files) and the React Compiler check all pass.
To see it on web: the cut runs over the thread's comments as <login>: <body> lines joined by newlines. For a thread with one comment, the emoji must start at code unit 994 − <login length> of the body, counted from 0 after leading whitespace is trimmed. That is 986 for the 8-character reviewer the test uses. Open a pull request with such an unresolved review comment, longer than 1,000 code units in all. In the pull request panel, choose Fix in a thread on that finding. Click the review comment chip on the composer to open the full card, then scroll to its end. On main it ends in � before the .... With the fix it ends in ....
What I could not check: the composer card in a running app. The test covers the handoff builder those buttons call.
Bug: a review finding handed to a thread ends in a broken character when an emoji sits at the 997-unit cut
This was written with Claude Opus 5.5 in Claude Code, driven from T3 Code.
…n character
Fixing pull request findings in a thread caps each piece of review text
at 1,000 UTF-16 code units, cutting at 997 and adding "...". When an emoji
straddled the cut, the text kept only its high surrogate and showed a
replacement character before the "...", in the review comment card on
the composer and in the handoff prompt.
The cut now steps back one unit when it would land between a high and a
low surrogate, the same check pingdotgg#29158 added to context chip labels and
pingdotgg#29563 added to thread titles.
Bug: a review finding handed to a thread ends in a broken character when an emoji sits at the 997-unit cut
Macroscope's review found this PR approvable — The change prevents truncated review handoff text from ending with half of a Unicode character. Existing length limits and handoff behavior remain intact, with no material operational exposure.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The review handoff truncation now avoids splitting a UTF-16 surrogate pair. A regression test checks the attachment when an emoji falls at the truncation boundary.
Changes: Review text truncation
Layer / File(s)
Summary
Bounded truncation and regression test apps/web/src/components/pullRequest/pullRequestDetail.logic.ts, apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts
bounded shortens the retained text by one code unit if the cutoff falls between a high and low surrogate. The regression test checks that the attachment ends with the preceding text and ....
The change safely handles surrogate pairs without introducing an identified merge-blocking risk.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title states the main change: review findings handed to a thread no longer end in a broken character. The changeset truncates the handoff text in bounded() so the cut does not split a UTF-16 surro…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description follows the repository template closely. It has all four sections: Problem, Change, Scope and approval, and Verification. The Problem section states the defect in bounded(), which cuts…
Full details: Title check
Explanation
The title states the main change: review findings handed to a thread no longer end in a broken character. The changeset truncates the handoff text in bounded() so the cut does not split a UTF-16 surrogate pair. The title is specific, follows the conventional commit format, and names the user-visible effect.
Full details: Description check
Explanation
The description follows the repository template closely. It has all four sections: Problem, Change, Scope and approval, and Verification. The Problem section states the defect in bounded(), which cuts at 997 UTF-16 code units and can leave a lone high surrogate before the "..." suffix. The Change section explains the one-code-unit step-back and why the shared truncate was not used. The Scope and approval section states that this is a very small, focused fix of an obvious bug, cites the precedents #29158 and #29563, and lists the overlapping open PRs. The Verification section gives a specific test assertion, the failing run on main, the passing results with the fix, the check:changed run, and what was not checked (the composer card in a running app). The description also names the model and harness used, as the template asks. The only gap is that the Scope section claims the fix is an obvious bug without a linked triaged issue. The template allows this for a very small, focused fix, and the author gives a reason. The UI-change screenshot requirement may apply to the composer chip, but the author states that the UI was not checked and the test covers the handoff builder. This is a minor gap, not a major one.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
verified unchanged head 591c3d54d99e19f4c436166078a7268e9d706a3c on linux using the dev web app, repository playwright, and /usr/bin/google-chrome.
reproduced the shared bounded defect on github/main (eee8ed4f23) before running the pr. in this pr's code tab, selected line 762, typed a request with 996 ascii units followed by 😀 and extra text, then clicked "add to agent". main's composer retained a lone high surrogate and displayed � before "...". the identical input on the pr drops the whole emoji and ends cleanly in "...". the request and both context chips survive sending.
sent the pr handoff through the actual codex gpt-6.1-sol provider using the local proxy. the provider returned HANDOFF_OK. two neighboring handoffs also completed: a short request retained 😀 and returned SHORT_OK; an emoji ending exactly at the cut stayed intact in a 1,000-unit prompt and returned INTACT_OK. provider protocol logs confirm all three turns completed on gpt-6.1-sol.
existing focused checks ran through blacksmith:
cd apps/web && vp test run src/components/pullRequest/pullRequestDetail.logic.test.ts
1 test file passed; 73 tests passed
ci also reports 30 successful contexts, zero failing or pending. no source changes, commits, or pushes were needed.
main, matching request:
pr, matching request:
Not verified
the specific "fix in a thread" finding-card path with an existing remote review comment whose emoji sits exactly at the cut. the real app verification above exercises the changed shared function through "add to agent"; the existing focused regression covers the review-body handoff. no artificial remote finding was posted.
the review-guide question path, electron desktop, native ios/android, and other providers.
main's provider completion: the baseline turn was stopped after reproducing and capturing the composer defect. all three pr turns completed.
This branch has not been deployed
No deployments
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
size:S10-29 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.
2 participants
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.
github-actions
Bot
added
the
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Problem
When you fix pull request findings in a thread,
boundedinapps/web/src/components/pullRequest/pullRequestDetail.logic.tscaps each piece of review text it hands over. It cuts anything longer than 1,000 UTF-16 code units at 997 code units and adds.... When an emoji straddles that cut, the slice keeps only the emoji's high surrogate (U+D83D for 😀). The handed-over text then ends in a replacement character (�) right before the..., where it should end on the last whole character before the emoji. Review bots often put emoji in long comments, so this cut can land on one.Where the broken end shows:
contextChipParts.tsx,UserMessageContextChips.tsx). The popover scrolls rather than clipping, so the � shows once you scroll to the end.boundedFieldit caps the quoted conversation comments, check names and descriptions, the pull request title and URL, and the branch names.These come from Fix in a thread on a finding, and from fixing all findings, in the pull request panel (
PullRequestDetailPanel.tsx) and in the thread details pull request row (ThreadDetailsPrRow.tsx).boundedalso caps text in two other handoffs in the same file: the request typed when adding a selection to an agent (buildAddSelectionToAgentHandoff), and the question, the stop summary and the attention line when asking about a review-guide stop (buildAskAboutGuideStopHandoff). This fix covers them the same way.Expected: the cut never splits a surrogate pair, as in the context chip labels #29158 fixed.
Change
When the 997-code-unit cut would land between a high and a low surrogate,
boundedsteps back one code unit, so the emoji is dropped whole. This is the same check #29158 added to context chip labels and #29563 added tot3 thread starttitles. The 1,000-code-unit limit, the...suffix and every text that does not split a pair are unchanged.The cut still works per code point, not per grapheme cluster. A multi-code-point emoji, such as a flag, a skin-tone modifier or a ZWJ family, can still be split between its code points. It never leaves a lone surrogate. That matches the precedents.
The shared
truncateinpackages/sharedis not used here. #24611 (open) fixes the same split in it, butboundeddoes not call it, and its arithmetic differs: it cuts at the limit and then adds..., so switching to it would make capped text 1,003 code units long instead of 1,000.Scope and approval
This is a very small, focused fix for an obvious bug, so there is no prior issue. It is the defect #29158 ("context chip labels no longer split an emoji at the 200-character cap") and #29563 ("t3 thread start titles no longer end in a broken character") fixed elsewhere, still present in the pull request findings handoff. It applies their check and changes no other behaviour, setting or contract.
Overlap check, by
bounded,buildFixFindingsHandoff,FINDING_BODY_MAX_LENGTHand by file. Open PRs #19955, #24990, #25803, #28841, #28884, #29011, #29400, #29412 and #29960 touchpullRequestDetail.logic.ts, but none changesbounded. #24990 rewrites parts ofbuildFixFindingsHandoffand its test file, but notboundedor the case this PR extends. #24611 changes the sharedtruncate, whichboundeddoes not call.Verification
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.tsgets one assertion in its existingbounds a hostile review body instead of attaching it wholecase. It hands over a review thread whose comment, byreviewer, is 986 ASCII characters, then😀, then more text. The text being cut isreviewer:followed by the body, so the emoji's high surrogate is the 997th code unit, the last one the cut keeps.On
mainwith only the test commit (a6c9a49), fromapps/web,vp test run src/components/pullRequest/pullRequestDetail.logic.test.ts:In words: on
mainthe attached review text isreviewer:, the 986 ASCII characters, the lone high surrogate U+D83D, then.... With the fix it isreviewer:, the 986 ASCII characters, then....With the fix:
vp test run src/components/pullRequest/pullRequestDetail.logic.test.tsinapps/web: 73 passed.vp run check:changedfrom the repo root: format, lint, knip, typecheck (2 packages), tests (2 files) and the React Compiler check all pass.To see it on web: the cut runs over the thread's comments as
<login>: <body>lines joined by newlines. For a thread with one comment, the emoji must start at code unit994 − <login length>of the body, counted from 0 after leading whitespace is trimmed. That is 986 for the 8-characterreviewerthe test uses. Open a pull request with such an unresolved review comment, longer than 1,000 code units in all. In the pull request panel, choose Fix in a thread on that finding. Click the review comment chip on the composer to open the full card, then scroll to its end. Onmainit ends in � before the.... With the fix it ends in....What I could not check: the composer card in a running app. The test covers the handoff builder those buttons call.
Bug: a review finding handed to a thread ends in a broken character when an emoji sits at the 997-unit cut
This was written with Claude Opus 5.5 in Claude Code, driven from T3 Code.