Studio: keep project chats in Recents, add a Projects sidebar section by shimmyshimmer · Pull Request #8932 · unslothai/unsloth · GitHub
Skip to content

Studio: keep project chats in Recents, add a Projects sidebar section - #8932

Merged
shimmyshimmer merged 19 commits into
mainfrom
studio-sidebar-projects-recents
Aug 16, 2026
Merged

Studio: keep project chats in Recents, add a Projects sidebar section#8932
shimmyshimmer merged 19 commits into
mainfrom
studio-sidebar-projects-recents

Conversation

@shimmyshimmer

@shimmyshimmer shimmyshimmer commented Aug 15, 2026

Copy link
Copy Markdown
Member

Problem

Project folders only appeared in the sidebar once you pinned a project, so a chat started inside a project had nowhere visible to live. It existed and was reachable, but nothing in the sidebar showed it.

That falls out of #5725, which added projects and filtered them out of the Recents list:

const recentChatItems = useMemo(
  () => allChatItems.filter((item) => !item.projectId),
  [allChatItems],
);

#6288 later extended the same filter to hold back pinned chats, and #7291 added pinned projects with nested chats, but the exclusion itself dates to #5725.

Changes

Projects is its own section. Every project appears as a folder with its chats nested underneath, pinned ones first in pin order and the rest by activity. Pinning now sorts a project to the top of that list instead of moving it into a separate section. Folders show 4 chats before "Show more", and the section shows 5 folders before its own. The folder menu gained a Pin project entry, since it was unpin only back when nothing but pinned projects rendered.

Whether that section exists is a setting. Chat settings has a Show projects section switch, and the sidebar header menu writes the same preference. On, which is the default, project chats live in their folder and are not repeated in Recents. Off, there is no Projects section and those chats list in Recents instead, so they are always somewhere visible rather than nowhere.

Project folders drag to reorder. Always available, not gated on the chat sort. A dragged order outranks the pinned-first-then-activity rule, and a project that was never dragged keeps that rule and stays on top, so a newly created project does not sink to the bottom of an old saved order.

Select several rows at once. Cmd click, or ctrl click on Windows and Linux, toggles a row. Shift click takes the block between the anchor and the row. Right clicking opens the bulk actions over the selection. Chat rows, in Pinned, Recents and inside a project, offer Pin chats or Unpin chats, Archive chats, Mark as unread and Delete chats. Project folders offer Pin projects or Unpin projects and Delete projects. Escape or any plain click clears the selection.

Folders and chats select separately: they share no bulk action, so picking one kind drops the other rather than leaving a mixed selection the menu cannot describe.

The modifier is read off the browser rather than the platform store, since that store gets corrected to the host's platform: a Mac browser talking to a Linux host still uses cmd. Ctrl is left alone on macOS, where ctrl click is the right click chord and would otherwise both extend the selection and open the menu.

Selection holds ids and resolves against the live rows, so a chat deleted from another tab leaves nothing behind to act on. Ranges are per list, because one chat can sit in both a project and Recents. Bulk delete goes through the same confirm dialog and honours the same skip-confirm preference as a single delete, and bulk archive shows one notice rather than one per chat.

Deleting a chat can always take its files. Data settings gains an Always delete files switch. With it on, the delete dialog opens with the files toggle already set, so what is about to happen is visible and can still be turned off for a single delete, and the paths that skip the confirm dialog remove the folder too. All three chat delete paths read it: the sidebar, the chat page and the archived chats dialog. Deleting a project keeps asking from scratch, since removing a whole workspace is a bigger thing than one chat's sandbox.

It is off by default, and a preferences payload written before the setting existed reads as off, so no existing install quietly starts deleting files.

Video is a top-level sidebar row. It sits under Images, where SIDEBAR_NAV_ITEM_IDS already orders it, so the media tabs stay together instead of Video hiding in the More flyout. The backend default moves with it, since a record predating sidebarNav is served the backend's own layout and a parity test compares the two.

The layout this replaces is recorded as a shipped default and the store version goes to 7, so an install still sitting on it adopts the new one while a sidebar the user arranged themselves is untouched.

Each list header reveals actions on hover. A "..." menu on Pinned, Projects and Recents, plus "+" on Projects and a compose button on Recents. The compose button starts a chat outside any project, whatever page is open.

The "..." menu carries:

  • Organize sidebar: By project or In one list, the same preference as the settings switch.
  • Sort chats by: Priority (default), Last updated, or Manual order. Priority lifts the rows wanting attention, generating first, then queued turns, then unread output, and settles ties by recency.

Pinned sorts independently and defaults to Manual order, because pin order already is a manual order built one pin at a time. Re-sorting the chat lists should not quietly rearrange rows that were placed by hand. Its menu is the sort half alone, labelled Sort pinned by.

Manual order is dragged into place, using native drag and drop with no new dependency. Order is saved per list, so dragging a chat inside a project cannot move it in Recents, and a drag started in one list is ignored if dropped in another. Rows that were never dragged keep the order their list already had rather than disappearing into an old saved order, and a drop while a list is collapsed behind "Show more" keeps the hidden rows in place. Touch and keyboard get Move up and Move down in the row menu, since neither can start a drag.

