refactor: Clear up VLM settings limitations - #2170
Conversation
WalkthroughAdds typed run-mode constants and propagates them through authentication and URL utilities. SaaS ingest settings now enforce mutual exclusivity between Langflow ingestion disabling and picture descriptions with automatic switching and tooltips. ChangesIngest mode controls
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@frontend/app/settings/_components/ingest-settings-section.tsx`:
- Around line 782-809: The settings form currently allows legacy SaaS state with
both disableIngestWithLangflow and pictureDescriptions enabled to be submitted
unchanged. Update the load/save handling around these state values and the
related save controls near the Switch to normalize the pair or prevent
submission until one is disabled, while preserving the existing mutual-exclusion
behavior for user changes.
- Around line 782-809: The disabled-state tooltip around the
“disable-ingest-with-langflow” Switch is not keyboard-accessible because
TooltipTrigger targets a non-focusable div. Update this TooltipTrigger wrapper,
and the corresponding tooltip block around the other affected Switch, to be
focusable and provide an accessible name when the tooltip is shown, while
preserving the existing disabled logic and tooltip condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fb8ebdc-c6ae-45b0-8c67-0ec1036a84db
📒 Files selected for processing (4)
frontend/app/settings/_components/ingest-settings-section.tsxfrontend/contexts/auth-context.tsxfrontend/lib/constants.tsfrontend/lib/url-utils.ts
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div> | ||
| <Switch | ||
| id="disable-ingest-with-langflow" | ||
| checked={disableIngestWithLangflow} | ||
| disabled={ | ||
| isSaas && | ||
| pictureDescriptions && | ||
| !disableIngestWithLangflow | ||
| } | ||
| onCheckedChange={(checked) => { | ||
| setDisableIngestWithLangflow(checked); | ||
| if (isSaas && checked) setPictureDescriptions(false); | ||
| }} | ||
| /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| {isSaas && | ||
| pictureDescriptions && | ||
| !disableIngestWithLangflow && ( | ||
| <TooltipContent> | ||
| <p>Unavailable while Picture Descriptions is on.</p> | ||
| </TooltipContent> | ||
| )} | ||
| </Tooltip> | ||
| </TooltipProvider> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce mutual exclusivity before saving.
If a legacy SaaS configuration loads with both settings enabled, both disabled conditions remain false, so the user can change an unrelated setting and submit both flags as true. Normalize the pair on load/save or block the save until one option is disabled.
Also applies to: 855-885
🤖 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 `@frontend/app/settings/_components/ingest-settings-section.tsx` around lines
782 - 809, The settings form currently allows legacy SaaS state with both
disableIngestWithLangflow and pictureDescriptions enabled to be submitted
unchanged. Update the load/save handling around these state values and the
related save controls near the Switch to normalize the pair or prevent
submission until one is disabled, while preserving the existing mutual-exclusion
behavior for user changes.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the disabled-state tooltip keyboard-accessible.
TooltipTrigger asChild targets a plain <div>, while the nested Switch is disabled. The explanation is therefore pointer-only; keyboard users cannot focus the trigger. Use a focusable wrapper with an accessible name when the tooltip is shown.
Also applies to: 855-885
🤖 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 `@frontend/app/settings/_components/ingest-settings-section.tsx` around lines
782 - 809, The disabled-state tooltip around the “disable-ingest-with-langflow”
Switch is not keyboard-accessible because TooltipTrigger targets a non-focusable
div. Update this TooltipTrigger wrapper, and the corresponding tooltip block
around the other affected Switch, to be focusable and provide an accessible name
when the tooltip is shown, while preserving the existing disabled logic and
tooltip condition.
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
2 participants
in SaaS disabling langflow ingestion does not work w/ picture descriptions this change clarifies that relationship for the users
Summary by CodeRabbit
New Features
Bug Fixes