Add SDK support for enhanced task status#1864
Conversation
WalkthroughThis PR extends both the Python and TypeScript SDKs with enhanced task status endpoints. New ChangesEnhanced Task Status Endpoints
Possibly related PRs
Suggested labels
Suggested reviewers
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
sdks/python/openrag_sdk/__init__.py (1)
97-97: ⚡ Quick winExport
IngestTaskStatusalongsideTaskListResponsefrom the root package API.
TaskListResponse.tasksand document status methods revolve aroundIngestTaskStatus; exporting onlyTaskListResponseforces deep imports for a core type.Suggested change
from .models import ( @@ IngestResponse, + IngestTaskStatus, @@ __all__ = [ @@ "IngestResponse", + "IngestTaskStatus", "TaskListResponse",🤖 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 `@sdks/python/openrag_sdk/__init__.py` at line 97, Add IngestTaskStatus to the root package exports so consumers can import it without deep imports; update the export list that currently contains "TaskListResponse" (in sdks/python/openrag_sdk/__init__.py) to also include "IngestTaskStatus" so both TaskListResponse and IngestTaskStatus are exposed from the package root.sdks/python/openrag_sdk/models.py (1)
95-95: ⚡ Quick winDefault
tasksto an empty list for cross-SDK response parity.
TaskListResponsecurrently requirestasks, whilesdks/typescript/src/documents.ts(Lines 131-133) gracefully defaults missing tasks to[]. The same payload can parse in TypeScript but fail in Python with a validation error; aligning defaults avoids SDK behavior drift.Suggested change
class TaskListResponse(BaseModel): """Response from listing ingestion tasks.""" - tasks: list[IngestTaskStatus] + tasks: list[IngestTaskStatus] = Field(default_factory=list)🤖 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 `@sdks/python/openrag_sdk/models.py` at line 95, In TaskListResponse in models.py, make the tasks field default to an empty list for parity with TypeScript: replace the required tasks: list[IngestTaskStatus] with a pydantic-safe default (use Field(default_factory=list)) so missing tasks in responses parse as [] rather than erroring; import Field from pydantic if not present and keep the element type as IngestTaskStatus.
🤖 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 `@sdks/typescript/src/documents.ts`:
- Around line 131-133: The code maps data.tasks without ensuring it's an array,
which can throw if the API returns a non-array; update the tasks assignment in
the relevant method in documents.ts to guard with Array.isArray(data.tasks) and
only map when true, otherwise default to an empty array (or preserve expected
fallback), calling this.parseTaskStatus for each item; reference the data.tasks
field and this.parseTaskStatus to locate and update the logic.
In `@tests/integration/sdk/test_documents.py`:
- Around line 119-125: The enhanced integration test is under-asserting contract
fidelity: update tests in tests/integration/sdk/test_documents.py to verify
enhanced metadata by asserting that enhanced_status.files is present and
non-empty and that the created task (task_response.task_id) appears in the
enhanced_tasks.tasks listing (e.g., at least one entry has matching task_id);
specifically add assertions after calling
client.documents.get_task_status_enhanced and
client.documents.list_tasks_enhanced to check enhanced_status.files (or
equivalent files field) exists and contains expected file entries and that any
task in enhanced_tasks.tasks has task_id == task_response.task_id to ensure the
created task is included.
---
Nitpick comments:
In `@sdks/python/openrag_sdk/__init__.py`:
- Line 97: Add IngestTaskStatus to the root package exports so consumers can
import it without deep imports; update the export list that currently contains
"TaskListResponse" (in sdks/python/openrag_sdk/__init__.py) to also include
"IngestTaskStatus" so both TaskListResponse and IngestTaskStatus are exposed
from the package root.
In `@sdks/python/openrag_sdk/models.py`:
- Line 95: In TaskListResponse in models.py, make the tasks field default to an
empty list for parity with TypeScript: replace the required tasks:
list[IngestTaskStatus] with a pydantic-safe default (use
Field(default_factory=list)) so missing tasks in responses parse as [] rather
than erroring; import Field from pydantic if not present and keep the element
type as IngestTaskStatus.
🪄 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: dda83fe6-62f2-41aa-a722-7e18b9096bca
📒 Files selected for processing (10)
sdks/python/README.mdsdks/python/openrag_sdk/__init__.pysdks/python/openrag_sdk/documents.pysdks/python/openrag_sdk/models.pysdks/typescript/README.mdsdks/typescript/src/documents.tssdks/typescript/src/index.tssdks/typescript/src/types.tssdks/typescript/tests/integration.test.tstests/integration/sdk/test_documents.py
| tasks: (data.tasks ?? []).map((task: Record<string, unknown>) => | ||
| this.parseTaskStatus(task) | ||
| ), |
There was a problem hiding this comment.
Guard data.tasks with Array.isArray before calling .map().
If the API returns a non-array truthy value for tasks, this will throw at runtime. A narrow shape guard keeps the method resilient.
Suggested change
async listTasksEnhanced(): Promise<TaskListResponse> {
const response = await this.client._request("GET", "/api/v1/tasks/enhanced");
const data = await response.json();
+ const tasksRaw = Array.isArray(data.tasks) ? data.tasks : [];
return {
- tasks: (data.tasks ?? []).map((task: Record<string, unknown>) =>
+ tasks: tasksRaw.map((task: Record<string, unknown>) =>
this.parseTaskStatus(task)
),
};
}📝 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.
| tasks: (data.tasks ?? []).map((task: Record<string, unknown>) => | |
| this.parseTaskStatus(task) | |
| ), | |
| async listTasksEnhanced(): Promise<TaskListResponse> { | |
| const response = await this.client._request("GET", "/api/v1/tasks/enhanced"); | |
| const data = await response.json(); | |
| const tasksRaw = Array.isArray(data.tasks) ? data.tasks : []; | |
| return { | |
| tasks: tasksRaw.map((task: Record<string, unknown>) => | |
| this.parseTaskStatus(task) | |
| ), | |
| }; | |
| } |
🤖 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 `@sdks/typescript/src/documents.ts` around lines 131 - 133, The code maps
data.tasks without ensuring it's an array, which can throw if the API returns a
non-array; update the tasks assignment in the relevant method in documents.ts to
guard with Array.isArray(data.tasks) and only map when true, otherwise default
to an empty array (or preserve expected fallback), calling this.parseTaskStatus
for each item; reference the data.tasks field and this.parseTaskStatus to locate
and update the logic.
| enhanced_status = await client.documents.get_task_status_enhanced( | ||
| task_response.task_id | ||
| ) | ||
| assert enhanced_status.status is not None | ||
|
|
||
| enhanced_tasks = await client.documents.list_tasks_enhanced() | ||
| assert isinstance(enhanced_tasks.tasks, list) |
There was a problem hiding this comment.
Shared root cause: enhanced integration tests are under-asserting contract fidelity in tests/integration/sdk/test_documents.py and sdks/typescript/tests/integration.test.ts.
Both tests only verify basic shape (status defined / tasks is array), so they can pass even when enhanced metadata regresses. Tighten both to assert enhanced files presence and that the created task appears in enhanced task listings.
🤖 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 `@tests/integration/sdk/test_documents.py` around lines 119 - 125, The enhanced
integration test is under-asserting contract fidelity: update tests in
tests/integration/sdk/test_documents.py to verify enhanced metadata by asserting
that enhanced_status.files is present and non-empty and that the created task
(task_response.task_id) appears in the enhanced_tasks.tasks listing (e.g., at
least one entry has matching task_id); specifically add assertions after calling
client.documents.get_task_status_enhanced and
client.documents.list_tasks_enhanced to check enhanced_status.files (or
equivalent files field) exists and contains expected file entries and that any
task in enhanced_tasks.tasks has task_id == task_response.task_id to ensure the
created task is included.
Summary
Added SDK support for enhanced ingestion task status.
What changed
client.documents.get_task_status_enhanced(task_id).client.documents.list_tasks_enhanced().client.documents.getTaskStatusEnhanced(taskId).client.documents.listTasksEnhanced().TaskListResponsetypes.Note: no internal APIs are exposed.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests