Skip to content

Commit 1aa7919

Browse files
authored
Merge branch 'main' into chore/type-safety-llm-calls
2 parents a8f9594 + efd2155 commit 1aa7919

18 files changed

Lines changed: 1117 additions & 144 deletions

File tree

.claude/agents/senior-engineer.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,19 @@ to what the task needs.
7979
- Naming: `list_*` plural fetch, `get_*` singleton; `Enum` suffix on enum classes.
8080
- Timestamps are `inserted_at` / `updated_at`, never `created_at`.
8181

82+
## Before finishing: type-check
83+
84+
After editing any file**always when you touched `app/models/`**type-check the files you changed:
85+
86+
```bash
87+
cd backend && bash scripts/pyright.sh <changed files> # e.g. app/models/foo.py
88+
```
89+
90+
Runs pyright via `uvx` (no `uv.lock` churn; config in `[tool.pyright]`). Fix real type errors you
91+
introduced before emitting the summary; a model change ripples into crud/service typing, so re-check
92+
those files too if you edited them. Distinguish real bugs from pre-existing stub noise — don't fix
93+
what you didn't touch. Same as the `/typecheck` command.
94+
8295
## After building
8396

8497
Emit ONE summary (not one per layer):

.claude/commands/typecheck.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
description: Run pyright type checking on Python files and triage the errors.
3+
---
4+
5+
Run pyright over the backend and report type errors in this FastAPI + SQLModel service.
6+
7+
## Run
8+
9+
From the repo root:
10+
11+
```bash
12+
cd backend && bash scripts/pyright.sh $ARGUMENTS
13+
```
14+
15+
- No `$ARGUMENTS` → checks all of `app/` (config in `backend/pyproject.toml` `[tool.pyright]`).
16+
- `$ARGUMENTS` → narrow to those paths/files, e.g. `/typecheck app/services/response`.
17+
- Pyright runs via `uvx` (ephemeral) so it never mutates `uv.lock`.
18+
19+
## Report
20+
21+
1. Group errors by file, most-affected first.
22+
2. For each: `file:line` — the error, then the concrete fix (narrow the type, add an annotation, guard the `None`, etc.).
23+
3. Separate real type bugs from noise (missing third-party stubs, `# type: ignore` candidates). Flag which is which.
24+
4. Do NOT auto-edit code — surface findings and let the user pick what to fix. If asked to fix, follow the layer conventions in `.claude/conventions/`.

