sync: port upstream web UI polish, prompt stash, appearance settings (#2109–#5004) - #193
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
|
@coderabbitai review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16e9018675
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const restoreStashEntry = useCallback( | ||
| (entry: PromptStashEntry) => { | ||
| // Remove first so a double activation (click + Enter) can't restore twice. | ||
| const { entry: taken, durable } = takeStashEntry(entry.id); |
There was a problem hiding this comment.
When a user restores an entry while pendingImageCount is nonzero, this removes the entry before its asynchronous image encoding finishes. The restore therefore contains no images, and finalizeEntryImages later finds that the entry is gone and discards the encoded attachments; the warning cannot recover an image that came from the clipboard. Disable restoration until encoding finishes, or coordinate restoration with the pending encoder instead of taking the entry immediately.
Useful? React with 👍 / 👎.
| const { durable } = persistEntries(nextEntries); | ||
| set(() => ({ entries: nextEntries })); |
There was a problem hiding this comment.
When the second-phase write exceeds the browser's storage quota, persistEntries reports failure but the oversized nextEntries payload is still installed in memory. Every subsequent stash write includes that same rejected payload (plus the new entry), so it also fails until the user deletes the offending entry or reloads; deleting other entries can likewise disappear only in memory and resurrect after reload. Preserve a persistable representation after a failed attachment write, even if session-only images are tracked separately.
Useful? React with 👍 / 👎.
…queue (pingdotgg#4453) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 200fa82)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 7248877)
…gdotgg#4655) (cherry picked from commit da11342)
(cherry picked from commit 32843c2)
(cherry picked from commit f0121f3)
(cherry picked from commit 6afbed3)
(cherry picked from commit 6a3df51)
(cherry picked from commit 3957a95)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit a2ffb12)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit c13a021)
(cherry picked from commit dd5ea32)
(cherry picked from commit 362f127)
(cherry picked from commit 4f584da)
(cherry picked from commit 9ccfd9d)
(cherry picked from commit 3c50a64)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 55dd016)
(cherry picked from commit 2deea7a)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 1ba3d01)
(cherry picked from commit 60af905)
Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 1b4830f)
(cherry picked from commit 08e4932)
(cherry picked from commit 3137c2b)
…otgg#4853) Co-authored-by: Simon Doba <simon.doba@orbit.de> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit e698796)
…gdotgg#4905) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 2d9066e)
(cherry picked from commit b125b76)
(cherry picked from commit 6efcf3e)
(cherry picked from commit 9146ed2)
…gdotgg#4964) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit f0c6ba9)
…oring (pingdotgg#4967) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit cbe8052)
(cherry picked from commit 4ba4871)
3463451 to
05bf93b
Compare
16e9018 to
fe91fe1
Compare
Review triageEvery finding on this PR was checked against Deliberately not fixing them here: patching upstream bugs inside a 101-commit port would diverge the fork in exactly the files this stack exists to realign, and would make the next sync harder. They are better raised upstream or tracked as fork-local follow-ups on their own branch, where they can be reviewed on their merits. Notably, the files where this PR did carry fork-specific conflict resolutions drew no findings. |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/aaditagrawal/t3code/blob/fe91fe166596ca544be082981f07b87f6f541772/apps/web/src/imageCompression.ts#L185
For an animated GIF or WebP above the composer or stash size budget, drawing the ImageBitmap onto a canvas and encoding that canvas produces a single-frame WebP/JPEG. The attachment is accepted without warning but every frame after the decoded frame is silently discarded, even though the backend accepts GIF/WebP attachments. Detect animated inputs and reject/preserve them, or use animation-aware transcoding instead of this canvas path.
https://github.com/aaditagrawal/t3code/blob/fe91fe166596ca544be082981f07b87f6f541772/apps/web/src/components/FileBrowserPanel.tsx#L281-L289
When the project-entry response has truncated: true, this replacement header no longer renders the previous “partial” indicator anywhere. Because useFileTreeSearch searches only the returned tree paths, users of large repositories can now interpret an absent search result as a missing file even though it was merely omitted by the server-side entry limit. Retain a visible partial-results warning alongside the new search field.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Terminal-context placeholders reference live sessions the stash can't | ||
| // round-trip, so they are stripped from the stashed prompt. | ||
| const prompt = promptRef.current.split(INLINE_TERMINAL_CONTEXT_PLACEHOLDER).join("").trim(); | ||
| const images = [...composerImagesRef.current]; |
There was a problem hiding this comment.
When a large pasted image is still being compressed, it is only represented in pendingImageCompressionsRef and is absent from composerImagesRef.current. Pressing the stash shortcut during that window therefore saves and clears the prompt without the image; once compression finishes, the image is added back to the now-empty draft instead of the stash. Block or coordinate stashing just as submitComposer already does for pending image compression.
Useful? React with 👍 / 👎.
| target.context.fillStyle = "#ffffff"; | ||
| target.context.fillRect(0, 0, width, height); | ||
| } | ||
| target.context.drawImage(bitmap, 0, 0, width, height); |
There was a problem hiding this comment.
For an animated GIF or WebP above the composer or stash size budget, drawing the ImageBitmap onto a canvas and encoding that canvas produces a single-frame WebP/JPEG. The attachment is accepted without warning but every frame after the decoded frame is silently discarded, even though the backend accepts GIF/WebP attachments. Detect animated inputs and reject/preserve them, or use animation-aware transcoding instead of this canvas path.
Useful? React with 👍 / 👎.
| <div className="surface-subheader gap-1 px-2" data-surface-subheader> | ||
| <RefreshFilesButton isPending={entriesQuery.isPending} onRefresh={entriesQuery.refresh} /> | ||
| <FileSearchField | ||
| name="project-files-search" | ||
| ariaLabel={`Search ${projectName} files`} | ||
| value={search.value} | ||
| onValueChange={handleSearchValueChange} | ||
| onClose={search.close} | ||
| /> |
There was a problem hiding this comment.
When the project-entry response has truncated: true, this replacement header no longer renders the previous “partial” indicator anywhere. Because useFileTreeSearch searches only the returned tree paths, users of large repositories can now interpret an absent search result as a missing file even though it was merely omitted by the server-side entry limit. Retain a visible partial-results warning alongside the new search field.
Useful? React with 👍 / 👎.
| // Electron's updater owns the remaining quit/install/relaunch sequence. | ||
| // Cancelling the following app "before-quit" event breaks that sequence, | ||
| // most visibly on macOS where the native updater performs the relaunch. | ||
| updaterQuitAllowed = true; |
There was a problem hiding this comment.
When before-quit-for-update sets this flag, the following before-quit event takes handleBeforeQuit's early-return path and never calls requestDesktopShutdownAndWait. DesktopApp is still blocked on shutdown.awaitRequest, so its backend-pool finalizer—the path that sends each backend its graceful stop—cannot run before Electron exits; installing an update can therefore hard-kill local/WSL backends and skip their persistence cleanup. Coordinate shutdown before invoking quitAndInstall rather than bypassing it.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| // Always claim the shortcut so the browser save dialog never opens, | ||
| // even when the composer is in a state that can't stash. | ||
| event.preventDefault(); | ||
| event.stopPropagation(); |
There was a problem hiding this comment.
When focus is in the right-panel file editor or another editable control within the chat route, this window capture listener still resolves the default mod+s binding because it only excludes terminal focus. It then prevents the event before the focused editor can handle Save and may clear an unrelated composer prompt into the stash. Gate the shortcut on composer focus or ignore non-composer editable targets.
Useful? React with 👍 / 👎.
| const stashCurrentPrompt = useCallback(async () => { | ||
| // Terminal-context placeholders reference live sessions the stash can't | ||
| // round-trip, so they are stripped from the stashed prompt. | ||
| const prompt = promptRef.current.split(INLINE_TERMINAL_CONTEXT_PLACEHOLDER).join("").trim(); |
There was a problem hiding this comment.
When a prompt begins with indentation, such as an indented Markdown code block, or intentionally ends in newlines, this .trim() mutates it before persistence and the composer is then cleared. Restoring the entry therefore cannot reproduce the original prompt and can change its Markdown interpretation, whereas the normal send path preserves promptRef.current. Use trimming only for the emptiness check while storing the original placeholder-stripped text.
Useful? React with 👍 / 👎.
10 participants
L4/7 of the 2026-07-30 upstream sync stack — the largest slice (35 commits). Stacked on #192.
What lands
Fork deviations preserved
ChatMarkdown.tsx: upstream's newcodehandler and the fork's older one merged as duplicate object keys with no textual conflict — the fork's later key silently won, which would have negated the ported commit. Kept upstream's implementation (itsfileLinkChipwiresopenInPreferredEditor+ preview, a superset of the fork's file-manager open) and removed the superseded duplicate.ChatComposer.tsx: dropped upstream's module-levelruntimeModeConfig; the fork uses provider-scopedgetRuntimeModeConfig(provider)for its medium-access/Auto traits. Also dropped the now-unusedIMAGE_SIZE_LIMIT_LABEL, which upstream removed along with its only usage.useSettings.test.ts: kept the fork's provider-migration tests alongside upstream's new identification-mode tests.CommandPalette.tsx: kept the fork's capture-phase keydown listener (so the shortcut survives xterm/contenteditable) with upstream's renamed handler and dep list.Ordering note
#4809(restore T3 Connect sign-in) depends on#4691(waitlist removal), which I had originally slotted into L5 — moved into this PR so the stack builds in order.#4992was deferred to L6 because it depends on#4903.Verification
vp run typecheckandvp checkpass; web suite green (193 files, 1724 tests).