Add scheduled Ingestion Service#95
Open
niedch wants to merge 9 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 9 minutes and 31 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds end-to-end RAG ingestion sources: types and REST client, React Query hooks with cache invalidation, editor and panel UI, integration into the RAG editor, MSW test handlers and test setup, extension slug/icon tweaks, and localization entries across 11 locales. ChangesRAG Ingestion Sources Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@src/components/editors/ingestion-source-editor.tsx`:
- Around line 195-212: The form save validation currently only enforces name and
ragConfigUri but misses validating the required web Start URL for web sources;
update the save/validation logic that checks name and ragConfigUri to also check
when type === "web" that webConfig?.startUrl is non-empty (and not just
whitespace) and block submission with an appropriate error state if missing; use
the existing webConfig?.startUrl field referenced in the input and ensure the
same validation is applied before constructing the payload/submitting in the
save handler.
- Around line 625-633: The handler handleSave currently calls
updateMutation.mutate or createMutation.mutate and then immediately calls
setEditing(null) and setIsAdding(false), which closes the editor even if the
mutation fails; change this so the editor is closed only on successful mutation
by moving those state updates into the mutation success path (use the onSuccess
callback or pass an options object to
updateMutation.mutate/createMutation.mutate) and keep the editor open on failure
(optionally handle errors via onError), referencing handleSave, updateMutation,
createMutation, setEditing, and setIsAdding.
In `@src/components/editors/rag-editor.tsx`:
- Around line 1013-1016: The Section component's label prop is hardcoded
("Ingestion Sources"); change it to use the i18n helper with an inline fallback
so the title is localized (e.g., replace the label value on the Section instance
with a call to t("editors.rag.ingestionSources", "Ingestion Sources")). Update
the Section usage at the same location (the Section element with icon={Globe}
and accent="text-sky-500") to pass the t() result for label, ensuring the file's
existing translation function (t) is used or imported if missing.
In `@src/i18n/locales/ar.json`:
- Around line 1131-1133: Translate the three untranslated keys —
"ingestionSources", "triggerSuccess", and "triggerError" — in the Arabic locale
so the UI is fully localized: replace their English values with appropriate
Arabic strings while keeping the keys unchanged (e.g., update the value for
ingestionSources, update triggerSuccess to an Arabic equivalent of "Ingestion
triggered", and update triggerError to an Arabic equivalent of "Failed to
trigger ingestion").
In `@src/i18n/locales/de.json`:
- Around line 1131-1133: The three locale entries "ingestionSources",
"triggerSuccess", and "triggerError" are still in English; update their German
translations in the de.json locale so the UI/toasts are fully localized —
replace "Ingestion Sources" with an appropriate German string (e.g.,
"Ingestionsquellen"), "Ingestion triggered" with a German message (e.g.,
"Ingestion ausgelöst"), and "Failed to trigger ingestion" with a German error
string (e.g., "Fehler beim Auslösen der Ingestion").
In `@src/i18n/locales/es.json`:
- Around line 1131-1133: Update the Spanish locale entries for the three
ingestion toast keys: replace "ingestionSources", "triggerSuccess", and
"triggerError" values with proper Spanish translations so users see consistent
language; specifically, set ingestionSources to "Fuentes de ingestión",
triggerSuccess to "Ingestión iniciada" (or "Ingesta iniciada"), and triggerError
to "Error al iniciar la ingesta" by editing the corresponding entries in the
es.json locale object (keys: ingestionSources, triggerSuccess, triggerError).
In `@src/i18n/locales/fr.json`:
- Around line 1130-1133: Update the French translations for the RAG ingestion
keys: change "addParam" to a proper French phrase like "Ajouter un paramètre",
translate "ingestionSources" to "Sources d'ingestion", change "triggerSuccess"
to "Ingestion déclenchée" (or similar concise success toast) and change
"triggerError" to "Échec du déclenchement de l'ingestion" (or "Échec lors du
déclenchement de l'ingestion") so all four keys (addParam, ingestionSources,
triggerSuccess, triggerError) contain correct French text.
In `@src/i18n/locales/hi.json`:
- Around line 1131-1133: Translate the three English values in the Hindi locale
JSON for the keys "ingestionSources", "triggerSuccess", and "triggerError" to
Hindi; update the value for ingestionSources to a Hindi phrase for "Ingestion
Sources" and similarly replace "Ingestion triggered" and "Failed to trigger
ingestion" with appropriate Hindi translations so the UI feedback is localized
for the hi locale.
In `@src/i18n/locales/ja.json`:
- Around line 1131-1133: The three new i18n keys ingestionSources,
triggerSuccess, and triggerError in the ja.json locale are still English; update
their values to proper Japanese translations (e.g. translate "Ingestion
Sources", "Ingestion triggered", and "Failed to trigger ingestion") while
preserving the JSON key names and punctuation in the file so the ingestion
section and trigger toasts display consistently in Japanese.
In `@src/i18n/locales/ko.json`:
- Around line 1131-1133: Replace the English values for the RAG ingestion keys
with Korean translations: update "ingestionSources" to "데이터 수집 소스",
"triggerSuccess" to "데이터 수집이 시작되었습니다", and "triggerError" to "데이터 수집 시작에 실패했습니다"
so the UI toasts/labels (ragEditor.triggerSuccess / ragEditor.triggerError) are
fully localized.
In `@src/i18n/locales/pt.json`:
- Around line 1131-1133: The Portuguese locale entries for the ingestion UI are
still in English; update the keys ingestionSources, triggerSuccess, and
triggerError in the pt.json locale to their Portuguese translations (e.g.,
"Ingestion Sources" → appropriate Portuguese label, "Ingestion triggered" →
Portuguese success message, and "Failed to trigger ingestion" → Portuguese error
message) so the visible UI/toast texts are localized; locate the three keys
ingestionSources, triggerSuccess, triggerError in the pt.json and replace their
English values with correct Portuguese strings.
In `@src/i18n/locales/th.json`:
- Around line 1131-1133: The three keys ingestionSources, triggerSuccess, and
triggerError in the th locale are still in English; update their string values
in src/i18n/locales/th.json to proper Thai translations (replace "Ingestion
Sources" → a Thai phrase for ingestion sources, "Ingestion triggered" → Thai for
ingestion triggered, and "Failed to trigger ingestion" → Thai for failed to
trigger ingestion) so the RAG ingestion UI/toasts show Thai; modify the values
for the keys ingestionSources, triggerSuccess, and triggerError accordingly
(ensure escaped/UTF-8-safe characters and keep JSON formatting).
In `@src/i18n/locales/zh.json`:
- Around line 1131-1133: The three i18n entries ingestionSources,
triggerSuccess, and triggerError in zh.json are still English; replace their
values with appropriate Chinese translations (e.g., "Ingestion Sources" ->
"摄取来源" or "导入来源", "Ingestion triggered" -> "已触发摄取", "Failed to trigger
ingestion" -> "触发摄取失败") by updating the values for the keys ingestionSources,
triggerSuccess, and triggerError in the JSON file so the user-facing ingestion
section and toast messages are localized.
In `@src/pages/__tests__/resource-detail-rag.test.tsx`:
- Around line 312-325: The test helper expandIngestionSources uses textContent
and role queries to find the ingestion toggle; change it to query by data-testid
attributes instead: locate the toggle using
screen.getByTestId("ingestion-sources-toggle") (or the testid used for the
toggle) rather than filtering sectionButtons by textContent, click that element,
and replace subsequent assertions that rely on class names or text with
screen.getByTestId("ingestion-sources-panel") and testids for source identity
and selected-state (e.g., "ingestion-source-<id>" and
"ingestion-source-<id>-selected") so the helper (expandIngestionSources) and the
other affected blocks (around lines 332-339 and 370-377) assert on stable
data-testid attributes instead of rendered text/CSS.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b13e3198-4b1f-468a-814f-9238d7357681
📒 Files selected for processing (22)
src/components/editors/add-extension-dialog.tsxsrc/components/editors/ingestion-source-editor.tsxsrc/components/editors/rag-editor.tsxsrc/hooks/use-extensions-store.tssrc/hooks/use-ingestion-sources.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/api/extensions.tssrc/lib/api/ingestion-sources.tssrc/pages/__tests__/resource-detail-rag.test.tsxsrc/pages/workflow-detail.tsxsrc/test/mocks/handlers.tssrc/test/setup.ts
Comment on lines
+195
to
+212
| {isWeb && ( | ||
| <Section label="Web Source Configuration" icon={GlobeLock} accent="text-emerald-500"> | ||
| <div className="space-y-3"> | ||
| <div> | ||
| <label className="mb-1 block text-[10px] font-semibold uppercase tracking-wider text-muted-foreground"> | ||
| Start URL <span className="text-destructive">*</span> | ||
| </label> | ||
| <input | ||
| type="url" | ||
| value={webConfig?.startUrl ?? ""} | ||
| onChange={(e) => | ||
| update({ sourceConfig: { ...webConfig, startUrl: e.target.value } as WebSourceConfig }) | ||
| } | ||
| readOnly={readOnly} | ||
| placeholder="https://docs.example.com" | ||
| className="h-8 w-full rounded-md border border-input bg-background px-3 text-sm text-foreground focus:outline-none focus:ring-1 focus:ring-ring" | ||
| data-testid="source-start-url" | ||
| /> |
There was a problem hiding this comment.
Block save when Web Start URL is empty.
Line 533 only validates name and ragConfigUri, but for type === "web" the Start URL is required in the form and model. This allows invalid payload submission.
Suggested fix
+ const canSave =
+ !!source.name.trim() &&
+ !!ragConfigUri &&
+ (!isWeb || !!webConfig?.startUrl?.trim());
...
- disabled={!source.name.trim() || !ragConfigUri}
+ disabled={!canSave}Also applies to: 533-535
🤖 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 `@src/components/editors/ingestion-source-editor.tsx` around lines 195 - 212,
The form save validation currently only enforces name and ragConfigUri but
misses validating the required web Start URL for web sources; update the
save/validation logic that checks name and ragConfigUri to also check when type
=== "web" that webConfig?.startUrl is non-empty (and not just whitespace) and
block submission with an appropriate error state if missing; use the existing
webConfig?.startUrl field referenced in the input and ensure the same validation
is applied before constructing the payload/submitting in the save handler.
Comment on lines
+1013
to
+1016
| <Section | ||
| label="Ingestion Sources" | ||
| icon={Globe} | ||
| accent="text-sky-500" |
There was a problem hiding this comment.
Localize the new section title via t() with fallback.
Line 1014 uses a hardcoded label, so this new UI text won’t follow locale selection.
- label="Ingestion Sources"
+ label={t("ragEditor.ingestionSources", "Ingestion Sources")}As per coding guidelines, use inline fallbacks in i18n calls: t("key", "Fallback").
📝 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.
Suggested change
| <Section | |
| label="Ingestion Sources" | |
| icon={Globe} | |
| accent="text-sky-500" | |
| <Section | |
| label={t("ragEditor.ingestionSources", "Ingestion Sources")} | |
| icon={Globe} | |
| accent="text-sky-500" |
🤖 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 `@src/components/editors/rag-editor.tsx` around lines 1013 - 1016, The Section
component's label prop is hardcoded ("Ingestion Sources"); change it to use the
i18n helper with an inline fallback so the title is localized (e.g., replace the
label value on the Section instance with a call to
t("editors.rag.ingestionSources", "Ingestion Sources")). Update the Section
usage at the same location (the Section element with icon={Globe} and
accent="text-sky-500") to pass the t() result for label, ensuring the file's
existing translation function (t) is used or imported if missing.
Source: Coding guidelines
There was a problem hiding this comment.
Pull request overview
This PR adds “Ingestion Sources” management for RAG knowledge bases, including API + React Query hooks, an editor/panel UI integrated into the existing RAG editor, and supporting test/MSW/i18n updates.
Changes:
- Added
ingestion-sourcesAPI module + React Query hooks for CRUD + trigger operations. - Integrated a new Ingestion Sources section into the RAG editor, with a full-featured source editor/panel UI.
- Extended MSW handlers, unit tests, and locale files to support the new ingestion-sources functionality.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/setup.ts | Adds a localStorage mock for the test environment. |
| src/test/mocks/handlers.ts | Adds MSW handlers + mock data for ingestion-sources endpoints. |
| src/pages/workflow-detail.tsx | Normalizes extension type to include eddi:// prefix when adding steps. |
| src/pages/tests/resource-detail-rag.test.tsx | Adds tests for the Ingestion Sources section and editor UI. |
| src/lib/api/ingestion-sources.ts | New API client module and types for ingestion-sources CRUD + trigger. |
| src/lib/api/extensions.ts | Updates RAG extension label/icon mapping (“Knowledge Bases”). |
| src/i18n/locales/ar.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/de.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/en.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/es.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/fr.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/hi.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/ja.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/ko.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/pt.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/th.json | Adds new ragEditor ingestion-source related keys. |
| src/i18n/locales/zh.json | Adds new ragEditor ingestion-source related keys. |
| src/hooks/use-ingestion-sources.ts | New React Query hooks for ingestion-sources operations. |
| src/hooks/use-extensions-store.ts | Minor formatting change in well-known types definition. |
| src/components/editors/rag-editor.tsx | Adds an “Ingestion Sources” section to the RAG editor. |
| src/components/editors/ingestion-source-editor.tsx | New ingestion source editor + list/trigger/delete panel UI. |
| src/components/editors/add-extension-dialog.tsx | Adds rag to the URI host mapping for new resource creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+46
| value: { | ||
| getItem: (key: string) => store.get(key) ?? null, | ||
| setItem: (key: string, value: string) => store.set(key, value), | ||
| removeItem: (key: string) => store.delete(key), | ||
| clear: () => store.clear(), | ||
| get length() { return store.size; }, | ||
| key: (i: number) => [...store.keys()][i] ?? null, | ||
| }, |
Comment on lines
+1013
to
+1016
| <Section | ||
| label="Ingestion Sources" | ||
| icon={Globe} | ||
| accent="text-sky-500" |
Comment on lines
+35
to
+40
| const EMPTY_SOURCE: RagIngestionSource = { | ||
| name: "", | ||
| type: "web", | ||
| sourceConfig: { startUrl: "" }, | ||
| ragConfigUri: "", | ||
| }; |
Comment on lines
+530
to
+534
| <button | ||
| type="button" | ||
| onClick={() => onSave(source)} | ||
| disabled={!source.name.trim() || !ragConfigUri} | ||
| className="rounded-lg bg-primary px-3 py-1.5 text-xs font-medium text-primary-foreground hover:bg-primary/90 transition-colors disabled:opacity-50" |
Comment on lines
+644
to
+657
| return ( | ||
| <div className="space-y-3" data-testid="ingestion-sources-panel"> | ||
| {isLoading && ( | ||
| <div className="flex items-center gap-2 text-xs text-muted-foreground"> | ||
| <Loader2 className="h-3 w-3 animate-spin" /> | ||
| Loading ingestion sources... | ||
| </div> | ||
| )} | ||
|
|
||
| {!isLoading && sources && sources.length === 0 && !isAdding && ( | ||
| <p className="text-xs text-muted-foreground italic" data-testid="no-sources-msg"> | ||
| No ingestion sources configured. Add one to automatically crawl content into this knowledge base. | ||
| </p> | ||
| )} |
Comment on lines
+1131
to
+1133
| "ingestionSources": "Ingestion Sources", | ||
| "triggerSuccess": "Ingestion triggered", | ||
| "triggerError": "Failed to trigger ingestion" |
Comment on lines
+1131
to
+1133
| "ingestionSources": "Ingestion Sources", | ||
| "triggerSuccess": "Ingestion triggered", | ||
| "triggerError": "Failed to trigger ingestion" |
Comment on lines
+1131
to
+1133
| "ingestionSources": "Ingestion Sources", | ||
| "triggerSuccess": "Ingestion triggered", | ||
| "triggerError": "Failed to trigger ingestion" |
Comment on lines
+1131
to
+1133
| "ingestionSources": "Ingestion Sources", | ||
| "triggerSuccess": "Ingestion triggered", | ||
| "triggerError": "Failed to trigger ingestion" |
Comment on lines
+1131
to
+1133
| "ingestionSources": "Ingestion Sources", | ||
| "triggerSuccess": "Ingestion triggered", | ||
| "triggerError": "Failed to trigger ingestion" |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/pages/__tests__/resource-detail-rag.test.tsx (1)
319-322:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
data-testidselectors/assertions throughout ingestion tests.This suite still relies on section header text (
textContent), toast text (getByText), and styling (className.includes("ring")), which makes tests fragile across locale and style changes. Switch these to stabledata-testidtargets.As per coding guidelines, "
**/*.test.{ts,tsx}: Assert ondata-testidattributes in unit tests instead of relying on rendered text or DOM structure."Also applies to: 341-341, 379-380, 461-464, 486-487
🤖 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 `@src/pages/__tests__/resource-detail-rag.test.tsx` around lines 319 - 322, The tests in resource-detail-rag.test.tsx are selecting elements by visible text and DOM structure (e.g., the sectionButtons/getAllByRole + sourcesBtn that uses textContent) which is fragile; update the tests to use stable data-testid selectors instead: replace getAllByRole/find(textContent) patterns with getByTestId or getAllByTestId (e.g., target the ingestion sources button via a new data-testid like "ingestion-sources-btn") and change other assertions that rely on getByText or className checks to use data-testid based queries and dedicated testids (for the toast, section headers, and ring state). Ensure the test file queries use the new testids (and coordinate adding those data-testid attributes in the corresponding components) and update the other occurrences mentioned (around the symbols used at lines ~341, ~379, ~461, ~486) to the same pattern.Source: Coding guidelines
src/i18n/locales/ja.json (1)
1133-1141:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUntranslated ingestion feedback keys across
src/i18n/locales/ja.json,src/i18n/locales/ko.json,src/i18n/locales/th.json, andsrc/i18n/locales/zh.json.All four locale files still ship English values for newly added
ragEditoringestion status/error keys, causing mixed-language user-facing toasts/messages. Localize the same key set consistently in each file.🤖 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 `@src/i18n/locales/ja.json` around lines 1133 - 1141, The locale files contain untranslated English values for the new ragEditor ingestion keys (e.g., "triggerError", "sourceCreated", "sourceUpdated", "sourceDeleted", "sourceCreateError", "sourceUpdateError", "sourceDeleteError", "ingestUploadError", "ingestReadError"); update each target locale entry with proper translations so the same key set across all locales uses localized strings, keeping keys identical, preserving JSON syntax/escaping, and verifying UTF-8 encoding and punctuation consistency.
🧹 Nitpick comments (1)
src/pages/__tests__/resource-detail-rag.test.tsx (1)
420-443: ⚡ Quick winReuse
renderPage(type)instead of introducing a second render helper.
renderPageWithToasterduplicates the same router/query/theme setup. Please extendrenderPage(e.g., optionalwithToaster) and keep a single canonical test render helper.As per coding guidelines, "
**/*.test.{ts,tsx}: UserenderPage(type)helper withMemoryRouter,QueryClient, andThemeProviderin unit tests."🤖 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 `@src/pages/__tests__/resource-detail-rag.test.tsx` around lines 420 - 443, The test adds a duplicate render helper renderPageWithToaster; update the existing renderPage to accept an optional flag/parameter (e.g., withToaster) or extra options so a single helper covers both cases, then remove renderPageWithToaster and update callers to use renderPage(type, { withToaster: true }) (or equivalent). Ensure the consolidated renderPage still sets up MemoryRouter (route /manage/resources/:type/:id), QueryClientProvider, ThemeProvider, and when withToaster is true includes <Toaster /> so ResourceDetailPage tests behave the same.Source: Coding guidelines
🤖 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 `@src/i18n/locales/fr.json`:
- Around line 1131-1141: The French locale file contains untranslated
ingestion-related keys (e.g., "ingestionSources", "triggerSuccess",
"triggerError", "sourceCreated", "sourceUpdated", "sourceDeleted",
"sourceCreateError", "sourceUpdateError", "sourceDeleteError",
"ingestUploadError", "ingestReadError" and any ragEditor ingestion messages) —
update src/i18n/locales/fr.json (and mirror the same work in
src/i18n/locales/hi.json) by replacing the English strings with proper French
(and Hindi) translations for each key so the UI/toasts are fully localized and
there are no mixed-language messages. Ensure you keep the exact key names
unchanged and only modify the string values.
---
Duplicate comments:
In `@src/i18n/locales/ja.json`:
- Around line 1133-1141: The locale files contain untranslated English values
for the new ragEditor ingestion keys (e.g., "triggerError", "sourceCreated",
"sourceUpdated", "sourceDeleted", "sourceCreateError", "sourceUpdateError",
"sourceDeleteError", "ingestUploadError", "ingestReadError"); update each target
locale entry with proper translations so the same key set across all locales
uses localized strings, keeping keys identical, preserving JSON syntax/escaping,
and verifying UTF-8 encoding and punctuation consistency.
In `@src/pages/__tests__/resource-detail-rag.test.tsx`:
- Around line 319-322: The tests in resource-detail-rag.test.tsx are selecting
elements by visible text and DOM structure (e.g., the
sectionButtons/getAllByRole + sourcesBtn that uses textContent) which is
fragile; update the tests to use stable data-testid selectors instead: replace
getAllByRole/find(textContent) patterns with getByTestId or getAllByTestId
(e.g., target the ingestion sources button via a new data-testid like
"ingestion-sources-btn") and change other assertions that rely on getByText or
className checks to use data-testid based queries and dedicated testids (for the
toast, section headers, and ring state). Ensure the test file queries use the
new testids (and coordinate adding those data-testid attributes in the
corresponding components) and update the other occurrences mentioned (around the
symbols used at lines ~341, ~379, ~461, ~486) to the same pattern.
---
Nitpick comments:
In `@src/pages/__tests__/resource-detail-rag.test.tsx`:
- Around line 420-443: The test adds a duplicate render helper
renderPageWithToaster; update the existing renderPage to accept an optional
flag/parameter (e.g., withToaster) or extra options so a single helper covers
both cases, then remove renderPageWithToaster and update callers to use
renderPage(type, { withToaster: true }) (or equivalent). Ensure the consolidated
renderPage still sets up MemoryRouter (route /manage/resources/:type/:id),
QueryClientProvider, ThemeProvider, and when withToaster is true includes
<Toaster /> so ResourceDetailPage tests behave the same.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d73a2ea-a02d-423f-9390-04478573c3b2
📒 Files selected for processing (14)
src/components/editors/ingestion-source-editor.tsxsrc/components/editors/rag-editor.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/pages/__tests__/resource-detail-rag.test.tsx
✅ Files skipped from review due to trivial changes (3)
- src/i18n/locales/pt.json
- src/i18n/locales/de.json
- src/i18n/locales/es.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/i18n/locales/ar.json
- src/components/editors/rag-editor.tsx
- src/components/editors/ingestion-source-editor.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/editors/ingestion-source-editor.tsx (1)
583-583:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing validation: Save button doesn't block empty Start URL for web sources.
Line 583 validates
nameandragConfigUri, but whensource.type === "web", thestartUrlfield is required per the API contract (WebSourceConfig.startUrlis a non-optional string). Submitting a web source with an empty Start URL will produce an invalid payload.🛡️ Proposed fix: add startUrl check to disabled condition
+ const isWeb = source.type === "web"; + const webConfig = source.sourceConfig as WebSourceConfig | undefined; <button type="button" onClick={() => onSave(source)} - disabled={!source.name.trim() || !ragConfigUri || isSaving} + disabled={!source.name.trim() || !ragConfigUri || (isWeb && !webConfig?.startUrl?.trim()) || isSaving} className="rounded-lg bg-primary px-3 py-1.5 text-xs font-medium text-primary-foreground hover:bg-primary/90 transition-colors disabled:opacity-50" data-testid="source-save-btn" >🤖 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 `@src/components/editors/ingestion-source-editor.tsx` at line 583, The disabled condition in the ingestion-source-editor component at line 583 does not validate the required startUrl field for web sources. When source.type is "web", the startUrl field must be non-empty per the API contract. Add an additional check to the disabled condition that also validates startUrl is present and non-empty when the source type is "web". This ensures the Save button remains disabled until all required fields (including startUrl for web sources) are properly filled.
🧹 Nitpick comments (1)
src/components/editors/ingestion-source-editor.tsx (1)
384-384: ⚡ Quick winInconsistent fallback:
requestDelayMsinput defaults to 500 vs. constant 100.Line 384 falls back to
500, butDEFAULT_CRAWL_SETTINGS.requestDelayMs(line 43) is100. IfmergeDefaultshas run, this fallback shouldn't trigger; if it hasn't, the mismatch is confusing.♻️ Align fallback with declared default
- value={crawlSettings?.requestDelayMs ?? 500} + value={crawlSettings?.requestDelayMs ?? DEFAULT_CRAWL_SETTINGS.requestDelayMs}🤖 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 `@src/components/editors/ingestion-source-editor.tsx` at line 384, The fallback value for requestDelayMs in the input field defaults to 500, but the declared constant DEFAULT_CRAWL_SETTINGS.requestDelayMs is set to 100. These inconsistent values create confusion about the intended default behavior. Change the fallback value on line 384 from 500 to 100 to align with the declared default constant, ensuring consistency across the codebase.
🤖 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 `@src/components/editors/ingestion-source-editor.tsx`:
- Around line 218-225: In the else block at line 224 of
ingestion-source-editor.tsx, the sourceConfig is being set to { startUrl: "" }
for non-web source types (file, git, api), but startUrl is only valid for
WebSourceConfig. Replace { startUrl: "" } with an empty object {} in the else
branch of the update call to fix the schema mismatch. When file, git, and api
source types are enabled, replace this empty object with their respective
type-specific default configurations to match the API contract.
- Around line 61-72: The `mergeDefaults` function unconditionally merges
web-specific fields (`scope` and `crawlSettings`) into all source types and then
force-casts the result to `WebSourceConfig`, which violates the type contract
for non-web sources like file, git, and api types. Fix this by adding a type
guard to check if the source is a web source before merging those web-specific
fields, remove the type assertion on line 68, and ensure the function returns
properly typed results for all source types in the union.
---
Duplicate comments:
In `@src/components/editors/ingestion-source-editor.tsx`:
- Line 583: The disabled condition in the ingestion-source-editor component at
line 583 does not validate the required startUrl field for web sources. When
source.type is "web", the startUrl field must be non-empty per the API contract.
Add an additional check to the disabled condition that also validates startUrl
is present and non-empty when the source type is "web". This ensures the Save
button remains disabled until all required fields (including startUrl for web
sources) are properly filled.
---
Nitpick comments:
In `@src/components/editors/ingestion-source-editor.tsx`:
- Line 384: The fallback value for requestDelayMs in the input field defaults to
500, but the declared constant DEFAULT_CRAWL_SETTINGS.requestDelayMs is set to
100. These inconsistent values create confusion about the intended default
behavior. Change the fallback value on line 384 from 500 to 100 to align with
the declared default constant, ensuring consistency across the codebase.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 037ff1b4-a3cf-4e3b-901b-06c9f07c9ee3
📒 Files selected for processing (1)
src/components/editors/ingestion-source-editor.tsx
Comment on lines
+218
to
+225
| if (st.value === "web") { | ||
| update({ | ||
| type: "web", | ||
| sourceConfig: { startUrl: "", scope: DEFAULT_SCOPE, crawlSettings: DEFAULT_CRAWL_SETTINGS }, | ||
| }); | ||
| } else { | ||
| update({ type: st.value, sourceConfig: { startUrl: "" } }); | ||
| } |
There was a problem hiding this comment.
Schema mismatch: non-web source types receive incorrect sourceConfig shape.
When switching to file, git, or api source types, line 224 sets sourceConfig: { startUrl: "" }, but startUrl is a WebSourceConfig-only field per the API contract. Although these types are currently disabled (lines 85–87), this will break when they're enabled.
♻️ Proposed fix: set empty config or type-specific defaults
if (st.value === "web") {
update({
type: "web",
sourceConfig: { startUrl: "", scope: DEFAULT_SCOPE, crawlSettings: DEFAULT_CRAWL_SETTINGS },
});
} else {
- update({ type: st.value, sourceConfig: { startUrl: "" } });
+ update({ type: st.value, sourceConfig: {} as any });
}(Replace as any with proper type-specific config shapes when file/git/api types are implemented.)
🤖 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 `@src/components/editors/ingestion-source-editor.tsx` around lines 218 - 225,
In the else block at line 224 of ingestion-source-editor.tsx, the sourceConfig
is being set to { startUrl: "" } for non-web source types (file, git, api), but
startUrl is only valid for WebSourceConfig. Replace { startUrl: "" } with an
empty object {} in the else branch of the update call to fix the schema
mismatch. When file, git, and api source types are enabled, replace this empty
object with their respective type-specific default configurations to match the
API contract.
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)
src/pages/__tests__/resource-detail-rag.test.tsx (1)
402-425:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the shared
renderPagehelper instead of introducing a second render helper.
renderPageWithToasterduplicates the same wrapper stack and diverges from the test-helper convention. Consider adding an optionalwithToasterflag torenderPage.Suggested refactor
-function renderPage(type: string, id = "res1") { +function renderPage(type: string, id = "res1", opts?: { withToaster?: boolean }) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false }, }, }); return render( <MemoryRouter initialEntries={[`/manage/resources/${type}/${id}`]}> <QueryClientProvider client={queryClient}> <ThemeProvider defaultTheme="light" storageKey="eddi-theme-test"> + {opts?.withToaster ? <Toaster /> : null} <Routes> <Route path="/manage/resources/:type/:id" element={<ResourceDetailPage />} /> </Routes> </ThemeProvider> </QueryClientProvider> </MemoryRouter> ); } @@ - function renderPageWithToaster(type: string, id = "res1") { - ... - } @@ - renderPageWithToaster("rag"); + renderPage("rag", "res1", { withToaster: true });As per coding guidelines, "
**/*.test.{ts,tsx}: UserenderPage(type)helper withMemoryRouter,QueryClient, andThemeProviderin unit tests."Also applies to: 437-437
🤖 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 `@src/pages/__tests__/resource-detail-rag.test.tsx` around lines 402 - 425, The renderPageWithToaster function duplicates the wrapper stack from the shared renderPage helper, violating the DRY principle and coding guidelines. Add an optional withToaster flag parameter to the existing renderPage helper that conditionally renders the Toaster component inside the wrapper stack when true, then remove the renderPageWithToaster function entirely and update all call sites (including line 437) to use renderPage with the withToaster flag set appropriately instead of using the duplicated helper.Source: Coding guidelines
♻️ Duplicate comments (1)
src/pages/__tests__/resource-detail-rag.test.tsx (1)
323-324:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace remaining text-based assertions with
data-testidassertions.These assertions still depend on rendered copy (
toHaveTextContent,getByText) instead of stable test IDs. Please switch to explicitdata-testidtargets for source label verification and error-toast verification.As per coding guidelines, "
**/*.test.{ts,tsx}: Assert ondata-testidattributes in unit tests instead of relying on rendered text or DOM structure."Also applies to: 462-464
🤖 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 `@src/pages/__tests__/resource-detail-rag.test.tsx` around lines 323 - 324, The test assertions are using `toHaveTextContent` which depends on rendered text content rather than stable test identifiers. Replace these text-based assertions with `data-testid` assertions by either verifying that the element with the specific test ID exists without checking its text content, or by asserting on data attributes instead of rendered text. This applies to the assertion at the shown location and also at the other affected locations mentioned in the comment (lines 462-464), ensuring all assertions in this test file rely on explicit `data-testid` targets for stability rather than rendered copy.Source: Coding guidelines
🤖 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 `@src/pages/__tests__/resource-detail-rag.test.tsx`:
- Around line 402-425: The renderPageWithToaster function duplicates the wrapper
stack from the shared renderPage helper, violating the DRY principle and coding
guidelines. Add an optional withToaster flag parameter to the existing
renderPage helper that conditionally renders the Toaster component inside the
wrapper stack when true, then remove the renderPageWithToaster function entirely
and update all call sites (including line 437) to use renderPage with the
withToaster flag set appropriately instead of using the duplicated helper.
---
Duplicate comments:
In `@src/pages/__tests__/resource-detail-rag.test.tsx`:
- Around line 323-324: The test assertions are using `toHaveTextContent` which
depends on rendered text content rather than stable test identifiers. Replace
these text-based assertions with `data-testid` assertions by either verifying
that the element with the specific test ID exists without checking its text
content, or by asserting on data attributes instead of rendered text. This
applies to the assertion at the shown location and also at the other affected
locations mentioned in the comment (lines 462-464), ensuring all assertions in
this test file rely on explicit `data-testid` targets for stability rather than
rendered copy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a55a147-261d-45dc-8645-d99bd91d9ecd
📒 Files selected for processing (3)
src/components/editors/ingestion-source-editor.tsxsrc/components/editors/rag-editor.tsxsrc/pages/__tests__/resource-detail-rag.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/editors/rag-editor.tsx
- src/components/editors/ingestion-source-editor.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
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.
Needed for labsai/EDDI#529
Summary by CodeRabbit
eddi://RAG URI extension resolution and normalized extension type handling in the add-extension flow.