Conversation
* address state in chat * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * update font size --------- Co-authored-by: himavarshagoutham <himavarshajan17@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request refines chat UI layout, message handling, and session-scoped build tracking. Changes include textarea resizing simplification, message component restructuring with improved layout, session-specific message caching, and adjustments to build duration tracking to scope per session context. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Layer
participant Store as Flow Store
participant Cache as Message Cache
participant API as Backend API
UI->>Store: User initiates build
Store->>Store: setBuildingSession(flowId, sessionId)
Store->>Cache: Store buildingFlowId & buildingSessionId
UI->>API: buildFlowVertices(flowId, sessionId)
API->>Cache: Process build events
API->>Store: Per-segment end_vertex event
Store->>Cache: findLastBotMessage(buildingFlowId, buildingSessionId)
Cache-->>Store: Return session-scoped message
Store->>Store: Calculate duration for this session only
API->>Store: Final build_duration event
Store->>Cache: Update session-specific cached message
Store->>API: Persist duration to backend
UI->>Store: Build completes
Store->>Store: setIsBuilding(false)
Store->>Store: Reset buildingFlowId & buildingSessionId to null
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (14.63%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11899 +/- ##
==========================================
+ Coverage 35.38% 35.39% +0.01%
==========================================
Files 1527 1527
Lines 73484 73499 +15
Branches 11041 11058 +17
==========================================
+ Hits 25999 26014 +15
- Misses 46073 46078 +5
+ Partials 1412 1407 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/frontend/src/pages/FlowPage/index.tsx (1)
136-148:⚠️ Potential issue | 🟡 MinorRemove debug
console.warn("unmounting")before merging.This is a leftover debug artifact in a cleanup effect that runs on every
idchange.🧹 Proposed fix
return () => { setOnFlowPage(false); - console.warn("unmounting"); - setCurrentFlow(undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/index.tsx` around lines 136 - 148, Remove the leftover debug call inside the cleanup of the useEffect in the FlowPage component: delete the console.warn("unmounting") statement from the effect that sets setOnFlowPage, setCurrentFlow, setSlidingContainerOpen, and setIsFullscreen (the effect with dependency [id]) so the cleanup only performs state resets without logging.src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsx (1)
16-16:⚠️ Potential issue | 🟠 MajorWiden
inputReftype to include| nullfor consistency withfileInputRefand React 19 semantics.
fileInputRefon Line 20 is correctly typed asReact.RefObject<HTMLInputElement | null>, butinputRefon Line 16 still uses the pre-widened formReact.RefObject<HTMLTextAreaElement>. This inconsistency requires the caller inchat-input.tsx(Line 48) to use thenull!non-null assertion hack:const inputRef = useRef<HTMLTextAreaElement>(null!);. Widening the type to include| nullmakes both ref types consistent and eliminates the need for that workaround.♻️ Proposed fix
- inputRef: React.RefObject<HTMLTextAreaElement>; + inputRef: React.RefObject<HTMLTextAreaElement | null>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsx` at line 16, The inputRef property is narrower than fileInputRef and requires callers to use a non-null assertion; update the inputRef type to React.RefObject<HTMLTextAreaElement | null> so it matches fileInputRef and React 19 semantics, then remove any non-null assertion usage in the chat-input.tsx caller (where useRef<HTMLTextAreaElement>(null!) is used); locate the declaration of inputRef in input-wrapper.tsx and change its type to include | null, ensuring all places that reference inputRef (and the fileInputRef comparison) compile without the null! hack.src/frontend/src/stores/flowStore.ts (1)
259-271:⚠️ Potential issue | 🟠 MajorCentralize build teardown so session scope cannot go stale.
Line 269 and Line 270 only clear session scope when teardown goes through
setIsBuilding(false), butstopBuilding(Line 131) andresetFlowStatecurrently bypass/omit that reset. This can leave stalebuildingFlowId/buildingSessionIdafter manual stop/reset.💡 Suggested fix
stopBuilding: () => { get().buildController.abort(); get().updateEdgesRunningByNodes( get().nodes.map((n) => n.id), false, ); - set({ isBuilding: false }); + get().setIsBuilding(false); get().revertBuiltStatusFromBuilding(); useAlertStore.getState().setErrorData({ title: "Build stopped", }); },resetFlowState: () => { set({ nodes: [], edges: [], flowState: undefined, hasIO: false, inputs: [], outputs: [], flowPool: {}, currentFlow: undefined, reactFlowInstance: null, lastCopiedSelection: null, verticesBuild: null, flowBuildStatus: {}, buildInfo: null, isBuilding: false, + buildingFlowId: null, + buildingSessionId: null, isPending: true, positionDictionary: {}, componentsToUpdate: [], rightClickedNodeId: null, }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/stores/flowStore.ts` around lines 259 - 271, The teardown that clears building session scope is only in setIsBuilding, so stopBuilding and resetFlowState can leave stale buildingFlowId/buildingSessionId; fix by centralizing teardown either by calling setIsBuilding(false) from stopBuilding and resetFlowState or extracting the clearing logic into a helper (eg. clearBuildingSession or teardownBuildingSession) and invoking that helper from setIsBuilding (when turning false), stopBuilding, and resetFlowState; update references to buildingFlowId and buildingSessionId in setIsBuilding, stopBuilding, and resetFlowState to use this shared teardown to ensure session IDs are always cleared.
🧹 Nitpick comments (7)
src/frontend/src/components/core/flowToolbarComponent/components/deploy-dropdown.tsx (1)
98-106: LGTM — removing the explicit color overrides from a ghost button is the right call.
variant="ghost"already owns the background/text styling contract. The formerbg-foreground text-backgroundclasses were fighting the variant and likely caused visual inconsistency depending on theme. Dropping them lets the token-based ghost styles apply cleanly.One minor note: the remaining
classNameis a plain string literal. Sincecn()is already imported you could use it for consistency, though it adds no real value here with a single unconditional string.🔧 Optional — wrap in cn() for consistency
- className="!px-2.5 font-normal" + className={cn("!px-2.5 font-normal")}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/flowToolbarComponent/components/deploy-dropdown.tsx` around lines 98 - 106, The Button uses a plain string literal for className; for consistency with the file's imports, wrap the classes with the cn() utility (e.g., replace the className value on the Button component in deploy-dropdown.tsx with cn("!px-2.5 font-normal")) and ensure the existing cn import is used — no other style changes are needed since the explicit bg-foreground/text-background overrides were correctly removed and variant="ghost" should drive colors.src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx (2)
85-91: Inconsistent properties handling vs.user-message.tsx— unsafeascast.
user-message.tsx(Lines 76-82) carefully narrowspropertiesto only includestatewhen it's"partial"or"complete". Here,...chat.propertiesis spread in full andstateis force-cast withas. Ifstateholds an unexpected runtime value, the cast silently passes it through.Consider aligning with the safer pattern from
user-message.tsx:♻️ Suggested alignment
properties: { - ...chat.properties, - state: chat.properties?.state as - | "complete" - | "partial" - | undefined, positive_feedback: evaluation, + ...(chat.properties?.state === "partial" || + chat.properties?.state === "complete" + ? { state: chat.properties.state } + : {}), + ...(chat.properties?.source + ? { source: { id: chat.properties.source.id } } + : {}), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx` around lines 85 - 91, The bot message spreads full chat.properties and force-casts state with "as", which can let invalid runtime values through; change the construction of the properties object in bot-message.tsx to mirror the safe narrowing used in user-message.tsx: do not spread chat.properties wholesale, instead build properties = { ...otherAllowedProps?, state: chat.properties?.state === "partial" || chat.properties?.state === "complete" ? chat.properties.state : undefined, positive_feedback: evaluation } (i.e., only include state when it equals "partial" or "complete" and avoid the unsafe as cast), referencing the properties creation in the Bot message component and aligning it with the pattern used in user-message.tsx.
137-151: Avatar conditionally hidden but condition may be too broad.The avatar renders when
thinkingActive || displayTime > 0 || chatMessage !== "". SincechatMessagedefaults to""only whenchat.messageis falsy (Line 43), virtually every bot message with any text will show the avatar. If the intent is to always show it for non-empty messages, the condition could be simplified. If the intent is to hide it for specific edge cases (e.g., empty building state), verify this is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx` around lines 137 - 151, The avatar visibility condition is overly broad: currently it shows when thinkingActive || displayTime > 0 || chatMessage !== "" which effectively displays for almost any non-empty bot text; decide the intended behavior and simplify accordingly — if you want the avatar to show for any non-empty message, replace the compound condition with chatMessage !== ""; if you need to hide it only during a specific "building/empty" state, add a clear boolean (e.g., isBuilding or isEmpty) to the component props/state and use that instead of the current combined condition; update references to thinkingActive, displayTime, chatMessage and chat.message to reflect the chosen simpler, explicit check.src/frontend/src/components/core/playgroundComponent/chat-view/utils/__tests__/message-utils.test.ts (2)
64-88: Consider adding a negative-path case: querying a non-existent session should returnnull.The current test only asserts correct isolation between two populated sessions. A complementary assertion would guard against false matches in the global-fallback path:
+ // Session that was never populated should return null when scoped + const resultMissing = findLastBotMessage("flow-1", "s-unknown"); + expect(resultMissing).toBeNull();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/utils/__tests__/message-utils.test.ts` around lines 64 - 88, Add a negative-path assertion to the existing test: after populating s1Key and s2Key with buildMessage and calling findLastBotMessage for "flow-1"/"s1" and "s2", also call findLastBotMessage("flow-1", "non-existent-session") and assert it returns null; locate the test that uses queryClient.setQueryData, s1Key/s2Key, buildMessage and findLastBotMessage and add the expect(result).toBeNull() for the non-existent session to ensure no global fallback occurs.
64-88: LGTM — session-scoped isolation is validated correctly; minor:s1Keyduplicates the module-levelQUERY_KEY.
s1Keyon Line 65 is structurally identical to the module-levelQUERY_KEY. Reusing it reduces maintenance surface:- const s1Key = ["useGetMessagesQuery", { id: "flow-1", session_id: "s1" }]; + const s1Key = QUERY_KEY; const s2Key = ["useGetMessagesQuery", { id: "flow-1", session_id: "s2" }];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/utils/__tests__/message-utils.test.ts` around lines 64 - 88, The test duplicates the module-level QUERY_KEY when building s1Key/s2Key; update the test to reuse QUERY_KEY to avoid duplication by constructing s1Key and s2Key from QUERY_KEY (merge/override the session_id and flow id into QUERY_KEY's params) before calling queryClient.setQueryData and asserting via findLastBotMessage, referencing the existing QUERY_KEY constant, the s1Key/s2Key variables, queryClient.setQueryData, and findLastBotMessage to locate where to change.src/frontend/src/utils/buildUtils.ts (1)
555-591: DuplicatefindLastBotMessage+ backend PUT block inend_vertexandendhandlers — extract to a helper.The ~20-line block that looks up the last bot message, conditionally updates its
build_duration, and fires aPUTto persist it appears nearly identically in both handlers. This creates a maintenance hazard.♻️ Suggested extraction
+function persistBuildDuration(durationMs: number) { + const flowState = useFlowStore.getState(); + const found = findLastBotMessage( + flowState.buildingFlowId ?? undefined, + flowState.buildingSessionId ?? undefined, + ); + if (found && !found.message.properties?.build_duration) { + updateMessageProperties(found.message.id!, found.queryKey, { + build_duration: durationMs, + }); + api + .put(`${getURL("MESSAGES")}/${found.message.id}`, { + ...found.message, + properties: { ...found.message.properties, build_duration: durationMs }, + }) + .catch((err: unknown) => { + console.warn("Failed to persist build_duration", { + messageId: found.message.id, + error: err instanceof Error ? err.message : String(err), + }); + }); + } +}Then in both handlers:
- const found = findLastBotMessage( - flowState.buildingFlowId ?? undefined, - flowState.buildingSessionId ?? undefined, - ); - if (found && !found.message.properties?.build_duration) { - updateMessageProperties(...) - api.put(...).catch(...) - } + persistBuildDuration(segmentDurationMs); // or durationMs in end handlerAlso applies to: 634-664
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/utils/buildUtils.ts` around lines 555 - 591, Extract the duplicated block that finds the last bot message, updates its properties with build_duration, and persists via api.put into a helper function (e.g., saveBuildDurationForLastBotMessage) and call it from both the end_vertex and end handlers: the helper should accept the flow/build identifiers and segmentDurationMs, call findLastBotMessage(flowId, sessionId), call updateMessageProperties(found.message.id, found.queryKey, { build_duration }), then perform the api.put using getURL("MESSAGES")/found.message.id and handle errors the same way (preserve the existing warning shape), and finally let the handlers continue to call flowState.setBuildStartTime(Date.now()) after invoking the helper. Ensure references to findLastBotMessage, updateMessageProperties, api.put and getURL("MESSAGES") are used inside the helper so both handlers simply compute segmentDurationMs and delegate to it.src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsx (1)
69-70: Switch class composition tocn()for component consistency.This component still uses
classNamesfor Tailwind class merging; prefercn()per repo convention.💡 Suggested fix
-import { classNames } from "../../../../../../utils/utils"; +import { cn } from "../../../../../../utils/utils"; ... - className={classNames(fileClass, additionalClassNames)} + className={cn(fileClass, additionalClassNames)}As per coding guidelines, "Use the cn() utility function for combining Tailwind CSS classes in components".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsx` around lines 69 - 70, Replace the raw/`classNames` class composition in TextAreaWrapper with the repo-standard cn() utility: import cn where needed and wrap the Tailwind class string (the "form-input block w-full border-0 custom-scroll focus:border-ring rounded-none shadow-none focus:ring-0 p-0 sm:text-sm !bg-transparent resize-none" usage) with cn(...) so the component uses cn() consistently instead of classNames or a plain string; update any adjacent class composition sites in the TextAreaWrapper component to the same cn() call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-header/components/session-selector.tsx`:
- Around line 114-115: In the SessionSelector component update the inner
container's Tailwind class in the JSX where className includes "w/full" to the
correct "w-full" so the div inside the outer <div className="flex h-8
items-center justify-between overflow-hidden w-full"> (look for the inner div in
session-selector.tsx) will properly expand to full width; simply replace the
typo "w/full" with "w-full" in that className.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/chat-input.tsx`:
- Line 48: Replace the non-null assertion in ChatInput's inputRef (change
useRef<HTMLTextAreaElement>(null!) to useRef<HTMLTextAreaElement | null>(null)
or useRef<HTMLTextAreaElement>(null)) and update the InputWrapper prop type to
accept a nullable ref (HTMLTextAreaElement | null) so the ref's type matches
React 19 behavior; then ensure all usages in InputWrapper and TextAreaWrapper
(e.g., any direct access to inputRef.current in InputWrapper and methods in
TextAreaWrapper) are guarded with optional chaining or explicit null checks to
satisfy the type change and avoid unsafe access.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx`:
- Around line 139-150: The avatar wrapper in BotMessage renders hardcoded
bg-white and text-black which breaks dark mode; update the JSX around the div
and the LangflowLogo usage in bot-message.tsx to use theme-aware classes (e.g.,
replace bg-white with bg-white dark:bg-<dark-bg> and text-black with text-black
dark:text-<light-text>) or conditionally apply classes using the existing
useDarkMode hook / dark store: import and call useDarkMode (or read the dark
store) and toggle the background/text classNames accordingly so the avatar
background and logo color adapt for dark theme.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/messages.tsx`:
- Line 29: Remove the unused bottomRef and its rendered anchor div: delete the
declaration const bottomRef = useRef<HTMLDivElement | null>(null) and remove the
conditional <div ref={bottomRef} /> (the anchor at lines ~78-84) since the
auto-scroll useEffect was removed and nothing reads or writes bottomRef; also
remove any imports/types that become unused after this deletion. Ensure no other
code references bottomRef before committing.
In
`@src/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsx`:
- Around line 167-168: The inner div uses an undefined Tailwind class "w-218";
update the div inside AnimatedConditional (the element with className "h-full
overflow-y-auto border-r border-border w-218") to use a valid width utility —
replace "w-218" with "w-full" (or "w-[236px]" if you want an explicit 236px
width) so the container matches AnimatedConditional's 236px width and uses a
defined Tailwind class.
In `@src/frontend/src/components/ui/simple-sidebar.tsx`:
- Around line 447-455: The trigger button doesn't expose its toggle state to
assistive tech—update the element that uses handleClick to include aria-expanded
tied to the open state (aria-expanded={open}) and provide a fallback accessible
label (e.g., aria-label derived from props or a default like "Expand
sidebar"/"Collapse sidebar" based on open) so screen readers know the control
and its current state; ensure you use the existing open variable and prefer any
incoming aria-label prop if present.
In
`@src/frontend/src/controllers/API/queries/messages/use-put-update-messages.ts`:
- Around line 83-92: In the update logic inside usePutUpdateMessages (the
old.map callback referencing existingIndex), avoid overwriting m.text when the
incoming message.text is undefined: set text to message.text only if
message.text !== undefined, otherwise keep m.text; compute textChanged as
message.text !== undefined && message.text !== m.text and use that to update
edit, and keep properties as message.properties ?? m.properties. This prevents
clearing cached text on partial updates.
- Around line 36-44: The code dereferences existingMessage after computing
messageIndex from messages which can be -1; add a guard right after computing
messageIndex to handle not-found cases (e.g., if (messageIndex === -1) { return
/* or handle appropriately: throw/log/append new message */ }), so you do not
access existingMessage.text or assign to messages[messageIndex] when
messageIndex is invalid; update the block around messageIndex / existingMessage
/ messages to early-return or perform the intended fallback behavior.
In `@src/frontend/src/utils/buildUtils.ts`:
- Around line 269-270: After the build "end" handler sets setIsBuilding(false),
also clear the stored building session so stale IDs don't persist; call the flow
store reset (useFlowStore.getState().setBuildingSession(...)) to clear
buildingFlowId/buildingSessionId (e.g., setBuildingSession(flowId, null) or
otherwise set both to null) right after setIsBuilding(false) in the end case so
findLastBotMessage and other event handlers no longer scope to the old session.
---
Outside diff comments:
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsx`:
- Line 16: The inputRef property is narrower than fileInputRef and requires
callers to use a non-null assertion; update the inputRef type to
React.RefObject<HTMLTextAreaElement | null> so it matches fileInputRef and React
19 semantics, then remove any non-null assertion usage in the chat-input.tsx
caller (where useRef<HTMLTextAreaElement>(null!) is used); locate the
declaration of inputRef in input-wrapper.tsx and change its type to include |
null, ensuring all places that reference inputRef (and the fileInputRef
comparison) compile without the null! hack.
In `@src/frontend/src/pages/FlowPage/index.tsx`:
- Around line 136-148: Remove the leftover debug call inside the cleanup of the
useEffect in the FlowPage component: delete the console.warn("unmounting")
statement from the effect that sets setOnFlowPage, setCurrentFlow,
setSlidingContainerOpen, and setIsFullscreen (the effect with dependency [id])
so the cleanup only performs state resets without logging.
In `@src/frontend/src/stores/flowStore.ts`:
- Around line 259-271: The teardown that clears building session scope is only
in setIsBuilding, so stopBuilding and resetFlowState can leave stale
buildingFlowId/buildingSessionId; fix by centralizing teardown either by calling
setIsBuilding(false) from stopBuilding and resetFlowState or extracting the
clearing logic into a helper (eg. clearBuildingSession or
teardownBuildingSession) and invoking that helper from setIsBuilding (when
turning false), stopBuilding, and resetFlowState; update references to
buildingFlowId and buildingSessionId in setIsBuilding, stopBuilding, and
resetFlowState to use this shared teardown to ensure session IDs are always
cleared.
---
Nitpick comments:
In
`@src/frontend/src/components/core/flowToolbarComponent/components/deploy-dropdown.tsx`:
- Around line 98-106: The Button uses a plain string literal for className; for
consistency with the file's imports, wrap the classes with the cn() utility
(e.g., replace the className value on the Button component in
deploy-dropdown.tsx with cn("!px-2.5 font-normal")) and ensure the existing cn
import is used — no other style changes are needed since the explicit
bg-foreground/text-background overrides were correctly removed and
variant="ghost" should drive colors.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsx`:
- Around line 69-70: Replace the raw/`classNames` class composition in
TextAreaWrapper with the repo-standard cn() utility: import cn where needed and
wrap the Tailwind class string (the "form-input block w-full border-0
custom-scroll focus:border-ring rounded-none shadow-none focus:ring-0 p-0
sm:text-sm !bg-transparent resize-none" usage) with cn(...) so the component
uses cn() consistently instead of classNames or a plain string; update any
adjacent class composition sites in the TextAreaWrapper component to the same
cn() call.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx`:
- Around line 85-91: The bot message spreads full chat.properties and
force-casts state with "as", which can let invalid runtime values through;
change the construction of the properties object in bot-message.tsx to mirror
the safe narrowing used in user-message.tsx: do not spread chat.properties
wholesale, instead build properties = { ...otherAllowedProps?, state:
chat.properties?.state === "partial" || chat.properties?.state === "complete" ?
chat.properties.state : undefined, positive_feedback: evaluation } (i.e., only
include state when it equals "partial" or "complete" and avoid the unsafe as
cast), referencing the properties creation in the Bot message component and
aligning it with the pattern used in user-message.tsx.
- Around line 137-151: The avatar visibility condition is overly broad:
currently it shows when thinkingActive || displayTime > 0 || chatMessage !== ""
which effectively displays for almost any non-empty bot text; decide the
intended behavior and simplify accordingly — if you want the avatar to show for
any non-empty message, replace the compound condition with chatMessage !== "";
if you need to hide it only during a specific "building/empty" state, add a
clear boolean (e.g., isBuilding or isEmpty) to the component props/state and use
that instead of the current combined condition; update references to
thinkingActive, displayTime, chatMessage and chat.message to reflect the chosen
simpler, explicit check.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/utils/__tests__/message-utils.test.ts`:
- Around line 64-88: Add a negative-path assertion to the existing test: after
populating s1Key and s2Key with buildMessage and calling findLastBotMessage for
"flow-1"/"s1" and "s2", also call findLastBotMessage("flow-1",
"non-existent-session") and assert it returns null; locate the test that uses
queryClient.setQueryData, s1Key/s2Key, buildMessage and findLastBotMessage and
add the expect(result).toBeNull() for the non-existent session to ensure no
global fallback occurs.
- Around line 64-88: The test duplicates the module-level QUERY_KEY when
building s1Key/s2Key; update the test to reuse QUERY_KEY to avoid duplication by
constructing s1Key and s2Key from QUERY_KEY (merge/override the session_id and
flow id into QUERY_KEY's params) before calling queryClient.setQueryData and
asserting via findLastBotMessage, referencing the existing QUERY_KEY constant,
the s1Key/s2Key variables, queryClient.setQueryData, and findLastBotMessage to
locate where to change.
In `@src/frontend/src/utils/buildUtils.ts`:
- Around line 555-591: Extract the duplicated block that finds the last bot
message, updates its properties with build_duration, and persists via api.put
into a helper function (e.g., saveBuildDurationForLastBotMessage) and call it
from both the end_vertex and end handlers: the helper should accept the
flow/build identifiers and segmentDurationMs, call findLastBotMessage(flowId,
sessionId), call updateMessageProperties(found.message.id, found.queryKey, {
build_duration }), then perform the api.put using
getURL("MESSAGES")/found.message.id and handle errors the same way (preserve the
existing warning shape), and finally let the handlers continue to call
flowState.setBuildStartTime(Date.now()) after invoking the helper. Ensure
references to findLastBotMessage, updateMessageProperties, api.put and
getURL("MESSAGES") are used inside the helper so both handlers simply compute
segmentDurationMs and delegate to it.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/frontend/src/components/core/chatComponents/ContentBlockDisplay.tsxsrc/frontend/src/components/core/chatComponents/ContentDisplay.tsxsrc/frontend/src/components/core/flowToolbarComponent/components/deploy-dropdown.tsxsrc/frontend/src/components/core/flowToolbarComponent/components/playground-button.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-header/components/session-selector.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-input/chat-input.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/upload-file-button.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/edit-message-field.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/user-message.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/messages.tsxsrc/frontend/src/components/core/playgroundComponent/chat-view/utils/__tests__/message-utils.test.tssrc/frontend/src/components/core/playgroundComponent/chat-view/utils/message-utils.tssrc/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsxsrc/frontend/src/components/ui/simple-sidebar.tsxsrc/frontend/src/constants/constants.tssrc/frontend/src/controllers/API/queries/messages/use-put-update-messages.tssrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarSegmentedNav.tsxsrc/frontend/src/pages/FlowPage/index.tsxsrc/frontend/src/stores/flowStore.tssrc/frontend/src/types/zustand/flow/index.tssrc/frontend/src/utils/buildUtils.ts
| <div className="flex h-8 items-center justify-between overflow-hidden w-full"> | ||
| <div className="flex w/full min-w-0 items-center px-2"> |
There was a problem hiding this comment.
Pre-existing typo: w/full on Line 115 is not a valid Tailwind class.
Line 115 has w/full instead of w-full. This is an invalid Tailwind class, so the inner container won't get full-width styling. Now that the outer container on Line 114 is w-full (expanded from w-52), this bug becomes more visible — the inner content div won't fill the wider parent.
🐛 Proposed fix
- <div className="flex w/full min-w-0 items-center px-2">
+ <div className="flex w-full min-w-0 items-center px-2">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex h-8 items-center justify-between overflow-hidden w-full"> | |
| <div className="flex w/full min-w-0 items-center px-2"> | |
| <div className="flex h-8 items-center justify-between overflow-hidden w-full"> | |
| <div className="flex w-full min-w-0 items-center px-2"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-header/components/session-selector.tsx`
around lines 114 - 115, In the SessionSelector component update the inner
container's Tailwind class in the JSX where className includes "w/full" to the
correct "w-full" so the div inside the outer <div className="flex h-8
items-center justify-between overflow-hidden w-full"> (look for the inner div in
session-selector.tsx) will properly expand to full width; simply replace the
typo "w/full" with "w-full" in that className.
| ); | ||
|
|
||
| const inputRef = useRef<HTMLTextAreaElement>(null); | ||
| const inputRef = useRef<HTMLTextAreaElement>(null!); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "chat-input.tsx" | head -5Repository: langflow-ai/langflow
Length of output: 233
🏁 Script executed:
find . -type f -name "input-wrapper.tsx" | head -5Repository: langflow-ai/langflow
Length of output: 261
🏁 Script executed:
cat -n ./src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/chat-input.tsx | head -70Repository: langflow-ai/langflow
Length of output: 3254
🏁 Script executed:
cat -n ./src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsx | head -30Repository: langflow-ai/langflow
Length of output: 1436
🌐 Web query:
React 19 useRef typing RefObject MutableRefObject null non-null assertion
💡 Result:
In React 19 TypeScript types, refs are typed more strictly around null, so you’ll often see RefObject<T | null> where older setups let you “pretend” it was always T. (reddit.com)
1) DOM element refs: include null (and narrow at use site)
In React, an element ref is null before mount (and can become null again on unmount), so the safe typing is:
const divRef = useRef<HTMLDivElement | null>(null);
// later
divRef.current?.focus(); // or any method/property
// or
if (divRef.current) divRef.current.focus();React’s own docs show useRef(null) for element refs, and current is set by React when attached to JSX. (react.dev)
2) Why your RefObject<T> no longer matches
With React 19 types, if you write:
const divRef = useRef<HTMLDivElement>(null);the returned type becomes RefObject<HTMLDivElement | null> (because you initialized with null). That can fail when passing into code that expects RefObject<HTMLDivElement> (non-nullable). (reddit.com)
Fix: change the receiving API type to accept nullable refs:
function takesRef(r: RefObject<HTMLDivElement | null>) {}3) Non-null assertion (!)—only when you can prove it’s mounted
If you know it’s non-null at that point (e.g., inside an effect after mount, or right after a null-check), you can assert:
divRef.current!.focus();…but it bypasses safety; prefer ?. or an if guard when possible. (stackabuse.com)
4) RefObject vs MutableRefObject (what to do in React 19)
Historically, React typings distinguished “mutable value container” refs (MutableRefObject<T>) vs element refs (RefObject<T>). In the React 19 type direction, refs are treated as mutable by default and useRef typing/overloads have been changing accordingly. (github.com)
Practical takeaway: don’t fight the types—store DOM refs as T | null, and narrow when reading.
Citations:
- 1: https://www.reddit.com/r/reactjs/comments/1g1u0cn?utm_source=openai
- 2: https://react.dev/reference/react/useRef?utm_source=openai
- 3: https://www.reddit.com/r/reactjs/comments/1g1u0cn?utm_source=openai
- 4: https://stackabuse.com/bytes/fixing-the-useref-object-is-possibly-null-error-in-react/?utm_source=openai
- 5: [react] Types for 19 DefinitelyTyped/DefinitelyTyped#64451
🌐 Web query:
React 19 useRef<T>(null) return type RefObject null handling TypeScript
💡 Result:
In React 19’s TypeScript types, useRef was changed so it always returns a mutable RefObject<T>, and there’s a special overload where useRef<T>(null) yields a nullable ref: RefObject<T | null>. This is intentional and makes null handling explicit. [1]
What useRef<T>(null) returns in React 19
const divRef = useRef<HTMLDivElement>(null);
// type: React.RefObject<HTMLDivElement | null>So divRef.current is possibly null and TypeScript will require checks. [1][2]
Correct null-handling patterns
1) Prefer: keep it nullable and check when using it
if (!divRef.current) return;
divRef.current.focus();2) If you know it’s set (e.g., inside an effect after mount), assert
useEffect(() => {
divRef.current!.focus();
}, []);3) If you must have a non-null RefObject<T> type (escape hatch)
If some API incorrectly requires RefObject<T> (non-nullable) and you can guarantee it won’t be read before assignment, you can force the type:
const divRef = useRef<HTMLDivElement>(null!); // type: RefObject<HTMLDivElement>This bypasses safety—use sparingly. The stricter nullable ref behavior is a known migration pain point with some libraries. [1][3]
Related React 19 ref typing changes to be aware of
useRef()with no argument is no longer allowed; it requires an initial value now. [1][2]- The old split between
MutableRefObjectvsRefObjectis effectively gone in favor of a single mutableRefObject. [1][2]
Sources: React 19 Upgrade Guide (refs + useRef overloads) [1], @types/react codemods documenting the changes [2], example of breakage due to nullable RefObject<T | null> [3].
🏁 Script executed:
cat -n ./src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/input-wrapper.tsx | grep -A 5 "inputRef"Repository: langflow-ai/langflow
Length of output: 1029
🏁 Script executed:
rg "useRef<.*>\(null!\)" src/frontend/src --type ts --type tsx -cRepository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
rg "useRef<.*>\(null!\)" src/frontend/src -t ts -t tsx -cRepository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
cat -n ./src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsx | sed -n '50,65p'Repository: langflow-ai/langflow
Length of output: 581
🏁 Script executed:
rg "useRef.*null!" src/frontend/srcRepository: langflow-ai/langflow
Length of output: 207
🏁 Script executed:
cat -n ./src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/components/text-area-wrapper.tsx | sed -n '75,95p'Repository: langflow-ai/langflow
Length of output: 849
Replace null! with null and update InputWrapper prop type to accept nullable ref.
In React 19, useRef<T>(null) returns RefObject<T | null>. The non-null assertion null! bypasses the type system and masks the actual nullable nature of the ref at runtime. The proper pattern is to accept | null in prop types and use optional chaining (already done at line 61), or explicit null checks (already done in text-area-wrapper.tsx).
Line 55–57 in input-wrapper.tsx shows unsafe direct access to inputRef.current after an optional chaining call; updating the prop type to nullable makes this safer by enforcing explicit narrowing.
♻️ Proposed changes
In chat-input.tsx:
- const inputRef = useRef<HTMLTextAreaElement>(null!);
+ const inputRef = useRef<HTMLTextAreaElement>(null);In input-wrapper.tsx:
- inputRef: React.RefObject<HTMLTextAreaElement>;
+ inputRef: React.RefObject<HTMLTextAreaElement | null>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const inputRef = useRef<HTMLTextAreaElement>(null!); | |
| const inputRef = useRef<HTMLTextAreaElement>(null); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-input/chat-input.tsx`
at line 48, Replace the non-null assertion in ChatInput's inputRef (change
useRef<HTMLTextAreaElement>(null!) to useRef<HTMLTextAreaElement | null>(null)
or useRef<HTMLTextAreaElement>(null)) and update the InputWrapper prop type to
accept a nullable ref (HTMLTextAreaElement | null) so the ref's type matches
React 19 behavior; then ensure all usages in InputWrapper and TextAreaWrapper
(e.g., any direct access to inputRef.current in InputWrapper and methods in
TextAreaWrapper) are guarded with optional chaining or explicit null checks to
satisfy the type change and avoid unsafe access.
| <div | ||
| className="relative hidden h-6 w-6 mt-[-1px] flex-shrink-0 items-center justify-center overflow-hidden rounded bg-white text-2xl @[45rem]/chat-panel:!flex border-0" | ||
| style={ | ||
| chat.properties?.background_color | ||
| ? { backgroundColor: chat.properties.background_color } | ||
| : {} | ||
| } | ||
| > | ||
| <div className="flex h-5 w-5 items-center justify-center"> | ||
| <LangflowLogo className="h-4 w-4 text-black" /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Hardcoded bg-white and text-black break dark mode.
Line 140 uses bg-white and Line 148 uses text-black, which won't adapt to the dark theme. Use theme-aware Tailwind classes instead.
🎨 Proposed fix
- className="relative hidden h-6 w-6 mt-[-1px] flex-shrink-0 items-center justify-center overflow-hidden rounded bg-white text-2xl @[45rem]/chat-panel:!flex border-0"
+ className="relative hidden h-6 w-6 mt-[-1px] flex-shrink-0 items-center justify-center overflow-hidden rounded bg-background text-2xl @[45rem]/chat-panel:!flex border-0"- <LangflowLogo className="h-4 w-4 text-black" />
+ <LangflowLogo className="h-4 w-4 text-foreground" />As per coding guidelines: "Implement dark mode support using the useDarkMode hook and dark store"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/components/bot-message.tsx`
around lines 139 - 150, The avatar wrapper in BotMessage renders hardcoded
bg-white and text-black which breaks dark mode; update the JSX around the div
and the LangflowLogo usage in bot-message.tsx to use theme-aware classes (e.g.,
replace bg-white with bg-white dark:bg-<dark-bg> and text-black with text-black
dark:text-<light-text>) or conditionally apply classes using the existing
useDarkMode hook / dark store: import and call useDarkMode (or read the dark
store) and toggle the background/text classNames accordingly so the avatar
background and logo color adapt for dark theme.
| @@ -28,15 +28,13 @@ export const Messages = ({ | |||
|
|
|||
| const bottomRef = useRef<HTMLDivElement | null>(null); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
bottomRef is dead code — the ref and its rendered anchor <div> are unused after the auto-scroll effect was removed.
bottomRef was only meaningful while a useEffect called .scrollIntoView() on it. Now that the scroll effect is gone, both the useRef and the conditional <div ref={bottomRef}> can be removed.
🧹 Proposed cleanup
- const bottomRef = useRef<HTMLDivElement | null>(null);
-
// Show thinking placeholder when building...- {isPlaygroundOpen && (
- <div
- ref={bottomRef}
- className="pointer-events-none absolute bottom-0 left-0 right-0 h-0 w-0 overflow-hidden"
- aria-hidden
- />
- )}Also applies to: 78-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/messages.tsx`
at line 29, Remove the unused bottomRef and its rendered anchor div: delete the
declaration const bottomRef = useRef<HTMLDivElement | null>(null) and remove the
conditional <div ref={bottomRef} /> (the anchor at lines ~78-84) since the
auto-scroll useEffect was removed and nothing reads or writes bottomRef; also
remove any imports/types that become unused after this deletion. Ensure no other
code references bottomRef before committing.
| <AnimatedConditional isOpen={sidebarOpen} width="236px"> | ||
| <div className="h-full overflow-y-auto border-r border-border w-218"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for w-218 definition in config and CSS files
rg -n "w-218" --type css --type js --type ts --type tsx -g '!node_modules'Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
# Check Tailwind config files for custom width definitions
fd -e "tailwind.config" -e "tailwind.config.ts" -e "tailwind.config.js" | head -5 | xargs catRepository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# Look at the actual file to understand the width context
head -200 src/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsx | tail -80Repository: langflow-ai/langflow
Length of output: 2922
🏁 Script executed:
# Search for all width-related usages (236px, 218px) in the file
rg -n "(236|218)" src/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsxRepository: langflow-ai/langflow
Length of output: 239
🏁 Script executed:
# Check if 236px is used elsewhere in the codebase
rg -n "236px" --type tsx --type ts -g '!node_modules' | head -20Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
# Search for any custom width class definitions (w-XXX pattern)
rg -n "w-\d+" --type css -g '!node_modules' | head -30Repository: langflow-ai/langflow
Length of output: 3264
🏁 Script executed:
# Look for arbitrary Tailwind width values w-[...]
rg "w-\[\d+px\]" -g '!node_modules' | head -20Repository: langflow-ai/langflow
Length of output: 3182
🏁 Script executed:
# Check if w-218 or similar custom utilities are defined anywhere
rg "(w-218|218px)" -g '!node_modules' | head -30Repository: langflow-ai/langflow
Length of output: 257
🏁 Script executed:
# Look for Tailwind extend config or custom class definitions
rg "extend.*width|width.*extend" --type js --type ts -g '!node_modules' | head -20Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# Check the entire flow-page-sliding-container file for context on width usage
cat -n src/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsx | grep -A 5 -B 5 "setWidth\|AnimatedConditional"Repository: langflow-ai/langflow
Length of output: 2864
Replace the invalid w-218 class with w-full to match the AnimatedConditional container width.
The w-218 class on line 168 is not defined anywhere in the project's Tailwind configuration or custom styles. The codebase uses arbitrary Tailwind values like w-[236px] for custom widths, not undefined utility classes. Since AnimatedConditional is now 236px wide, the inner div should either use w-[236px] to explicitly match or w-full to fill the container naturally.
🔧 Proposed fix
- <div className="h-full overflow-y-auto border-r border-border w-218">
+ <div className="h-full overflow-y-auto border-r border-border w-full">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <AnimatedConditional isOpen={sidebarOpen} width="236px"> | |
| <div className="h-full overflow-y-auto border-r border-border w-218"> | |
| <AnimatedConditional isOpen={sidebarOpen} width="236px"> | |
| <div className="h-full overflow-y-auto border-r border-border w-full"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/components/core/playgroundComponent/sliding-container/components/flow-page-sliding-container.tsx`
around lines 167 - 168, The inner div uses an undefined Tailwind class "w-218";
update the div inside AnimatedConditional (the element with className "h-full
overflow-y-auto border-r border-border w-218") to use a valid width utility —
replace "w-218" with "w-full" (or "w-[236px]" if you want an explicit 236px
width) so the container matches AnimatedConditional's 236px width and uses a
defined Tailwind class.
| className={cn( | ||
| "!px-2 !font-normal !justify-start !gap-1.5 !w-[7.2rem]", | ||
| className, | ||
| )} | ||
| onClick={handleClick} | ||
| {...props} | ||
| > | ||
| {open ? ( | ||
| <PanelLeftOpen strokeWidth={1.5} /> | ||
| ) : ( | ||
| <PanelRightOpen strokeWidth={1.5} /> | ||
| )} | ||
| {children && ( | ||
| <AnimatedConditional isOpen={!open}> | ||
| <div className="pl-2">{children}</div> | ||
| </AnimatedConditional> | ||
| )} | ||
| <Play strokeWidth={1.5} /> | ||
| {children} |
There was a problem hiding this comment.
Expose toggle state to assistive tech on the trigger button.
This control toggles sidebar visibility, but its expanded/collapsed state is not announced. Add aria-expanded (using open) and a fallback label.
Suggested patch
return (
<Button
ref={ref}
data-sidebar="trigger"
data-testid="playground-btn-flow-io"
variant="ghost"
size="md"
className={cn(
"!px-2 !font-normal !justify-start !gap-1.5 !w-[7.2rem]",
className,
)}
+ aria-expanded={open}
+ aria-label={props["aria-label"] ?? "Toggle sidebar"}
onClick={handleClick}
{...props}
>
<Play strokeWidth={1.5} />
{children}
</Button>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={cn( | |
| "!px-2 !font-normal !justify-start !gap-1.5 !w-[7.2rem]", | |
| className, | |
| )} | |
| onClick={handleClick} | |
| {...props} | |
| > | |
| {open ? ( | |
| <PanelLeftOpen strokeWidth={1.5} /> | |
| ) : ( | |
| <PanelRightOpen strokeWidth={1.5} /> | |
| )} | |
| {children && ( | |
| <AnimatedConditional isOpen={!open}> | |
| <div className="pl-2">{children}</div> | |
| </AnimatedConditional> | |
| )} | |
| <Play strokeWidth={1.5} /> | |
| {children} | |
| className={cn( | |
| "!px-2 !font-normal !justify-start !gap-1.5 !w-[7.2rem]", | |
| className, | |
| )} | |
| aria-expanded={open} | |
| aria-label={props["aria-label"] ?? "Toggle sidebar"} | |
| onClick={handleClick} | |
| {...props} | |
| > | |
| <Play strokeWidth={1.5} /> | |
| {children} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/components/ui/simple-sidebar.tsx` around lines 447 - 455,
The trigger button doesn't expose its toggle state to assistive tech—update the
element that uses handleClick to include aria-expanded tied to the open state
(aria-expanded={open}) and provide a fallback accessible label (e.g., aria-label
derived from props or a default like "Expand sidebar"/"Collapse sidebar" based
on open) so screen readers know the control and its current state; ensure you
use the existing open variable and prefer any incoming aria-label prop if
present.
| const existingMessage = messages[messageIndex]; | ||
| const textChanged = | ||
| message.text !== undefined && message.text !== existingMessage.text; | ||
| messages[messageIndex] = { | ||
| ...messages[messageIndex], | ||
| ...existingMessage, | ||
| ...message, | ||
| flow_id: flowId, | ||
| edit: true, | ||
| edit: textChanged ? true : existingMessage.edit, | ||
| }; |
There was a problem hiding this comment.
Guard missing cached message before dereferencing existingMessage.
When findIndex returns -1, Line 38 reads existingMessage.text and crashes. Line 39 also writes to an invalid array index path. Add an index guard before merge.
💡 Suggested fix
- const messages = JSON.parse(sessionStorage.getItem(flowId) || "");
+ const raw = sessionStorage.getItem(flowId);
+ if (!raw) return;
+ const messages: Message[] = JSON.parse(raw);
const messageIndex = messages.findIndex(
(m: Message) => m.id === message.id,
);
+ if (messageIndex === -1) return;
const existingMessage = messages[messageIndex];
const textChanged =
message.text !== undefined && message.text !== existingMessage.text;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/messages/use-put-update-messages.ts`
around lines 36 - 44, The code dereferences existingMessage after computing
messageIndex from messages which can be -1; add a guard right after computing
messageIndex to handle not-found cases (e.g., if (messageIndex === -1) { return
/* or handle appropriately: throw/log/append new message */ }), so you do not
access existingMessage.text or assign to messages[messageIndex] when
messageIndex is invalid; update the block around messageIndex / existingMessage
/ messages to early-return or perform the intended fallback behavior.
| return old.map((m, idx) => { | ||
| if (idx !== existingIndex) return m; | ||
| const textChanged = | ||
| message.text !== undefined && message.text !== m.text; | ||
| return { | ||
| ...m, | ||
| text: message.text, | ||
| edit: textChanged ? true : m.edit, | ||
| properties: message.properties ?? m.properties, | ||
| }; |
There was a problem hiding this comment.
Avoid clearing cached text on partial updates.
Line 89 sets text to message.text even when it is undefined, which can erase message text when only properties are updated.
💡 Suggested fix
return {
...m,
- text: message.text,
+ text: message.text ?? m.text,
edit: textChanged ? true : m.edit,
properties: message.properties ?? m.properties,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return old.map((m, idx) => { | |
| if (idx !== existingIndex) return m; | |
| const textChanged = | |
| message.text !== undefined && message.text !== m.text; | |
| return { | |
| ...m, | |
| text: message.text, | |
| edit: textChanged ? true : m.edit, | |
| properties: message.properties ?? m.properties, | |
| }; | |
| return old.map((m, idx) => { | |
| if (idx !== existingIndex) return m; | |
| const textChanged = | |
| message.text !== undefined && message.text !== m.text; | |
| return { | |
| ...m, | |
| text: message.text ?? m.text, | |
| edit: textChanged ? true : m.edit, | |
| properties: message.properties ?? m.properties, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/messages/use-put-update-messages.ts`
around lines 83 - 92, In the update logic inside usePutUpdateMessages (the
old.map callback referencing existingIndex), avoid overwriting m.text when the
incoming message.text is undefined: set text to message.text only if
message.text !== undefined, otherwise keep m.text; compute textChanged as
message.text !== undefined && message.text !== m.text and use that to update
edit, and keep properties as message.properties ?? m.properties. This prevents
clearing cached text on partial updates.
| // Scope build_duration updates to this flow/session (fixes wrong session showing duration) | ||
| useFlowStore.getState().setBuildingSession(flowId, session ?? null); |
There was a problem hiding this comment.
setBuildingSession is never reset after build completion — stale session IDs persist in the store.
After the end event handler calls setIsBuilding(false) (Line 667), buildingFlowId and buildingSessionId are left set. If a subsequent build starts on a different flow/session before the store updates, findLastBotMessage in event handlers could scope to the wrong session.
🛡️ Proposed fix — reset after build ends
In the end case handler (around Line 667):
onBuildComplete && onBuildComplete(allNodesValid);
useFlowStore.getState().setIsBuilding(false);
+ useFlowStore.getState().setBuildingSession(null, null);
return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/utils/buildUtils.ts` around lines 269 - 270, After the build
"end" handler sets setIsBuilding(false), also clear the stored building session
so stale IDs don't persist; call the flow store reset
(useFlowStore.getState().setBuildingSession(...)) to clear
buildingFlowId/buildingSessionId (e.g., setBuildingSession(flowId, null) or
otherwise set both to null) right after setIsBuilding(false) in the end case so
findLastBotMessage and other event handlers no longer scope to the old session.
The playground now opens in fullscreen mode, which covers the toolbar button (playground-btn-flow-io). Updated the freeze test to use the close button inside the playground header instead.
The playground now opens in fullscreen, covering the toolbar button. Use playground-close-button instead to close the playground.
Use playground-close-button to close and exit fullscreen before accessing the more menu which is hidden in fullscreen mode.
The chat-header-more-menu is hidden in fullscreen mode. Use the session sidebar more menu instead to access message logs.
The clear-chat option is only available in the chat-header-more-menu which is hidden in fullscreen mode. Exit fullscreen first.
Use sidebar new-chat button and session more menu instead of header elements that are hidden in fullscreen mode.
Use sidebar session more menu and new-chat button instead of header elements that are hidden or overlapped in fullscreen mode.
|
@keval718 Hi, I was just wondering that why right panel playground css would be deleted since 1.8.1? |
Overview
This PR refactors the playground experience with improved state management, UI enhancements, and code quality improvements across the chat interface and message handling components.
Summary of Changes
🔧 Core Fixes
Fixed playground state management - Resolved state synchronization issues in chat messages
Resolved ESLint errors - Cleaned up code quality issues across chat components
Enhanced message utilities - Added robust message handling with comprehensive test coverage
UI/UX Improvements
Simplified and optimized chat message components
Enhanced content display and message editing
Improved sidebar and navigation UI
Refined input handling and text area components
Better visual consistency across playground interface
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Style