Repository navigation
feat(web): Spaces sidebar grouping with thread tabs - #242
Conversation
…iB page kernels (pingdotgg#25825) TEMPORARY workaround for an upstream Node bug: nodejs/node#67048 Node 26.9+ --build-sea writes linux-arm64 executables that 16/64 KiB page kernels cannot load. linux-arm64 now injects with postject, and the archive build rejects incompatible ELF layouts. One-time exception. Revert once Node ships a fix: grep for the Node issue URL and drop postject from scripts/package.json. Fixes pingdotgg#23433. Replaces pingdotgg#23443. Co-authored-by: Lakshmi Tanmay <lakshmi@voltcrash.com> Co-authored-by: maria-rcks <maria@kuuro.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ingdotgg#25666) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…s 4x faster (pingdotgg#25824) Co-authored-by: Omar <76748259+omar9848492@users.noreply.github.com> Co-authored-by: Carlos D. Escobar-Valbuena <carlosdavidescobar@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com> Co-authored-by: maria-rcks <maria@kuuro.net> Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…ingdotgg#25816) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…h no list open (pingdotgg#25809) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…saves a draft twice (pingdotgg#25768) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ering (pingdotgg#25846) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rning (pingdotgg#25819) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…first commit (pingdotgg#25817) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rom screen effects (pingdotgg#25820) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…otgg#25837) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…otgg#25860) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ingdotgg#25812) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ngdotgg#25882) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…d one (pingdotgg#25839) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…s it unavailable (pingdotgg#25632) Co-authored-by: Theo Browne <me@theo.li>
…otgg#25841) Co-authored-by: jason chien <3936511+jason5545@users.noreply.github.com>
…b checks agent-device 0.21.32 folds the iPhone Duo in Simlock's device set. The folded Duo keeps its side rail inside a thread, so the hide-on-push tab checks only apply to bottom tab bars. The Duo poses stay pending: alert accept cannot see the open-link prompt on the Duo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gdotgg#25854) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…derive instead of syncing (pingdotgg#25813) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…effects (pingdotgg#25895) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ingdotgg#25900) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Theo Browne <me@theo.li>
…ount connects (pingdotgg#25868) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…aint (pingdotgg#25909) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ingdotgg#25915) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…wn panel (pingdotgg#25908) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…tgg#25920) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Merges 1069 upstream commits. Upstream split ChatView, ChatComposer and MessagesTimeline into hooks and row modules, so those files were rebuilt from upstream's versions with the fork's fenced intents re-applied at their new homes (useComposerSend, useThreadLifecycle, ThreadActivityBanners, useComposerSelection, chatRouteCommands, Timeline*Row, AppearanceSettingsPanel). Notable fork-side follow-ups: - GitHub PR automation provides its own GitHubRest, which upstream now builds only inside its GitHub driver. - Window fill colours follow upstream's new #000000 dark background. - New upstream lint rules are deferred for fork-owned code with baselines; over-long fork comments in upstream files were shortened. - Upstream-replaced files are fenced into knip ignores; dead fork CSS for removed upstream elements is deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream's new error-level comment rule fails three fork comments in the vite configs, which would turn CI's `vp check` red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sage Upstream now keeps the thread mounted under the Settings and Usage modals and gates its global shortcuts on BackgroundThreadContext. Fork hooks that own global input did not: - pending-question digits 1-9 could answer the hidden thread's question - the dictation hotkey recorded into the hidden thread's draft instead of the fallback toast; a session still preparing went on to record unseen (stop() is a no-op before recording, so it is now cancelled) Also restores the pre-sync ChatView focus effect that reads splitPane.focused without depending on it, so clicking into a split pane keeps its caret. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream's new budget test pins the MCP tool list at its own size; the fork's GitHub PR automation adds 342 bytes to schedule_task, so the test failed. The budget now records the fork's measured size, and the budget gate compares a PR with its base branch (custom) instead of main, the upstream mirror, which would otherwise fail every fork PR. Folded into ci-on-custom with a guard row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ault mode Upstream moved the permissions chip from a Select to a Menu. The fork's ComposerRuntimeModeTrigger still rendered a Base UI Select.Trigger, which throws without a Select root, so any thread whose mode differed from the default crashed the composer. It now renders upstream's ComposerControl. Upstream's ScheduledTasksSettings case for a user-opened New task editor racing a late-loading link cannot happen in the fork: New task opens the fork's /scheduled page. The case is fenced and replaced with one pinning that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the sync found fork behavior lost where upstream moved code: - Side chat: upstream starts every app-wide service from main.tsx, so the side chat window doubled notifications, sounds and toasts; it now stops after workspace focus and skips main-app location tracking. Its root no longer nests a second toast provider pair. - New worktree from a previous worktree's draft clears the stored path again. - Multi-model sends detach and restore design-change pills with the cleared draft (in useMultipleModelSend), not after the await, so a failure keeps them and a send during thread start cannot repeat them. - Find keeps one attached timeline per thread, so a streaming split pane no longer steals the focused pane's. - "Compact idle threads: Never" hides the resume card, as upstream's setting does. - "Open automation" on a PR-review task opens /scheduled, like a list click. - Startup recovery links to the fork's releases. - Glass Settings nav rows and buttons match inside the new Settings modal, and the Settings button restyle no longer reaches the chat behind it. - One useOwnsGlobalInput() hook for fork window listeners; the stale git-config-lock-retry manifest entry (replaced upstream) is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e GitHub sign-in Upstream's new webhook coalescing path dispatches without the GitHub trigger, so a PR-review task that kept a stored dedupe launched in the task's own project with no PR context. Saving a GitHub task now drops dedupe; its deliveries are already one per PR event. Upstream builds GitHubRest only inside its GitHub driver, so the automation built a second GitHubAppSignIn with its own refresh lock. The source-control layer now builds GitHubRest first in the server's memo map, which the driver's forked map and the automation both reuse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- PR-review automations saved with a dedupe before the save-time strip still took the coalesced path and lost their PR trigger. Delivery now treats any GitHub task as no-dedupe. - The fork's resume-compaction card also stands down for held queued runs and a multi-model selection, as upstream's offer does. - The side chat skips agent-opened previews and preview title recording; both follow every thread and the window shows no panels. - Glass Settings buttons match any popup, so dialogs and menus portaled out of the Settings modal keep the chip look. - The stash shortcut, open-in-editor and chat shortcuts read useOwnsGlobalInput(); dictation's focus-loss effect and settleWithoutControls share one settleSession; the new-worktree fence asks resolveEffectiveEnvMode; the split-pane deps comment is fenced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… after the sync Upstream's simpler composer (pingdotgg#25869) changed three things the fork leaned on without a conflict: - composer-host paints its own glass ::before when a context row is present, which drew a dark tray behind the fork's vessel. It joins the shell pseudo-elements the fork already switches off. - The shell's context marker is data-context-row now, not data-with-context, so the 8px gap between the chips and the vessel never applied. - The chips' and the model trigger's carets went with ComposerControlChevron. The worktree and branch chips and the fork's model picker shadow draw them again. Guards in forkComposerShell.test.ts pin all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… PR review A GitHub task saved with a dedupe before the save-time strip still passed its replay key to the immediate path, so the same PR number in a second watched repository read as a duplicate. Delivery hands that path the task without its stored dedupe; capacity and rate reservations still apply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fork lint gate holds no-low-value-comments at its 876 baseline, and the "(pingdotgg#25869)" reference made it 877, failing Lint on the sync PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds "Spaces" as a fourth V2 sidebar grouping. Each project becomes a space with Threads, PRs, Changelog and Automations rows. The space's name opens its Home: the new-thread draft, with an overview of the space beneath the composer. A browser-like tab row at the top of the chat column holds the threads you opened, ranked by status. Splits share one row and light both threads together. Client-only (fork-spaces). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NoahHendrickson
left a comment
There was a problem hiding this comment.
Code quality review — commit 216bce6a6c1d513e199afc9b8572d77fcbf1759a
Verdict: changes requested (posted as a comment because the reviewing account is the PR author). The behavior looks deliberate and the new logic modules (spaceTabs.ts, changelog.logic.ts, spaceSidebar.ts) are tidy, tested, pure functions. The integration into existing code is the problem. Spaces is wired in as a mode flag threaded through shared paths (Sidebar.tsx, SidebarV2ProjectGroupHeader, the list animation, and ~30 CSS rules), when it should be its own section model and its own components. Several helpers are copy-pasted across the new files.
No file crosses 1k lines for the first time: Sidebar.tsx (5384→5621) and ChatView.tsx were already far past it. But +237 lines of spacesGrouped branches in Sidebar.tsx is exactly the growth the fork should be keeping out of upstream files.
1. SidebarV2ProjectGroupHeader became a two-mode component (structural)
variant?: "project" | "space" plus four space-only optional props (onOpenSpace, spaceActive, collapsedSummary, spaceProject). Each one means nothing unless variant === "space". Nearly every element now branches on space:
- the hit button spreads one of two prop bags (open vs. collapse semantics, different aria)
- the mark is either a favicon or the folder/chevron stack
- a wrapper span is
display: contentsin one mode and a flex box in the other - the actions get an extra reveal wrapper in space mode
- the caret button exists only in space mode
rounded-fulloverrides are sprinkled on top
These are two different headers: one is a collapse toggle with a folder, the other a navigation pill with a favicon and a separate fold caret. Suggested fix:
- Extract the shared menu+plus cluster (
sectionActions) into a smallSidebarV2SectionActionscomponent. - Add a dedicated
SidebarV2SpaceHeaderincustom/spaces/with requiredproject,active,onOpen,summaryprops. - Restore the project header to its pre-PR shape.
That removes the variant flag, the ?? undefined optionality, and the contents trick.
2. Sidebar.tsx: spacesGrouped spliced into ~12 unrelated flows (spaghetti growth)
New spacesGrouped conditionals appear in:
- the
threadLastVisitedAtByIdselector - section building
threadsVisibleInProjectSection(collapsedandkeepThread)afterOpenSectionprojectReorderEnabled- both shelf toggles
- five header props
- pre/post row injection
header.projectKey !== UNGROUPED_PROJECT_KEY is restated 5 times. projectGroups.find(...) runs twice for the same header (space and spaceProject). Two separate new Map(activeSections.flatMap(...)) passes compute per-space data.
Code-judo: build one space model per section in spaceSidebar.ts, for example buildSpaceSections({ sections, projectGroups, expanded, kindOf }) → Map<projectKey, { group, summary, openPullRequests, threadsFolded }>, which returns nothing for the ungrouped section. The render site then does one const space = spaceSections.get(header.projectKey) lookup. Every UNGROUPED_PROJECT_KEY check, both finds, both Maps and spaceThreadsFolded collapse into it. Better still, a single <SidebarV2SpaceSection> that takes the rendered cards as children would cut the header / Threads row / cards / link rows splice down to one fenced call.
3. Spaces mode inferred from DOM shape in CSS and JS (magic / brittle)
ul:has(> li[data-fork-space-header])appears in about a dozentheme.custom.cssrules, andisSidebarV2SpaceListItemruns aquerySelectoron every animated item to rediscover the same thing.Sidebaralready knows the grouping. Putdata-fork-grouping={grouping}on the list once and key CSS and the animation plugin off it. That's cheaper than a broad:has()on the whole list on every recalc, and it can't break when the headerlichanges shape.- The shelf toggles render both the upstream chevron and a
SidebarV2SpaceCaret, and CSS (& > svg:not([data-fork-shelf-caret]) { display: none }) hides one. Render one or the other with a single ternary. - The split-pair tray radius matches Tailwind utility class names (
.rounded-\[16px\],.rounded-full,.rounded-md) on the cards, and other rules targetspan.h-pxandspan:first-child. A radius tweak on the card silently breaks the tray. Expose the card radius as a CSS variable (or a data attribute) instead.
4. A parallel status vocabulary that can drift from the cards
spaceSidebar.ts defines SpaceThreadMark + MARK_BY_KIND, SidebarV2SpaceRows.tsx hand-writes SPACE_MARK_STATUS (label/tone/mark), and spaceTabs.ts adds TAB_RANK_BY_KIND. The docs say the summary is "read off the cards' own status ladder, so the row and the cards cannot disagree". It isn't: the labels are re-typed (for example "Needs you" vs. the card's "Needs approval" / "Needs input"), and any new kind or tone needs edits in three tables. Export sidebarV2CardTopStatus (or dedupe each card's own topStatus by tone:mark) and drop SpaceThreadMark entirely.
Relatedly, resolveSidebarV2CardStatus(shell, { lastVisitedAt: resolveThreadLastVisitedAt(...), wokeAt: threadWokeAt(...) }) is repeated three times: the Sidebar summary, ThreadStatusMark, and rankSpaceTabs. The tab copies read new Date() in render while the sidebar uses its snoozeNow tick, so a tab and its card can briefly disagree on "woke". One resolveThreadCardStatus(shell, localVisitedAt, now) helper fixes both.
5. Copy-paste across the new spaces files
- The filled-caret SVG (
M2.71 4.75H11.29L7 9.04Z) now exists in 4 places: the status header, the space header, the Threads row, andSidebarV2SpaceCaret.SidebarV2SpaceCaretwas added in this PR but isn't reused by the other two new copies. Give it aclassNameand use it everywhere. - The space PR query (
projectIdsByEnvironment→EnvironmentQueryTarget[]→usePullRequestList→ filter byspaceHasProject→ sort byupdatedAt) is duplicated almost line-for-line inSpaceOverview.SpacePullRequestsandChangelogList. ExtractuseSpacePullRequests(space, state, limit). - Route search objects are duplicated: "space's open PRs" (
SidebarV2SpaceLinkRowsandSpaceOverview), "one PR" (SpaceOverviewandChangelogPage), plus/scheduled?space=and/changelog?space=. One smallspaceRoutes.tskeeps the "fresh view, not saved filters" decision in one place. useSpaceOfProjectre-implementsspaceHasProjectinline, in the same file.SpaceOverviewimportsThreadStatusMark/useThreadProviderfrom the 531-lineSpaceThreadTabs.tsx. Move the shared thread-presentation bits into their own module so the overview doesn't depend on the tab row.
6. SpaceTabRow orchestration is more indirect than it needs to be
onOpenHomeis passed in by both callers (ChatViewandSplitSpaceTabRow) only to be wrapped again inuseOpenSpaceHome(space.key, props.onOpenHome).Sidebardoes the same with a() => handleNewThreadInProject(...)closure. The "pass a fallback callback" API is the smell. LetuseOpenSpaceHomeown the fallback (calluseNewThreadHandlerwith the space's representative ref) and take a space. That deletes theonOpenHomeprop, theopenNewparameter and both caller closures.- Tab persistence serializes sets to
"\n"-joined strings (openable,closedSignature) so they can serve as effect deps, then splits them back apart. Two effects reconcile one stored list. One purereconcileSpaceTabs(keys, { opened, closed })inspaceTabs.ts(unit-tested with the others) plus one effect is simpler and avoids the string round-trip.
Smaller
useSpaceThreadsExpandedstores an expanded set throughuseSidebarV2CollapsedKeys. Rename the generic hook (it's a persisted key set) rather than reading it backwards.buildActiveThreadSectionsgains an optionalkeepEmptyGroups?: boolean. Fine as a param, but make it required (or fold it into the spaces branch's own builder) so the default isn't an implicit second mode.- The
forkSpaces.test.tsdocblock says the chat header draws the tabs "either side of its breadcrumb". The tabs moved toChatView/SplitThreadPanes, and the test itself now assertsChatHeaderhas nofork-spacesfence. Update the comment.
Reviewed commit: 216bce6a6c1d513e199afc9b8572d77fcbf1759a.
| folder mark. The label opens the space's Home (its draft) instead of | ||
| folding it; status grouping's filled caret, a button of its own on the row's | ||
| far end, folds it. */ | ||
| readonly variant?: "project" | "space"; |
There was a problem hiding this comment.
This mode flag (plus four space-only optional props) turns the header into two components behind space ? … : … at nearly every element. Can we split out a SidebarV2SpaceHeader with required props and share only the actions cluster? See the review summary, item 1.
There was a problem hiding this comment.
Done in a82425f. New custom/spaces/SidebarV2SpaceHeader.tsx takes required project, active, onOpen and summary props. The project header is back to its pre-PR shape. Both share SidebarV2SectionActions (the menu and plus cluster, exported from the header file so its pinned strings stay put); the space header passes it pill buttons. The two headers share no mode flag.
| !previousSection.collapsed)); | ||
| /* fork:end fork-spaces */ | ||
| /* fork:begin fork-spaces — see .fork/customizations.yaml#fork-spaces */ | ||
| const space = |
There was a problem hiding this comment.
This is the second projectGroups.find for this header (spaceProject below repeats it), and the fifth UNGROUPED_PROJECT_KEY guard. One precomputed spaceSections.get(header.projectKey) from spaceSidebar.ts would replace this, spaceProject, the two summary Maps, and spaceThreadsFolded. See the review summary, item 2.
There was a problem hiding this comment.
Done in a82425f. buildSpaceSections in spaceSidebar.ts returns Map<projectKey, { group, summary, openPullRequests, threadsFolded }> and leaves out the unresolved section. The render does one spaceSections.get(header.projectKey), and the fold filter and afterOpenSection read the same map. That removes both projectGroups.find calls, both summary Maps, spaceThreadsFolded, and the UNGROUPED_PROJECT_KEY guards. Unit tests added.
| } | ||
|
|
||
| /** True for an item in a list that draws spaces: their headers mark it. */ | ||
| export function isSidebarV2SpaceListItem(el: Element): boolean { |
There was a problem hiding this comment.
This re-infers the grouping from DOM shape with a querySelector per animated item. Sidebar already knows grouping: put data-fork-grouping on the list and read that here and in theme.custom.css (which repeats ul:has(> li[data-fork-space-header]) about a dozen times).
There was a problem hiding this comment.
Done in a82425f. The list ul carries data-fork-grouping={grouping}. theme.custom.css keys off ul[data-fork-grouping="spaces"], and the animation plugin reads the parent's attribute instead of running a querySelector.
| background: transparent; | ||
| } | ||
|
|
||
| & > svg:not([data-fork-shelf-caret]) { |
There was a problem hiding this comment.
This renders two carets and hides one with CSS. A ternary in Sidebar.tsx that renders only the one we want removes this rule.
There was a problem hiding this comment.
Done in a82425f. The shelves render spacesGrouped ? <SidebarV2SpaceCaret> : <ChevronDownIcon>. The hairline and the Snoozed ink are set in TSX too, so the span.h-px, span:first-child and hide-the-chevron rules are gone.
| background: rgb(0 0 0 / 0.04); | ||
| } | ||
|
|
||
| & > li[data-fork-split-pair] > ul:has(> li > .rounded-\[16px\]) { |
There was a problem hiding this comment.
The tray radius matches Tailwind class names on the card (.rounded-[16px], .rounded-full, .rounded-md). Any radius tweak on the card silently breaks this. Can the card expose its radius as a CSS variable instead?
There was a problem hiding this comment.
Done in a82425f, slightly differently. The tray is an ancestor of the cards, so it can't read a variable set on them (custom properties only inherit downward). Instead the pair row gets its cards' ThreadRowShape from threadRowShape() and stamps data-fork-row-shape; the CSS keys off that, not Tailwind class names. The radii sit next to a comment pointing at THREAD_ROW_RADIUS.
| ] as const; | ||
| export type SpaceThreadMark = (typeof SPACE_THREAD_MARKS)[number]; | ||
|
|
||
| const MARK_BY_KIND: Record<SidebarV2CardStatusKind, SpaceThreadMark> = { |
There was a problem hiding this comment.
This, SPACE_MARK_STATUS in SidebarV2SpaceRows, and TAB_RANK_BY_KIND re-encode the card status ladder by hand (and the labels already differ from the card's). Can we derive the marks from the card's own topStatus and drop SpaceThreadMark?
There was a problem hiding this comment.
Done in a82425f. summarizeSpaceThreads takes each card's own topStatus and dedupes by mark and tone, so labels and tones come from sidebarV2CardStatus. SPACE_MARK_STATUS and the hand-written mark union are gone; only a display order remains. TAB_RANK_BY_KIND stays on purpose: it's the tab order you asked for (needs you, done, working, monitoring, idle), which isn't the card ladder. The tab row and the overview now compute status through one threadCardStatus helper in the new spaceThreadMarks.tsx.
| function SpacePullRequests(props: { readonly space: Space; readonly state: "open" | "merged" }) { | ||
| const navigate = useNavigate(); | ||
| const { space, state } = props; | ||
| const projectIdsByEnvironment = new Map< |
There was a problem hiding this comment.
This target-building, filter and sort block is duplicated almost verbatim in ChangelogPage.tsx:61. Extract useSpacePullRequests(space, state, limit).
There was a problem hiding this comment.
Done in 7ff76e6: useSpacePullRequests(space, state, limit) in spacePullRequests.ts, used by the overview and the Changelog. It also dedupes per environment.
| const lastVisitedAtById = useUiStateStore((store) => store.threadLastVisitedAtById); | ||
| const { selectedKeys } = props; | ||
| const openSpaceHome = useOpenSpaceHome(); | ||
| const onOpenHome = () => |
There was a problem hiding this comment.
onOpenHome comes in from both callers only to be wrapped as useOpenSpaceHome's fallback. If useOpenSpaceHome owned the new-thread fallback, this prop, the openNew parameter, and the closures in ChatView, SplitSpaceTabRow and Sidebar would all go away.
There was a problem hiding this comment.
Done in a82425f. useOpenSpaceHome() owns the fallback: it calls useNewThreadHandler itself, in the caller's project when the space has it, else the space's representative. The onOpenHome prop, the openNew parameter and the closures in ChatView, SplitSpaceTabRow and Sidebar are gone.
| ); | ||
|
|
||
| // Store what is on screen as a tab, and drop tabs whose thread settled. | ||
| const openable = threads |
There was a problem hiding this comment.
Serializing to \n-joined strings for effect deps and splitting back is a workaround. One pure reconcileSpaceTabs(keys, { opened, closed }) in spaceTabs.ts plus a single effect would be clearer and testable alongside the existing spaceTabs tests.
There was a problem hiding this comment.
Done in a82425f. New pure reconcileSpaceTabs(keys, { opened, closed }) in spaceTabs.ts returns the same list when nothing changes. One effect writes only when the result differs by identity. Tests added; the string round-trip is gone.
NoahHendrickson
left a comment
There was a problem hiding this comment.
Reviewed commit 216bce6a6c1d513e199afc9b8572d77fcbf1759a against base e63be3e7f92f64f7e095131a6ec6b39442ea3dda. Found three issues, detailed inline.
Validation: the focused Spaces, sidebar grouping/animation, scheduled-page logic, and Spaces guard tests passed (7 files, 71 tests); the web typecheck passed. Review used source inspection and focused checks; no browser verification was performed.
| // and plus, so it is not one of its threads. | ||
| threads: activeThreads.filter( | ||
| (thread) => | ||
| !draftIdByThreadKey.has( |
There was a problem hiding this comment.
[P2] Keep older unsent drafts reachable in Spaces
This removes every draft, although Home can reopen only the single draft currently mapped to the logical project. For example, type a prompt into a space's draft, click that space header's + to create another draft, and navigate away: useNewThreadHandler replaces the logical-project mapping while setLogicalProjectDraftThreadId deliberately retains the older draft containing text. Home now opens the newer draft, this filter hides the older one, and the tab strip reads server shells only, so the original prompt has no recovery entry within Spaces. Keep non-Home drafts visible or provide a way to select all retained drafts.
There was a problem hiding this comment.
Fixed in 7ff76e6. Spaces now hides only each logical project's current Home draft (logicalProjectDraftThreadKeyByLogicalProjectKey). An older draft the Home has moved on from stays listed as a draft card, so nothing written is stranded.
| }) { | ||
| const navigate = useNavigate(); | ||
| const space = useSpaceOfProject(scopeProjectRef(props.project.environmentId, props.project.id)); | ||
| const shells = useThreadShellsForProjectRefs(space?.memberProjectRefs ?? NO_PROJECT_REFS); |
There was a problem hiding this comment.
[P2] Include selected split threads from outside the focused space
A supported split can contain threads from two different logical projects, but this query loads only the focused space's member projects. The later selectedKeys filter cannot add a shell absent from that query, and resolveSpaceTabs skips active keys missing from its thread map. Pair a thread from project A with one from project B: while A is focused, B has no tab or tab close/menu actions, and focusing B makes A disappear instead. Load the selected refs independently and merge them with the space shells so both on-screen panes remain represented.
There was a problem hiding this comment.
Fixed in 7ff76e6. SplitSpaceTabRow passes both panes' own shells as onScreenShells, and SpaceTabRow merges them with the space's shells. A partner from another space keeps its tab, close, and menu.
| /* The header's panel controls are pinned to the window's top edge; under a | ||
| space's tab row they drop to the header's own row, never sharing the tabs'. */ | ||
| [data-chat-column]:has(> [data-fork-space-tab-row]) { | ||
| --workspace-controls-top: calc(env(titlebar-area-y, 0px) + 2.75rem); |
There was a problem hiding this comment.
[P2] Avoid applying the window offset to header-relative controls
With Spaces enabled and an inline right panel open, ChatView renders threadPanelHeaderControl inside its position: relative chat header. That control is position: absolute with top: var(--workspace-controls-top), so it also inherits this new 2.75rem offset. The tab row has already moved the header down by 2.75rem; adding the offset again places the thread-details toggle below the header, over the chat body. Apply the extra offset only to viewport-positioned controls, keeping controls positioned relative to the header at its own top.
There was a problem hiding this comment.
Fixed in 7ff76e6. The absolute header control (threadPanelHeaderControl) gets --workspace-controls-top reset under the tab row. Only the window-fixed controls take the 2.75rem drop.
NoahHendrickson
left a comment
There was a problem hiding this comment.
Code review of 216bce6a6c1d513e199afc9b8572d77fcbf1759a (Claude Code /code-review high, findings checked by hand against the source).
Correctness
- Scratch space Home never resumes its draft or lights its header (space key vs. draft logical key mismatch).
spaceHome.ts - Archived/deleted threads' tabs are never pruned from storage and come back on unarchive.
SpaceThreadTabs.tsx - In a split, the trailing pane's fixed panel controls cover the shared tab row.
theme.custom.css - A split whose focused pane is a draft shows the partner's space.
SpaceThreadTabs.tsx - PRs are duplicated per environment in Home's PR sections and the Changelog.
SpaceOverview.tsx/ChangelogPage.tsx - Automations card shows one Loading/empty row per environment.
SpaceOverview.tsx
Performance
7. The tab row (and Home's thread list) subscribe to non-quiet shells and the whole visited map, so they re-render per streamed token.
Minor
- Changelog week keys/labels lack a year.
- Home's Threads section has no
SECTION_LIMITslice or link through, unlike the other sections, so a busy space pushes PRs/automations far below the composer. - Duplication: the env→projectIds PR target builder is copy-pasted between
SpacePullRequestsandChangelogList; the filled caret SVG is inlined inSidebarV2SpaceThreadsRowinstead of reusingSidebarV2SpaceCaret;spaceSidebar.tsredefinesisAnyKeyfromsidebarV2ProjectGrouping.ts.
Items 3 and 4 are reasoned from the code and not reproduced in a client. Nothing was run.
🤖 Generated with Claude Code
| const navigate = useNavigate(); | ||
| return (spaceKey: string, openNew: () => void) => { | ||
| const store = useComposerDraftStore.getState(); | ||
| const draft = store.getDraftSessionByLogicalProjectKey(spaceKey); |
There was a problem hiding this comment.
Scratch space never resumes or lights its Home. Space keys come from buildProjectGroups({ groupScratchProjects: true }), so the Scratch space's key is SCRATCH_PROJECT_SCOPE_KEY. useHandleNewThread stores the draft under deriveLogicalProjectKeyFromSettings(project, settings) with groupScratchProjects off, i.e. the scratch project's own key.
Result: type into the Scratch space's Home, click away, click the space name (or the Home tab / plus) → getDraftSessionByLogicalProjectKey(spaceKey) is null, so it calls openNew instead of resuming the written draft. The same mismatch keeps spaceActive (Sidebar.tsx:5313, activeDraftThread.logicalProjectKey === header.projectKey) from ever lighting the Scratch header. Look the draft up by the space's member project refs (or derive the key with groupScratchProjects: true on both sides).
There was a problem hiding this comment.
Fixed in 7ff76e6. useOpenSpaceHome takes the space and finds its Home among the drafts whose project is a member of the space. The header's lit state uses the same membership test (isSpaceDraft). Scratch now resumes and lights.
|
|
||
| // The tabs hold what the sidebar would show (no subagents, no automation | ||
| // runs) plus the threads on screen, whatever they are. | ||
| const visible = new Set(filterSidebarV2VisibleThreads(shells, null)); |
There was a problem hiding this comment.
Archived tabs off screen are never pruned. filterSidebarV2VisibleThreads drops archivedAt !== null shells before resolveSpaceTabs runs, so an archived thread that isn't selected never reaches byKey, never lands in closedKeys, and its stored key is treated as "another space / not loaded yet". The closed: … || shell.archivedAt !== null branch only fires for the thread on screen.
Repro: open A as a tab, navigate to B, archive A from the sidebar. A's tab disappears, but its key stays in t3code:fork:space-tabs:v1; unarchiving A later brings the tab back unasked. Deleted threads behave the same. Keys pile up until the cap evicts live tabs. Computing closed over all of the space's shells (before the visibility filter) would fix it.
There was a problem hiding this comment.
Fixed in 7ff76e6. closed is computed over all of the space's shells: archived, settled, or hidden and not on screen. Stored keys for those are pruned. Deleted threads have no shell, so they still wait for the cap; that's left as a follow-up.
| /* The header's panel controls are pinned to the window's top edge; under a | ||
| space's tab row they drop to the header's own row, never sharing the tabs'. */ | ||
| [data-chat-column]:has(> [data-fork-space-tab-row]) { | ||
| --workspace-controls-top: calc(env(titlebar-area-y, 0px) + 2.75rem); |
There was a problem hiding this comment.
Split view: trailing pane's panel controls sit on top of the shared tab row. This drop only applies to [data-chat-column]:has(> [data-fork-space-tab-row]). In a split, SplitSpaceTabRow is rendered by SplitThreadPanes as a sibling before the panes, outside every data-chat-column, so each pane keeps --workspace-controls-top: 0. ChatView's panelLayoutControls are fixed top-[var(--workspace-controls-top)] right-[…] (ChatView.tsx:8516), so they render at the window's top-right over the 44px tab row, covering the plus / last tabs and taking their clicks, and no longer line up with the pane headers below. Reasoned from code, not checked in a client.
There was a problem hiding this comment.
Not reproducible: in a split, each pane contains its own fixed controls. Measured in the dev app with a split open: both [data-workspace-titlebar-controls] sit at y=44, the top of each pane's header, below the 44px tab row (x 679–803 and 1184–1308). They do not overlap the shared row.
| // An unsent draft in the other pane is the space's Home too. | ||
| const partnerDraftId = useComposerDraftStore((store) => store.getDraftIdByRef(props.partnerRef)); | ||
| const partnerIsDraft = partnerShell === null && partnerDraftId !== null; | ||
| const shell = focusedShell ?? partnerShell; |
There was a problem hiding this comment.
Wrong space when the focused split pane is a draft. A draft has no thread shell, so focusedShell is null and shell falls back to the partner's. If the draft belongs to project X and the partner thread to project Y, the row draws Y's tabs, lights Y's Home (homeActive is true via focusedIsDraft), and the plus/Home open Y's draft while the user is composing in X. The focused draft's project is available from the composer draft store (getDraftSessionByRef / the draft session's project ref) and should win.
There was a problem hiding this comment.
Fixed in 7ff76e6. SplitSpaceTabRow names the row after the focused pane: its shell, else its draft session (getDraftSessionByRef), and only then the partner.
| ); | ||
| const list = usePullRequestList(targets); | ||
| const entries = (list.data?.entries ?? []) | ||
| .filter((entry) => spaceHasProject(space, entry.environmentId, entry.projectId)) |
There was a problem hiding this comment.
PRs are listed once per machine in a multi-environment space. A space is explicitly "a repository checked out on several machines", and the targets query every member environment for the same repository. mergePullRequestLists concatenates per-environment answers without deduping, so Open pull requests / Recently merged here (and the Changelog at ChangelogPage.tsx:84) show each PR twice when the repo is on, say, the local Mac and a remote host. The React keys include environmentId so nothing warns. The sidebar's spaceOpenPullRequests already dedupes by host/repository#number; these lists should too (before the SECTION_LIMIT slice, so duplicates don't eat the limit).
There was a problem hiding this comment.
Fixed in 7ff76e6. New useSpacePullRequests / spacePullRequestEntries in spacePullRequests.ts filter by the space, sort newest first, and dedupe by host/repository#number before the section limit. Home's two PR sections and the Changelog share it. Test added.
| const tasks = (query.data?.tasks ?? []).filter((task) => | ||
| spaceHasProject(space, environmentId, task.projectId), | ||
| ); | ||
| if (query.data === null) return <Empty>{query.error ?? "Loading…"}</Empty>; |
There was a problem hiding this comment.
Duplicated placeholder rows. SpaceAutomations renders once per environment (line 68), and each instance returns its own Loading… / No automations.. A two-environment space with no automations shows "No automations." twice; with tasks on one env only, the list gets a stray "No automations." from the other. Fetch per environment but decide the empty/loading state once in the parent.
There was a problem hiding this comment.
Fixed in 7ff76e6. Each environment draws only its rows, or one pending row while it loads (only the first pending row shows). A single only: empty row stands for the whole section.
| }) { | ||
| const navigate = useNavigate(); | ||
| const space = useSpaceOfProject(scopeProjectRef(props.project.environmentId, props.project.id)); | ||
| const shells = useThreadShellsForProjectRefs(space?.memberProjectRefs ?? NO_PROJECT_REFS); |
There was a problem hiding this comment.
Perf: the tab row re-renders on every streamed token in the space. useThreadShellsForProjectRefs is the non-quiet shell list, and the row also subscribes to the whole threadLastVisitedAtById map. Each update reruns the visibility filter, the status resolution per tab, rankSpaceTabs, and the signature joins, in the chat column while an agent streams. spaceProjects.ts deliberately avoids exactly this ("reading every thread shell … would re-render them on every streamed token"). Worth a quiet selector (the sameApartFromStreaming approach used by quietThreadShellAtom) or a memoized derived atom for the row's inputs. SpaceThreads in SpaceOverview has the same shape.
There was a problem hiding this comment.
Declining for this PR. Upstream's BranchToolbar subscribes to the same non-quiet useThreadShellsForProjectRefs in the same chat column. A quiet multi-project variant needs a new atom in packages/client-runtime, which is shared code outside this client-only fork change. Worth a follow-up that fixes both.
| return ( | ||
| <div className="flex flex-col gap-6" data-testid="space-changelog"> | ||
| {weeks.map((week) => ( | ||
| <section key={week.label} className="flex flex-col gap-1"> |
There was a problem hiding this comment.
Nit: week sections are keyed by week.label ("Week of Oct 5"), which has no year. If the 100 merged PRs span more than a year and two Mondays share a date, React gets a duplicate key and the reader sees two identical headings. Key by week.start, and consider adding the year to labels outside the current year.
There was a problem hiding this comment.
Fixed in 7ff76e6: weeks are keyed by their Monday (week.start). Adding the year to old labels is left as a follow-up.
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
… lists - Hide only a space's current Home draft; older drafts stay listed. - Find a space's Home by its member projects, so Scratch resumes and lights. - Prune archived and hidden tabs from storage. - Keep a cross-space split partner as a tab. - Name the row after a focused draft's own space. - Leave the inline right panel's header-relative control at the header's top. - List each PR once across environments (shared useSpacePullRequests). - One Loading/empty row for a multi-environment automations section. - Key changelog weeks by their start. - Unexport four symbols knip flagged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- SidebarV2SpaceHeader replaces the project header's space mode; both share SidebarV2SectionActions, and the project header is back to its own shape. - buildSpaceSections models each space once (group, summary, open PRs, folded Threads row); the list looks a section up instead of re-deriving it. - The list carries data-fork-grouping; CSS and the list animation read it instead of inferring Spaces from a header's presence. - The Threads row's marks are the cards' own topStatus, deduped by mark. - Shelves draw one caret; the split tray reads its cards' shape from the pair. - useOpenSpaceHome owns its new-thread fallback; tab storage reconciles in one pure, tested step; thread marks and PR routes live in shared modules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The space header destructures its sortable state, so the compiler sees plain values rather than reads off a ref-holding object. - Its pill's focus ring is inset, not drawn outside where the row clips it. - The space model is a real hook (useSpaceSections reads the visit watermarks itself, only while grouping by space), so the compiler memoizes it without a manual useMemo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #240 (landing draft cleanup) and #241 (design mode fixes). Upstream moved the right panel's width hook from ChatView into ChatRightPanel, so design mode's panel-room floor now reaches it through a fenced minWidth prop; manifest watch entry and guard follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The landing-draft header and a guard header were over four lines, the panel-room effect had no stated reason, and the boot browser test cleaned up in afterEach instead of a disposable. All four tripped the lint-owned ratchet once custom merged in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Takes the upstream sync (#239) ahead of it landing on custom. The one conflict is the draft overlay in ChatView: both the Space Home stamp and the new-agent-draft className fence stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sync moved or removed what Spaces built on: project grouping now lives in client-runtime, the sidebar visibility filter takes the delegated-child set, the thread menu needs canShareWithGuest, ChatHeader no longer exports its rename helper, formatRelativeTimeUntil is gone, and cards gained a "request" status (ranked with what needs you). Spaces' comments are cut to the new four-line limit, and its render-time clocks and tab-list effect carry the reasons upstream's lint now asks for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oject The revert dropped the project-section marker #242 put on split-pair rows, which the retained project grouping's hover reveal and drop edge also rely on. Restore just the marker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Threads, PRs, changelogs and automations for a project live in separate places, so there's no one spot to work "in" a project. Spaces gives each project one.
What changed
/changelog?space=page of merged PRs by week. Automations opens/scheduled?space=.The work is client-only (
fork-spacesin.fork/customizations.yaml, guard testforkSpaces.test.ts). The Feedback and Figma rows are deferred because they need server work.Ported onto the upstream sync
This branch now carries #239 (04f2e74) so it lands after the sync. 838c9a2 ports Spaces to what the sync changed: project grouping moved into client-runtime, the sidebar visibility filter takes the delegated-child set, the thread menu needs
canShareWithGuest, the rename helper andformatRelativeTimeUntilare gone, and the new "request" card status ranks with what needs you. Comments are cut to the new four-line limit, and render-time clocks and the tab-list effect carry reasoned lint disables, as upstream does in the sidebar.Reviewed with sol-loop: 3 rounds with gpt-6.1-sol on high. Sol verdict: approved
Built with Claude Opus 5.5 in Claude Code (running in T3 Code).
🤖 Generated with Claude Code