Feat/async attachment parsing - #961
Closed
kissghosts wants to merge 2 commits into
Closed
Conversation
Backend: - Add parse_async query parameter to upload endpoint - Support async parsing for all file types via FastAPI BackgroundTasks - File saved immediately with PARSING status, parsing happens in background - Add attachment_status and attachment_error_message to KnowledgeDocumentResponse - Update list_documents to batch query SubtaskContext for attachment status Frontend: - Add parseAsync parameter to uploadAttachment API - Add isTimeoutError and retryFileAsync to useBatchAttachment hook - Add \"Async Submit\" button for timeout errors in DocumentUpload - Add PARSING status display with hint text - Add attachment_status display in DocumentItem (PARSING/FAILED indicators) - Add i18n translations for async upload flow Usage flow: 1. Try sync upload first (default) 2. On 504 timeout, show \"Async Submit\" button 3. Click to retry with parse_async=true 4. PARSING status shown in KB list with yellow indicator 5. FAILED status shown with red indicator and error message Closes: async attachment parsing requirement
Contributor
📝 WalkthroughWalkthroughAdds optional asynchronous attachment parsing: upload endpoints accept Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Endpoint as API Endpoint
participant Service as ContextService
participant Background as BackgroundTask
participant DB as Database
participant Parser as DocumentParser
Client->>Endpoint: POST /api/attachments/upload?parse_async=true
Endpoint->>Service: upload_attachment(file, async_parse=True, background_tasks)
Service->>DB: create context/set status=PARSING, commit
Service->>Background: add_task(_parse_attachment_background, context_id, user_id, data, ext)
Service-->>Endpoint: return context (status=PARSING)
Endpoint-->>Client: respond with attachment PARSING status
Background->>DB: open new session, fetch context
Background->>Parser: parse(binary_data, extension)
alt parse succeeds
Parser-->>Background: extracted_text, images
Background->>DB: update context (READY, extracted_text, text_length), commit
else parse fails
Parser-->>Background: raise DocumentParseError
Background->>DB: update context (FAILED, error_message), commit
end
Background->>DB: close session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/services/knowledge/test_orchestrator.py (1)
451-512:⚠️ Potential issue | 🟡 MinorAssert the new attachment fields in result objects.
Line 465–466 and Line 634–635 add fixture fields, but the tests never assert
result.attachment_status/result.attachment_error_message. That weakens regression protection for the new contract.✅ Suggested test assertion patch
@@ assert result.truncated is True assert result.summary == {"summary": "hello"} + assert result.attachment_status is None + assert result.attachment_error_message is None @@ assert result.truncated is True + assert result.attachment_status is None + assert result.attachment_error_message is NoneAlso applies to: 620-668
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/services/knowledge/test_orchestrator.py` around lines 451 - 512, The test test_get_document_detail_maps_content_length_and_truncated (and the similar test around lines 620-668) builds a paged object that includes attachment_status and attachment_error_message but never asserts them; update these tests to assert that the returned result includes result.attachment_status and result.attachment_error_message with the expected values coming from the paged fixture (e.g., None in this case). Locate assertions after the existing content/summary checks in test_get_document_detail_maps_content_length_and_truncated and add two assertions verifying result.attachment_status == paged.attachment_status and result.attachment_error_message == paged.attachment_error_message (and mirror the same additions in the other test).
🧹 Nitpick comments (7)
backend/app/services/rag/local_data_plane/indexing.py (1)
66-70: Tighten the log wording to match execution order.Line 67 says binary data is available before the fetch/check at Line 72–77. Consider wording this as an attempted fetch to avoid misleading diagnostics.
📝 Suggested wording change
- "Indexing attachment context %s with non-ready status %s because binary data is available", + "Attachment context %s has non-ready status %s; attempting binary fetch for indexing",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/rag/local_data_plane/indexing.py` around lines 66 - 70, The log message currently states "binary data is available" prematurely; update the logger.info call that logs attachment_id and context.status to indicate that a fetch/attempt to retrieve binary data is being performed (e.g., "Attempting to fetch binary data for attachment %s with status %s") so it accurately reflects the subsequent fetch/check block that verifies binary presence.frontend/src/types/knowledge.ts (1)
174-177: Tightenattachment_statustyping to a literal union.
string | nullis too loose here and bypasses strict-mode safety for status-driven UI logic.♻️ Suggested refactor
+export type AttachmentParsingStatus = 'uploading' | 'parsing' | 'ready' | 'failed' + export interface KnowledgeDocument { @@ - attachment_status?: string | null + attachment_status?: AttachmentParsingStatus | null @@ } export interface DocumentDetailResponse { @@ - attachment_status?: string | null + attachment_status?: AttachmentParsingStatus | null }As per coding guidelines,
**/*.{ts,tsx}code MUST use TypeScript strict mode.Also applies to: 287-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/types/knowledge.ts` around lines 174 - 177, The attachment_status field currently typed as "string | null" is too loose; change it to a literal union of allowed statuses such as "uploading" | "parsing" | "ready" | "failed" | null in the type/interface that declares attachment_status (look for the attachment_status property in frontend/src/types/knowledge.ts), and make the same change for the similar occurrence noted at lines 287-290 so all status-driven fields use the strict literal union to satisfy TypeScript strict-mode and prevent invalid values in UI logic.backend/app/services/context/context_service.py (3)
344-348: Remove redundant import.
osis already imported at module level (line 13). This inline import is unnecessary.🧹 Remove redundant import
- import os - # Validate async requirements if async_parse and background_tasks is None: raise ValueError("background_tasks is required for async parsing")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/context/context_service.py` around lines 344 - 348, Remove the redundant inline "import os" inside the validation block; since os is already imported at the module level, delete that import line and leave the async requirements check (the if block that raises ValueError when async_parse is true and background_tasks is None) unchanged so only the unnecessary import is removed.
1629-1629: Minor: Use f-string conversion flag.Per static analysis hint, prefer
{e!s}over{str(e)}for explicit string conversion.🧹 Use conversion flag
- error_message = f"Parse failed: {str(e)}" + error_message = f"Parse failed: {e!s}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/context/context_service.py` at line 1629, Replace the explicit str() call in the exception formatting with the f-string conversion flag: locate the assignment to error_message that currently builds the message using f"Parse failed: {str(e)}" and change it to use f"Parse failed: {e!s}" so the exception is converted to string via the conversion flag (in the context of context_service.py where error_message is set).
319-320: Consider adding type hint forbackground_tasks.While the current code works, adding a type hint would improve IDE support and documentation.
📝 Add type hint
+from typing import Any, Dict, List, Optional, Tuple, Union, TYPE_CHECKING + +if TYPE_CHECKING: + from fastapi import BackgroundTasks + ... def upload_attachment( self, db: Session, user_id: int, filename: str, binary_data: bytes, subtask_id: int = 0, async_parse: bool = False, - background_tasks=None, + background_tasks: Optional["BackgroundTasks"] = None, ) -> Tuple[SubtaskContext, Optional[TruncationInfo]]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/context/context_service.py` around lines 319 - 320, The parameter background_tasks in the function signature is missing a type hint; update it to use Optional[BackgroundTasks] (e.g., background_tasks: Optional[BackgroundTasks] = None) and add the necessary imports (from typing import Optional and from fastapi.background import BackgroundTasks) so IDEs and linters recognize the type; apply this change where the parameter is declared (background_tasks) in context_service.py.backend/app/api/endpoints/adapter/attachments.py (1)
12-12: Unused import:osThis import was added but is not used in this file. The
os.path.splitextcall happens incontext_service.py, not here.🧹 Remove unused import
import logging -import os from typing import List, Optional🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/endpoints/adapter/attachments.py` at line 12, Remove the unused import "os" from the top of the module (it's not referenced in this file; os.path.splitext is used in context_service.py), so delete the line importing os to clean up the module and avoid linter warnings.frontend/src/hooks/useBatchAttachment.ts (1)
360-392: Consider adding status guard for consistency.Unlike
retryFile(line 312),retryFileAsyncdoesn't verify that the file is in'error'status before retrying. While the UI currently only shows the async retry button for error states, adding the guard would make the function more robust against misuse.♻️ Optional: Add status check for consistency
const retryFileAsync = useCallback( async (id: string) => { const fileItem = state.files.find(f => f.id === id) - if (!fileItem) return + if (!fileItem || fileItem.status !== 'error') return // Reset the file status to pending🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useBatchAttachment.ts` around lines 360 - 392, The retryFileAsync function should guard against non-error states like retryFile does: locate retryFileAsync and add an early return that checks the found fileItem's status (e.g., if (fileItem.status !== 'error') return), so you only reset and call uploadSingleFile for files in the 'error' state; keep the rest of the logic (setState resetting status, building resetFileItem, calling uploadSingleFile, and updating state with result) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/tests/services/knowledge/test_document_read_service.py`:
- Around line 28-30: The fixtures create SimpleNamespace entries with
status="READY" which doesn't match runtime lowercase statuses; update those
fixtures (the SimpleNamespace entries for ids 101, 102, 201 and any other places
using "READY") to use the enum-backed value instead—import the DocumentStatus
enum used by the code and set status to DocumentStatus.READY.value (or
DocumentStatus.READY.name.lower()) so the test fixtures use the same
enum-aligned lowercase status as production.
---
Outside diff comments:
In `@backend/tests/services/knowledge/test_orchestrator.py`:
- Around line 451-512: The test
test_get_document_detail_maps_content_length_and_truncated (and the similar test
around lines 620-668) builds a paged object that includes attachment_status and
attachment_error_message but never asserts them; update these tests to assert
that the returned result includes result.attachment_status and
result.attachment_error_message with the expected values coming from the paged
fixture (e.g., None in this case). Locate assertions after the existing
content/summary checks in
test_get_document_detail_maps_content_length_and_truncated and add two
assertions verifying result.attachment_status == paged.attachment_status and
result.attachment_error_message == paged.attachment_error_message (and mirror
the same additions in the other test).
---
Nitpick comments:
In `@backend/app/api/endpoints/adapter/attachments.py`:
- Line 12: Remove the unused import "os" from the top of the module (it's not
referenced in this file; os.path.splitext is used in context_service.py), so
delete the line importing os to clean up the module and avoid linter warnings.
In `@backend/app/services/context/context_service.py`:
- Around line 344-348: Remove the redundant inline "import os" inside the
validation block; since os is already imported at the module level, delete that
import line and leave the async requirements check (the if block that raises
ValueError when async_parse is true and background_tasks is None) unchanged so
only the unnecessary import is removed.
- Line 1629: Replace the explicit str() call in the exception formatting with
the f-string conversion flag: locate the assignment to error_message that
currently builds the message using f"Parse failed: {str(e)}" and change it to
use f"Parse failed: {e!s}" so the exception is converted to string via the
conversion flag (in the context of context_service.py where error_message is
set).
- Around line 319-320: The parameter background_tasks in the function signature
is missing a type hint; update it to use Optional[BackgroundTasks] (e.g.,
background_tasks: Optional[BackgroundTasks] = None) and add the necessary
imports (from typing import Optional and from fastapi.background import
BackgroundTasks) so IDEs and linters recognize the type; apply this change where
the parameter is declared (background_tasks) in context_service.py.
In `@backend/app/services/rag/local_data_plane/indexing.py`:
- Around line 66-70: The log message currently states "binary data is available"
prematurely; update the logger.info call that logs attachment_id and
context.status to indicate that a fetch/attempt to retrieve binary data is being
performed (e.g., "Attempting to fetch binary data for attachment %s with status
%s") so it accurately reflects the subsequent fetch/check block that verifies
binary presence.
In `@frontend/src/hooks/useBatchAttachment.ts`:
- Around line 360-392: The retryFileAsync function should guard against
non-error states like retryFile does: locate retryFileAsync and add an early
return that checks the found fileItem's status (e.g., if (fileItem.status !==
'error') return), so you only reset and call uploadSingleFile for files in the
'error' state; keep the rest of the logic (setState resetting status, building
resetFileItem, calling uploadSingleFile, and updating state with result)
unchanged.
In `@frontend/src/types/knowledge.ts`:
- Around line 174-177: The attachment_status field currently typed as "string |
null" is too loose; change it to a literal union of allowed statuses such as
"uploading" | "parsing" | "ready" | "failed" | null in the type/interface that
declares attachment_status (look for the attachment_status property in
frontend/src/types/knowledge.ts), and make the same change for the similar
occurrence noted at lines 287-290 so all status-driven fields use the strict
literal union to satisfy TypeScript strict-mode and prevent invalid values in UI
logic.
🪄 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: 039ab6bf-3e8b-428a-8342-5d2ca7e4358d
📒 Files selected for processing (20)
backend/app/api/endpoints/adapter/attachments.pybackend/app/schemas/knowledge.pybackend/app/services/context/context_service.pybackend/app/services/knowledge/document_read_service.pybackend/app/services/knowledge/orchestrator.pybackend/app/services/rag/local_data_plane/indexing.pybackend/tests/api/endpoints/test_knowledge_document_detail_endpoints.pybackend/tests/services/knowledge/test_document_read_service.pybackend/tests/services/knowledge/test_orchestrator.pybackend/tests/services/rag/test_local_data_plane_indexing.pybackend/tests/services/test_context_service.pyfrontend/src/__tests__/features/knowledge/document/document-detail-dialog.test.tsxfrontend/src/apis/attachments.tsfrontend/src/features/knowledge/document/components/DocumentDetailDialog.tsxfrontend/src/features/knowledge/document/components/DocumentItem.tsxfrontend/src/features/knowledge/document/components/DocumentUpload.tsxfrontend/src/hooks/useBatchAttachment.tsfrontend/src/i18n/locales/en/knowledge.jsonfrontend/src/i18n/locales/zh-CN/knowledge.jsonfrontend/src/types/knowledge.ts
Comment on lines
+28
to
+30
| 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status="READY", error_message=""), | ||
| 102: SimpleNamespace(id=102, extracted_text="lmnopqrstuv", status="READY", error_message=""), | ||
| 201: SimpleNamespace(id=201, extracted_text="wxyz0123456", status="READY", error_message=""), |
Contributor
There was a problem hiding this comment.
Use enum-aligned status values in fixtures.
Line 28–30, Line 100–101, and Line 171 use "READY"; runtime status values are lowercase ("ready"). Prefer enum values to keep tests contract-accurate.
🧪 Suggested fixture update
+from app.models.subtask_context import ContextStatus
from app.services.context.context_service import context_service
@@
- 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status="READY", error_message=""),
- 102: SimpleNamespace(id=102, extracted_text="lmnopqrstuv", status="READY", error_message=""),
- 201: SimpleNamespace(id=201, extracted_text="wxyz0123456", status="READY", error_message=""),
+ 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status=ContextStatus.READY.value, error_message=""),
+ 102: SimpleNamespace(id=102, extracted_text="lmnopqrstuv", status=ContextStatus.READY.value, error_message=""),
+ 201: SimpleNamespace(id=201, extracted_text="wxyz0123456", status=ContextStatus.READY.value, error_message=""),
@@
- 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status="READY", error_message=""),
- 201: SimpleNamespace(id=201, extracted_text="lmnopqrstuv", status="READY", error_message=""),
+ 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status=ContextStatus.READY.value, error_message=""),
+ 201: SimpleNamespace(id=201, extracted_text="lmnopqrstuv", status=ContextStatus.READY.value, error_message=""),
@@
- 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status="READY", error_message=""),
+ 101: SimpleNamespace(id=101, extracted_text="abcdefghijk", status=ContextStatus.READY.value, error_message=""),Also applies to: 100-101, 171-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/services/knowledge/test_document_read_service.py` around lines
28 - 30, The fixtures create SimpleNamespace entries with status="READY" which
doesn't match runtime lowercase statuses; update those fixtures (the
SimpleNamespace entries for ids 101, 102, 201 and any other places using
"READY") to use the enum-backed value instead—import the DocumentStatus enum
used by the code and set status to DocumentStatus.READY.value (or
DocumentStatus.READY.name.lower()) so the test fixtures use the same
enum-aligned lowercase status as production.
kissghosts
force-pushed
the
feat/async-attachment-parsing
branch
from
April 10, 2026 14:02
74bba8d to
5a80a3f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/services/context/context_service.py (1)
319-320: Type the new scheduler dependency explicitly.
background_tasksis part of the method contract now, but it is left untyped even though the code immediately calls.add_task(...)on it.BackgroundTasks | Noneor a small protocol would make the contract clear and satisfy the repo’s Python typing rule.As per coding guidelines "Python code MUST follow PEP 8, Black formatter (line length: 88), and isort standards with type hints required".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/context/context_service.py` around lines 319 - 320, The background_tasks parameter in the function signature is untyped but immediately used with .add_task(), so update the signature to include an explicit type such as background_tasks: BackgroundTasks | None (or Optional[BackgroundTasks]) and import BackgroundTasks from starlette.background or fastapi.background; alternatively define a small Protocol with add_task(self, func: Callable[..., Any], *args: Any, **kwargs: Any) -> None and type background_tasks against that Protocol. Ensure the annotation is applied where async_parse: bool = False, background_tasks=None appears (the function in context_service.py that calls .add_task) and update imports accordingly to satisfy typing rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/context/context_service.py`:
- Around line 390-404: The async parse scheduling must attach and verify a
per-parse generation token to avoid stale writes: when setting context.status =
ContextStatus.PARSING.value and calling
background_tasks.add_task(_parse_attachment_background, ...), persist a new
generation/token on the Context row (e.g., context.parse_generation = uuid or
incrementing int), include that token in the add_task args, and in
_parse_attachment_background verify the token against the current
Context.parse_generation before applying extracted_text/status updates; also
ensure overwrite_attachment(...) and any retry flows rotate/increment the
generation so older jobs self-discard if tokens mismatch.
---
Nitpick comments:
In `@backend/app/services/context/context_service.py`:
- Around line 319-320: The background_tasks parameter in the function signature
is untyped but immediately used with .add_task(), so update the signature to
include an explicit type such as background_tasks: BackgroundTasks | None (or
Optional[BackgroundTasks]) and import BackgroundTasks from starlette.background
or fastapi.background; alternatively define a small Protocol with add_task(self,
func: Callable[..., Any], *args: Any, **kwargs: Any) -> None and type
background_tasks against that Protocol. Ensure the annotation is applied where
async_parse: bool = False, background_tasks=None appears (the function in
context_service.py that calls .add_task) and update imports accordingly to
satisfy typing rules.
🪄 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: 310a2ed8-cd03-4613-83b5-d47ef521aff1
📒 Files selected for processing (8)
backend/app/services/context/context_service.pybackend/app/services/rag/local_data_plane/indexing.pybackend/tests/services/rag/test_local_data_plane_indexing.pybackend/tests/services/test_context_service.pyfrontend/src/__tests__/features/knowledge/document/document-detail-dialog.test.tsxfrontend/src/features/knowledge/document/components/DocumentDetailDialog.tsxfrontend/src/i18n/locales/en/knowledge.jsonfrontend/src/i18n/locales/zh-CN/knowledge.json
✅ Files skipped from review due to trivial changes (1)
- backend/tests/services/test_context_service.py
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/tests/services/rag/test_local_data_plane_indexing.py
- frontend/src/features/knowledge/document/components/DocumentDetailDialog.tsx
- frontend/src/i18n/locales/en/knowledge.json
- frontend/src/i18n/locales/zh-CN/knowledge.json
- frontend/src/tests/features/knowledge/document/document-detail-dialog.test.tsx
Comment on lines
+390
to
+404
| if async_parse: | ||
| # Async mode: set PARSING status and schedule background task | ||
| context.status = ContextStatus.PARSING.value | ||
| db.commit() | ||
| db.refresh(context) | ||
|
|
||
| # Schedule background parsing | ||
| _, ext = os.path.splitext(filename) | ||
| background_tasks.add_task( | ||
| _parse_attachment_background, | ||
| context_id=context.id, | ||
| user_id=user_id, | ||
| binary_data=binary_data, | ||
| extension=ext.lower(), | ||
| ) |
Contributor
There was a problem hiding this comment.
Protect the async writeback from stale parse jobs.
The background worker updates by context_id alone. If this record is overwritten or retried while the first async parse is still running, the older job can commit stale extracted_text/status onto the newer binary and leave the row internally inconsistent. Please persist a per-parse generation/token when scheduling, rotate it on overwrite/retry, and check it before applying the final update.
🔒 Minimal shape of the fix
if async_parse:
+ parse_generation = (
+ (context.type_data or {}).get("parse_generation", 0) + 1
+ )
+ context.type_data = {
+ **(context.type_data or {}),
+ "parse_generation": parse_generation,
+ }
context.status = ContextStatus.PARSING.value
db.commit()
db.refresh(context)
background_tasks.add_task(
_parse_attachment_background,
context_id=context.id,
user_id=user_id,
+ parse_generation=parse_generation,
binary_data=binary_data,
extension=ext.lower(),
) def _parse_attachment_background(
context_id: int,
user_id: int,
+ parse_generation: int,
binary_data: bytes,
extension: str,
) -> None:
...
if not context:
logger.error(f"Context {context_id} not found for async parsing")
return
+ current_generation = (context.type_data or {}).get("parse_generation", 0)
+ if current_generation != parse_generation:
+ logger.info(
+ "Skipping stale async parse result for context %s", context_id
+ )
+ return
context.extracted_text = extracted_textAlso bump the same generation in overwrite_attachment(...) and any retry flow so older jobs self-discard.
Also applies to: 1635-1654
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/context/context_service.py` around lines 390 - 404, The
async parse scheduling must attach and verify a per-parse generation token to
avoid stale writes: when setting context.status = ContextStatus.PARSING.value
and calling background_tasks.add_task(_parse_attachment_background, ...),
persist a new generation/token on the Context row (e.g.,
context.parse_generation = uuid or incrementing int), include that token in the
add_task args, and in _parse_attachment_background verify the token against the
current Context.parse_generation before applying extracted_text/status updates;
also ensure overwrite_attachment(...) and any retry flows rotate/increment the
generation so older jobs self-discard if tokens mismatch.
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
2 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.
Summary by CodeRabbit
New Features
Enhancements