🧰 feat: Unify Tool Activity, Reasoning, Search, and Agent Workflows - #14546
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba586d37fc
ℹ️ 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".
ba586d3 to
db34b71
Compare
berry-13
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fbee3ff08
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee28e0ff4
ℹ️ 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".
f228ab0 to
dfa51d3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfa51d325b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7f9349492
ℹ️ 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".
f4877fd to
8abb379
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8abb379cdc
ℹ️ 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".
| {options.map((option, index) => { | ||
| const isChecked = multiSelect && checked.includes(index); | ||
| return ( | ||
| <button |
There was a problem hiding this comment.
When a deployment uses a non-default theme, this new option row bypasses the existing @librechat/client Button choice variant and recreates hover, focus, disabled, and selection appearance locally, including the raw text-white color below. Both ask surfaces can therefore drift from shared button changes or lose contrast when surface-submit is customized; compose the existing primitive or add the required shared variant instead.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5a98cc. The option row now composes the shared Button primitive (variant="ghost"), so hover, focus-visible ring and disabled appearance all come from the shared recipe instead of being recreated locally. Only the row geometry and the selected fill stay local, both on semantic roles. The raw text-white on the checked badge is now text-on-status, matching how the submit variant pairs with surface-submit.
| line.type === 'add' && 'bg-green-500/10', | ||
| line.type === 'del' && 'bg-red-500/10', |
There was a problem hiding this comment.
When a custom or versioned theme changes its success and destructive roles, these raw Tailwind palette backgrounds—and the corresponding raw marker colors below—remain fixed to green/red instead of adapting. Add semantic diff addition/deletion roles or a shared variant rather than feature-local palette values.
AGENTS.md reference: AGENTS.md:L6-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5a98cc. Replaced the raw diff palette with the existing semantic status roles: bg-status-success-subtle / bg-status-error-subtle for the row fills and text-status-success / text-status-error for the markers. These resolve through CSS variables in both themes (green-50/green-950, red-50/red-950 for the subtle fills; green-700/green-300, red-700/red-300 for the markers), so a customized or versioned theme moves them. Also applied the same roles to the +/- counts in CodeWindowHeader, which carried the identical raw pair, and updated the FileAuthoringCall assertions.
| * (`Summary` renders null without text) and matches the persisted | ||
| * message, which retains the part server-side. | ||
| */ | ||
| return { ...part, summarizing: false } as SummaryContentPart; |
There was a problem hiding this comment.
When summarization emits one or more deltas and then completes with completeData.error, this fallback preserves the partial content but sets summarizing to false. Summary renders any nonempty, non-summarizing part with the “Conversation summarized” label, so the failed and potentially incomplete summary is presented as successful despite the failure announcement; preserve the slot without treating its partial text as completed, or carry an explicit failed state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5a98cc. You were right that flipping summarizing alone only hides an empty row: with deltas already streamed in there is text, so Summary rendered it under com_ui_conversation_summarized. SummaryContentPart now carries an explicit failed flag, the error branch sets it while still keeping the slot (preserving the content-position == step-index invariant), and Summary labels it com_ui_summarize_failed instead. A failed round with no text at all still renders nothing. Covered by a new Summary test suite.
| const completed = !!tc.output || tc.progress === 1; | ||
| const name = tc.name ?? ''; | ||
| const iconName = isBashProgrammaticToolCall(name, tc.args) ? Tools.bash_tool : name; | ||
| return { name, iconName, hasOutput: completed }; | ||
| return { name, iconName, hasOutput: completed, failed: hasFailedOutput(tc.output) }; |
There was a problem hiding this comment.
When a grouped tool settles with runStepStatus: 'failed' but returns empty or otherwise non-isError output, this metadata still marks it as not failed and often as incomplete. The individual tool card uses the authoritative run-step status and displays a failure, while the group can remain “Running” until the whole response ends and then report that it ran successfully with no failure count; derive both completion and failure from runStepStatus as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5a98cc. getToolMeta now reads runStepStatus off the outer tool_call object and routes it through the same resolveToolCallPhase helper the individual card uses, deriving hasOutput from phase !== 'running' and failed from phase === 'failed' across all four tool-call branches. A step closed as failed with empty or benign output is therefore settled and counted as a failure, and cancellation still outranks error-shaped output. Parts with no terminal status keep the legacy output/progress signal, so older endpoints are unaffected. Covered by a new ToolCallGroup test that fails on the previous code for both the tense and the failure count.
| const label = streaming | ||
| ? localize('com_ui_thinking') | ||
| : localize('com_ui_thoughts'); |
There was a problem hiding this comment.
When a THINK part with a populated reasoning_label is adjacent to a tool and therefore enters the new grouped rendering path, this replaces that generated label with the generic “Thinking” or “Thoughts” text. Standalone reasoning passes part.reasoning_label through to Reasoning, so the same authored activity label disappears only when grouping occurs; prefer the part's label here and use the generic text only as a fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5a98cc. The grouped THINK path now mirrors the standalone Reasoning path exactly: the trimmed part.reasoning_label wins, and the generic com_ui_thinking / com_ui_thoughts text is only the fallback when it is absent or whitespace.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5a98ccaba
ℹ️ 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".
| <button | ||
| type="button" | ||
| aria-label={localize('com_ui_ask_move_to_composer')} | ||
| className="rounded-md p-1 text-text-secondary transition-colors hover:bg-surface-hover hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy" | ||
| onClick={expand} |
There was a problem hiding this comment.
Replace this locally styled <button> with the existing @librechat/client Button primitive and an appropriate variant; the equivalent move controls in AskUserQuestionPopover.tsx repeat the same hover, focus-ring, color, and rounding recipe, so future shared button or theme changes will not reach these controls. Keep only the icon-button geometry local.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 164430e. All three move controls now use the shared Button primitive (variant="ghost" size="icon"), keeping only the icon geometry local, matching how Summary.tsx already composes it. That covers this chevron plus both AskUserQuestionPopover controls you pointed at, so the duplicated hover/focus/color/rounding recipe is gone from all of them.
| if (!data) { | ||
| continue; | ||
| } | ||
| images.push(...(data.images ?? [])); |
There was a problem hiding this comment.
Filter image entries that have neither thumbnailUrl nor imageUrl before adding them here. Both properties are optional in the shared ImageResult contract, but an unfiltered entry still makes images.length nonzero and ImageStrip renders an anchor and <img> with undefined destinations, leaving users with a blank or broken result card instead of omitting the unusable provider result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 164430e. collectSearchVerticals now only keeps entries with a thumbnailUrl or imageUrl, so an entry with neither no longer makes images.length nonzero and ImageStrip never renders an img with an undefined src.
| const askAnswerTextAtom = atom<{ actionId: string | null; text: string }>({ | ||
| key: 'askAnswerModeText', | ||
| default: { actionId: null, text: '' }, |
There was a problem hiding this comment.
Store these drafts per action rather than in one global { actionId, text } slot. When Save drafts is disabled, a user can edit a collapsed question in conversation A, visit another paused conversation and collapse or edit its question, and then return to A; the second question overwrites this atom, so A's card becomes empty and its unsent answer is permanently lost. An action-keyed map or atom family would preserve each pending question independently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 164430e. The atom is now a Record<string, string> keyed by action id: reads take the entry for the live pause, writes merge into it, and the answered question's entry is dropped on submit so the map stays bounded. Added a regression test covering exactly your scenario (a second paused question being edited must not drop the first one's unsent answer); it fails on the previous single-slot code.
| {!isSave && ( | ||
| <div className="text-sm italic text-text-secondary"> | ||
| {localize('com_ui_memory_deleted')} | ||
| </div> |
There was a problem hiding this comment.
When a delete_memory step is cancelled after its key has arrived, hasPanel remains true and this non-failed branch renders “Memory deleted” even though the header correctly says “Cancelled.” That falsely tells users the memory was removed; gate the deletion confirmation on phase === 'completed' and show only the attempted input or cancellation state otherwise.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 164430e. The deletion confirmation is now gated on phase === 'completed'. Worth noting it was also wrong while the call was still running: the panel opens as soon as the key streams in, so 'Memory deleted' appeared before anything had been deleted. The gate covers both. The key above it still shows what was attempted, and the header keeps reporting Cancelled.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 164430e37f
ℹ️ 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".
| if (toolCallCount >= 2 || (toolCallCount >= 1 && hasReasoning)) { | ||
| result.push({ type: 'tool-group', parts: [...currentBlock] }); |
There was a problem hiding this comment.
When activity labels are disabled, blank, or absent on older conversations, currentBlock can contain phase: 'commentary' text alongside two tool calls. This branch now puts the entire block into ToolCallGroup, which auto-collapses after completion and hides the assistant's commentary behind a generic action header; the previous fallback emitted commentary separately while grouping only the tools. Group only reasoning and tool calls here, or explicitly split commentary before constructing the unlabeled group.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 71a0c89. flushWithoutLabel now splits the block at commentary and groups each surrounding run on its own, so commentary is emitted as its own part in transcript order and only reasoning plus tool calls end up inside the collapsing group. I checked origin/dev to confirm the shape: the old fallback split at every non-tool-call part, so this restores its commentary behavior while keeping this PR's deliberate reasoning folding. Two new groupToolCalls tests cover it.
| images.push(image); | ||
| } | ||
| } | ||
| shopping.push(...(data.shopping ?? [])); |
There was a problem hiding this comment.
When a provider returns a partial shopping result without link—which is valid because every ShoppingResult field is optional—this unfiltered entry reaches ShoppingStrip and renders as a product <a> with no href. The card looks actionable but cannot be opened, and an entirely sparse entry can produce a blank tile; filter unusable shopping records here or render linkless results as non-interactive content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 71a0c89. collectSearchVerticals now requires a link on shopping results. I also applied the same guard to places (they need a name or address), since the identical reasoning holds and mapLink would otherwise build a maps URL from an empty query for an entirely sparse entry. All three verticals are now filtered at the same seam.
| <TooltipAnchor | ||
| description={copyLabel} | ||
| render={ | ||
| <button |
There was a problem hiding this comment.
Replace this locally styled copy button and its duplicated icon-state treatment with the existing CopyButton/shared button composition. This new control independently recreates hover, focus-ring, opacity, rounding, and copied-state appearance, so shared accessibility or theme changes applied to the established copy primitive will not reach handoff details.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 71a0c89. Now composes the shared CopyButton with iconOnly, so the icon crossfade, tooltip, hover, focus ring and copied-state labels all come from CopyButton/ActionButton. Only the reveal-on-hover opacity stays local, passed through className. The existing AgentHandoff copy test still passes, so the copied text is unchanged.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71a0c8956f
ℹ️ 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".
| if (image.thumbnailUrl || image.imageUrl) { | ||
| images.push(image); |
There was a problem hiding this comment.
The current guard still accepts a thumbnail-only ImageResult with no link, googleUrl, or imageUrl. ImageStrip then computes an undefined href while rendering the preview as a styled anchor, so the visible result looks actionable but cannot be opened. This thumbnail-only case is fresh evidence beyond the earlier missing-image-source issue; require a navigation target as well or render such previews non-interactively.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f8f71e. Agreed, my earlier guard only covered the image source. The filter now requires both a source (thumbnailUrl or imageUrl) and a navigation target (link, googleUrl or imageUrl), so a thumbnail-only entry no longer renders as an anchor with an undefined href.
| return ( | ||
| typeof standardToolCall.auth === 'string' && standardToolCall.auth.length > 0 && progress < 1 | ||
| ); |
There was a problem hiding this comment.
When a persisted OAuth call has a terminal runStepStatus but its progress is absent or remains below 1, this predicate still classifies it as pending. ToolCall itself honors the terminal status and hides the sign-in button, while the group suppresses the per-call warning and appends a shared trust warning, leaving a warning with no authentication action. Fresh evidence in the current head is that grouped outcome metadata now treats runStepStatus as authoritative, but this parallel predicate does not; closed steps should be excluded here as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f8f71e. hasPendingAuth now returns false whenever runStepStatus is set, so it agrees with the outcome metadata and with ToolCall: a closed step is never awaiting a sign-in regardless of its recorded progress.
|
|
||
| const hasInfo = useMemo(() => (args?.trim()?.length ?? 0) > 2, [args]); | ||
| const handleCopy = useCallback(() => { | ||
| navigator.clipboard.writeText(copyText).then( |
There was a problem hiding this comment.
On LibreChat deployments served from a non-secure LAN origin, navigator.clipboard can be undefined, so this dereference throws synchronously before the rejection handler runs and the new handoff copy control does nothing. Fresh evidence after the shared-button refactor is that the handler still bypasses the copy-to-clipboard fallback used by the surrounding message-content controls; use that fallback or guard the Clipboard API before calling it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f8f71e. Now uses copy from 'copy-to-clipboard' with format text/plain, matching BashCall, CodeWindowHeader and the other message-content copy controls, and the copied state only flips when it returns true. Added a test covering the unavailable-clipboard path alongside the existing copy test.
| export const disclosureChevronClassName = | ||
| 'shrink-0 text-text-secondary opacity-0 transition-transform duration-200 ease-out group-focus-within/disclosure:opacity-100 group-hover/disclosure:opacity-100 motion-reduce:transition-none'; |
There was a problem hiding this comment.
This new export is a feature-local alias for a reusable disclosure-chevron appearance, and it is now copied across tool, search, memory, and reasoning surfaces instead of being expressed by an @librechat/client primitive or variant. Shared hover, focus, motion, or theme changes therefore still require editing Chat-specific class strings; deepen the shared disclosure/trigger primitive and leave only feature layout local.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f8f71e. The chevron appearance now lives in @librechat/client as disclosureChevronVariants, a cva recipe owning the hover/focus reveal, the motion and an expanded variant for the rotation. All four surfaces (ProgressText, WebSearch, MemoryArtifacts, ReasoningCompact) pass their expanded state and keep only icon size and optical offset local. The feature-local disclosure.ts now holds just toolPanelSpacingClassName, which is genuine layout.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f8f71ee9f
ℹ️ 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".
| }, []); | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| navigator.clipboard.writeText(reasoningText); |
There was a problem hiding this comment.
When LibreChat is served from a non-secure LAN origin, navigator.clipboard can be undefined, so clicking the newly added copy control for reasoning inside a tool group throws synchronously and copies nothing. Use the copy-to-clipboard fallback already used by the surrounding message-content controls, or guard the Clipboard API before dereferencing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in afb7701. ReasoningCompact now copies through copy-to-clipboard with format text/plain, and only flips the copied state when it succeeds, matching the handoff control and the other message-content copy buttons.
| <HoverCardTrigger | ||
| tabIndex={0} | ||
| className={cn( | ||
| 'ml-auto inline-flex cursor-help items-center justify-center rounded-md p-1 text-text-secondary opacity-0 transition-all duration-200 ease-out', | ||
| 'hover:bg-surface-hover hover:text-text-primary', | ||
| 'group-focus-within/websearch:opacity-100 group-hover/websearch:opacity-100', | ||
| 'focus-visible:opacity-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy', |
There was a problem hiding this comment.
Replace this locally styled hover-card trigger with the shared Button primitive composed through HoverCardTrigger; the new control independently recreates rounding, hover colors, focus rings, motion, and icon-button spacing, so shared accessibility or appearance changes will not reach it. Keep only the hover-card reveal behavior and geometry local.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in afb7701. Now composes the shared Button through HoverCardTrigger's asChild, so rounding, hover fill, focus ring and motion all come from the shared recipe. Side benefit: it is now a real rather than Radix's default anchor with tabIndex=0. Only the hover-card reveal (the group-hover/focus opacity) and the icon geometry stay local.
| const PEEK_FADE = | ||
| 'linear-gradient(to bottom, transparent, #000 1.25rem, #000 calc(100% - 1.25rem), transparent)'; |
There was a problem hiding this comment.
This new feature-local gradient hard-codes #000 and is applied as arbitrary inline theme CSS without explaining why the shared system cannot express it. Define the reusable mask through the shared primitive or versioned theme-token registry, or document a narrowly scoped custom-CSS exception, rather than embedding a fixed color in the reasoning component.
AGENTS.md reference: AGENTS.md:L6-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in afb7701, but taking the second branch you offered rather than the first. Pushing back on the theme-token route: PEEK_FADE is applied as style={{ maskImage, WebkitMaskImage }}, so it is an alpha stencil, not paint. Only the alpha channel is sampled, #000 means 'keep this pixel' and transparent means 'hide it'; the hue never reaches the screen, so no theme could meaningfully restyle it, and routing it through a theme role would invite a token carrying alpha that would silently wash out the very text the mask exists to keep. Tailwind also has no mask-image utility expressing a four-stop gradient with calc() offsets. So I documented it as a narrowly scoped custom-CSS exception at the constant, per the AGENTS.md clause requiring that justification. Happy to revisit if you see a shared-system expression I have missed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afb770173a
ℹ️ 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".
| for (const raw of diff.replace(/\n$/, '').split('\n')) { | ||
| if (!inHunk && (raw.startsWith('--- ') || raw.startsWith('+++ '))) { | ||
| continue; |
There was a problem hiding this comment.
When edit_file contains two or more edits, formatEditPreview inserts a new --- old_text / +++ new_text pair before each @@, but inHunk never resets, so this condition skips only the first pair. Every later synthetic header is rendered and counted as one deletion and one addition, inflating the displayed statistics and adding bogus diff rows; recognize the separator pair for every batched edit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 439d908. Good catch on inHunk never resetting. The parser now matches the synthetic pair by name (^(?:---|+++) (?:old_text|new_text)( \d+)?$) before the inHunk-gated file-header skip, so every batch is recognized. Anchoring on the exact marker names keeps the existing behavior for content lines like '--- deleted text', which the suite already pins. Added a batched-edit test asserting 2 additions and 2 deletions where the old code reported 3 and 3.
| <button | ||
| type="button" | ||
| className="text-xs italic text-text-secondary hover:text-text-primary hover:underline" | ||
| className="cursor-text rounded-md text-xs text-text-secondary transition-colors hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy" |
There was a problem hiding this comment.
This edit adds a feature-local rounding, hover, transition, and focus-ring recipe to the raw hint button even though the shared Button primitive is already imported and used by the adjacent controls. Compose that primitive and keep only the hint-specific geometry locally so shared accessibility and theme changes also reach this control.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 439d908. Composes the shared Button with variant ghost and suppresses only the hover fill, matching how Summary's quiet text buttons compose it, so this stays a hint rather than a control with a surface while the focus ring and disabled handling come from the shared recipe. Only the hint geometry (h-auto, p-0, text-xs, cursor-text) is local.
| {answerBox && (answerBox.title || answerBox.snippet) && ( | ||
| <div className="border-t border-border-light pt-2"> | ||
| {answerBox.title && ( | ||
| <div className="text-sm font-medium text-text-primary"> | ||
| {answerBox.title} | ||
| </div> | ||
| )} | ||
| {answerBox.snippet && ( |
There was a problem hiding this comment.
When the provider returns an answer box containing only snippetHighlighted—a valid AnswerBoxResult shape—or the persisted answer field already exercised by the message projection tests, hasDetails still exposes this trigger but this condition renders neither value. The details card then shows only the query/source count and silently drops the direct answer; normalize and render every supported answer-box text field before deciding that the card has content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly fixed in 439d908. snippetHighlighted was a real gap: it is independently optional from snippet and the card opens for any answer box, so a highlights-only result rendered a heading with an empty body. It now falls back to joining the highlights, and the section gate uses that derived text. On the 'persisted answer field': I could not corroborate it. AnswerBoxResult in packages/data-provider/src/types/web.ts:423-429 declares only title, snippet, snippetHighlighted, link and date, and no fixture or projection test in the repo carries an answer key on an answerBox. If you can point at the shape you saw, I will add it to the type and render it too.
The rebase onto dev landed both halves of #15643's row rail except in `WebSearch`, whose completed row had since moved its glyph into a shared `Button` and whose streaming row kept a 10px gap. Restore the shared `ROW_GLYPH_SLOT` around the completed row's favicon stack or globe and drop the streaming row's gap to 8px, so both search rows put their glyph on the avatar's axis and their text where the header's name starts.
b423515 to
6e16282
Compare
`showThinking` is an app-global display preference the group only reads, and `ToolCallGroup` already receives it from its host. `ReasoningCompact` still reached for `showThinkingAtom` itself, coupling a grouped thought to `~/store` and blocking the move to a feature-owned workspace. It takes the preference as a required prop now, from the value the group already holds.
|
@codex review |
The rebase onto dev silently dropped `ROW_GLYPH_SLOT` from the completed row — its glyph had since moved inside a shared `Button` — and left the streaming row on a 10px gap, so nothing caught either. `Part.tsx` routes web search past `ProgressText`, so this renderer owns its glyph and has to take the slot itself; the test fails on both regressions.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 638a55635c
ℹ️ 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".
| if (part.type === ContentTypes.THINK) { | ||
| const think = part.think; | ||
| const reasoning = typeof think === 'string' ? think : (think?.value ?? ''); |
There was a problem hiding this comment.
When a grouped THINK part has reasoning_unavailable: true and empty content, as used by detached-subagent projections, this branch bypasses Part's ReasoningMarker path and passes the empty text to ReasoningCompact, which returns null. The unavailable-reasoning indicator therefore disappears whenever that part is folded into a tool group; delegate these parts to the normal renderer or render the marker here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9441da4. ToolCallGroup now delegates a THINK part with empty text and reasoning_unavailable back through renderPart, so Part keeps rendering ReasoningMarker for it; ReasoningCompact only handles parts that have text. Regression test added in ToolCallGroup.test.tsx ('delegates an unavailable-reasoning part to the standalone renderer') — it fails on the previous head.
| </div> | ||
| </div> | ||
| )} | ||
| {!hideAttachments && <SearchVerticals attachments={attachments} />} |
There was a problem hiding this comment.
When a completed activity phase contains this web-search call, ContentParts renders the nested segment with hideAttachments=true, so this condition suppresses its images, shopping results, and places. The phase-level hoist renders only AttachmentGroup, which explicitly discards Tools.web_search attachments, so these results disappear instead of moving outside the collapsed phase; render SearchVerticals at the phase level or keep them enabled here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9441da4. ActivityPhaseGroup now renders SearchVerticals beside AttachmentGroup in its hoisted media row, mirroring ToolCallGroup's own hoist, so a folded search's images, products and places lift out of the fold instead of vanishing. Regression test added in ActivityPhaseGroup.test.tsx ('hoists a folded search call verticals out of the card') — it fails on the previous head.
Two things a fold could swallow. A detached-subagent projection is an empty THINK part flagged `reasoning_unavailable`, which `Part` renders as a `ReasoningMarker`. `ToolCallGroup` sent every THINK part to `ReasoningCompact`, which has no text to show and returns null, so the marker disappeared the moment its call joined a group. Those parts go back through the standalone renderer. A phase hoists its span's media out of the fold, and the nested segment renders with `hideAttachments` so a `WebSearch` row inside it stands down for that hoist. The hoist was `AttachmentGroup` alone, which drops `web_search` attachments outright, so a folded search lost its images, products and places instead of lifting them out. The card renders `SearchVerticals` beside the attachment row, mirroring `ToolCallGroup`.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9441da45c3
ℹ️ 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".
| lastAnswerableRef.current = null; | ||
| if (!previous || previous.conversationId !== conversationId) { | ||
| return; |
There was a problem hiding this comment.
When drafts are disabled and a released composer message is stashed, navigating away while the answer resume is in flight causes this branch to retain the stash but clear lastAnswerableRef; if the resume then succeeds or expires while the user is elsewhere, returning to the original conversation starts with answerable === false and previous === null, so the guard exits again. The ordinary unsent message remains permanently unreachable in releasedComposerText; associate the stash with its conversation and recover it when that conversation is revisited after the action has settled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 27c457d. releasedComposerTextAtom entries now carry their conversation (ReleasedComposerStash = { conversationId, text }), and recovery keys on that instead of on lastAnswerableRef, so revisiting the conversation after the action settled elsewhere hands the message back rather than exiting on the null ref. Only the exit this hook armed knows what the composer held when the resume started, so on a revisit an empty composer is the restore target; either way the entry is dropped so the map stays bounded. Regression test added ('hands a stash back when its conversation is revisited after the answer settled') — it fails with the old ref gate in place.
Recovery of the ordinary unsent message stashed while the card owns the composer was gated on `lastAnswerableRef` — what this hook happened to observe. Navigating away mid-resume clears that ref while the stash rightly stays put, so a resume that succeeded or expired while the user was elsewhere left nobody to hand the message back: returning to the conversation found `answerable === false` with no ref, exited early, and the text sat unreachable in `releasedComposerTextAtom` for the session. The stash carries its conversation now, and recovery keys on that, so a revisit is a hand-back like any other exit. Only the exit this hook armed knows what the composer held when the resume started; on a revisit an empty composer is the only target that cannot overwrite something newer, and either way the entry is dropped.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9d208ad3b
ℹ️ 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".
| return current; | ||
| } | ||
| const next = { ...current }; | ||
| delete next[actionId]; |
There was a problem hiding this comment.
When drafts are disabled and the user navigates away while an answer is submitting, the intentionally reused ChatForm can still contain the submitted answer or text from the other conversation because useAutoSave.ts:273-275 skips conversation switching entirely. On returning after the question settles, previous is null, so the nonempty composer fails the restorable check, but this unconditional deletion still discards the stashed ordinary message permanently. Fresh evidence at this exact head is the drafts-off early return combined with the explicitly reused form; keep the stash when restoration is deferred rather than treating every nonempty value as a newer edit.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| context.toolCallId = runnableConfig.toolCall.id; | ||
| context.agentId = runnableConfig.metadata.agent_id ?? runnableConfig.metadata.agentId; |
There was a problem hiding this comment.
When a handoff agent runs web_search, this reads the invocation's outer agent_id, not the per-call owner: createToolExecuteHandler passes the original metadata unchanged to tool.invoke at packages/api/src/agents/handlers.ts:6645-6648, while the canonical agentId is added as executingAgentId only later for the tool-end callback. The streaming/highlight snapshots are therefore owned by the outer agent while the final snapshot is owned by the handoff agent; because snapshot identity now includes agentId, the final event does not replace the incorrect snapshots, and repeated call IDs can leave results duplicated or routed to another card. Fresh evidence beyond the earlier callback fix is that the invocation path still never supplies that canonical owner to this producer; thread executingAgentId into the tool invocation and prefer it here.
Useful? React with 👍 / 👎.
The server has stamped `failed` on an errored summarize round only since #14546, and `summarizing` never reaches storage, so every round that errored on v0.8.7 or rc2 was persisted as a plain summary holding its partial deltas. The flag checks cannot tell that row from a checkpoint, so upgraded conversations kept losing the history before it. The aggregator settles it structurally, identically in every SDK release that has shipped compaction (3.1.63 through 3.8.6): deltas stream `content` blocks into the part, and only a completed round replaces the part with the final block, which is the sole writer of `boundary`. A `content`-block summary without a boundary therefore never finished. `isUsableSummaryPart` and the shared `isCompactedLeaf` both apply that rule; bare-`text` rows predate the aggregator and stay governed by the flags. Fixtures that stand for completed summaries now carry the boundary a real run writes, and the flag fixtures keep one so they still test the flags.
…15913) * 🧷 fix: Skip Failed Summaries When Resolving the Conversation Checkpoint * 🧷 fix: Keep a Failed Summary Out of the Model-Facing History Boundary * 🧭 refactor: Own the Summary Checkpoint Predicate in packages/api * 🧯 fix: Reject Unfinished Summaries Everywhere and Recount Stripped Turns * 🧬 fix: Version Event-Actor Summary State So Legacy Prefixes Cannot Warm * 🏷️ fix: Stamp Inherited Summaries Where Actor State Is Assembled * ⚖️ fix: Drop Unusable Summaries While Building the Prompt * 🧷 fix: Repoint Prompt Content Instead of Splicing Stored Parts * 🧭 fix: Treat a Streamed Summary Without a Boundary as Unfinished The server has stamped `failed` on an errored summarize round only since #14546, and `summarizing` never reaches storage, so every round that errored on v0.8.7 or rc2 was persisted as a plain summary holding its partial deltas. The flag checks cannot tell that row from a checkpoint, so upgraded conversations kept losing the history before it. The aggregator settles it structurally, identically in every SDK release that has shipped compaction (3.1.63 through 3.8.6): deltas stream `content` blocks into the part, and only a completed round replaces the part with the final block, which is the sole writer of `boundary`. A `content`-block summary without a boundary therefore never finished. `isUsableSummaryPart` and the shared `isCompactedLeaf` both apply that rule; bare-`text` rows predate the aggregator and stay governed by the flags. Fixtures that stand for completed summaries now carry the boundary a real run writes, and the flag fixtures keep one so they still test the flags. * 🔁 fix: Strip Unusable Summaries From Responses API Continuations `previous_response_id` loads the stored conversation and hands each message's `content` array straight to `formatAgentMessages`, whose summary scan takes the last summary part carrying text as the history boundary. The chat client strips unusable summaries before that scan; this entry point did not, so a conversation continued through the Responses API still lost everything before a failed round. The same packages/api helper now runs ahead of the formatter. * 🧭 refactor: Own Event-Actor Summary Selection in packages/api `getLatestEventActorSummary` decides which summary a warm continuation carries forward, how a missing token count is recorded, and how the state is stamped. That is compaction behavior, and every helper it relies on already lives in `packages/api/src/agents`, so it moves beside `isUsableSummaryPart` and the controller imports it. * 🧪 test: Give the Finished Compaction Leaf the Boundary a Completed Round Writes `isCompactedLeaf` now requires the boundary only a completed summary block carries, so the client fixture for a finished compaction needs one, and a boundaryless leaf joins the interrupted compactions that can be retried. --------- Co-authored-by: Danny Avila <danny@librechat.ai>
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail.
…Card (#16394) * 💭 feat: Bring the Streaming Thought Peek Back Under a Collapsed Live Card #14546 previewed streaming reasoning as its trailing sentences in a short fading window under the thought's header. The live fold (#16118) put that thought inside a collapsed card, which unmounts its rows, so the peek went with them and one throttled sentence on the header stood for a paragraph of live reasoning. A collapsed live card whose tail is a streaming thought now renders the same peek under its header, outside the fold, in the cursor's place. It gives way to the rows when the card opens and to the cursor when a call, not a thought, is at the tail. * 🧪 test: Watch Live Reasoning Stream Through the Real App A mock-lane scenario streams three sentences of reasoning at a readable pace; the spec samples the collapsed card's header and asserts every line was the generic one or a whole sentence, sees the thought peek under the card, opens the card mid-stream and checks the header became a title, then checks the settled thought's header sits at the shared row scale. The peek strips the thought's tags itself: handed the stream straight from a live card, it had shown a literal `<think>` before the first words. * 🎨 style: Sort Activity Phase Imports After Branch Rewrite --------- Co-authored-by: Lia <lia@librechat.ai>
Summary
This PR turns tool use, reasoning, search, memory, subagents, and clarification questions into one coherent activity system across settled, streaming, and interrupted responses.
ask_user_questionuses the same numbered option UI in chat and the composer, with a single movable control and a view-transition morph between those locations.General before / after
Activity lifecycle and streaming
Reasoning and tool use were summarized separately; the new summary reports one activity batch with human-readable action names.
The activity group now keeps a stable shell and meaningful in-progress label as tools arrive and settle.
Instead of hiding all intermediate reasoning behind a generic row, the live tail is visible with a symmetric fade while generation continues.
Completed reasoning gets consistent disclosure, copy, and spacing behavior.
Tool surfaces
Shared row hierarchy and disclosure treatment.
Friendly action label and the correct path field.
Consistent file action row and path presentation.
Structured unified diff with file stats and line numbers replaces the generic output treatment.
Execution uses the same compact result hierarchy as the other first-party tools.
Command/output presentation is aligned with the shared row, with copy controls kept available.
Third-party tool calls follow the same disclosure and status language without losing their server identity.
Search, retrieval, and memory
Results now include snippets and dates instead of title/domain alone.
The query, result count, and answer context are available in a focused details surface.
Image, shopping, and place results remain visible and places get direct map links.
Retrieval output uses the shared hierarchy and no longer gets stuck collapsed after an empty result.
Raw tool output becomes a dedicated card that exposes the saved key and value.
Deletion gets the same purpose-built, readable result treatment.
Artifacts render as stable memory content instead of leaking generic tool structure.
Questions, subagents, and handoffs
Chip buttons become numbered option rows with selection state, skip, and a clear freeform answer path.
The composer uses those same shared rows and one semantic move control; the control morphs between composer and message instead of duplicating UI.
Inline progress shows useful live work and a concise settled summary.
A blocking modal becomes a right-side panel so the parent conversation remains visible.
The transferred context is extracted from tool arguments and rendered as readable prose, with a compact disclosure rail and copy behavior instead of a nested JSON panel.
Generation status participates in the same action-row system.
Verification
47Jest suites passed (632tests total) across message content, tool renderers/grouping, subagent panel behavior, and ask/answer mode.tsc --noEmit.50matched screenshots without page errors.