DO NOT MERGE: ui work and stuff like that - #6275
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI 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
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| <span | ||
| className="font-medium tabular-nums" | ||
| style={{ color: PROVIDER_COLOR[provider] }} | ||
| > | ||
| {formattedTotal} |
There was a problem hiding this comment.
🟡 Medium usage/UsagePage.tsx:189
The Codex provider total is rendered with style={{ color: PROVIDER_COLOR[provider] }}, and PROVIDER_COLOR.codex is #e6e6e6 — near-white on a light background — so the total is effectively unreadable. This color was meant for graphical fills, not text. Consider using text-foreground or a theme-aware accessible text color for these totals.
| <span | |
| className="font-medium tabular-nums" | |
| style={{ color: PROVIDER_COLOR[provider] }} | |
| > | |
| {formattedTotal} | |
| <span | |
| className="font-medium tabular-nums text-foreground" | |
| style={{ color: PROVIDER_COLOR[provider] }} | |
| > |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/usage/UsagePage.tsx around lines 189-193:
The Codex provider total is rendered with `style={{ color: PROVIDER_COLOR[provider] }}`, and `PROVIDER_COLOR.codex` is `#e6e6e6` — near-white on a light background — so the total is effectively unreadable. This color was meant for graphical fills, not text. Consider using `text-foreground` or a theme-aware accessible text color for these totals.
| suppressed: true, | ||
| clearCustomLabel: true, | ||
| }), | ||
| reconcileTerminalIds: (threadRef, nextIds) => |
There was a problem hiding this comment.
🟡 Medium src/terminalUiStateStore.ts:784
reconcileTerminalIds filters out terminal IDs that have disappeared from server metadata from the terminal list, but does not remove their entries from terminalCustomLabelsByThreadKey. This leaves persisted labels for terminals that no longer exist. When a reusable ID such as term-1 is later allocated for a new terminal, the new terminal inherits the old terminal's custom label because the stale label was never cleaned up. The reconcileTerminalIds action should filter terminalCustomLabelsByThreadKey to only include labels for the reconciled terminal IDs.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/terminalUiStateStore.ts around line 784:
`reconcileTerminalIds` filters out terminal IDs that have disappeared from server metadata from the terminal list, but does not remove their entries from `terminalCustomLabelsByThreadKey`. This leaves persisted labels for terminals that no longer exist. When a reusable ID such as `term-1` is later allocated for a new terminal, the new terminal inherits the old terminal's custom label because the stale label was never cleaned up. The `reconcileTerminalIds` action should filter `terminalCustomLabelsByThreadKey` to only include labels for the reconciled terminal IDs.
| <span className="truncate">{windowLabel}</span> | ||
| </WorkspaceBreadcrumbItem> | ||
| </WorkspaceBreadcrumb> | ||
| <div className="ms-auto flex shrink-0 items-center gap-3"> |
There was a problem hiding this comment.
🟠 High usage/UsagePage.tsx:109
The top-bar action group uses shrink-0 so the segmented controls and refresh button cannot wrap or shrink. On narrow browser widths they exceed the available header width and are clipped by the page's overflow-hidden shell, making the rightmost period options and refresh button unreachable. Previously these controls lived in a wrapping content row. Consider allowing the action group to wrap (e.g., removing shrink-0 and adding flex-wrap).
- <div className="ms-auto flex shrink-0 items-center gap-3">
+ <div className="ms-auto flex flex-wrap items-center justify-end gap-3">🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/usage/UsagePage.tsx around line 109:
The top-bar action group uses `shrink-0` so the segmented controls and refresh button cannot wrap or shrink. On narrow browser widths they exceed the available header width and are clipped by the page's `overflow-hidden` shell, making the rightmost period options and refresh button unreachable. Previously these controls lived in a wrapping content row. Consider allowing the action group to wrap (e.g., removing `shrink-0` and adding `flex-wrap`).
There was a problem hiding this comment.
🟡 Medium
The rename state (renamingTerminalId and terminalRenameDraft) is not reset when threadRef changes. Since ThreadTerminalDrawer is reused across thread switches — the height state already handles this via a threadId check — a terminal being renamed in one thread persists into the next thread if the terminal ID also exists there. The rename input shows the old thread's draft, and pressing Enter or blurring calls setTerminalCustomLabel with the new threadRef, saving the stale label to the wrong thread. Consider resetting renamingTerminalId and terminalRenameDraft when threadRef changes.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ThreadTerminalDrawer.tsx around line 1281:
The rename state (`renamingTerminalId` and `terminalRenameDraft`) is not reset when `threadRef` changes. Since `ThreadTerminalDrawer` is reused across thread switches — the height state already handles this via a `threadId` check — a terminal being renamed in one thread persists into the next thread if the terminal ID also exists there. The rename input shows the old thread's draft, and pressing Enter or blurring calls `setTerminalCustomLabel` with the *new* `threadRef`, saving the stale label to the wrong thread. Consider resetting `renamingTerminalId` and `terminalRenameDraft` when `threadRef` changes.
| }: { | ||
| selected: boolean; | ||
| density?: "default" | "compact"; | ||
| } & Omit<ButtonHTMLAttributes<HTMLButtonElement>, "aria-pressed">) { |
There was a problem hiding this comment.
🟡 Medium ui/segmented-tabs.tsx:28
SegmentedTab sets type="button" but then spreads ...props afterward, so a caller passing type="submit" overrides the safety default and clicking the tab submits any enclosing form. The Omit on ButtonHTMLAttributes omits aria-pressed but not type, so type remains an accepted prop. Consider omitting type from the accepted props or spreading props before the fixed type="button".
| } & Omit<ButtonHTMLAttributes<HTMLButtonElement>, "aria-pressed">) { | |
| } & Omit<ButtonHTMLAttributes<HTMLButtonElement>, "aria-pressed" | "type">) { |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ui/segmented-tabs.tsx around line 28:
`SegmentedTab` sets `type="button"` but then spreads `...props` afterward, so a caller passing `type="submit"` overrides the safety default and clicking the tab submits any enclosing form. The `Omit` on `ButtonHTMLAttributes` omits `aria-pressed` but not `type`, so `type` remains an accepted prop. Consider omitting `type` from the accepted props or spreading `props` before the fixed `type="button"`.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| url.search = ""; | ||
| url.hash = ""; | ||
| return url.toString(); | ||
| } |
There was a problem hiding this comment.
Repo link lowercases path
Medium Severity
changeRequestRepositoryUrl rebuilds the repo path from parseChangeRequestUrl, which lowercases repository for identity matching. That lowercased path is then opened via openExternal, so mixed-case GitLab or Azure repo URLs can 404 even when the original pull request URL was valid.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 78bc072. Configure here.
| ) ?? []); | ||
| const isStackedPullRequest = | ||
| matchingBaseBranchRefs.length > 0 && | ||
| !matchingBaseBranchRefs.some((refName) => refName.isDefault); |
There was a problem hiding this comment.
Stacked badge over-matches bases
Medium Severity
isStackedPullRequest treats any resolved base ref that is not the git default as stacked. Ordinary PRs into long-lived non-default bases like develop get the stacked layers badge, and remote-only non-origin default mirrors can also misclassify.
Reviewed by Cursor Bugbot for commit 78bc072. Configure here.
ApprovabilityVerdict: Needs human review 7 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
maria-rcks
changed the title
maria-rcks
changed the title
| if (entry.kind === "message" && entry.message.role === "assistant") { | ||
| return [entry.id]; | ||
| } | ||
| if (entry.kind === "work" && entry.entry.agentSpawn === undefined) { |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.logic.ts:594
When isWorking is true, non-tool informational work entries (e.g. { tone: "info" }) disappear from the timeline. activeWindowEntryIds includes every non-spawn work entry, so the main loop skips them at their original position. But appendActiveRows only emits work entries that pass workLogEntryIsToolLike, so neutral or info-tone entries are never rendered by either path until the turn settles and isWorking becomes false. Consider excluding non-tool-like entries from activeWindowEntryIds (or rendering them in appendActiveRows) so they remain visible during the active turn.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.logic.ts around line 594:
When `isWorking` is true, non-tool informational work entries (e.g. `{ tone: "info" }`) disappear from the timeline. `activeWindowEntryIds` includes every non-spawn `work` entry, so the main loop skips them at their original position. But `appendActiveRows` only emits `work` entries that pass `workLogEntryIsToolLike`, so neutral or info-tone entries are never rendered by either path until the turn settles and `isWorking` becomes false. Consider excluding non-tool-like entries from `activeWindowEntryIds` (or rendering them in `appendActiveRows`) so they remain visible during the active turn.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 07db7d2. Configure here.
| group.anchorEntry = entry; | ||
| group.startBoundary = pendingUserBoundary; | ||
| group.reanchoredAfterUserMessage = true; | ||
| pendingUserBoundary = null; |
There was a problem hiding this comment.
Expanded reanchor breaks chronology
Medium Severity
Reanchoring a reused turn moves the fold below a later user message, but pre-reanchor entries stay in hiddenEntryIds and reappear at their original positions when the fold expands. Expanding Worked for… after a mid-turn steer therefore inserts earlier tool/commentary rows above that user prompt while the fold control stays below it.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 07db7d2. Configure here.
| workEntryIndicatesToolNeutralStatus(activeEntry.entry) || | ||
| !workLogEntryIsToolLike(activeEntry.entry) | ||
| ) { | ||
| continue; |
There was a problem hiding this comment.
Active turn drops non-tool work
Medium Severity
While isWorking, every non-spawn work entry for the unsettled turn enters activeWindowEntryIds and is skipped in the main timeline, but appendActiveRows only emits tool-like, non-neutral rows. Info work such as context compaction (which carries a turnId) disappears for the whole active turn, and those invisible rows can still clear showThinking.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 07db7d2. Configure here.
There was a problem hiding this comment.
🟡 Medium
appendActiveRows extracts active assistant messages and tool entries and emits them all at activeTurnHeaderIndex, but proposed-plan, turn-plan, and agent-spawn CTA rows stay in their original positions in the outer loop. An active sequence like assistant → proposed-plan → assistant renders as both assistants followed by the proposed-plan, and a tool after an agent-spawn CTA can render before it. This reorders active timeline rows whenever those non-extracted types appear between assistant messages or tool entries. Consider either including those row types in the extracted active window or skipping their outer-loop emission while the turn is active so chronology is preserved.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.logic.ts around line 532:
`appendActiveRows` extracts active assistant messages and tool entries and emits them all at `activeTurnHeaderIndex`, but `proposed-plan`, `turn-plan`, and agent-spawn CTA rows stay in their original positions in the outer loop. An active sequence like assistant → proposed-plan → assistant renders as both assistants followed by the proposed-plan, and a tool after an agent-spawn CTA can render before it. This reorders active timeline rows whenever those non-extracted types appear between assistant messages or tool entries. Consider either including those row types in the extracted active window or skipping their outer-loop emission while the turn is active so chronology is preserved.
1 participant
What Changed
Why
The workspace-level pages had drifted into separate layout and navigation patterns, while terminal and pull-request controls used inconsistent placement and menu behavior. This consolidates those surfaces around the same page frame, segmented controls, sidebar utilities, and action conventions so moving between them feels like one application.
UI Changes
origin/main)All comparisons use the same backend state and an identical 1440×900 viewport crop. “Before” is latest
origin/main; “After” is this branch.Validation
vp test runon the seven affected test files: 164 tests passedChecklist
Built with GPT-5.6 Sol via Codex in T3 Code.
Note
Medium Risk
Large UI surface area with non-trivial messages-timeline ordering/folding logic and persisted terminal label/sidebar state; regressions would mainly affect chat readability and terminal UX rather than auth or data integrity.
Overview
Unifies workspace page framing and refreshes several high-traffic surfaces so chat, terminals, PRs, settings, and sidebar actions feel like one app.
Terminal drawer replaces floating action buttons with a persistent resizable right sidebar. Users can rename terminals (double-click or menu), persist custom labels, and hide the drawer from the sidebar.
Messages timeline adds live
work-liverows for the latest active tool, collapses historical tool runs into summarizedwork-togglerows, and reanchors turn folds below intervening user steers so mid-turn follow-ups stay readable.Shared shell + menus introduce
WorkspacePageContainer/WorkspacePageHeaderfor settings and related pages, densify PR detail chrome (including stacked-PR detection), scope sidebar “New thread” to the current project when filtered, and nest copy actions under aCopysubmenu withseparatorBeforesupport in both web and Electron menus.Reviewed by Cursor Bugbot for commit 459778d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Redesign chat timeline, terminal drawer, usage page, and sidebar UI
work-live), animated 'Thinking' state, re-anchored turn folds below intervening user messages, and collapsible tool groups with human-readable summaries and failure indicators in MessagesTimeline.logic.ts and MessagesTimeline.tsx.SidebarUtilityMenuthat shows Settings/PRs/Usage icons and toggles to 'Back' on the active page.separatorBeforesupport to the contract, Electron menu builder, and fallback renderer; thread action menu groups copy actions into a submenu with icons.SettingsPageContainernow accepts awidthprop (readable/wide/expanded) via a newWorkspacePageContainerwrapper; settings sidebar replaces the Back button with the utility menu.Macroscope summarized 459778d.