backend/app/api/routes/documents.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
from app.services.documents.helpers import (
3434
calculate_file_size,
3535
schedule_transformation,
36-
pre_transform_validation,
36+
validate_upload,
3737
build_document_schema,
3838
build_document_schemas,
3939
)
@@ -126,8 +126,8 @@ async def upload_doc(
126126
if callback_url:
127127
validate_callback_url(callback_url)
128128

129-
source_format, actual_transformer = pre_transform_validation(
130-
src_filename=src.filename,
129+
source_format, actual_transformer = validate_upload(
130+
src=src,
131131
target_format=target_format,
132132
transformer=transformer,
133133
)

backend/app/crud/rag/open_ai.py

Lines changed: 127 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,47 @@
11
import json
22
import logging
3+
import time
34
import functools as ft
45

56
import openai
67
from openai import OpenAI, OpenAIError
8+
from openai.types import VectorStore
9+
from openai.types.vector_stores import VectorStoreFileBatch
710
from pydantic import BaseModel
11+
from tenacity import (
12+
RetryCallState,
13+
retry,
14+
retry_if_exception_type,
15+
stop_after_attempt,
16+
wait_exponential,
17+
)
818

919
from app.models import Document, ProviderType
1020

1121
logger = logging.getLogger(__name__)
1222

1323
OPENAI_PROVIDER = ProviderType.openai.value
1424

25+
# Under the Celery soft time limit so a hung call can't eat the whole task window.
26+
# SDK-level retries are off (registry.py: max_retries=0); tenacity is the sole
27+
# retry layer, wrapping batch create+index.
28+
OPENAI_TIMEOUT_SECONDS = 30
29+
30+
BATCH_POLL_INTERVAL_SECONDS = 2
31+
32+
# Retry batch create+index on any OpenAI/indexing failure, exponential backoff
33+
# (~2s, 4s, 8s), all inside one Celery soft-time-limit window.
34+
BATCH_INDEX_MAX_ATTEMPTS = 4
35+
BATCH_RETRY_BACKOFF_BASE_SECONDS = 2
36+
37+
38+
def _log_batch_retry(retry_state: RetryCallState) -> None:
39+
logger.warning(
40+
f"[OpenAIVectorStoreCrud._create_and_index_batch] Batch attempt failed, retrying | "
41+
f"attempt={retry_state.attempt_number}, "
42+
f"error={retry_state.outcome.exception() if retry_state.outcome else None}"
43+
)
44+
1545

1646
def vs_ls(client: OpenAI, vector_store_id: str):
1747
kwargs = {}
@@ -85,7 +115,7 @@ def __init__(self, client):
85115

86116

87117
class OpenAIVectorStoreCrud(OpenAICrud):
88-
def create(self):
118+
def create(self) -> VectorStore:
89119
logger.info(
90120
f"[OpenAIVectorStoreCrud.create] Creating vector store | {{'action': 'create'}}"
91121
)
@@ -101,6 +131,100 @@ def read(self, vector_store_id: str):
101131
)
102132
yield from vs_ls(self.client, vector_store_id)
103133

134+
def _create_file_batch(self, vector_store_id: str, file_ids: list[str]) -> str:
135+
"""Returns the vsfb_ id. poll()'s return deserializes a vector-store body,
136+
so its .id is the vs_ id - take the batch id from create()."""
137+
created = self.client.vector_stores.file_batches.create(
138+
vector_store_id=vector_store_id,
139+
file_ids=file_ids,
140+
)
141+
return created.id
142+
143+
def _retrieve_file_batch(
144+
self, batch_id: str, vector_store_id: str
145+
) -> VectorStoreFileBatch:
146+
return self.client.vector_stores.file_batches.retrieve(
147+
batch_id, vector_store_id=vector_store_id
148+
)
149+
150+
def _poll_file_batch(
151+
self, batch_id: str, vector_store_id: str
152+
) -> VectorStoreFileBatch:
153+
"""Poll until indexing finishes; the Celery soft time limit is the deadline."""
154+
while True:
155+
batch = self._retrieve_file_batch(batch_id, vector_store_id)
156+
if batch.status != "in_progress":
157+
return batch
158+
time.sleep(BATCH_POLL_INTERVAL_SECONDS)
159+
160+
def _raise_if_batch_incomplete(
161+
self,
162+
batch: VectorStoreFileBatch,
163+
batch_id: str,
164+
vector_store_id: str,
165+
docs: list[Document],
166+
) -> None:
167+
"""Raise on any indexing failure so the batch attempt is retried."""
168+
if batch.file_counts.failed > 0:
169+
try:
170+
failed_files = self.client.vector_stores.file_batches.list_files(
171+
vector_store_id=vector_store_id,
172+
batch_id=batch_id,
173+
filter="failed",
174+
)
175+
doc_by_file_id = {d.file_id[OPENAI_PROVIDER]: d for d in docs}
176+
parts = []
177+
for f in failed_files:
178+
d = doc_by_file_id.get(f.id)
179+
label = d.fname if d else f.id
180+
msg = f.last_error.message if f.last_error else "no error detail"
181+
parts.append(f"{label}: {msg}")
182+
logger.error(
183+
f"[OpenAIVectorStoreCrud._raise_if_batch_incomplete] Files failed to index | "
184+
f"{{'batch_id': '{batch_id}', 'failed_files': '{', '.join(parts)}'}}"
185+
)
186+
raise RuntimeError("; ".join(parts))
187+
except OpenAIError as err:
188+
logger.warning(
189+
f"[OpenAIVectorStoreCrud._raise_if_batch_incomplete] Could not fetch per-file errors | "
190+
f"{{'batch_id': '{batch_id}', 'error': '{str(err)}'}}"
191+
)
192+
raise
193+
194+
# Only 'completed' is success; a 'cancelled'/'failed' batch with no per-file
195+
# failures slips past the failed-count check above.
196+
if batch.status != "completed":
197+
error_message = (
198+
f"[OPENAI] Vector store indexing did not complete "
199+
f"(status: {batch.status}). Retry the collection."
200+
)
201+
logger.error(
202+
f"[OpenAIVectorStoreCrud._raise_if_batch_incomplete] {error_message} | "
203+
f"vector_store_id={vector_store_id}, batch_id={batch_id}, "
204+
f"status={batch.status}"
205+
)
206+
raise RuntimeError(error_message)
207+
208+
@retry(
209+
reraise=True,
210+
stop=stop_after_attempt(BATCH_INDEX_MAX_ATTEMPTS),
211+
wait=wait_exponential(multiplier=BATCH_RETRY_BACKOFF_BASE_SECONDS),
212+
retry=retry_if_exception_type((OpenAIError, RuntimeError)),
213+
before_sleep=_log_batch_retry,
214+
)
215+
def _create_and_index_batch(
216+
self, vector_store_id: str, docs: list[Document]
217+
) -> tuple[VectorStoreFileBatch, str]:
218+
"""Create the file batch, wait for indexing, verify it completed. Retried as
219+
a unit on any OpenAI/indexing failure; SoftTimeLimitExceeded is not retried
220+
(not an OpenAIError/RuntimeError) so it aborts the task inside the window."""
221+
batch_id = self._create_file_batch(
222+
vector_store_id, [doc.file_id[OPENAI_PROVIDER] for doc in docs]
223+
)
224+
batch = self._poll_file_batch(batch_id, vector_store_id)
225+
self._raise_if_batch_incomplete(batch, batch_id, vector_store_id, docs)
226+
return batch, batch_id
227+
104228
def update(
105229
self,
106230
vector_store_id: str,
@@ -115,11 +239,7 @@ def update(
115239
)
116240

117241
try:
118-
batch = self.client.vector_stores.file_batches.upload_and_poll(
119-
vector_store_id=vector_store_id,
120-
files=[],
121-
file_ids=[doc.file_id[OPENAI_PROVIDER] for doc in docs],
122-
)
242+
batch, batch_id = self._create_and_index_batch(vector_store_id, docs)
123243
except openai.RateLimitError as e:
124244
error_message = (
125245
f"[OPENAI] Rate limit exceeded (code: {e.status_code}): "
@@ -215,30 +335,9 @@ def update(
215335

216336
logger.info(
217337
f"[OpenAIVectorStoreCrud.update] Batch complete | "
218-
f"{{'vector_store_id': '{vector_store_id}', "
338+
f"{{'vector_store_id': '{vector_store_id}', 'batch_id': '{batch_id}', "
219339
f"'completed': {batch.file_counts.completed}, 'failed': {batch.file_counts.failed}}}"
220340
)
221-
if batch.file_counts.failed > 0:
222-
try:
223-
failed_files = self.client.vector_stores.file_batches.list_files(
224-
vector_store_id=vector_store_id,
225-
batch_id=batch.id,
226-
filter="failed",
227-
)
228-
doc_by_file_id = {d.file_id[OPENAI_PROVIDER]: d for d in docs}
229-
parts = []
230-
for f in failed_files:
231-
d = doc_by_file_id.get(f.id)
232-
label = d.fname if d else f.id
233-
msg = f.last_error.message if f.last_error else "no error detail"
234-
parts.append(f"{label}: {msg}")
235-
raise RuntimeError("; ".join(parts))
236-
except OpenAIError as err:
237-
logger.warning(
238-
f"[OpenAIVectorStoreCrud.update] Could not fetch per-file errors | "
239-
f"{{'batch_id': '{batch.id}', 'error': '{str(err)}'}}"
240-
)
241-
raise
242341

243342
def delete(self, vector_store_id: str, retries: int = 3):
244343
if retries < 1:

backend/app/services/collections/create_collection.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -403,9 +403,11 @@ def execute_batch_job(
403403
collection_job_crud = CollectionJobCrud(session, project_id)
404404
collection_job = collection_job_crud.read_one(job_uuid)
405405
already_uploaded = collection_job.documents_uploaded or []
406-
now_uploaded = already_uploaded + [
407-
str(d) for d in all_doc_ids_this_batch
408-
]
406+
now_uploaded = list(
407+
dict.fromkeys(
408+
already_uploaded + [str(d) for d in all_doc_ids_this_batch]
409+
)
410+
)
409411

410412
collection_job = collection_job_crud.update(
411413
job_uuid,
@@ -507,7 +509,10 @@ def execute_batch_job(
507509
webhook_secret=webhook_secret,
508510
)
509511

510-
except (Timeout, SoftTimeLimitExceeded) as err:
512+
except (Timeout, SoftTimeLimitExceeded):
513+
# Batch-level retries happen in-task (tenacity in OpenAIVectorStoreCrud),
514+
# so hitting the soft time limit means the window is spent — fail, don't
515+
# re-queue.
511516
timeout_err = TimeoutError("Task exceeded soft time limit")
512517
logger.warning(
513518
"[create_collection.execute_batch_job] Collection Creation Timed Out | {'collection_job_id': '%s', 'error': '%s'}",

backend/app/services/collections/providers/registry.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from openai import OpenAI
66

77
from app.crud import get_provider_credential
8+
from app.crud.rag.open_ai import OPENAI_TIMEOUT_SECONDS
89
from app.services.collections.providers.base import BaseProvider
910
from app.services.collections.providers.gemini import GeminiAIStudioProvider
1011
from app.services.collections.providers.openai import OpenAIProvider
@@ -63,7 +64,11 @@ def get_llm_provider(
6364
if provider == LLMProvider.OPENAI:
6465
if "api_key" not in credentials:
6566
raise ValueError("OpenAI credentials not configured for this project.")
66-
client = OpenAI(api_key=credentials["api_key"])
67+
client = OpenAI(
68+
api_key=credentials["api_key"],
69+
max_retries=0,
70+
timeout=OPENAI_TIMEOUT_SECONDS,
71+
)
6772
elif provider == LLMProvider.GOOGLE_AISTUDIO:
6873
if "api_key" not in credentials:
6974
raise ValueError(

backend/app/services/doctransform/registry.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ class TransformationError(Exception):
3939
".markdown": "markdown",
4040
".csv": "csv",
4141
".json": "json",
42+
".xlsx": "xlsx",
43+
".xls": "xls",
4244
}
4345

4446
# Map format names to file extensions
@@ -51,6 +53,8 @@ class TransformationError(Exception):
5153
"markdown": ".md",
5254
"csv": ".csv",
5355
"json": ".json",
56+
"xlsx": ".xlsx",
57+
"xls": ".xls",
5458
}
5559

5660

backend/app/services/documents/helpers.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from typing import Optional, Tuple, Iterable, Union
23
from uuid import UUID
34

@@ -11,6 +12,10 @@
1112
)
1213
from app.crud import DocTransformationJobCrud, DocumentCrud
1314
from app.services.doctransform import job as transformation_job
15+
from app.services.documents.validator import (
16+
DocumentValidationError,
17+
validate_document_content,
18+
)
1419
from app.models import (
1520
DocTransformJobCreate,
1621
TransformationStatus,
@@ -23,6 +28,39 @@
2328
)
2429

2530

31+
logger = logging.getLogger(__name__)
32+
33+
34+
def validate_upload(
35+
*,
36+
src: UploadFile,
37+
target_format: str | None,
38+
transformer: str | None,
39+
) -> Tuple[str, str | None]:
40+
"""
41+
Full pre-storage gate: extension and transformer validation plus a content
42+
sanity check. Returns (source_format, actual_transformer_or_none).
43+
44+
Raises: HTTPException(400) on client errors.
45+
"""
46+
source_format, actual_transformer = pre_transform_validation(
47+
src_filename=src.filename,
48+
target_format=target_format,
49+
transformer=transformer,
50+
)
51+
52+
try:
53+
validate_document_content(file=src, source_format=source_format)
54+
except DocumentValidationError as e:
55+
logger.warning(
56+
f"[validate_upload] Document failed sanity check | "
57+
f"filename: {e.filename} | format: {source_format} | reason: {e.reason}"
58+
)
59+
raise HTTPException(status_code=400, detail=e.client_message)
60+
61+
return source_format, actual_transformer
62+
63+
2664
def calculate_file_size(file: UploadFile) -> float:
2765
"""
2866
Calculate the size of an uploaded file in kilobytes.

0 commit comments

Comments
 (0)