All of it persists in localStorage behind a validating merge, so an old or half written payload falls back to defaults per field.

Notes

Pinned projects no longer render in the Pinned section, since Projects lists every project. Pin state is untouched and now decides ordering instead.

Mark as unread writes to the session-level unread state the sidebar already keeps for finished background runs, so it clears on reload the way that state always has. Persisting unread across reloads would change the existing feature and is left out of this PR.

sidebar-action-rows-inert.test.ts counts the row groups sharing the scroll padding. Projects is a new group, so the count went from 4 to 5.

Wrapping each chat row in a context menu re-indented the row renderer, so that commit reads as 545 added and 323 removed lines where the real change is 236 and 14. git diff -w cuts the noise out.

The only backend change is the sidebar nav default in routes/settings.py, which has to match the frontend. No training or inference paths are touched.

Testing

  • npm run typecheck clean
  • npm test: 2724 pass, 0 fail, with new cases covering the delete-files preference default and its reach across all three delete paths, reorder semantics, drop indicator edges against the reorder they describe, menu moves matching drags, undragged rows, per list isolation, the Recents rule in both modes, shift-click ranges, cmd-click toggling and bulk pin ordering
  • npm run i18n:check:strict clean, with every new string keyed and added to all 11 overlays
  • npm run build clean
  • pytest studio/backend/tests/test_personalization_settings.py: 33 pass, covering the moved backend nav default

