ask: nine UI fixes (padding, session switching, share, graypaper link, shared cache) - #250
Merged
Conversation
Nine scoped fixes: padding, session switching, text-foreground drift, dropdown parity, session list sync, graypaper URL, "new chat" removal, "regenerate title" removal, dropdown share → flip public + copy + toast. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ten tasks covering padding, session switching, text-foreground drift, dropdown parity, session-list sync via react-query, graypaper URL, dropdown share → flip+copy+toast. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cache Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…YAGNI helper, stronger test) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for jam-search2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGraypaper URL generation now includes the active search query; frontend adds Sonner toasts and mounts a global Toaster; share behavior consolidated to an onShare flow that may mark sessions public, copies a share URL, and reports outcomes; session state migrated to TanStack Query with optimistic updates/invalidation; related UI, tests, and docs updated. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SessionRow
participant AskLayout
participant Supabase as API
participant Clipboard
participant Sonner
User->>SessionRow: Click "Share..."
SessionRow->>AskLayout: onShare(sessionId)
alt session not public
AskLayout->>Supabase: PATCH /ask_sessions/:id { isPublic: true }
Supabase-->>AskLayout: updated session
end
AskLayout->>AskLayout: build share URL (window.location)
AskLayout->>Clipboard: navigator.clipboard.writeText(url)
alt copy success
AskLayout->>Sonner: show "Link copied" / "Link copied. Session is public now"
else copy failure
AskLayout->>Sonner: show "Couldn't copy link…" (reason)
end
Sonner-->>User: display toast
sequenceDiagram
actor User
participant UI as Ask UI
participant useSessions
participant ReactQuery as Cache
participant Supabase as API
User->>UI: create/update/delete session
UI->>useSessions: call mutation
useSessions->>Supabase: perform DB operation
Supabase-->>useSessions: success
useSessions->>ReactQuery: optimistically update cache
useSessions->>ReactQuery: invalidate ["ask_sessions", userId]
ReactQuery->>Supabase: refetch sessions list
Supabase-->>ReactQuery: new list
ReactQuery-->>UI: UI updates via useQuery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/components/ask/AskLayout.tsx`:
- Around line 41-55: In the onShare handler, separate the session-publication
step from the clipboard step: first call sessions.update(id, { isPublic: true })
inside its own try/catch (or await it and handle its error) so you can confirm
the session became public using toast.success or toast.error about publishing;
then in a second try/catch attempt
navigator.clipboard.writeText(shareUrlFor(id)) and show a clipboard-specific
toast (e.g., "Link copied" on success or "Could not copy link, but session is
now public" on failure). Reference the onShare async handler, sessions.update,
navigator.clipboard.writeText, shareUrlFor, and toast.success/toast.error when
applying this change.
In `@docs/superpowers/specs/2026-04-24-ask-ui-fixes-design.md`:
- Around line 47-50: The spec incorrectly claims createUseSessions was removed;
update the design note to reflect the actual implementation where
createUseSessions is still exported and the default useSessions() delegates to
it. Edit the paragraph to state that sessions are implemented on top of
`@tanstack/react-query` with shared cache via queryKey: ["ask_sessions", userId],
mutations call queryClient.invalidateQueries, and explicitly mention that
createUseSessions and useSessions remain in the codebase (so test helpers can
call createUseSessions). Ensure the spec references the symbols
createUseSessions and useSessions and the behavior of
queryClient.invalidateQueries/queryKey accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de4abcce-37f6-4458-ae4f-a479c33d2a23
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
backend/src/ask/tools.tsclient/package.jsonclient/src/App.tsxclient/src/components/ask/AskLayout.tsxclient/src/components/ask/SessionRow.tsxclient/src/components/ask/SessionsSidebar.tsxclient/src/components/ask/__tests__/SessionsSidebar.test.tsxclient/src/hooks/__tests__/useSessions.test.tsclient/src/hooks/useSessions.tsclient/src/pages/ask.tsxclient/src/pages/askShared.tsxdocs/superpowers/plans/2026-04-24-ask-ui-fixes.mddocs/superpowers/specs/2026-04-24-ask-ui-fixes-design.md
- Split the onShare handler into two try/catches so a clipboard failure after a successful isPublic flip reports accurately: "Couldn't copy link — session is public now: …" instead of a blanket "Couldn't share". - Fix spec doc: createUseSessions factory is still exported and the default useSessions() still delegates to it (for test injection). Earlier draft wording implied it was removed. Addresses CodeRabbit review on PR #250. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause of the "UI doesn't switch / refresh loses the answer / new
chat breaks" symptoms: the hydrate and save effects in ask.tsx listed
the whole `sessions` API object in their deps. With react-query, that
object's identity churns on every mutation/refetch — both in AskPage
and in AskLayout's sibling useSessions. A single autosave invalidated
the list, triggered a refetch across both hooks, re-rendered AskPage
through the Outlet, cancelled the 100ms debounce, and rescheduled it
in perpetuity. Net effect: the answer never got saved, so refresh
brought back only the question.
Fixes:
- sessionsRef in ask.tsx carries the live `sessions` API into effects
without having it in the dep array. Hydrate/save effects now only
run when sessionId or state actually change.
- lastSavedStateRef dedups saves: the save effect skips when the
reducer state already matches what we persisted. Set on successful
save and on hydrate (the hydrated state is already in the DB).
- Nav-away flush: when sessionId goes from X to undefined, persist
the in-flight state snapshot to X before dispatch(reset) so
"New chat" mid-stream doesn't orphan the session with just the
question.
- useSessions.update only invalidates the shared cache when the patch
touches something the sidebar shows (title, isPublic). State-only
autosaves are silent — breaking the loop at the source.
- Optimistic cache writes: create() inserts the new summary at the
top, update({title|isPublic}) patches in place, remove() drops the
row — sidebar reflects changes instantly instead of waiting for the
refetch round-trip.
- Removed the dead createdRef branch in the hydrate effect; with
hydratedRef pre-set inside send(), the branch was unreachable.
- send() uses stateRef.current to avoid pulling `state` and
`sessions` into its useCallback deps, so its identity stays stable.
Tests cover: state-only update does not invalidate, optimistic
create/update/remove, and the existing shared-cache invariant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
client/src/hooks/useSessions.ts (2)
228-230: Empty dependency array is intentional but consider adding an eslint-disable comment.The
supabasereference from the factory closure is stable for the hook instance lifetime, as explained in lines 95-97. Adding an inline eslint-disable would make the intent explicit and prevent future linter warnings.📝 Suggested explicit disable
const get = useCallback(async (id: string) => { return fetchSessionById(supabase, id); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/hooks/useSessions.ts` around lines 228 - 230, The useCallback wrapper for get captures the supabase variable intentionally but has an empty dependency array; to make this explicit and silence linter warnings, add an inline eslint-disable comment for the react-hooks/exhaustive-deps rule immediately above the get useCallback declaration (referencing the get function and the supabase variable in useSessions.ts) so future reviewers and the linter know the omission is intentional.
126-143: Clarify comment terminology: this is an immediate cache update, not an optimistic update.The update happens in
onSuccess(after the mutation succeeds), not inonMutate(before). True optimistic updates would also need anonErrorrollback. The current approach is perfectly valid—it provides instant UI feedback after success without waiting for refetch—but calling it "optimistic" may confuse future maintainers familiar with React Query's optimistic update pattern.📝 Suggested comment clarification
onSuccess: (_data, args) => { - // Optimistic insert: put the new row at the top of the cached list - // so the sidebar reflects it immediately, before the refetch lands. + // Immediate cache update: put the new row at the top of the cached list + // so the sidebar reflects it immediately, before the refetch lands. queryClient.setQueryData<AskSessionSummary[]>(key, (old) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/hooks/useSessions.ts` around lines 126 - 143, The inline comment inside the onSuccess handler of useSessions.ts incorrectly calls the behavior an "Optimistic insert" even though the cache is updated after the mutation succeeds; update the comment to say this is an immediate cache update (or similar) and mention it happens in onSuccess rather than an optimistic onMutate flow that would require onError rollback (reference the onSuccess handler and onMutate terminology to locate the code and clarify intent).client/src/hooks/__tests__/useSessions.test.ts (1)
192-217: Consider usingvi.useFakeTimers()instead of real timeout.The 20ms
setTimeoutat line 215 is timing-dependent and could be flaky under CPU pressure. Using fake timers would make the assertion deterministic.📝 Suggested refactor using fake timers
- it("update({state}) does NOT invalidate the sessions query (autosave loop guard)", async () => { + it("update({state}) does NOT invalidate the sessions query (autosave loop guard)", async () => { + vi.useFakeTimers(); // The autosave path fires sessions.update(id, {state}) on a 100ms // debounce. If that invalidated the list query, the resulting // refetch would render back into the save effect and reschedule // the timer in perpetuity. State-only updates must be silent. const { client, builder } = makeClient([]); const { wrapper } = wrapperFactory(); const useHook = createUseSessions({ supabase: client as never, userId: "u1", }); const hook = renderHook(() => useHook(), { wrapper }); await waitFor(() => expect(hook.result.current.sessions).not.toBeUndefined() ); // After the initial load, no further `order()` calls should occur. builder.order.mockClear(); await hook.result.current.update("abc", { state: { model: "m", cards: {}, messages: [] }, }); - // Wait a tick in case an unwanted invalidation is queued. - await new Promise((resolve) => setTimeout(resolve, 20)); + // Advance timers to flush any queued invalidation. + await vi.advanceTimersByTimeAsync(50); expect(builder.order).not.toHaveBeenCalled(); + vi.useRealTimers(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/hooks/__tests__/useSessions.test.ts` around lines 192 - 217, This test uses a real setTimeout(20) making it flaky; switch to fake timers by calling vi.useFakeTimers() at the start of the test and vi.useRealTimers() in a finally/after block, then after await hook.result.current.update(...) advance the timers deterministically (e.g., vi.advanceTimersByTime(20) or vi.runOnlyPendingTimers()) and await any pending microtasks before asserting builder.order was not called; update the "update({state}) does NOT invalidate the sessions query (autosave loop guard)" test to wrap the timer setup/teardown around the renderHook/update calls and replace the manual Promise-based setTimeout with advancing the fake timers so the assertion is deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/src/hooks/__tests__/useSessions.test.ts`:
- Around line 192-217: This test uses a real setTimeout(20) making it flaky;
switch to fake timers by calling vi.useFakeTimers() at the start of the test and
vi.useRealTimers() in a finally/after block, then after await
hook.result.current.update(...) advance the timers deterministically (e.g.,
vi.advanceTimersByTime(20) or vi.runOnlyPendingTimers()) and await any pending
microtasks before asserting builder.order was not called; update the
"update({state}) does NOT invalidate the sessions query (autosave loop guard)"
test to wrap the timer setup/teardown around the renderHook/update calls and
replace the manual Promise-based setTimeout with advancing the fake timers so
the assertion is deterministic.
In `@client/src/hooks/useSessions.ts`:
- Around line 228-230: The useCallback wrapper for get captures the supabase
variable intentionally but has an empty dependency array; to make this explicit
and silence linter warnings, add an inline eslint-disable comment for the
react-hooks/exhaustive-deps rule immediately above the get useCallback
declaration (referencing the get function and the supabase variable in
useSessions.ts) so future reviewers and the linter know the omission is
intentional.
- Around line 126-143: The inline comment inside the onSuccess handler of
useSessions.ts incorrectly calls the behavior an "Optimistic insert" even though
the cache is updated after the mutation succeeds; update the comment to say this
is an immediate cache update (or similar) and mention it happens in onSuccess
rather than an optimistic onMutate flow that would require onError rollback
(reference the onSuccess handler and onMutate terminology to locate the code and
clarify intent).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be467198-80d6-4425-a83f-c95d3f8ea428
📒 Files selected for processing (3)
client/src/hooks/__tests__/useSessions.test.tsclient/src/hooks/useSessions.tsclient/src/pages/ask.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/pages/ask.tsx
Session switches were blocking the UI on the Supabase round-trip for the full record. During that window the previous session's messages and citations stayed on screen — read as "lag" or a broken switch. Introduces an isHydrating flag set while the hydrate effect is fetching, and renders a loading skeleton in place of the stale messages plus three skeleton cards in the citations aside. The sticky header stays visible with whatever title the sidebar cache already has, so the user gets immediate context-of-where-they-are even before the content lands. The chat input is disabled with a "Loading conversation…" placeholder so follow-ups don't queue against a half-hydrated state. Extracts SessionStickyHeader (reused between hydrating and hydrated branches) and SessionLoadingSkeleton (placeholder bubbles + text rows). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/src/pages/ask.tsx (1)
371-395: Consider extracting the duplicatedonToggleSharecallback.The same inline callback is defined identically in both the hydrating and non-empty branches. Extracting it would reduce duplication.
♻️ Proposed refactor
Add before the return statement:
const handleToggleShare = useCallback( (next: boolean) => { if (sessionId) sessions.update(sessionId, { isPublic: next }); }, [sessionId, sessions] );Then use
onToggleShare={handleToggleShare}in bothSessionStickyHeaderinstances.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/pages/ask.tsx` around lines 371 - 395, Duplicate inline onToggleShare callbacks are used in both SessionStickyHeader instances; extract a memoized handler instead. Create a handleToggleShare function using useCallback that accepts (next: boolean) and calls sessions.update(sessionId, { isPublic: next }) only when sessionId is set, with dependencies [sessionId, sessions]; then replace both onToggleShare inline props with onToggleShare={handleToggleShare} on the SessionStickyHeader components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/pages/ask.tsx`:
- Around line 101-116: Wrap the async hydration IIFE in a try/catch so any
exception from sessionsRef.current.get(sessionId) is handled; in the catch
ensure you call setIsHydrating(false), optionally log the error (e.g.,
console.error or process logger) and perform the same fallback flow as the "no
record" case (navigate("/ask", { replace: true })) to avoid leaving the UI
stuck, keeping dispatch({ type: "hydrate", ... }), hydratedRef.current and
lastSavedStateRef.current assignments only inside the success path.
---
Nitpick comments:
In `@client/src/pages/ask.tsx`:
- Around line 371-395: Duplicate inline onToggleShare callbacks are used in both
SessionStickyHeader instances; extract a memoized handler instead. Create a
handleToggleShare function using useCallback that accepts (next: boolean) and
calls sessions.update(sessionId, { isPublic: next }) only when sessionId is set,
with dependencies [sessionId, sessions]; then replace both onToggleShare inline
props with onToggleShare={handleToggleShare} on the SessionStickyHeader
components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6a8647f-efe9-4836-9f2d-e3746ed5ff29
📒 Files selected for processing (1)
client/src/pages/ask.tsx
Six UX tweaks: - SessionRow uses text-xs + text-muted-foreground for inactive rows (matches the citation card preview weight); active row is text-foreground font-medium so the current session stands out. - SharePopover button now reads "Public" when the session is public (was "Shared"), with a gap-1.5 between the icon and label. - "New chat" button uses gap-1.5 between icon and text. - Sidebar search input replaces the "Filter…" placeholder with "Search", adds a Search icon on the left, and sits in a fixed h-12 header at the top of the aside. - Dropdown borders (session options, ModelPicker, any dropdown) drop from `border` to `border-border/40` for lower contrast. - All three column headers now share the same h-12 + border-b/60 styling: sidebar "Search" (with "New chat" immediately below), main section session-title + Share, and sources aside "Sources". The bottom-of-header line aligns across columns. CitationsPanel's internal "Sources" heading moved up into the aside header (shown with citation count), so the inline title no longer duplicates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
client/src/pages/ask.tsx (1)
100-116:⚠️ Potential issue | 🟠 MajorCatch rejected hydration fetches.
If
sessionsRef.current.get(sessionId)throws at Line 103,setIsHydrating(false)is never reached and the page stays on the loading skeleton indefinitely. This looks like the same unresolved issue raised on the earlier revision.Suggested fix
let cancelled = false; (async () => { - const record = await sessionsRef.current.get(sessionId); - if (cancelled) return; - if (!record) { - setIsHydrating(false); - navigate("/ask", { replace: true }); - return; - } - dispatch({ type: "hydrate", state: record.state }); - hydratedRef.current = sessionId; - // The hydrated state is by definition what's in the DB; mark it so - // the save effect doesn't immediately re-save it on the next render. - lastSavedStateRef.current = record.state; - setIsHydrating(false); + try { + const record = await sessionsRef.current.get(sessionId); + if (cancelled) return; + if (!record) { + setIsHydrating(false); + navigate("/ask", { replace: true }); + return; + } + dispatch({ type: "hydrate", state: record.state }); + hydratedRef.current = sessionId; + // The hydrated state is by definition what's in the DB; mark it so + // the save effect doesn't immediately re-save it on the next render. + lastSavedStateRef.current = record.state; + setIsHydrating(false); + } catch (err) { + if (cancelled) return; + setIsHydrating(false); + setSaveError((err as Error).message); + navigate("/ask", { replace: true }); + } })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/pages/ask.tsx` around lines 100 - 116, The hydration async IIFE can throw when calling sessionsRef.current.get(sessionId), so wrap the await call (and subsequent logic inside that IIFE) in try/catch/finally: call setIsHydrating(false) in a finally block to ensure the loader is cleared, handle errors in the catch (e.g., log via console or a logger and navigate("/ask", { replace: true }) if record retrieval failed), and preserve the cancelled check and dispatch({ type: "hydrate", state: record.state }), hydratedRef.current and lastSavedStateRef.current assignments only when no error occurred; specifically edit the anonymous async function around sessionsRef.current.get to add try/catch/finally and reference the existing setIsHydrating, sessionsRef.current.get, navigate, dispatch, hydratedRef.current, and lastSavedStateRef.current symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/pages/ask.tsx`:
- Around line 412-418: The Sources count currently reads directly from
lastAssistant causing stale counts during session switches; change the JSX in
the Sources header to show 0 while the panel is hydrating/loading (e.g., replace
the value expression with a conditional that returns 0 when the component is
hydrating/loading and otherwise returns lastAssistant?.citations?.length ?? 0).
Use the existing loading/hydration state variable (or add one if missing)
referenced by the panel/skeleton rendering to drive this logic so the count
resets to 0 whenever the skeletons are shown.
- Around line 456-460: The UI currently treats a null activeSession.title as
"loading" and renders the Skeleton forever; change the render logic so the
Skeleton is only shown when the session object is absent/loading (e.g.,
activeSession === undefined or a dedicated loading flag), and for existing
sessions render activeSession.title with a fallback label (like "Untitled
session" or "Untitled") when title is null/empty. Update the JSX around
sessionId / activeSession?.title and the Skeleton usage to check for presence of
activeSession (or a loading boolean) before falling back to the Skeleton, and
otherwise render a non-skeleton fallback string.
- Around line 368-375: The onToggleShare inline handler currently calls
sessions.update(...) without awaiting or handling rejections; change the handler
to be async, await sessions.update(sessionId, { isPublic: next }), and wrap it
in try/catch so any error is caught and forwarded to the page's error UI (either
call the existing page error handler like setError/showError or rethrow the
error to let the error boundary handle it). Locate the SessionSectionHeader prop
onToggleShare and update that handler to await sessions.update and surface
failures instead of letting them become unhandled background errors.
---
Duplicate comments:
In `@client/src/pages/ask.tsx`:
- Around line 100-116: The hydration async IIFE can throw when calling
sessionsRef.current.get(sessionId), so wrap the await call (and subsequent logic
inside that IIFE) in try/catch/finally: call setIsHydrating(false) in a finally
block to ensure the loader is cleared, handle errors in the catch (e.g., log via
console or a logger and navigate("/ask", { replace: true }) if record retrieval
failed), and preserve the cancelled check and dispatch({ type: "hydrate", state:
record.state }), hydratedRef.current and lastSavedStateRef.current assignments
only when no error occurred; specifically edit the anonymous async function
around sessionsRef.current.get to add try/catch/finally and reference the
existing setIsHydrating, sessionsRef.current.get, navigate, dispatch,
hydratedRef.current, and lastSavedStateRef.current symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6973e0fd-756d-407f-9974-c5da0a5aed26
📒 Files selected for processing (7)
client/src/components/ask/SessionRow.tsxclient/src/components/ask/SessionsSidebar.tsxclient/src/components/ask/SharePopover.tsxclient/src/components/ask/__tests__/SessionsSidebar.test.tsxclient/src/components/chat/CitationsPanel.tsxclient/src/components/ui/dropdown-menu.tsxclient/src/pages/ask.tsx
✅ Files skipped from review due to trivial changes (2)
- client/src/components/ask/SharePopover.tsx
- client/src/components/ui/dropdown-menu.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/components/ask/tests/SessionsSidebar.test.tsx
The existing section↔aside divider used a 2-pixel bevel (dark #D4D4D4/#181818 on one edge + light white/#353535 on the adjacent edge) for a subtle carved look. Every other column and fixed-row divider in the /ask layout was using a single-pixel border-border/60 and looked out of place next to it. Applies the same two-pixel bevel pattern everywhere: - Sidebar → main section: sidebar right = dark, section left = light (section now has both border-l and border-r). - Sidebar "Search" header → "New chat" row: dark bottom + light top. - Sidebar "New chat" row → scrolling list: dark bottom + light top. - Main section header → messages: dark bottom + light top. - Sources aside header → citations: dark bottom + light top. Colors match the original section/aside bevel exactly so the whole frame reads as one consistent "inset" treatment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The main home page had no visible path to /ask — an authenticated user with existing conversations couldn't reach them without typing /ask into the URL. Adds a subtle muted link below the search form, shown only when the user is logged in AND has at least one ask_session row, with the session count inline so the user knows there's something to go back to. Wraps the useSessions call in an inner component that only mounts when a supabase user is present, since the hook throws on unauthenticated contexts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Set font-light at the root of AskPage, the sidebar aside, and the dropdown surfaces (ModelPicker, SessionRow menu) so everything descending inherits it. Strip the explicit font-medium / font-semibold overrides that were previously competing: session title, "Sources" label, empty-state heading, citation number badge, citation title, tool-step toolname, "[N]" inline citation pill, sidebar group label. Markdown headings stay readable by dropping one step (semibold → normal): 400 reads as a heading next to a 300 body without shouting. Strong/em and table headers get the same 400 treatment. Buttons are still in the Button component which ships font-medium by default; individual button instances on /ask (Ask, Share, New chat, Model picker trigger) get a font-light className to override. Active session row no longer needs a font-medium bump to stand out — the color shift from muted to foreground does the work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the dark shadow half of the 3D bevel under the sidebar Search header and under the Sources header in the aside — only the light highlight line on the adjacent content remains. The vertical column dividers stay as the full bevel. Removes the divider between the sidebar "New chat" row and the session list entirely; they now read as one continuous block, while the Search → New chat divider keeps the single muted line. Sticky behavior and heights are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The backend's fallback title model was "anthropic/claude-haiku-4-5" but OpenRouter's actual slug uses a dot: "anthropic/claude-haiku-4.5" (matching the client's models list). Wherever TITLE_MODEL wasn't set explicitly, OpenRouter rejected the request, generateTitle threw, /ask/title returned 502, and requestTitle() swallowed that into null — so the session title was never replaced with the generated topic and the user saw their full question forever. Updates the default in askTitle.ts, the .env.example, and the unit test that happened to use the broken slug as a placeholder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the dark shadow line below the session title + Share header so the three column headers all use the same muted "light highlight line only" divider style. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The private note ("anyone with the link will see it as unavailable")
stays on its own — the readOnly input only renders once isPublic flips
to true, and uses text-xs so the URL fits without wrapping.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Hydrate effect now catches thrown errors from sessions.get() so a network / supabase failure clears isHydrating and shows the save-error banner; previously the skeleton was stuck indefinitely with no recovery path. - The header's onToggleShare chained .catch(setSaveError) so a failed isPublic mutation no longer becomes an unhandled background rejection — the user sees the banner and can retry. Addresses CodeRabbit review on PR #250. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
tomusdrw
added a commit
that referenced
this pull request
Apr 27, 2026
- getGraypaperLatest invalidates its cache on versions.md mtime change (graypaperJob runs out-of-process and rewrites the file). - writeGraypaperVersions now treats `latest === undefined` as "preserve" so callers like exportToMarkdown.ts don't wipe the pinned ref, and `latest === null` as "clear" so the metadata-no-longer-has-latest case drops the stale pin. - updateGraypaperVersions detects null↔value transitions both ways. Declined the suggestion to URL-encode params: prior PR #250 explicitly removed encoding to match the reader's hash-router contract — changing that needs reader-side verification, not a drive-by edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tomusdrw
added a commit
that referenced
this pull request
Apr 28, 2026
* graypaper: pin reader URLs to latest hash + version Embed the latest graypaper hash and version in result links (`/#/<shortHash>?v=<version>&...`) so the reader skips its client-side redirect. The latest ref is persisted in `versions.md` frontmatter and exposed via `GET /graypaper/latest`; both backend and frontend share a single URL composer in `shared/graypaper.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * graypaper: sort imports in useGraypaperLatest hook Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * graypaper: address review — invalidate cache, preserve pin, clear pin - getGraypaperLatest invalidates its cache on versions.md mtime change (graypaperJob runs out-of-process and rewrites the file). - writeGraypaperVersions now treats `latest === undefined` as "preserve" so callers like exportToMarkdown.ts don't wipe the pinned ref, and `latest === null` as "clear" so the metadata-no-longer-has-latest case drops the stale pin. - updateGraypaperVersions detects null↔value transitions both ways. Declined the suggestion to URL-encode params: prior PR #250 explicitly removed encoding to match the reader's hash-router contract — changing that needs reader-side verification, not a drive-by edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses nine UI/UX defects on
/ask. Spec and plan are committed atdocs/superpowers/specs/2026-04-24-ask-ui-fixes-design.mdanddocs/superpowers/plans/2026-04-24-ask-ui-fixes.md. One commit per fix so each change is independently reviewable.fullBleednow matches any/asksubroute; session and shared views no longer double-pad.createdRefis consumed on first use, so create A → visit B → click back A re-hydrates from DB instead of keeping B's state.text-foreground— added where drift was visible (sidebar aside, session row,/askshells, citations aside).SessionRow's menu now usesDropdownMenuLabel+DropdownMenuSeparatorto matchModelPicker.useSessionsmoved to@tanstack/react-querywith a shared["ask_sessions", userId]cache, so sidebar and page now observe the same data after any mutation.SharePopoverto show.buildGraypaperUrlnow emits?search=<q>§ion=<title>to match the search-page format so the reader's hash router resolves."Link copied"/"Link copied. Session is public now". Addedsonnerfor toasts; mounted<Toaster />inApp.tsx.askShared.tsx::forkAndGonow callsinvalidateSessions(queryClient, userId)after the direct insert, so a forked session appears in the sidebar immediately instead of after a stale-time refetch.Test plan
npm run qaat repo root — clean (lint + biome).npm testinclient/— 82/82 pass.npm run typecheckin bothclient/andbackend/— clean./askempty state and/ask/:idboth full-bleed, no double padding."Link copied. Session is public now", link in clipboard./ask/s/:idas a logged-in user and fork — sidebar shows the fork immediately.🤖 Generated with Claude Code
Summary by CodeRabbit