Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion backend/app/services/collections/providers/openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,14 @@ def upload_files(
if doc.file_size_kb is None:
doc.file_size_kb = round(tmp.tell() / 1024, 2)
tmp.seek(0)
if "." in doc.fname:
filename, extension = doc.fname.rsplit(".", 1)
normalized_fname = f"{filename}.{extension.lower()}"
else:
normalized_fname = doc.fname
uploaded = self.client.files.create(
file=(doc.fname, tmp), purpose="assistants"
file=(normalized_fname, tmp),
purpose="assistants",
)
except Exception as err:
logger.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,15 +296,25 @@ def test_upload_files_empty_docs_is_noop() -> None:
client.files.create.assert_not_called()


def test_upload_files_file_name_matches_doc_fname() -> None:
"""The file tuple passed to OpenAI must carry the original filename."""
@pytest.mark.parametrize(
("fname", "expected"),
[
("report.pdf", "report.pdf"),
("Report.PDF", "Report.pdf"),
("README", "README"),
],
)
def test_upload_files_file_name_lowercases_only_extension(
fname: str, expected: str
) -> None:
"""The file tuple passed to OpenAI keeps the stem's case, lowercases the extension."""
client = MagicMock()
client.files.create.return_value = MagicMock(id="file-abc")
provider = OpenAIProvider(client=client)

storage = _make_storage(b"data")
doc = _make_doc(file_size_kb=1.0)
doc.fname = "report.pdf"
doc.fname = fname

session_p, crud_p = _patch_session_and_crud()
with session_p as MockSession, crud_p:
Expand All @@ -313,8 +323,8 @@ def test_upload_files_file_name_matches_doc_fname() -> None:
provider.upload_files(storage, [doc], project_id=1)

_, kwargs = client.files.create.call_args
fname, _ = kwargs["file"]
assert fname == "report.pdf"
uploaded_fname, _ = kwargs["file"]
assert uploaded_fname == expected


def test_upload_files_raises_on_db_update_failure() -> None:
Expand Down
1 change: 1 addition & 0 deletions docs/wiki/modules/knowledge-base.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ All paths relative to `backend/app/`.
## Gotchas
- `signed_url` behaviour is chosen per request by the `download` query param (default false = inline, as before). `put()` stores the upload's `Content-Type`, so a bare presigned URL for a PDF opens in a tab (CSV/XLSX datasets happen to download, which is why only KB docs looked broken). `get_signed_url(..., filename=...)` adds `ResponseContentDisposition: attachment`; `_signed_url()` in `services/documents/helpers.py` passes `fname` only when `download` is set, and the flag threads through `build_document_schema(s)` / `build_job_schema(s)` from `api/routes/documents.py`, `doc_transformation_job.py` and `collections.py`. An attachment URL will not render in an `<iframe>`, so preview callers must leave `download` off. The upload response and the doc-transform callback have no request to carry the flag and always sign inline. Leave `filename` unset for URLs meant to play inline (TTS audio in `api/routes/llm.py`).
- Uploads de-duplicate by provider file ID (see deep dive §7).
- Extension case: `get_file_format` lowercases the suffix, so `report.PDF` uploads fine and is stored with its original case in `fname` (the `download` attachment name depends on it). OpenAI file search may not match an uppercase suffix, so `services/collections/providers/openai.py` lowercases only the extension of the name it sends to `files.create`.
- Collections are immutable-ish: deletion semantics in deep dive §10.
- OpenAI file-batch id: the SDK's `file_batches.poll()` / `upload_and_poll()` final return deserializes a vector-store body, so its `.id` is the `vs_` id, not the `vsfb_` batch id. `crud/rag/open_ai.py` captures the batch id from `create()` before polling and uses it for `list_files`. Any failed file is a hard failure (whole vector store rolled back); partial indexing needs an add-documents endpoint first.
- The SDK's `file_batches.poll()` never times out. `_poll_file_batch` polls `retrieve` in a loop with no internal deadline — the Celery soft time limit bounds it, and its `SoftTimeLimitExceeded` aborts the task. An earlier version took a deadline from the caller via a `task_budget` `ContextVar` (and a fixed `BATCH_POLL_TIMEOUT_SECONDS`); both were deleted. Don't reintroduce caller coupling here.
Expand Down
Loading