feat: task ingest dialog and retry flow#1700
Merged
Merged
Conversation
Task dialog, retry API, Langflow retry handling, and related frontend/backend changes from task-ingest-dialog at 5e94758. Omits connector/orphan paths that only appear in the PR diff due to main merge/revert baseline, matching the original 32-file PR against main. Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
added
frontend
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a backend retry endpoint and schemas, client retry mutation and TaskDialog UI (with polling, selection, and optimistic overlays), stable React Query keys and enhanced task queries, structured ingestion-failure analysis, overlay reconciliation in TaskContext, processor/Langflow and index changes, styling tokens, and tests. ChangesTask Retry Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
github-actions
Bot
added
enhancement
Wallgau
changed the title
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/app/api/mutations/useDeleteDocument.ts (1)
16-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle non-JSON error bodies in delete failures.
Line 28 assumes every error response is JSON. If the server/proxy returns HTML/plain text, this throws before your intended
Errormessage is created.Proposed fix
export async function deleteDocumentByFilename( filename: string, ): Promise<DeleteDocumentResponse> { @@ if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || "Failed to delete document"); + const error = await response + .json() + .catch(() => ({} as { error?: string; message?: string })); + throw new Error( + error.error ?? error.message ?? "Failed to delete document", + ); }🤖 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/api/mutations/useDeleteDocument.ts` around lines 16 - 33, In deleteDocumentByFilename, the error handling assumes response.json() always succeeds; wrap the JSON parse in a try/catch (or attempt response.json() and fall back to response.text()) when response.ok is false, extract a meaningful message from parsed.error if present or from the plain text fallback, and then throw new Error(...) with that message (while keeping the existing DeleteDocumentResponse/Request types intact). Ensure you update the error branch to first attempt JSON parse and gracefully handle non-JSON bodies before throwing.src/models/processors.py (1)
540-554:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDelete-by-document_id uses the wrong identifier for connector cleanup.
This cleanup now deletes by
file_id, but indexed chunks inprocess_document_standardusedocument_id = file_hash, so source-deleted connector files can remain searchable instead of being removed.Suggested direction
- # File gone at source — remove indexed chunks by document_id (= connector file_id) + # File gone at source — remove indexed chunks using the same stable identifier + # used at index time (currently document_id=file_hash), or persist a dedicated + # connector identifier field for reliable cleanup.A durable fix is to align ingestion and deletion on one stable key (either restore a dedicated connector identifier in index docs or ensure connector document_id is derived from connector file_id consistently).
🤖 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/models/processors.py` around lines 540 - 554, The cleanup is deleting indexed chunks by file_id but indexing uses document_id = file_hash (see process_document_standard), so connector-deleted files remain searchable; fix by passing the same document_id key used at ingest to delete_chunks_by_document_ids (i.e., use the file_hash value used when creating document_id) or alternatively change the indexing to use a connector-stable id (e.g., file_id) consistently—update the call to delete_chunks_by_document_ids (and any surrounding logic) to use document_id/file_hash to align with process_document_standard, keeping references to delete_chunks_by_document_ids, process_document_standard, document_id, file_hash, file_id, opensearch_client and get_index_name.
🧹 Nitpick comments (2)
frontend/components/task-error-content.tsx (1)
240-249: 💤 Low valueMinor:
onClosemay be redundant withonOpenChange.Both
onOpenChange={setIsTaskDialogOpen}andonClose={() => setIsTaskDialogOpen(false)}accomplish the same thing when the dialog closes. Consider whetheronCloseis needed, or if TaskDialog uses it for a distinct purpose (e.g., cleanup actions distinct from state updates).🤖 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/components/task-error-content.tsx` around lines 240 - 249, The TaskDialog is being passed both onOpenChange={setIsTaskDialogOpen} and a redundant onClose={() => setIsTaskDialogOpen(false)}; inspect the TaskDialog component to see if it expects an onClose for extra cleanup—if not, remove the onClose prop from the call site (the TaskDialog JSX) to avoid duplicate state updates, otherwise implement a single handler that calls setIsTaskDialogOpen(false) and then performs any cleanup and pass that handler to onClose while keeping onOpenChange for state synchronization (referencing TaskDialog, onOpenChange, onClose, and setIsTaskDialogOpen).frontend/app/knowledge/page.tsx (1)
787-791: 💤 Low valueConsider removing redundant refetch calls.
invalidateQueriesmarks queries as stale and triggers automatic refetch for active observers. The subsequentrefetchQueriescalls are redundant since this component actively observes both queries.♻️ Suggested simplification
await refreshTasks(); await queryClient.invalidateQueries({ queryKey: ["search"] }); await queryClient.invalidateQueries({ queryKey: ["listFiles"] }); - await queryClient.refetchQueries({ queryKey: ["search"] }); - await queryClient.refetchQueries({ queryKey: ["listFiles"] });🤖 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/knowledge/page.tsx` around lines 787 - 791, Remove the redundant refetch calls: after calling refreshTasks() keep the two await queryClient.invalidateQueries({ queryKey: ["search"] }) and await queryClient.invalidateQueries({ queryKey: ["listFiles"] }) and delete the subsequent await queryClient.refetchQueries({ queryKey: ["search"] }) and await queryClient.refetchQueries({ queryKey: ["listFiles"] }) lines; invalidateQueries will mark those queries stale and active observers in the component will refetch automatically (look for refreshTasks and the queryClient.invalidateQueries/refetchQueries calls in frontend/app/knowledge/page.tsx).
🤖 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/api/queries/useGetTaskQuery.ts`:
- Line 21: The fetch call in useGetTaskQuery directly interpolates taskId into
the URL which can break for IDs with reserved characters; update the URL
construction in the function (the fetch(`/api/tasks/${taskId}/enhanced`) usage)
to encode the ID using encodeURIComponent (i.e., interpolate
encodeURIComponent(taskId)) before calling fetch so reserved characters are
percent-encoded.
In `@frontend/components/task-dialog/category-chips.tsx`:
- Around line 38-45: This button acting as a toggle should expose its pressed
state for accessibility; update the button in category-chips.tsx (the element
that uses onStatusCategoryChange, isActive, chip.id and
ALL_TASK_STATUS_CATEGORIES) to include an aria-pressed attribute bound to the
toggle state (aria-pressed={isActive}) so screen readers get the correct
pressed/unpressed status while preserving existing behavior.
In `@frontend/components/task-dialog/file-list.tsx`:
- Around line 348-370: The panel with role="tabpanel" (id
"task-dialog-panel-task-ingestions") must always be rendered even when entries
is empty; currently the empty branch returns renderEmptyPanel which skips the
panel and breaks aria-controls semantics. Change the JSX so renderListHeader
remains conditional as needed but always output the div with id
"task-dialog-panel-task-ingestions", role="tabpanel",
aria-labelledby="task-dialog-tab-task-ingestions" and
className={listScrollClass}, and inside that div render either
renderFileRows(entries) or renderEmptyPanel("No files match your filters.").
Apply the same change to the other tab block (lines 373-397) so both isTabActive
branches always render their tabpanel containers.
In `@frontend/contexts/task-context.tsx`:
- Around line 486-511: The filter in setFiles (inside the prevFiles.filter
callback) currently removes "active" overlays solely because
indexedFilenames.has(filename) is true, which can drop distinct files that share
a filename; update the logic in the "active" branch to first compute the file
identity via getKnowledgeFileIdentity({ filename: file.filename, source_url:
file.source_url }) and check indexedIdentities.has(identity) to decide removal,
and only if source_url is missing or identity is undefined fall back to checking
indexedFilenames.has(filename); keep the existing behavior for statuses "failed"
and "processing" and ensure you reference the same symbols (setFiles,
indexedIdentities, indexedFilenames, getKnowledgeFileIdentity, currentTask) when
making the change.
---
Outside diff comments:
In `@frontend/app/api/mutations/useDeleteDocument.ts`:
- Around line 16-33: In deleteDocumentByFilename, the error handling assumes
response.json() always succeeds; wrap the JSON parse in a try/catch (or attempt
response.json() and fall back to response.text()) when response.ok is false,
extract a meaningful message from parsed.error if present or from the plain text
fallback, and then throw new Error(...) with that message (while keeping the
existing DeleteDocumentResponse/Request types intact). Ensure you update the
error branch to first attempt JSON parse and gracefully handle non-JSON bodies
before throwing.
In `@src/models/processors.py`:
- Around line 540-554: The cleanup is deleting indexed chunks by file_id but
indexing uses document_id = file_hash (see process_document_standard), so
connector-deleted files remain searchable; fix by passing the same document_id
key used at ingest to delete_chunks_by_document_ids (i.e., use the file_hash
value used when creating document_id) or alternatively change the indexing to
use a connector-stable id (e.g., file_id) consistently—update the call to
delete_chunks_by_document_ids (and any surrounding logic) to use
document_id/file_hash to align with process_document_standard, keeping
references to delete_chunks_by_document_ids, process_document_standard,
document_id, file_hash, file_id, opensearch_client and get_index_name.
---
Nitpick comments:
In `@frontend/app/knowledge/page.tsx`:
- Around line 787-791: Remove the redundant refetch calls: after calling
refreshTasks() keep the two await queryClient.invalidateQueries({ queryKey:
["search"] }) and await queryClient.invalidateQueries({ queryKey: ["listFiles"]
}) and delete the subsequent await queryClient.refetchQueries({ queryKey:
["search"] }) and await queryClient.refetchQueries({ queryKey: ["listFiles"] })
lines; invalidateQueries will mark those queries stale and active observers in
the component will refetch automatically (look for refreshTasks and the
queryClient.invalidateQueries/refetchQueries calls in
frontend/app/knowledge/page.tsx).
In `@frontend/components/task-error-content.tsx`:
- Around line 240-249: The TaskDialog is being passed both
onOpenChange={setIsTaskDialogOpen} and a redundant onClose={() =>
setIsTaskDialogOpen(false)}; inspect the TaskDialog component to see if it
expects an onClose for extra cleanup—if not, remove the onClose prop from the
call site (the TaskDialog JSX) to avoid duplicate state updates, otherwise
implement a single handler that calls setIsTaskDialogOpen(false) and then
performs any cleanup and pass that handler to onClose while keeping onOpenChange
for state synchronization (referencing TaskDialog, onOpenChange, onClose, and
setIsTaskDialogOpen).
🪄 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
Run ID: 70797592-01d0-41a9-b813-1e63e52b347c
📒 Files selected for processing (32)
frontend/app/api/mutations/useCancelTaskMutation.tsfrontend/app/api/mutations/useDeleteDocument.tsfrontend/app/api/mutations/useRetryTaskMutation.tsfrontend/app/api/queries/useGetTaskQuery.tsfrontend/app/api/queries/useGetTasksQuery.tsfrontend/app/globals.cssfrontend/app/knowledge/page.tsxfrontend/components/task-dialog/category-chips.tsxfrontend/components/task-dialog/constants.tsfrontend/components/task-dialog/file-error-details.tsxfrontend/components/task-dialog/file-list.tsxfrontend/components/task-dialog/filters.tsxfrontend/components/task-dialog/header.tsxfrontend/components/task-dialog/index.tsfrontend/components/task-dialog/task-dialog.tsxfrontend/components/task-dialog/use-task-dialog.tsfrontend/components/task-error-content.tsxfrontend/components/tasks_details.tsxfrontend/contexts/task-context.tsxfrontend/lib/task-error-display.tsfrontend/lib/task-utils.tsfrontend/tailwind.config.tsfrontend/tests/core/tasks-unified-panel.spec.tssrc/api/schemas/tasks.pysrc/api/tasks.pysrc/app/routes/internal.pysrc/config/settings.pysrc/models/processors.pysrc/services/task_service.pysrc/utils/telemetry/message_id.pytests/unit/test_task_service_get_task_status2.pytests/unit/test_task_service_retry_failed_files.py
…ng accessibility mainly
github-actions
Bot
added
enhancement
Contributor
|
Actionable comments posted: 0 |
github-actions
Bot
added
enhancement
ricofurtado
approved these changes
May 28, 2026
Collaborator
ricofurtado
left a comment
ricofurtado
left a comment
There was a problem hiding this comment.
LGTM.
I have created another PR #1702 to implement sync task logs enhancement and harden the Task Logs API.
mfortman11
requested changes
May 28, 2026
| {CATEGORY_CHIPS.map((chip) => { | ||
| const Icon = chip.icon; | ||
| const isActive = statusCategory === chip.id; | ||
| const count = counts[chip.id]; |
Contributor
There was a problem hiding this comment.
If this count is 0 I don't think we should show the option. For example no need to show Indexing for a completed task
| )} | ||
| > | ||
| <TaskDialogHeader | ||
| isCloudBrand={isCloudBrand} |
Contributor
There was a problem hiding this comment.
Because we have a hook for isCloudBrand no need to pass it as a prop to each component
Comment on lines
+27
to
38
| "task-dialog": "var(--task-dialog-width)", | ||
| }, | ||
| maxHeight: { | ||
| "task-dialog": "var(--task-dialog-max-height)", | ||
| }, | ||
| width: { | ||
| "task-dialog": "var(--task-dialog-width)", | ||
| "task-dialog-file-type": "var(--task-dialog-file-type-width)", | ||
| }, | ||
| zIndex: { | ||
| "task-dialog-menu": "var(--z-task-dialog-menu)", | ||
| }, |
Contributor
There was a problem hiding this comment.
nit: because these are a one off per the name it doesn't matter if they are global
Comment on lines
+58
to
+93
| <button | ||
| type="button" | ||
| disabled={disabled} | ||
| className="inline-flex min-h-10 w-task-dialog-file-type shrink-0 items-center justify-between gap-2 bg-layer-contextual px-4 text-sm text-muted-foreground transition-colors hover:text-foreground disabled:pointer-events-none disabled:opacity-50" | ||
| > | ||
| <span className="truncate">{fileTypeLabel}</span> | ||
| {open ? ( | ||
| <ChevronUp className="h-4 w-4 shrink-0 opacity-70" aria-hidden /> | ||
| ) : ( | ||
| <ChevronDown | ||
| className="h-4 w-4 shrink-0 opacity-70" | ||
| aria-hidden | ||
| /> | ||
| )} | ||
| </button> | ||
| ) : ( | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| disabled={disabled} | ||
| className="min-h-10 h-10 w-task-dialog-file-type shrink-0 justify-between font-normal" | ||
| > | ||
| <span className="truncate">{fileTypeLabel}</span> | ||
| {open ? ( | ||
| <ChevronUp | ||
| className="ml-2 h-4 w-4 shrink-0 opacity-50" | ||
| aria-hidden | ||
| /> | ||
| ) : ( | ||
| <ChevronDown | ||
| className="ml-2 h-4 w-4 shrink-0 opacity-50" | ||
| aria-hidden | ||
| /> | ||
| )} | ||
| </Button> | ||
| )} |
Contributor
There was a problem hiding this comment.
nit: because these 2 are so similar inline conditionals might be a bit cleaner
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent | ||
| className={cn( | ||
| "flex max-h-task-dialog w-task-dialog max-w-task-dialog flex-col gap-0 overflow-hidden p-0 sm:rounded-lg", |
Contributor
There was a problem hiding this comment.
There is a lot of vertical shifting when a filter is applied or unapplied let's use a fixed height to address that
| /> | ||
| <TaskDialogCategoryChips | ||
| isCloudBrand={isCloudBrand} | ||
| counts={categoryCounts} |
Contributor
There was a problem hiding this comment.
These counts don't update when another filter is applied ex: search
github-actions
Bot
added
enhancement
Contributor
|
Actionable comments posted: 0 |
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
Contributor
|
Actionable comments posted: 0 |
github-actions
Bot
added
enhancement
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/tailwind.config.ts (1)
29-34: 💤 Low valueClarify TaskDialog sizing:
h-task-dialogandmax-h-task-dialogboth map to--task-dialog-max-height(60vh)
frontend/tailwind.config.tssets bothheight.task-dialogandmaxHeight.task-dialogtovar(--task-dialog-max-height), andfrontend/app/globals.cssdefines--task-dialog-max-height: 60vh.frontend/components/task-dialog/task-dialog.tsxapplies bothh-task-dialogandmax-h-task-dialogon the dialog container, making the height effectively fixed at 60vh (not just capped).- Since
frontend/components/task-dialog/file-list.tsxusesoverflow-hiddenon wrappers andoverflow-y-auto(listScrollClass) for the panels, this fixed height likely supports intended internal scrolling.If the goal is “max only” sizing, drop
h-task-dialog(keepmax-h-task-dialog) or introduce a separate height token; otherwise themax-hutility is redundant.🤖 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/tailwind.config.ts` around lines 29 - 34, The Tailwind config maps both height.task-dialog and maxHeight.task-dialog to --task-dialog-max-height, and the component applies both h-task-dialog and max-h-task-dialog in TaskDialog (frontend/components/task-dialog/task-dialog.tsx), which fixes the dialog to 60vh instead of capping it; decide one behavior and implement it: if you want a max-only behavior, remove the use of the h-task-dialog class from task-dialog.tsx (keep max-h-task-dialog) and leave tailwind.config.ts with only maxHeight mapping (or delete the height.task-dialog key); if you want a fixed height, keep h-task-dialog and remove/ignore max-h-task-dialog or create a separate CSS var/utility for a true fixed height to avoid redundancy—also verify file-list.tsx overflow handling (overflow-hidden and listScrollClass) still works with the chosen sizing.
🤖 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 `@frontend/tailwind.config.ts`:
- Around line 29-34: The Tailwind config maps both height.task-dialog and
maxHeight.task-dialog to --task-dialog-max-height, and the component applies
both h-task-dialog and max-h-task-dialog in TaskDialog
(frontend/components/task-dialog/task-dialog.tsx), which fixes the dialog to
60vh instead of capping it; decide one behavior and implement it: if you want a
max-only behavior, remove the use of the h-task-dialog class from
task-dialog.tsx (keep max-h-task-dialog) and leave tailwind.config.ts with only
maxHeight mapping (or delete the height.task-dialog key); if you want a fixed
height, keep h-task-dialog and remove/ignore max-h-task-dialog or create a
separate CSS var/utility for a true fixed height to avoid redundancy—also verify
file-list.tsx overflow handling (overflow-hidden and listScrollClass) still
works with the chosen sizing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a4a1e54-e6cc-4b88-8019-0b51351be221
📒 Files selected for processing (3)
frontend/components/task-dialog/filters.tsxfrontend/components/task-dialog/task-dialog.tsxfrontend/tailwind.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/components/task-dialog/filters.tsx
- frontend/components/task-dialog/task-dialog.tsx
github-actions
Bot
added
enhancement
ConnectorFileProcessor calls from main resolve after the merge conflict. Fixes lint-backend mypy failure on unexpected keyword argument.
github-actions
Bot
added
enhancement
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
3 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.
Screen.Recording.2026-05-27.at.11.35.07.PM.mov
Screen.Recording.2026-05-27.at.11.50.17.PM.mov
Task dialog, retry API, Langflow retry handling, and related frontend/backend changes from task-ingest-dialog at 5e94758. Omits connector/orphan paths that only appear in the PR diff due to main merge/revert baseline, matching the original 32-file PR against main.
Summary by CodeRabbit