feat: Openrag improved tasks logs backend#1671
Conversation
* update style for oss of the failed task in the task panel * keep logic on click, remove unecessary useeffect * fix padding * wip implementing Saas style * utils to reshape error until backend provide info we need * utils to reshape error until backend provide info we need * utils to reshape error until backend provide info we need and fixinf fallbacks of isTotalFailure * utils to reshape error until backend provide into * have Saas style for failed and complete labelstatus and width and border * few style adjustment to follow codebase pattern * adjust succeed and partially succeed case * adding comment for TODO implementation or more clarity * remove carbon icon package and replace carbon icon * add incident-reporter-icon --------- Co-authored-by: Olfa Maslah <[email protected]>
* Encode IBM API key as Basic auth header Add base64 encoding for the IBM auth path: import base64, construct a Basic auth token from X-Username and X-Api-Key (username:apikey), and store it in user.jwt_token and user.opensearch_credentials. Also set request.state.user before attaching the DB user ID so downstream code can access the created user object. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* restart deployment if env changes * unit test * lint
…S_TO_GET_FROM_ENVIRONMENT (#1667) * Ensure we dynamically update the list of Langflow .env environment variables with default values when the comma separated list defined in LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT changes * fix tests * fix additional linting errors --------- Co-authored-by: rodageve <[email protected]>
* Retire openrag-mcp; switch docs to streamable HTTP Remove the stdio-based MCP server and all in-repo MCP tooling, and update README to mark the package as retired. Deleted module files include the MCP entrypoint, server, config, registry and individual tools (chat, search, documents, settings). The README was rewritten to announce that openrag-mcp is retired, explain migration to the built-in streamable-HTTP /mcp endpoint, update Cursor/Claude examples to use URL+headers auth, list the new v1 API tools, and note that the last PyPI release is final. This change consolidates MCP functionality into the OpenRAG core and removes the subprocess/stdio implementation and its source code. * Mark MCP SDK retired and clean package metadata Update package metadata to reflect retirement and integration into the OpenRAG backend. Bump version to 0.3.0 and replace the project description with a retirement/migration note. Set Development Status to Inactive, remove explicit Python version classifiers, and clear runtime dependencies and the CLI script entrypoint. Also remove the hatch env pip-args setting; build-system and wheel package target remain unchanged. * chore: update uv.lock files after version bump * Update uv.lock --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add filename-based duplicate handling for connectors Add end-to-end support for filename-based duplicate handling on connector ingests. Frontend: send a new replace_duplicates flag with connector sync requests, perform a pre-sync duplicate check, and show a DuplicateHandlingDialog that lets users overwrite or skip duplicates when uploading from provider UI. Backend: propagate replace_duplicates through connector_router, request models, and connector services into the file processors. ConnectorFileProcessor and LangflowConnectorFileProcessor now check whether a filename already exists in the index and either fail the file task or delete the existing document before ingesting when replace_duplicates is true. Utilities/tests: clean_connector_filename now preserves original spacing/slashes and only enforces MIME-mapped extensions; get_filename_aliases adds underscore/sanitized variants so lookups match connector-indexed names. Add unit tests covering filename dedupe logic and filename alias behavior. * Use duplicateNames list and display names Replace numeric duplicateCount with a duplicateNames string[] across upload and dropdown flows so the UI can show the actual file names that would be overwritten. The duplicate-handling dialog now accepts duplicateNames, derives an effective count, and lists up to 5 duplicate filenames with an "… and N more" indicator; message labels and button text use the effective count. Toast messages and pending state in upload/[provider]/page.tsx and knowledge-dropdown.tsx were updated to pass and consume duplicateNames and to use duplicateNames.length for counts. * Update page.tsx * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This reverts commit bc8be33.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds Docling transient-error classification and bounded result-fetch retry, infers structured failure metadata for failed files in TaskService, exposes enhanced task endpoints returning enriched payloads, updates route ordering to avoid path shadowing, and adds unit tests for polling and task-status enrichment. ChangesStructured task failure metadata
Sequence DiagramsequenceDiagram
participant Client
participant API as Task API (task_status_enhanced / all_tasks_enhanced)
participant TaskService
participant Polling as DoclingPollingService
participant Docling as docling_service.fetch_task_result
Client->>API: GET /tasks/{id}/enhanced or /tasks/enhanced
API->>TaskService: get_task_status2 / get_all_tasks2(user_id)
TaskService->>Polling: when checking docling status -> poll_until_ready
Polling->>Docling: fetch_task_result(task_id)
alt fetch returns document
Docling-->>Polling: document.json_content
Polling-->>TaskService: SUCCESS (result returned)
TaskService-->>API: enriched task payload (includes failure metadata where applicable)
API-->>Client: 200 JSON
else transient/network error
Docling-->>Polling: raises DoclingTransientError
Polling->>Docling: retry fetch_task_result (bounded by budget and remaining time)
Docling-->>Polling: returns document or raises permanent error
else permanent error / exhausted budget
Docling-->>Polling: raises DoclingServeError or budget exhausted
Polling-->>TaskService: FAILED with detail
TaskService-->>API: task payload (failed file entries include inferred metadata if applicable)
API-->>Client: 200 JSON (or 404 if task missing)
end
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
🧹 Nitpick comments (2)
src/services/docling_polling_service.py (2)
108-109: 💤 Low valueConsider returning
TIMEOUTfor consistency with the outer loop timeout.The outer polling timeout (line 212) returns
PollOutcome.TIMEOUT, but this inner result-fetch timeout returnsPollOutcome.FAILED. Both scenarios are timeout conditions. While downstream consumers handle them identically, usingTIMEOUThere would improve consistency for debugging and observability.♻️ Proposed change for consistency
return DoclingPollResult( - outcome=PollOutcome.FAILED, + outcome=PollOutcome.TIMEOUT, detail=detail, last_snapshot=snapshot, elapsed_seconds=elapsed, )🤖 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/services/docling_polling_service.py` around lines 108 - 109, The inner result-fetch timeout currently constructs and returns DoclingPollResult(outcome=PollOutcome.FAILED); change that to DoclingPollResult(outcome=PollOutcome.TIMEOUT) so the inner timeout outcome matches the outer loop timeout behavior; locate the code that returns DoclingPollResult in the result-fetch branch (the block constructing DoclingPollResult with PollOutcome.FAILED) and replace the PollOutcome enum value to PollOutcome.TIMEOUT while keeping the rest of the returned payload unchanged.
26-31: ClarifyStrEnumcompatibility and timeout outcome semantics
StrEnumis safe here: the project setsrequires-python = ">=3.13"and the backend runtime usespython:3.13-slim, so there’s no Python 3.10/3.11 import-time risk.- Minor consistency: the inner “result fetch” timeout returns
PollOutcome.FAILEDwhile the outer polling timeout returnsPollOutcome.TIMEOUT; returningTIMEOUTfrom the inner path would improve observability (downstream still treats it as a non-EXPIREDfailure).🤖 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/services/docling_polling_service.py` around lines 26 - 31, StrEnum usage is fine given Python 3.13 target (no change needed for PollOutcome’s base), but update the inner "result fetch" timeout path to return PollOutcome.TIMEOUT instead of PollOutcome.FAILED so timeout semantics are consistent with the outer polling timeout; locate the PollOutcome enum and the function/method where the inner fetch handles a timeout (the branch currently returning PollOutcome.FAILED) and change that return to PollOutcome.TIMEOUT, preserving any logging or metrics emission to indicate a timeout occurred.
🤖 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 `@src/services/docling_polling_service.py`:
- Around line 108-109: The inner result-fetch timeout currently constructs and
returns DoclingPollResult(outcome=PollOutcome.FAILED); change that to
DoclingPollResult(outcome=PollOutcome.TIMEOUT) so the inner timeout outcome
matches the outer loop timeout behavior; locate the code that returns
DoclingPollResult in the result-fetch branch (the block constructing
DoclingPollResult with PollOutcome.FAILED) and replace the PollOutcome enum
value to PollOutcome.TIMEOUT while keeping the rest of the returned payload
unchanged.
- Around line 26-31: StrEnum usage is fine given Python 3.13 target (no change
needed for PollOutcome’s base), but update the inner "result fetch" timeout path
to return PollOutcome.TIMEOUT instead of PollOutcome.FAILED so timeout semantics
are consistent with the outer polling timeout; locate the PollOutcome enum and
the function/method where the inner fetch handles a timeout (the branch
currently returning PollOutcome.FAILED) and change that return to
PollOutcome.TIMEOUT, preserving any logging or metrics emission to indicate a
timeout occurred.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b98d7148-78fc-487d-8a27-6cbddac76a50
📒 Files selected for processing (1)
src/services/docling_polling_service.py
mpawlow
left a comment
There was a problem hiding this comment.
Code Review 1
- ✅ Approved / LGTM 🚀
- See: PR comments:
- (1a) needs to be addressed (Ruff failures)
- (1b) to (1e) for optional consideration
| "actionable_by": "RETRYABLE", | ||
| } | ||
|
|
||
| if phase == IngestionPhase.LANGFLOW: |
There was a problem hiding this comment.
(1c) [Normal] _infer_failure_metadata ordering: LANGFLOW phase check shadows the "already exists" branch
Problem
- The priority-ordered early-return chain in
_infer_failure_metadataplaces thephase == IngestionPhase.LANGFLOWcheck (item 5) before the"already exists" in errorcheck (item 6). - A file whose failure occurs during the Langflow phase but whose
errorstring contains"already exists"will be classified aslangflow / unknown / RETRYABLEinstead ofopenrag / file_validation / USER_ACTIONABLE. - The ordering implies "if it failed during Langflow, the component is always langflow/unknown" — but duplicate-file rejections propagated via the Langflow error message would silently be misclassified.
- No test covers this ordering edge case.
Background Information
- The docstring says "Priority order: docling_status enum first (stable), error string patterns second (fallback for edge cases)." The LANGFLOW branch should arguably rank below the error-pattern checks if those patterns represent globally valid classifications regardless of phase.
Potential Solution
Move the "already exists" (and any other globally-classified error-string checks) above the phase-based catch-all:
# Check stable docling status enums first (as today)
if docling_status == DoclingPhaseStatus.FAILED: ...
if docling_status == DoclingPhaseStatus.EXPIRED: ...
# Check globally-applicable error patterns before any phase catch-alls
if "already exists" in error:
return {
"component": "openrag",
"failure_phase": "file_validation",
"user_facing_message": "A file with this name already exists.",
"actionable_by": "USER_ACTIONABLE",
}
# Phase-specific and phase-dependent checks last
if phase == IngestionPhase.DOCLING and "Docling conversion did not complete" in error: ...
if phase == IngestionPhase.DOCLING and docling_status == DoclingPhaseStatus.PROCESSING: ...
if phase == IngestionPhase.LANGFLOW: ...
return None| ) | ||
|
|
||
|
|
||
| async def all_tasks_enhanced_endpoint( |
There was a problem hiding this comment.
(1e) [Minor] get_task_status2/get_all_tasks2 and get_all_tasks2 filter completed files differently than get_task_status2
Problem
get_task_status2(line 728) iterates all files inupload_task.file_tasksincluding completed ones — consistent withget_task_status.get_all_tasks2(line 777) explicitly skips completed files (if file_task.status.value != "completed":) — consistent withget_all_tasks.- Consumers of the
/tasks/{task_id}/enhancedendpoint (which callsget_task_status2) will see completed files in the"files"dict; consumers of/tasks/enhanced(which callsget_all_tasks2) will not see completed files. - This behavioral asymmetry is present in the pre-existing methods and is perpetuated in the new "enhanced" variants without being documented in the endpoint docstrings, which could confuse API consumers.
Code References
src/services/task_service.py:728—get_task_status2: all files includedsrc/services/task_service.py:777—get_all_tasks2: completed files excludedsrc/api/v1/documents.py:68–81—all_tasks_enhanced_endpointdocstring (no mention of completed-file filtering)src/api/v1/documents.py:96–114—task_status_enhanced_endpointdocstring (no mention)
Potential Solution
Document the divergent behaviour in both endpoint docstrings:
async def all_tasks_enhanced_endpoint(...):
"""...
Note: completed files are omitted from each task's ``files`` dict to
reduce payload size; use GET /v1/tasks/{task_id}/enhanced for the full
file list of a specific task.
"""
This pull request introduces a more robust and user-friendly workflow for handling duplicate file uploads, especially when syncing files to connectors or uploading folders. The main improvements include duplicate detection before syncing, a dialog that lists duplicate filenames, and options for users to overwrite or skip duplicates. Additionally, several new CSS variables and a new icon component were added to support these UI enhancements.
Duplicate Handling Workflow Improvements:
Added pre-upload duplicate checks for selected files when syncing to connectors, prompting the user with a dialog listing duplicate filenames and options to overwrite or skip them. This prevents accidental overwrites and improves user awareness. (frontend/app/upload/[provider]/page.tsxR415-R423, frontend/app/upload/[provider]/page.tsxL430-R451, frontend/app/upload/[provider]/page.tsxR460, frontend/app/upload/[provider]/page.tsxR478-R560, frontend/app/upload/[provider]/page.tsxL655-R749, frontend/app/upload/[provider]/page.tsxR769-R776, [1] [2] [3]
Enhanced the
DuplicateHandlingDialogto display up to five duplicate filenames and indicate if there are more, making it easier for users to review which files are affected. [1] [2] [3]Updated the folder upload duplicate handling logic in
KnowledgeDropdownto track and display duplicate filenames instead of just counts, improving feedback and consistency with connector uploads. [1] [2] [3] [4] [5] [6]API and Mutation Updates:
useSyncConnectormutation to accept areplace_duplicatesflag, allowing the backend to distinguish between overwrite and skip actions for duplicates. (frontend/app/api/mutations/useSyncConnector.tsR75-R76, frontend/app/upload/[provider]/page.tsxR460)UI and Design Enhancements:
Added new CSS variables for task status colors and duplicate/failure states to improve visual feedback in the UI. [1] [2] [3] [4] [5]
Added a new
IncidentReporterIconReact component for consistent iconography related to incident reporting.Other Minor Updates:
select_accounttoconsentin the connector authorization flow to better align with user expectations.These changes collectively make file uploads and syncs safer and more transparent for users, while also improving the look and feel of the app.
Summary by CodeRabbit
New Features
Bug Fixes
Tests