Fix: When adding a chat in the main interface, a warning will automatically pop up #15685
Merged
Conversation
…ically pop up (even if embedding and LLM model have already been configured).
dosubot
Bot
added
size:S
Contributor
📝 WalkthroughWalkthroughAdds loading-aware suppression for empty-model warnings, adjusts chunk/testing list scroll layouts and pagination spacing, reduces memory fetch page_size, and applies small UI tweaks to popover sizing and command-item keyword props. ChangesLoading-aware empty-model warning suppression
Scroll/container layout updates
Memory request page size change
Popover and command-item UI tweaks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15685 +/- ##
=======================================
Coverage 93.16% 93.16%
=======================================
Files 10 10
Lines 717 717
Branches 118 118
=======================================
Hits 668 668
Misses 29 29
Partials 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dosubot
Bot
added
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/dataset/testing/testing-result.tsx (1)
89-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore a constrained scroll container for chunk list.
Line 89 sets
overflow-autoon the list, but this node is no longer height-constrained after removing the wrapper. In the current hierarchy (article flex-col→ header + section + pagination), the section can expand with content, so internal scrolling may not engage and pagination can be pushed out of view.Suggested fix
- <> + <div className="flex-1 min-h-0 flex flex-col"> {data.chunks?.length > 0 && !loading && ( <> - <section className="px-5 pb-5 flex flex-col gap-5 overflow-auto scrollbar-thin min-h-0"> + <section className="flex-1 min-h-0 px-5 pb-5 flex flex-col gap-5 overflow-auto scrollbar-thin"> {data.chunks?.map((x) => ( <article key={x.chunk_id}> <Card className="px-5 py-2.5 bg-transparent shadow-none"> <ChunkTitle item={x}></ChunkTitle> <p className="!mt-2.5"> {x.content_with_weight}</p> </Card> </article> ))} </section> <div className="p-2"> <RAGFlowPagination total={data.total} onChange={onPaginationChange} current={page} pageSize={pageSize} ></RAGFlowPagination> </div> </> )} {!data.chunks?.length && !loading && ( <div className="size-full p-5 flex justify-center items-center"> <div> <Empty type={EmptyType.SearchData} iconWidth={80}> <div className="text-text-secondary text-sm"> {t( data.isRuned ? 'knowledgeDetails.noTestResultsForRuned' : 'knowledgeDetails.noTestResultsForNotRuned', )} </div> </Empty> </div> </div> )} - </> + </div>As per coding guidelines: “When fixing CSS/layout issues… inspect the full parent hierarchy for
flex-shrink,min-width, andoverflowconstraints before applying fixes.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/pages/dataset/testing/testing-result.tsx` around lines 89 - 106, The chunk list section can grow and push pagination out of view because its parent flex container isn't height-constrained; make the list a constrained scrollable area by ensuring the parent article (or the section’s immediate flex container) is a column flex with a constrained height (e.g., flex-1 with min-h-0) and then keep overflow-auto on the section so internal scrolling works; update the component containing ChunkTitle and the section that maps data.chunks (the article/Card/section hierarchy) to add those flex/min-height constraints and preserve RAGFlowPagination placement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/src/pages/dataset/testing/testing-result.tsx`:
- Around line 89-106: The chunk list section can grow and push pagination out of
view because its parent flex container isn't height-constrained; make the list a
constrained scrollable area by ensuring the parent article (or the section’s
immediate flex container) is a column flex with a constrained height (e.g.,
flex-1 with min-h-0) and then keep overflow-auto on the section so internal
scrolling works; update the component containing ChunkTitle and the section that
maps data.chunks (the article/Card/section hierarchy) to add those
flex/min-height constraints and preserve RAGFlowPagination placement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06de0bf3-5318-44cc-9da4-f2bc49d8a7c4
📒 Files selected for processing (2)
web/src/pages/chunk/parsed-result/add-knowledge/components/knowledge-chunk/index.tsxweb/src/pages/dataset/testing/testing-result.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/tree-select.tsx (1)
238-238: ⚡ Quick winConsider adding a max-width constraint for responsive behavior.
The change from fixed width to
w-autoallows the popover to expand and show full labels without truncation, which improves UX. However, without amax-widthconstraint, extremely long tree node labels could make the popover very wide, potentially causing layout issues in narrow containers or on mobile devices.📐 Suggested improvement
- className="p-0 w-auto min-w-[var(--radix-popover-trigger-width)]" + className="p-0 w-auto min-w-[var(--radix-popover-trigger-width)] max-w-md"This maintains the flexible width while preventing excessive expansion. Adjust the max-width value (
max-w-md,max-w-lg,max-w-xl, etc.) based on your design requirements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/tree-select.tsx` at line 238, The popover trigger's className in the TreeSelect component currently uses "p-0 w-auto min-w-[var(--radix-popover-trigger-width)]" which can grow unbounded; add a responsive max-width class (e.g., "max-w-md" or "max-w-lg") to constrain expansion on small screens while preserving w-auto behavior. Update the className string on the same element in tree-select.tsx (where className="p-0 w-auto min-w-[var(--radix-popover-trigger-width)]") to include the chosen max-w utility and adjust the breakpoint-specific max-width if needed for your design. Ensure the new class is compatible with your tailwind config or replace with an inline maxWidth style if utilities aren’t available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/src/components/tree-select.tsx`:
- Line 238: The popover trigger's className in the TreeSelect component
currently uses "p-0 w-auto min-w-[var(--radix-popover-trigger-width)]" which can
grow unbounded; add a responsive max-width class (e.g., "max-w-md" or
"max-w-lg") to constrain expansion on small screens while preserving w-auto
behavior. Update the className string on the same element in tree-select.tsx
(where className="p-0 w-auto min-w-[var(--radix-popover-trigger-width)]") to
include the chosen max-w utility and adjust the breakpoint-specific max-width if
needed for your design. Ensure the new class is compatible with your tailwind
config or replace with an inline maxWidth style if utilities aren’t available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efababbb-399b-4faa-98c2-fcdbe4e239b7
📒 Files selected for processing (2)
web/src/components/tree-select.tsxweb/src/pages/agent/form/components/select-with-secondary-menu.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Fix: When adding a chat in the main interface, a warning will automatically pop up (even if embedding and LLM model have already been configured).
Type of change