fix: missing ingest settings for individual files in COS#2066
Conversation
Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly.
Changes (frontend-only, no backend changes needed — it already threads shared uniformly):
- frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path.
- frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
mpawlow
left a comment
There was a problem hiding this comment.
Code Review 1
- ✅ Approved / LGTM 🚀
- See non-blocking PR comment (1a) for consideration
Functional Review 1
- Not performed yet (I don't have a MinIO instance in HMAC mode setup locally)
- I can set this up tomorrow OR we can manually verify the fix (using two separate users) in a shared OpenRAG instance with IBM COS already setup)
| [allFiles, selectedFileIds], | ||
| ); | ||
|
|
||
| const handleIngest = useCallback(async () => { |
There was a problem hiding this comment.
(1a) [Normal] Individual-file ingest path skips chunk-settings client-side validation that the bucket-level path enforces
Problem
SharedBucketView.ingestSelected(the whole-bucket path,frontend/components/connectors/shared-bucket-view.tsx:107-111) validatesingestSettingsviagetIngestChunkSettingsErrorbefore submitting, blocking with a toast error ifchunkSize < 1orchunkOverlap >= chunkSize.FileBrowserDialog.handleIngest(frontend/components/file-browser-dialog.tsx:128-150), which now forwards the exact sameingestSettingsobject to the backend viasettings: ingestSettings, does not perform this same validation before callingsyncMutation.mutateAsync.
Background Information
- The
IngestSettingspanel (chunk size / chunk overlapNumberInputfields,frontend/components/cloud-picker/ingest-settings.tsx:206-226) has nomin/maxenforcement, so a user can freely setchunkOverlap >= chunkSizein the UI. - That same settings panel and its session state (
ingestSettings) are shared between both ingest entry points inSharedBucketView— a user can set an invalid chunk overlap, then click "Browse Files" (which opensFileBrowserDialog) instead of the whole-bucket "Ingest N Buckets" button, and submit without ever hittinggetIngestChunkSettingsError. - Backend search (
src/connectors/service.py,src/api/connectors.py) shows no server-side re-validation of thechunkSize/chunkOverlaprelationship, so an invalid submission would proceed to task creation and likely surface as a confusing downstream ingestion failure rather than the clear, immediate toast the bucket path provides. - This gap pre-dates the "shared" fix but is newly exposed by this PR, since before this change
FileBrowserDialogdidn't forwardsettingsat all — it had no reason to validate. Now that it forwards the same object, the two ingest paths have diverged in validation behavior.
Code References
frontend/components/file-browser-dialog.tsx:128-150(missing validation beforesyncMutation.mutateAsync)frontend/components/connectors/shared-bucket-view.tsx:107-111(existing validation pattern to mirror)frontend/components/cloud-picker/types.ts:119-129(getIngestChunkSettingsError)
Potential Solution
- In
handleIngest, callgetIngestChunkSettingsError(ingestSettings)(guarding foringestSettingsbeingundefined) beforesyncMutation.mutateAsync, and short-circuit withtoast.error("Could not start ingest", { description: chunkErr })on failure — identical toingestSelected's existing pattern.
Alternative Solutions
- Extract the validate-then-toast logic into a small shared helper (e.g.
validateIngestSettingsOrToast(settings)) used by bothingestSelectedandhandleIngest, to prevent this same divergence from recurring as more ingest entry points are added. This is the more idiomatic fix given there are now two call sites duplicating the same submit-guard pattern.
There was a problem hiding this comment.
@coderabbitai create an issue with the above suggestions. also a potential PR also.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="langflow-ai/openrag"
body=$(cat <<'EOF'
## Summary
`FileBrowserDialog.handleIngest` (in `frontend/components/file-browser-dialog.tsx`) now forwards the same `ingestSettings` object used by `SharedBucketView.ingestSelected` (`frontend/components/connectors/shared-bucket-view.tsx`) to the backend via `settings: ingestSettings`, but unlike the whole-bucket ingest path, it does not run the client-side chunk-settings validation (`getIngestChunkSettingsError`) before submitting.
## Problem
- `SharedBucketView.ingestSelected` validates `ingestSettings` via `getIngestChunkSettingsError` before submitting, blocking with a toast error if `chunkSize < 1` or `chunkOverlap >= chunkSize`.
- `FileBrowserDialog.handleIngest` performs no equivalent validation before calling `syncMutation.mutateAsync`.
- The `IngestSettings` panel (`frontend/components/cloud-picker/ingest-settings.tsx`) chunk size / chunk overlap `NumberInput` fields have no `min`/`max` enforcement, so a user can freely set an invalid `chunkOverlap >= chunkSize`.
- Since `ingestSettings` state is shared between both entry points in `SharedBucketView`, a user can set invalid values, then use "Browse Files" (opening `FileBrowserDialog`) instead of the whole-bucket "Ingest N Buckets" button, bypassing validation entirely.
- No server-side re-validation of the `chunkSize`/`chunkOverlap` relationship was found in `src/connectors/service.py` or `src/api/connectors.py`, so an invalid submission proceeds to task creation and likely surfaces as a confusing downstream ingestion failure instead of an immediate, clear toast.
## Affected areas
- `frontend/components/file-browser-dialog.tsx` (`handleIngest`, missing validation)
- `frontend/components/connectors/shared-bucket-view.tsx` (`ingestSelected`, existing validation pattern to mirror)
- `frontend/components/cloud-picker/types.ts` (`getIngestChunkSettingsError`)
## Suggested fix
- In `handleIngest`, call `getIngestChunkSettingsError(ingestSettings)` (guarding for `ingestSettings` being `undefined`) before `syncMutation.mutateAsync`, and short-circuit with a `toast.error(...)` on failure — mirroring `ingestSelected`'s existing guard pattern.
- Preferably, extract the validate-then-toast logic into a small shared helper (e.g. `validateIngestSettingsOrToast(settings)`) used by both `ingestSelected` and `handleIngest`, to prevent this divergence from recurring as more ingest entry points are added.
## Acceptance criteria
- [ ] `FileBrowserDialog.handleIngest` rejects invalid chunk settings (`chunkSize < 1` or `chunkOverlap >= chunkSize`) with a toast error, without calling `syncMutation.mutateAsync`.
- [ ] Both ingest entry points (`SharedBucketView.ingestSelected` and `FileBrowserDialog.handleIngest`) use consistent validation logic, ideally via a shared helper.
## References
- PR: https://github.com/langflow-ai/openrag/pull/2066
- Review comment: https://github.com/langflow-ai/openrag/pull/2066#discussion_r3555097191
- Reported by: `@mpawlow`
- Requested by: `@edwinjosechittilappilly`
EOF
)
gh issue create \
--repo "$repo" \
--title "Individual-file ingest path skips chunk-settings client-side validation enforced by the bucket-level path" \
--body "$body" \
--assignee "edwinjosechittilappilly"Length of output: 3485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' frontend/components/connectors/shared-bucket-view.tsx
echo "-----TYPES-----"
sed -n '1,160p' frontend/components/cloud-picker/types.tsLength of output: 13437
@edwinjosechittilappilly Done — I've opened the issue with the full write-up: #2070
Now here's the plan for a potential fix PR:
Task: Add chunk-settings validation to FileBrowserDialog.handleIngest
- Import
getIngestChunkSettingsErroralongside the existingIngestSettingstype import infrontend/components/file-browser-dialog.tsx. - Guard
handleIngestwith the same validate-then-toast pattern used inSharedBucketView.ingestSelected, short-circuiting beforesyncMutation.mutateAsyncwheningestSettingsis invalid (skip validation only wheningestSettingsitself is not provided, since it's optional on this dialog).
This keeps validation logic duplicated between the two call sites for now (matching the existing pattern exactly), which is the minimal fix. If you'd prefer the more idiomatic route — extracting a shared validateIngestSettingsOrToast(settings) helper used by both ingestSelected and handleIngest — let me know and I can adjust the plan accordingly.
🧬 Code Graph Analysis Results
frontend/components/file-browser-dialog.tsx
- (Context for the reviewer) Key behaviors implemented in this file:
- Fetches remote file listing via
useBrowseConnectionFileswhenopenis true (see lines 58-66). - Filters results client-side by
search(lines 70-76) and determines selectable rows based on!is_ingested || is_stale(lines 77-81). - Sends ingest request via
syncMutation.mutateAsync(...), including conditionalreplace_duplicateswhen any selected file is stale (lines 123-163). - Disables selection for already-ingested, non-stale files (
locked = file.is_ingested && !file.is_stale) (lines 303-305, 312-318).
- Fetches remote file listing via
frontend/components/ui/dialog.tsx
Relevant for understanding the modal structure used by FileBrowserDialog.
- DialogHeader (lines 56-67):
DialogHeader = ({
className,
...props
}: React.HTMLAttributes<HTMLDivElement>) => (
<div
className={cn(
"flex flex-col space-y-1.5 text-center sm:text-left",
className,
)}
{...props}
/>
)- DialogFooter (lines 70-81):
DialogFooter = ({
className,
...props
}: React.HTMLAttributes<HTMLDivElement>) => (
<div
className={cn(
"flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2",
className,
)}
{...props}
/>
)frontend/components/ui/badge.tsx
Relevant because this file uses Badge variants to label ingest status.
- Badge component (lines 30-34):
function Badge({ className, variant, ...props }: BadgeProps) {
return (
<div className={cn(badgeVariants({ variant }), className)} {...props} />
);
}frontend/app/api/queries/useBrowseConnectionFiles.ts
Relevant for understanding the remote file data shape (RemoteFile) and the fetch/error behavior.
RemoteFileinterface (lines 7-19):
interface RemoteFile {
id: string;
name: string;
bucket: string;
key: string;
size: number;
modified_time: string;
is_ingested: boolean;
/** Ingested, but the source version is newer than what we indexed. Such files
* stay selectable (to re-ingest the newer version) while unchanged ingested
* files are disabled. */
is_stale: boolean;
}useBrowseConnectionFileshook (lines 48-97) — summary:
/**
* useBrowseConnectionFiles(params, options?)
* - Parameters:
* - params: { connectorType, connectionId, bucket?, search?, pageToken?, maxFiles? }
* - options: react-query UseQueryOptions excluding queryKey/queryFn.
* - Returns: react-query query result for BrowseConnectionFilesResponse.
* - Fetch behavior:
* - Builds URLSearchParams for bucket/search/page_token/max_files.
* - GETs:
* /api/connectors/${connectorType}/${connectionId}/browse?...
* - Error handling:
* - If response is not ok, tries response.json() and throws:
* new Error(errorData.error || `Failed to browse files: ${response.status}`)
* - Important implementation details:
* - queryKey: ["browseConnectionFiles", params]
* - retry: false
* - enabled: Boolean(connectorType && connectionId) (overridable by caller via options)
* - staleTime: 0
* - refetchOnMount: "always"
* - Passes through ...options
*/frontend/app/api/mutations/useSyncConnector.ts
Relevant because the ingest action in FileBrowserDialog calls useSyncConnector().mutateAsync(...).
- useSyncConnector hook (lines 112-122):
export const useSyncConnector = () => {
const queryClient = useQueryClient();
return useMutation({
mutationFn: syncConnector,
onSettled: () => {
// Immediately refetch tasks so new sync jobs appear in the task list
queryClient.invalidateQueries({ queryKey: ["tasks"], exact: false });
},
});
};frontend/components/cloud-picker/types.ts
Relevant because FileBrowserDialog accepts ingestSettings?: IngestSettings and forwards settings/shared to the ingest mutation.
- IngestSettings interface (lines 110-118):
export interface IngestSettings {
chunkSize: number;
chunkOverlap: number;
ocr: boolean;
pictureDescriptions: boolean;
embeddingModel: string;
/** When true, index without an owner so all users in the instance can retrieve the document. COS only. */
shared?: boolean;
}frontend/lib/file-format.ts
Relevant because file sizes are displayed in the dialog.
- formatFileSize (lines 13-21):
export function formatFileSize(bytes?: number | null): string {
if (!bytes || bytes < 0) return "0 B";
const units = ["B", "KB", "MB", "GB", "TB"];
const i = Math.min(
Math.floor(Math.log(bytes) / Math.log(1024)),
units.length - 1,
);
return `${(bytes / 1024 ** i).toFixed(i === 0 ? 0 : 1)} ${units[i]}`;
}✏️ Learnings added
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/openrag PR: 2066
File: frontend/components/file-browser-dialog.tsx:128-128
Timestamp: 2026-07-09T22:54:09.716Z
Learning: In frontend/components/file-browser-dialog.tsx, `FileBrowserDialog.handleIngest` forwards `ingestSettings` to the backend but (as of PR `#2066`) does not validate chunk settings via `getIngestChunkSettingsError` before submitting, unlike `SharedBucketView.ingestSelected` in frontend/components/connectors/shared-bucket-view.tsx which does perform this validation. Tracked as follow-up in https://github.com/langflow-ai/openrag/issues/2070.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Failed to handle agent chat message. Please try again.
29783ab
into
release-saas-ga-0.6.2
…#2095) * feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) * fix: cos share all (#2050) * fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * missing ingest settings for individual files (#2066) Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly. Changes (frontend-only, no backend changes needed — it already threads shared uniformly): - frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path. - frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…ors (backport of #2077) (#2096) * feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) * fix: cos share all (#2050) * fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * missing ingest settings for individual files (#2066) Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly. Changes (frontend-only, no backend changes needed — it already threads shared uniformly): - frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path. - frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog. * fix: introduces a user-facing duplicate handling workflow for bucket-based connectors (#2077) * fix japanese file names Root cause: COS/Azure/S3 connectors build document.id as the raw, non-ASCII object key ("bucket::報告書.pdf"), and that value was reused verbatim as document_id in the default Langflow ingestion path — the only ingestion path that puts non-ASCII text into ASCII-only HTTP headers and an unbounded-length OpenSearch identifier. Google Drive (opaque ASCII ID) and manual upload (content hash) were structurally immune; COS/Azure/S3 were the only paths exposed. Fix applied (matches the already-proven split used by the non-Langflow ingestion path): - document_index_writer.py — DocumentIndexContext gained a connector_file_id field, now written onto every indexed chunk when present. - langflow_file_service.py — _resolve_document_id now hashes any supplied connector_file_id into a stable ASCII document_id, instead of using it verbatim; connector_file_id threads through run_ingestion_flow → _configure_ingest_callback → the JWT context. - processors.py — the Langflow-path connector branch now passes connector_file_id=document.id (not document_id=document.id), and the post-ingest existence check / metadata sync now match on connector_file_id. - connectors/service.py — stale comments corrected (dedupe/ACL-sync already matched both fields, no logic change needed there). - Added 13 new unit tests (test_resolve_document_id_connector_file_id.py, test_document_index_writer_connector_file_id.py, test_bucket_connector_unicode_file_ids.py) — confirmed they fail on the old code and pass on the fix. - Ran the full unit suite: same 14 pre-existing failures before and after (unrelated to this change — auth/settings/encryption tests), no new regressions. * style: ruff autofix (auto) * fix connector File name issue * ingest fix * Update processors.py * review fix * fix connector dialog on rewrite * overwrite dialog box * Update connectors.py --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly.
Changes (frontend-only, no backend changes needed — it already threads shared uniformly):