Filing a chat under a project removed it from Recents (#5725), so a new
project chat looked like it went nowhere. Recents is the whole history
again, project chats included, and only pinned chats are held back since
the Pinned section renders those.

Projects is now its own section listing every project as a folder with
its chats nested underneath, rather than folders only appearing once a
project was pinned. Pinning sorts a project to the top of that list.

Each list header (Pinned, Projects, Recents) reveals actions on hover:
a "..." menu, plus "+" on Projects and a compose button on Recents.
The menu carries Organize sidebar (By project, In one list) and a sort
(Priority, Last updated, Manual order). Pinned sorts on its own and
defaults to manual, since pin order already is a manual order.

Manual order is dragged into place and is per list, so reordering a
chat inside a project cannot move it in Recents.
@chatgpt-codex-connector

Copy link
Copy Markdown

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

Folders drag regardless of the chat sort, the way the chat rows do under
Manual order. A dragged order outranks the pinned-first-then-activity
rule; projects never dragged keep that rule and stay on top.

Pulls the drag handlers out of the chat row into one helper both rows
use, and replaces the manual-sort comparator with a shared
applyManualOrder. That also fixes Pinned: an undragged pinned chat now
keeps pin order instead of falling back to last updated.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

- Order projects by their chats' activity. A project's own updatedAt only
  moves when its name, instructions or archived flag change, so an old
  project with a busy chat was sorting below one that was merely renamed.
- Add Move up and Move down to the row menus. Touch browsers never fire
  dragstart and a keyboard cannot drag, so manual order was unreachable
  there. moveIdBy is tested to agree with dragging onto the neighbour.
- Build each list's id array once instead of per row, and pass the row
  index in, so rendering N rows no longer allocates N arrays of length N.
- Key the new menu strings and add them to all 11 locale overlays.
- Paint the drop indicator on the edge the row actually lands on. Dragging
  down inserts after the target, so a cue always drawn above it was wrong.
  dropEdgeFor is checked against reorderIds for every pair.
- Re-measure the scroll fade when regrouping, collapsing a folder or
  revealing more projects changes the row count.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

Went through all six suggestions. Every one of them held up under checking, so all six are fixed in dd5cfec.

Sort projects by their chats' activity. Confirmed: update_chat_project is only ever called from the PATCH route, so chat_projects.updated_at moves on rename, instructions and archive, never on a message. Ordering now takes the max of the project's own timestamp and its member chats'.

Drop indicator edge. Confirmed, and my own test asserted the contradiction: reorderIds(["a","b","c","d"], "a", "c") returns ["b","c","a","d"], so the row lands after the target while the cue was always painted above it. Rather than change the insertion index, which would make the last slot unreachable when dragging down, the cue is now direction aware. dropEdgeFor is checked against reorderIds for every ordered pair, so the two cannot drift apart.

Touch reorder. Correct, and it applies to keyboard users too. Rows in a manually ordered list now carry Move up and Move down in their menu, and project folders always do. moveIdBy is tested to produce the same order as dragging onto the neighbour, so the two paths cannot disagree.

One ID array per list. Correct. Hoisted into memos per list. While fixing it I hit the same shape in the new menu items, since JSX children are evaluated even while a menu is closed, so the row index is passed in rather than looked up with indexOf per row.

i18n. Correct, and the README is explicit that a key must land in every overlay in the same change. The strings are keyed under shell.organize and added to all 11 overlays. i18n:check:strict passes.

Scroll fade. Confirmed: the effect deps were recentChatItems.length, runItems.length, projects.length, the three disclosure flags, isStudioRoute and showUpdateCard. Switching between By project and In one list adds or removes every nested row with no scroll and no collapsible animation to re-measure from. It now depends on the rendered project row count, which also covers collapsing a folder and Show more.

Checks: typecheck clean, 2712 tests pass with 3 new cases for the edge and move helpers, i18n:check:strict clean, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

chatgpt-codex-connector[bot]

This comment was marked as resolved.

The folder's actions lacked sidebar-touch-reveal, and the coarse-pointer
rule only un-hides row actions carrying it, so the Project options menu
was inert on touch. That menu is the only reorder path there, since touch
never fires dragstart, leaving folders unorderable on a touch sidebar.

Reveals both folder actions on touch and reserves the same padding hover
does, or the name runs under them. Test covers the rule, the rows that
carry reorder controls, and the padding.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

Checked this one and it is right, fixed in 7d76021.

The chain is exactly as described. .sidebar-row-action is opacity-0 pointer-events-none, and the @media (pointer: coarse) block only un-hides .sidebar-row-action.sidebar-touch-reveal. The chat rows carry that class, but the folder row's Project options trigger and its compose button never did, so on touch the menu could not be opened at all. Since touch never fires dragstart, that menu is the only reorder path there, and folders were unorderable on a touch sidebar.

Both folder actions now carry sidebar-touch-reveal, and the row reserves pr-16 on coarse pointers to match what hover already reserves, otherwise the always-visible buttons sit on top of the project name. The comment on the coarse-pointer rule said only chat rows reserve touch padding, which is no longer true, so it was updated.

Added sidebar-touch-reorder.test.ts covering the CSS rule itself, every row action belonging to a row that carries reorder controls, and the reserved padding. Confirmed it fails without the fix rather than passing vacuously.

Worth noting for scope: the Images, nav Projects and training run rows also lack the class. That predates this PR and none of them own reorder controls, so I left them alone rather than widening the diff.

Checks: typecheck clean, 2715 tests pass, i18n:check:strict clean, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 7d76021e9e

ℹ️ 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".

shimmyshimmer added 4 commits August 15, 2026 18:41
Cmd click (ctrl on Windows and Linux) toggles a row, shift click takes
the block from the anchor. Right click opens Pin chats, Archive chats,
Mark as unread and Delete chats over the whole selection, and Escape or
any plain click clears it.

The modifier is read off the browser rather than the platform store,
which the backend corrects: a Mac browser on a Linux host still uses cmd,
and ctrl stays free on macOS where ctrl click is the right click chord.

Selection holds ids and resolves against the live rows, so a chat deleted
elsewhere leaves nothing behind to act on. Ranges are per list, since one
chat can sit in both a project and Recents.

Mark as unread writes to the same session-level unread state the sidebar
already keeps, so it clears on reload like the rest of it.
Project chats no longer appear in both their folder and Recents. With the
Projects section on they stay in the folder; with it off there is no
section and they list in Recents, so a new project chat is still visible
somewhere. Appearance settings carries the switch, and the header menu
writes the same preference.

A selected row now takes the plain hover fill, with no outline.
Data settings gains an Always delete files switch. With it on, the delete
dialog opens with the files toggle already set, so what will happen is
visible and can still be turned off for one delete, and the paths that
skip the dialog remove the folder too.

All three chat delete paths read it: the sidebar, the chat page and the
archived chats dialog. Deleting a project keeps asking from scratch,
since removing a whole workspace is a bigger thing than one sandbox.

Off by default, and a preferences payload written before this setting
existed reads as off, so no install starts deleting files on its own.
Video ships as a top-level row again instead of sitting in the More
flyout. It lands under Images, where SIDEBAR_NAV_ITEM_IDS already puts
it, so the media tabs stay together.

The backend default moves with it, since a record predating sidebarNav is
served the backend's own layout, and the parity test compares the two.

Recorded the version 6 layout as a shipped default and bumped the store
to 7, so an install still on that layout adopts this one while a sidebar
the user arranged themselves is left alone.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Two more paths delete chats and both ignored the setting. Clear all chats
left every sandbox behind, and after it there is no chat row left to
reach those folders from. Deleting a chat from Voice, Recent Dictations
did the same for one chat.

The DELETE endpoint already takes delete_files; only clearAllChats and
clearStoredChats dropped it on the way through, so both now pass it on.

The test that covers this now names all five delete paths, and checks
each hop of the clear-all chain, since a dropped hop still compiles.
@shimmyshimmer

Copy link
Copy Markdown
Member Author

Checked both halves of this and both are right, fixed in 4b41fff.

Recent Dictations. Confirmed: it builds a SidebarItem and calls deleteChatItem with three arguments, so the options object never arrives. It now passes the preference like the other row-level deletes.

Clear all chats. Confirmed, and worse than the single-chat case, because after a clear there is no row left to reach those folders from. The endpoint itself was never the problem: clearBackendChats already takes deleteFiles and appends ?delete_files=true. The option was being dropped in the two wrappers above it, clearAllChats and clearStoredChats, both of which took no arguments at all. Both thread it through now.

One thing I left as is: clearStoredChats dedupes a clear that is already in flight, so a second call would return the first one's result and ignore its own options. Only one call site can start a clear, and it is behind a confirm dialog and a busy flag, so this is unreachable rather than fixed. The dedupe is what stops two clears racing, so I did not want to key it on arguments.

The offerToDeleteKeptSandboxes call after a clear stays either way. Those are sandboxes the backend could not remove even when asked, exactly as on the single-delete path, so the offer is still the only notice. I updated its comment, which claimed clear-all has no switch.

My earlier test asserted three delete paths read the preference, which is what let these two through. It now names all five, and separately checks each hop of the clear-all chain, since a hop that drops the option still compiles and still passes a "does this file mention the preference" check. Confirmed it fails when a hop is removed.

Checks: typecheck clean, 2725 tests pass, i18n:check:strict clean, build clean.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

The Show projects section switch is a chat setting, not an appearance
one, so it moves to the Chat tab and its keys move with it.

Project folders now select like chat rows: cmd click toggles, shift click
takes a block, and right click offers Pin projects and Delete projects
over the selection. Folders and chats select separately, since a mixed
selection has no action they share, so picking one kind drops the other.

Test names the lists that hand rows a selection context. The argument is
optional, so dropping it compiles and only stops the row selecting.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Pinning a project chat adds a Pinned row while Recents never held it and
the folder still counts it, so no dependency of the fade effect moved and
the bottom fade kept a stale value.

Regrouping has the same hole when the projects hold no chats: the folder
rows appear and disappear while every count stays at zero.

Test names every row-count input the effect must depend on. This is the
second time a missing one has gone unnoticed, and it never shows up as a
failure, only as a fade that is wrong until the next scroll.
@shimmyshimmer

Copy link
Copy Markdown
Member Author

Right again, fixed in eb06332.

Traced it through: with grouping on, a project chat is already excluded from recentChatItems, so pinning it does not change that length. chatsByProjectId keeps pinned chats, so projectChatRowCount does not move either, and neither does visibleProjectRecords.length or projects.length. Pinned still gains a row, and if it was the first pin the whole section appears. So the list grows with nothing in the dependency array changing, and the fade keeps its old value until an unrelated scroll or resize.

Worth noting this is a consequence of an earlier change in this PR. Before it, pinning a project chat pulled it out of the folder, so the folder count moved and the effect happened to re-run.

Chasing it turned up a second hole in the same array: regrouping when the projects hold no chats. projectChatRowCount is zero either way, recentChatItems has no project chats to gain or lose, and visibleProjectRecords.length is unchanged, yet the folder rows appear and disappear. organizeBy is now a dependency too.

Added sidebar-scroll-fade-deps.test.ts naming every row-count input the effect has to depend on. This is the second time a missing one has gone unnoticed here, and it never surfaces as a failure, only as a fade that is wrong until the next scroll. Confirmed it fails when a dependency is removed.

The other comment on this file is the delete-files one from the last round, already fixed in 4b41fff; it just re-anchored to a new line.

Checks: typecheck clean, 2729 tests pass, i18n:check:strict clean, build clean.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Folders stayed highlighted when a chat menu opened.
@shimmyshimmer

Copy link
Copy Markdown
Member Author

Confirmed, fixed in e906acc.

selectForContextMenu was the only one of the four selection entry points that did not clear the other kind. The other three do: both click handlers clear on the way in, and selectProjectForContextMenu clears chats. So cmd-clicking two folders and then right-clicking a chat left all three rows highlighted while the menu only ever acted on the chat.

Rather than patch the one function, I pulled the clearing into dropChatSelection and dropProjectSelection and routed all four through them, so the invariant lives in one place instead of being restated four times. Both context-menu helpers now clear before their early return, which covers the already-selected case you called out.

Two things the helpers also fix, since they clear the anchor along with the ids. The anchor used to survive a clear, so selecting a folder, clicking a chat, then shift-clicking another folder selected the whole range from a folder that no longer looked selected. Same on the chat side. The anchor now always matches what is visible.

Three tests: all four entry points drop the other kind, both right-click paths drop it before the early return, and each helper clears its anchor. Verified the first two fail with the original shape, and that the ordering one specifically fails when the drop is moved below the early return.

Checks: typecheck clean, 2734 tests pass, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

Both correct. Fixed in e8810d3.

Preference reset: unsloth_sidebar_organization was not in PREFS_KEYS, so grouping, sort and the manual row order survived a reset that promises to clear everything. The store now exports SIDEBAR_ORGANIZATION_STORAGE_KEY and both the persist config and the reset list read it, so the two cannot drift apart. Added the same guard test the panel-prefs and sidebar-width keys already have, and confirmed it fails with the key removed from the list.

Archive batch: the try wrapped the loop, so the first failure exited it and every later chat was left unarchived, with the selection already cleared and nothing to retry from. The success toast was skipped too, even if nine of ten had gone through. It now catches per item and reports on the count: the "view archived chats" notice if any succeeded, the error toast if any did not, and a partial batch gets both. That also matches the two bulk loops next to it, since deleteChatWithCleanup catches per chat and the project loop has its own try inside.

I kept the error description as the last failure's message rather than a count, to avoid a new plural key across twelve locales for a case the existing key already covers.

Test asserts the try sits inside the loop and that the toasts key off archived, not off whether the loop threw. Confirmed it fails against the previous shape.

Checks: typecheck clean, 2736 tests pass, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

Correct, fixed in 1177b3f.

The Projects expander is new here and was rendering literal English. Added shell.navigation.showMore and showLess across all twelve locales and pointed it at them.

I converted the per-project chat expander in the same pass. That one predates this PR, but the two controls sit inches apart and render identical text, so translating one and leaving the other is the more visible half of the problem. It reuses the same keys, so no extra strings.

Test asserts neither expander still renders the literal ternary and that both read both keys. Confirmed it fails before the change, and the strict parity check passes with the new keys in every overlay.

Checks: typecheck clean, i18n strict parity clean, 2737 tests pass, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Two dialogs deleted files the preference had turned on without saying so.
@shimmyshimmer

Copy link
Copy Markdown
Member Author

Confirmed, fixed in 5024733.

Both dialogs read alwaysDeleteChatFiles at delete time while showing nothing but "This cannot be undone", so with the preference on they removed a sandbox the user was never shown. The sidebar dialog already preselects a switch from the same preference and lets you turn it off for one delete, so the three confirmations disagreed about the same setting.

Rather than copy that block twice, the switch is now a small shared component and both dialogs render it, preselected from the preference and reset per open. The executor takes the confirmed value as an argument instead of reading the store, so turning the switch off actually reaches the request. Without that the switch would have been decorative.

The sidebar keeps its own richer version, since it also covers projects and bulk selections, but its label and single-chat description now read the same two keys the shared component does, so the copy has one source. Those were literals, so this also removes two more untranslated strings; new keys added across all twelve locales.

Three assertions: every confirmation renders a switch bound to the state, each preselects it from the preference, and neither executor still reads the preference past its own switch. My first version of the first assertion matched leftover state rather than the rendered control, so it passed with the switch deleted; it now matches checked={deleteFilesOnDelete} and I confirmed it fails when the switch is removed.

Checks: typecheck clean, i18n strict parity clean, 2739 tests pass, build clean.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

Both correct, fixed in e5d008e. These were the last two of the five paths that can reach a chat delete, and I should have swept them all in the previous commit rather than only the two that were flagged.

Clear chats: the confirmation showed only clearChatsConfirmDescription while handleClear passed the preference straight through, so with it on the button silently authorised removing every chat's sandbox. It now renders the switch and clears with whatever the switch says. The shared component takes an optional description for this one, since the per-chat wording does not fit an operation covering every chat; it uses the existing settings.data-adjacent plural key rather than a new string.

Linked dictation: same shape on "Delete chat and dictation". The switch renders only in that branch, since clearing dictation history and deleting an unlinked dictation never touch a chat, and confirmDeleteWithChat now takes the confirmed value instead of reading the store.

The coverage test now walks all five dialogs and checks each renders a switch bound to its own state, preselects it from the preference, and hands the confirmed value to the executor rather than re-reading the preference. Confirmed both new entries fail when reverted to the previous shape.

Checks: typecheck clean, i18n strict parity clean, 2739 tests pass, build clean.

@shimmyshimmer

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

@shimmyshimmer
shimmyshimmer merged commit 2643d0e into main Aug 16, 2026
52 of 60 checks passed
@shimmyshimmer
shimmyshimmer deleted the studio-sidebar-projects-recents branch August 16, 2026 05:48
danielhanchen added a commit that referenced this pull request Aug 16, 2026
…ering

#8932 added SIDEBAR_ORGANIZATION_STORAGE_KEY to the @/features/chat barrel and
had general-tab.tsx read it back out of that barrel. The key is used at MODULE
scope, in the storage-key list, and the barrel is part of an import cycle that
reaches this file, so the binding is still in its temporal dead zone when the
list is built:

  Cannot access 'SIDEBAR_ORGANIZATION_STORAGE_KEY' before initialization

That kills the whole module graph, so the page renders nothing. It is why
Frontend CI has been failing on main with a Playwright locator that finds no
elements, which reads as a flaky browser test rather than a module-init error.

Importing the key straight from its module breaks the cycle. Verified by
bisection with the real browser smoke: it passes at cfee137 (before #8932),
fails on main with the TDZ error above, and passes again with this one-line
change. Typecheck clean, 2756 frontend tests pass.

#8932's own branch was already red with this exact failure before it merged.
danielhanchen added a commit that referenced this pull request Aug 16, 2026
Both fail on pristine origin/main, so they are not from this branch. #8932
moved the sidebar's user-visible copy into the locale file and rewrote the
delete-switch predicate to cover its new bulk targets. Neither change breaks a
contract; both broke a grep of app-sidebar.tsx.

test_the_delete_switch_does_not_promise_project_files greps for the sentence
"This chat's own sandbox folder is removed from disk." It is still there,
verbatim, in studio/frontend/src/i18n/locales/en.ts. The promise is the
contract, not its address, so the test now searches the frontend sources and
survives the next move while still failing on a reworded promise.

test_the_delete_switch_reaches_a_chat_moved_into_a_project pinned one spelling
of deleteTargetHasFiles:

    -return target.kind === "project" || target.kind === "chat";
    +return target.kind !== "run";

Same answer for a chat and for a project, plus the new "chats" / "projects"
bulk kinds. What must hold is that a run is excluded and that project
membership is never consulted, so the test reads the brace-matched function
body and asserts that instead of the old one-liner.

Both were checked by mutation: reinstating the misleading copy, and gating
deleteTargetHasFiles on target.item.projectId, each fail the rewritten test.
danielhanchen added a commit that referenced this pull request Aug 16, 2026
Both fail on pristine origin/main, so neither comes from this branch, and in
both the source is right and the test-side constant is stale.

INLINE_ROW_IDS in tests/studio/playwright_mac_tab_capabilities.py still listed
four rows. #7863 had put Video under "More" as layout v5, so the script dropped
it: an unpinned row renders no data-testid and every assertion on it silently
observes nothing. #8932 pins Video under Images again, and records it in the
migration history as v7, so it is deliberate and versioned rather than an
accidental revert. Video is observable again, so the script samples it again.
test_inline_row_ids_match_the_frontends_default_pinned_set exists precisely to
catch this tuple drifting from the store in either direction, and it did its job.

test_multi_chat_prompt_queue_contract.py pinned the zero-argument spelling
"return await clearStoredChats();". #8932 gave the call an options argument,
which changes nothing about the ordering the assertion is there to hold, so it
now matches on the call prefix, the same way the sibling assertion three lines
up already does.

Both mutation-checked: unpinning Video in the store fails the first, and moving
requestPromptQueueStop after clearStoredChats fails the second.
danielhanchen added a commit that referenced this pull request Aug 16, 2026
Self-audit of an earlier fix in this branch, under the same standard applied to
everything else here.

test_the_delete_switch_does_not_promise_project_files greps for a promise the
delete dialog must make. #8932 moved that copy into the locale file, so the
original grep of app-sidebar.tsx broke, and my fix widened the search to the
whole frontend tree. That is the same defect the review caught in the rollback
ordering assertion: a search wide enough to be satisfied by an unrelated
occurrence proves nothing. The sentence appearing in any of ~1200 files, a
comment or a dead module included, would have passed it.

Now scoped by intent. The promise the dialog MUST make is looked for where the
sidebar's user-visible copy lives, the locales and the component. The promise it
must NOT make is still looked for across all of src, since breadth only makes a
negative stricter.

Mutation-checked: rewording the real string and planting the original in an
unrelated module fails the test, where the whole-tree form passed.

The two scopes are also read through an lru_cache. The wide one concatenates
about 1200 files, and it was re-reading every one of them on each call.
danielhanchen added a commit that referenced this pull request Aug 16, 2026
…ce defects (#8956)

* Fix CI on main: stale test doubles, a stale router stub, and two source defects

main has been red since Aug 14 and every open PR inherits it. Five clusters, none
of them caused by the PRs that were showing them.

context_length (13 tests, plus 4 more in disguise). #8700 added an unguarded
llama_backend.context_length read to the chat-completions path and updated five
test files, missing three. The real LlamaCppBackend has had the property for a
long time, so no user was ever affected; the doubles were simply
under-specified. The four gguf_stream_slot_release failures are the SAME bug:
those doubles reach the same line, but the AttributeError is swallowed into the
response task and surfaces as a 20 second timeout, which reads as a flake.

Nine hand-written doubles across five files each re-declared the same attribute
block with no shared base, so one new read broke whichever files happened not to
be updated. They now share FakeLlamaCppBackend, and a canary drives the real
route with a bare double so the next such read fails in one place, named, at the
point of the change. The stream waits no longer discard the driving task's
exception, so that class of failure cannot present as a bare timeout again.

youtube_router (2 tests). routes/__init__.py exports it and main.py imports it;
the app is fine. test_desktop_auth stubs sys.modules[routes] with a hardcoded
list of 17 routers, deliberately, to avoid importing the ML stack. #8648 added a
router and did not update it, the second time this has happened after
openai_codex_auth_router in #8511. The stub is now derived from main.py's own
import block, so it cannot go stale.

Repo tests (CPU), 6 failures, of which two are real source defects:
llama-extra-args.ts put the Studio brand into a user-visible validation message,
which the desktop branding contract forbids in runtime surfaces, and
test_playwright_server_lifecycle.py read checked-in files without an encoding,
which is a real Windows cp1252 crash the lint exists to catch. Both fixed in the
source. The other four are stale assertions chasing text that #8702 legitimately
moved or reflowed; they now assert the behaviour instead, via the real
override_lookup_candidates() and the element-scan pattern their own siblings
already use.

Not addressed here: pip scan-packages :: hf-stack reports 173 findings in
third-party deps under SCAN_ENFORCE=1 and is red on main too. Baselining a
supply-chain scanner to get green is the wrong reflex, so it wants its own look.

* Studio: break the settings/chat import cycle that stopped the UI rendering

#8932 added SIDEBAR_ORGANIZATION_STORAGE_KEY to the @/features/chat barrel and
had general-tab.tsx read it back out of that barrel. The key is used at MODULE
scope, in the storage-key list, and the barrel is part of an import cycle that
reaches this file, so the binding is still in its temporal dead zone when the
list is built:

  Cannot access 'SIDEBAR_ORGANIZATION_STORAGE_KEY' before initialization

That kills the whole module graph, so the page renders nothing. It is why
Frontend CI has been failing on main with a Playwright locator that finds no
elements, which reads as a flaky browser test rather than a module-init error.

Importing the key straight from its module breaks the cycle. Verified by
bisection with the real browser smoke: it passes at cfee137 (before #8932),
fails on main with the TDZ error above, and passes again with this one-line
change. Typecheck clean, 2756 frontend tests pass.

#8932's own branch was already red with this exact failure before it merged.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Security: fix the filesystem-enumeration false positives in scan-packages

The hf-stack shard of pip scan-packages has been red on main. One CRITICAL
was blocking it, and tracking it down turned up a pattern bug behind nine of
the entries already in the baseline.

The blocking finding, unsloth-zoo/llama_cpp.py under "Harvests environment
variables/secrets AND makes network calls", is first-party: _github_auth_headers()
reads GH_TOKEN so the llama.cpp releases API call is not rate limited. That
file and check were already reviewed and baselined. It reopened because
unsloth-zoo 2026.8.12 replaced

    keynames = "\n" + "\n".join(os.environ.keys())

with a targeted _is_colab_environment() helper. Every other matched line is
byte identical to what was reviewed, so the current code is a strict subset of
the approved evidence. The entry is refreshed, with the hash generated by
--write-baseline rather than by hand. No PR caused this; it is the baseline's
reopen-on-change working as designed against an upstream release.

The pattern bug is in RE_FS_ENUM:

    r"|\bhistory\b.*\bread\b"  # reading shell history

Under re.DOTALL that .* spans the whole file, so any module containing the
word "history" anywhere before the word "read" anywhere is filesystem
enumeration, and with a network call in the same file that is a CRITICAL.
That is httpx's Response.history, retries.history in urllib3, IPython's
history module, torch's CUDA memory history. Nine of the eleven baselined
CRITICALs under this check were that one alternative, each suppressing a whole
file for the check.

Meanwhile the precise half was dead. \b\.bash_history\b puts \b between "/"
and "." in "~/.bash_history", where neither side is a word character, so it
could never match; same for \b\.zsh_history\b. This is the unsatisfiable-\b
bug already fixed once for /proc/self/status. Checked against the old pattern:
it matched none of five real history-file reads and all four benign cases.

Naming the files instead inverts that. The nine dead baseline entries are
removed, which narrows the allowlist rather than widening it, and three tests
pin both directions.

Verified by running all three shards locally against the same requirements
transform CI uses: hf-stack now exits 0 (was 1), studio and extras stay at 0,
and no removed entry resurfaced.

* Fix two more stale sidebar contract tests left by #8932

Both fail on pristine origin/main, so they are not from this branch. #8932
moved the sidebar's user-visible copy into the locale file and rewrote the
delete-switch predicate to cover its new bulk targets. Neither change breaks a
contract; both broke a grep of app-sidebar.tsx.

test_the_delete_switch_does_not_promise_project_files greps for the sentence
"This chat's own sandbox folder is removed from disk." It is still there,
verbatim, in studio/frontend/src/i18n/locales/en.ts. The promise is the
contract, not its address, so the test now searches the frontend sources and
survives the next move while still failing on a reworded promise.

test_the_delete_switch_reaches_a_chat_moved_into_a_project pinned one spelling
of deleteTargetHasFiles:

    -return target.kind === "project" || target.kind === "chat";
    +return target.kind !== "run";

Same answer for a chat and for a project, plus the new "chats" / "projects"
bulk kinds. What must hold is that a run is excluded and that project
membership is never consulted, so the test reads the brace-matched function
body and asserts that instead of the old one-liner.

Both were checked by mutation: reinstating the misleading copy, and gating
deleteTargetHasFiles on target.item.projectId, each fail the rewritten test.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address the review: scope a contract test, keep a parity check, close a scanner gap

Four items, all confirmed against the code at head before changing anything.

1. tests/studio/test_model_picker_contracts.py: the rollback ordering assertion
searched the whole of chat-page.tsx, which takes the same snapshot in the hub
auto-load path at line 2558. The only applyModelLoadConfigToRuntime call is at
3262, so the index comparison was satisfied by the unrelated occurrence: deleting
the snapshot inside selectWithConfig outright still passed both assertions.
Verified by mutation. Now scoped to the selectWithConfig body, which fails on
that deletion.

2. studio/backend/tests/test_research_internal_call_tool_gate.py: dropping
perf_callback from the kwargs comparison hid presence as well as identity, so the
opt-out losing its callback on one path would have gone unnoticed and cost that
path its tok/s readout. Assert both are callable (or both absent) first, then
exclude. Verified by mutation at routes/inference.py:14512.

3. tests/studio/test_model_picker_contracts.py was source-only and ran without the
backend environment; calling the real ladder for a standalone .gguf pulled in
hub.utils.gguf, then loggers, then structlog, so a bare pytest run failed after
183 passes. CI installs studio.txt and is unaffected, so coverage there is
unchanged. The helper now skips on a missing third-party package only; a missing
first-party module still fails.

4. scripts/scan_packages.py: fish stores history at $XDG_DATA_HOME/fish/fish_history
with no leading dot (fishshell.com/docs/current/cmds/history.html), so the dotted
alternative could never match the real path and narrowing the pattern left an
exfiltration blind spot. Added a non-dotted form. The fish read plus a network call
is a CRITICAL again, the dotted shells still match, and the false positives stay
suppressed.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Tighten comments added by the CI repair

* Fix two more stale studio contract constants left by #8932

Both fail on pristine origin/main, so neither comes from this branch, and in
both the source is right and the test-side constant is stale.

INLINE_ROW_IDS in tests/studio/playwright_mac_tab_capabilities.py still listed
four rows. #7863 had put Video under "More" as layout v5, so the script dropped
it: an unpinned row renders no data-testid and every assertion on it silently
observes nothing. #8932 pins Video under Images again, and records it in the
migration history as v7, so it is deliberate and versioned rather than an
accidental revert. Video is observable again, so the script samples it again.
test_inline_row_ids_match_the_frontends_default_pinned_set exists precisely to
catch this tuple drifting from the store in either direction, and it did its job.

test_multi_chat_prompt_queue_contract.py pinned the zero-argument spelling
"return await clearStoredChats();". #8932 gave the call an options argument,
which changes nothing about the ordering the assertion is there to hold, so it
now matches on the call prefix, the same way the sibling assertion three lines
up already does.

Both mutation-checked: unpinning Video in the store fails the first, and moving
requestPromptQueueStop after clearStoredChats fails the second.

* Narrow a contract search I made too wide, and read it once

Self-audit of an earlier fix in this branch, under the same standard applied to
everything else here.

test_the_delete_switch_does_not_promise_project_files greps for a promise the
delete dialog must make. #8932 moved that copy into the locale file, so the
original grep of app-sidebar.tsx broke, and my fix widened the search to the
whole frontend tree. That is the same defect the review caught in the rollback
ordering assertion: a search wide enough to be satisfied by an unrelated
occurrence proves nothing. The sentence appearing in any of ~1200 files, a
comment or a dead module included, would have passed it.

Now scoped by intent. The promise the dialog MUST make is looked for where the
sidebar's user-visible copy lives, the locales and the component. The promise it
must NOT make is still looked for across all of src, since breadth only makes a
negative stricter.

Mutation-checked: rewording the real string and planting the original in an
unrelated module fails the test, where the whole-tree form passed.

The two scopes are also read through an lru_cache. The wide one concatenates
about 1200 files, and it was re-reading every one of them on each call.

* Address the second review: real scoping, not the appearance of it

Five items, each reproduced against the code at head before changing anything.
Three are cases where my own earlier fix looked scoped but was not.

deleteTargetHasFiles: the negative assertions did not establish the contract.
`return target.kind === "run";` -- the exact inversion, which hides the delete
switch for every chat and project -- mentions "run", mentions no projectId, and
contains neither prohibited expression, so it passed all four checks. Confirmed
by construction. Now the direction is pinned: run is the kind excluded, never
the one included.

The selectWithConfig slice ran to end of file, not to the callback's closing
brace: 13,566 characters rather than 579. Moving applyModelLoadConfigToRuntime
out of the callback while leaving the snapshot behind still passed. Brace-matched
now, and that mutation fails.

asgi_stream_helpers returned on the frame without inspecting the task, so a
send() that sets the event and then raises left both futures done, the frame
branch won, and the caller's gather(return_exceptions = True) discarded the
exception. That is the silence the helper exists to break. Reproduced directly.
The task is checked first now, and the message says whether the failure came
before or after the frame.

Two scanner gaps, both from narrowing RE_FS_ENUM. Constructed fish paths put a
quote rather than a separator before the basename, so Path.home() / "fish" /
"fish_history" and os.path.join(h, "fish", "fish_history") did not match. And the
dotted list omitted PowerShell's ConsoleHost_history.txt, Ruby's .irb_history and
SQLite's .sqlite_history. A quote now counts as a boundary and those names are
covered, case-insensitively for the Windows one. Ten read forms match, and the
httpx, urllib3, IPython and torch false positives stay suppressed.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant