feat: add support for skipped tasks and warnings in task processing#1769
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds "skipped" status for duplicate uploads; backend marks duplicates as SKIPPED when replace_duplicates=false; frontend types, utilities, and components treat skipped files as warnings and render warning UI; tests updated to expect skipped/warning behavior. ChangesSkipped file status and warning display
Sequence Diagram(s)sequenceDiagram
participant Connector as ConnectorFileProcessor
participant OpenSearch as OpenSearchClient
participant Upload as UploadTask
Connector->>OpenSearch: check filename exists
alt filename exists and replace_duplicates=false
Connector-->>Upload: mark FileTask SKIPPED, clear error, result={status: "skipped", reason: "duplicate_filename", warning}
Connector->>Upload: increment successful_files
else filename exists and replace_duplicates=true
Connector->>OpenSearch: delete_document_by_filename(owner_user_id)
Connector->>Upload: proceed with replacement flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/task-utils.ts`:
- Around line 252-257: The function hasFailedFileEntries now returns true for
both failed and warning file entries (it calls getTaskIssueFileEntries), which
mismatches its name and causes callers like isCompletedWithFailures to
misclassify warnings as failures; either rename hasFailedFileEntries to reflect
the broader check (e.g., hasIssueFileEntries) and update all callers (including
isCompletedWithFailures) to the new name, or restore strict failure semantics by
changing hasFailedFileEntries to only check task.failed_files and add a new
helper (e.g., hasWarningOrIssueFileEntries) that uses getTaskIssueFileEntries
for warning-aware checks, then update callers accordingly.
🪄 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: 8a6d4d1b-6bb0-48c2-ac80-bfec5655eabb
📒 Files selected for processing (7)
frontend/app/api/queries/useGetTasksQuery.tsfrontend/components/task-dialog/constants.tsfrontend/components/task-error-content.tsxfrontend/lib/task-error-display.tsfrontend/lib/task-utils.tssrc/models/processors.pytests/unit/test_connector_processor_filename_dedupe.py
lucaseduoli
left a comment
There was a problem hiding this comment.
Really good work, just some minor things
Wallgau
left a comment
There was a problem hiding this comment.
Hey @ricofurtado, is this something we need for GA? If not, maybe we could postpone it until after GA, just to be on the safe side and avoid introducing additional complexity or potential bugs this late in the cycle.
…ogic for task file checks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/task-utils.ts`:
- Around line 259-261: hasIssueFileEntries currently just checks
getTaskIssueFileEntries(task).length > 0 and ignores task.failed_files; update
the function hasIssueFileEntries to also return true when task.failed_files (or
task.failedFiles if that naming is used elsewhere) is a positive number so tasks
with missing/partial task.files still report issues; locate hasIssueFileEntries
and getTaskIssueFileEntries in frontend/lib/task-utils.ts and add the fallback
check (e.g., return getTaskIssueFileEntries(task).length > 0 ||
(task.failed_files && task.failed_files > 0)) to preserve the original behavior.
🪄 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: daea4582-a330-449c-9e1c-50996eaa07bb
📒 Files selected for processing (3)
frontend/components/task-error-content.tsxfrontend/components/task-notification-menu.tsxfrontend/lib/task-utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/task-error-content.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
lucaseduoli
left a comment
There was a problem hiding this comment.
LGTM! but we should consider Olfa's comment
|
merged |
|
Merged |
This pull request introduces a new "skipped" status for task files to represent files that are intentionally not processed (such as due to duplicate filenames), and updates both backend and frontend logic to support this status. The UI now distinguishes between failed and warning (skipped) files, providing clearer feedback to users. Tests and utility functions are updated to reflect these changes.
Backend logic changes:
result, and incrementssuccessful_filesinstead offailed_filesin these cases.Frontend and UI changes:
TaskFileEntryandTasktypes, and introduced a new "warning" category for task file status. [1] [2] [3]Error message improvements:
Summary by CodeRabbit
New Features
Bug Fixes