Skip to content

Studio: keep chat input visible and fix compare pane clipping#4924

Merged
danielhanchen merged 5 commits intounslothai:mainfrom
Imagineer99:fix/chat-visible-scrolling
Apr 9, 2026
Merged

Studio: keep chat input visible and fix compare pane clipping#4924
danielhanchen merged 5 commits intounslothai:mainfrom
Imagineer99:fix/chat-visible-scrolling

Conversation

@Imagineer99
Copy link
Copy Markdown
Collaborator

Improves chat layout so the input stays usable while scrolling in normal chat, and fixes compare mode where the thread footer visually covered the bottom of the chat content.

Chat:
image

Compare:
image

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91066df24b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/frontend/src/components/assistant-ui/thread.tsx Outdated
@Imagineer99 Imagineer99 marked this pull request as draft April 8, 2026 16:04
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the UI layout for the thread viewport and chat composer components to improve visibility and positioning. The reviewer suggested refactoring the CSS class logic in the thread component for better clarity and recommended extracting the duplicated sticky composer styles into a shared constant to improve maintainability.

Comment thread studio/frontend/src/components/assistant-ui/thread.tsx
Comment thread studio/frontend/src/features/chat/chat-page.tsx Outdated
Comment thread studio/frontend/src/features/chat/chat-page.tsx Outdated
- Remove unused `compact` prop from ThreadScrollToBottom call site
  (component is FC with no props, passing it caused TS2322)
- Extract shared classes (sticky, bottom-0, z-20, bg-transparent) from
  ternary branches into the unconditional className string
- Restore `relative` on normal-mode footer so the inner absolute
  bg-background strip has a positioning context
- Remove redundant md:pb-3 / md:pb-4 (same value as base pb-3 / pb-4)
- Remove no-op `sticky bottom-0` from SharedComposer wrapper in both
  LoraCompareContent and GeneralCompareContent (flex layout with
  shrink-0 already pins it at the bottom; parent has no scrollable
  overflow for sticky to bind to)
- Fix truncated comment on pointer-events rationale
@danielhanchen danielhanchen merged commit dc16e0c into unslothai:main Apr 9, 2026
1 check failed
danielhanchen pushed a commit to Datta0/unsloth-staging-3 that referenced this pull request Apr 13, 2026
Two reviewer.py reports flagged that workers had pr_diff.json /
pr_changes.diff but no `_pre_pr/` clone, forcing them to fall back to
diff-only review without a real before/after tree to simulate against.

Two root causes:

1. Partial-clone-of-partial-clone fragility. _clone_single_repo uses
   `git clone --filter=blob:none` (treeless), and the previous
   _setup_pre_pr_artifacts path then ran `git clone --no-hardlinks`
   from THAT treeless source followed by `git checkout --detach
   <merge_base_sha>`. On a partial clone the checkout step can't
   always materialize blobs at an arbitrary historic SHA, and we
   silently swallowed the failure.

2. Silent failure mode. The post-clone status block printed
   `<repo>_pre_pr/: present` only on success - the failure path
   printed nothing, so reports of "no pre_pr clone" couldn't be
   correlated with a specific reviewer.py log line.

Changes:

- Replace clone+checkout with `git archive --format=tar <sha> | tar
  -x -C pre_dir`. git archive only needs the trees + blobs for the
  one specified commit and will promisor-fetch them on demand from
  the partial-clone source. The result is a flat directory of files
  with no .git overhead, which is exactly what the worker needs
  (it diffs files - it doesn't need git history on the pre_pr tree).

- Add a fallback path: if the first archive attempt fails (genuinely
  missing blobs), explicitly `git fetch --filter=blob:none origin
  <merge_base_sha>` to populate the source's object store, then
  retry. GitHub supports `uploadpack.allowReachableSHA1InWant=true`
  so fetching by SHA works.

- Wrap both attempts in detailed error capture so any remaining
  failure surfaces with the actual git/tar stderr in the reviewer.py
  log instead of being silently dropped.

- Always print `<repo>_pre_pr/: present (N files)` OR `<repo>_pre_pr/:
  MISSING (workers will fall back to pr_diff.json...)` in the
  post-clone status block, parallel to the existing `.venv:
  present|MISSING` line. Now if the user sees "no _pre_pr clone" in
  a worker, they can scan the reviewer.py log for the matching line
  and the WARNING that explains why.

Smoke tested on unslothai/unsloth#4924 with a fresh treeless partial
clone: git archive extracts the merge-base tree (80 top-level files,
including install.sh) successfully.
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.

2 participants