Repository navigation
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dd57cd08e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cddc4e3fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1aa3520ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dccc4b2c49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95d4a599b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c1ec96f32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 185e2b657a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b17419725
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7df75cc142
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11562c7692
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87cc745ea1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ec7543db0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8682c4e904
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18185de7e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| snapshotFiles: { $ne: false }, | ||
| $or: [ | ||
| { fileSnapshots: { $exists: false } }, | ||
| { snapshotVersion: { $exists: false } }, |
There was a problem hiding this comment.
Guard backfills against legacy-replica republishes
At reviewed head 18185de7e8ba37e1a40af53e1f6145b91bed4464, during a rolling mixed-version deployment, an older replica can republish this share after the viewer read it, replacing fileSnapshots while leaving snapshotVersion absent; this predicate still matches, so the viewer's stale backfill overwrites the republished snapshot and can re-authorize files the owner just removed. Include the originally read snapshot or publication revision in the compare-and-set rather than treating every absent version as unchanged.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b34a507: persistBackfilledSnapshots now compares the stored fileSnapshots against the array the viewer read (or requires it absent), so a republish by an older replica that leaves snapshotVersion unset no longer matches. share.test.ts 'keeps a legacy replica republish that lands during a version upgrade' fails without the fix and passes with it.
|
@codex review Please review the current PR head b34a507. State the exact reviewed commit and ignore findings that apply only to earlier heads. Purpose supplied by the requester: 4 fixes since reviewed head 18185de: upload fallback parser bounds, fallback partial-text policy, streamed OpenAI upload, share backfill CAS on the read snapshot |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b34a50798d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? extractors.parseDocument({ | ||
| file, | ||
| maxFileSize: documentParser?.fileSizeLimit, |
There was a problem hiding this comment.
Cancel fallback parsing when the upload disconnects
At reviewed commit b34a50798d08307122df7a2fadb41d216fdf68ef, when textFallbackWithoutTools parses a none-routed document and the client disconnects, this separate parser invocation still receives no request-owned AbortSignal; unlike the primary Context parser, it can retain an admission slot until its deadline and then continue into storage after the upload was cancelled. Fresh evidence beyond the resolved primary-parser thread is this fallback path at process.js:1307-1316; thread the response-close signal through here and rethrow cancellation instead of converting it to a best-effort missing fallback.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c55cdda: resolveUploadFallbackText takes the upload response as its connection, aborts the parse on close (freeing the admission slot) and rethrows cancellation instead of returning a missing fallback; the listener is removed in finally. Covered by fallback.spec.ts 'cancels the parse when the upload closes'.
| fileSnapshots: readSnapshots === undefined ? { $exists: false } : readSnapshots, | ||
| $or: [ | ||
| { snapshotVersion: { $exists: false } }, | ||
| { snapshotVersion: { $lt: FILE_SNAPSHOT_VERSION } }, |
There was a problem hiding this comment.
Compare the publication revision during snapshot backfill
When a version-1 share contains snapshot [A] but its old snapshot omitted a newly supported text file B, a viewer computes [A,B]; if an older replica concurrently republishes the link after removing B, it can write [A] with no snapshotVersion, which is byte-for-byte equal to readSnapshots. This predicate therefore still matches and restores B, re-authorizing a file the owner removed. Include the read publication revision (updatedAt, messages/target revision, or equivalent) in the compare-and-set rather than relying solely on an array that a legitimate republish can reproduce.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 659772d: the backfill compare-and-set now also requires the updatedAt the viewer read, which every republish bumps and the backfill itself never changes (timestamps: false). share.test.ts 'keeps a legacy replica republish that writes back the snapshot the viewer read' fails without the fix and passes with it.
| return documentParserMimeTypes.some((pattern) => pattern.test(route.mimeType)) | ||
| ? UPLOAD_FALLBACK_TEXT_PLANS.documentParser | ||
| : null; |
There was a problem hiding this comment.
Normalize the MIME type before selecting fallback parsing
When textFallbackWithoutTools handles a document reported as application/octet-stream, application/zip, or another alias—common for DOCX/PDF uploads—this test uses the raw Multer MIME and returns no parser plan even though parseDocument supports routing the same file by its extension. The primary Context path already calls resolveUploadMimeType, but the fallback path passes file.mimetype directly, so a tool-less turn silently loses otherwise extractable fallback text; select the plan using resolveEffectiveMimeType(file.originalname, file.mimetype) or pass the already resolved type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c55cdda: the fallback plan now uses resolveEffectiveMimeType(originalname, mimetype), the same resolution the upload path uses. Covered by fallback.spec.ts 'plans a document reported under a generic type by its resolved type'.
| const uploadedFile = await openai.files.create({ | ||
| file: fs.createReadStream(file.path), | ||
| file: new File([await fs.openAsBlob(file.path)], file.originalname), | ||
| purpose: isImage ? FilePurpose.Vision : FilePurpose.Assistants, |
There was a problem hiding this comment.
Move streamed upload construction into packages/api
This change implements the new filesystem-to-file-backed-Blob upload behavior directly in legacy /api CJS rather than leaving that layer as wiring. Move the uploadable construction and provider invocation behind a TypeScript service in packages/api, with this file limited to importing and calling it; otherwise backend upload behavior continues to accumulate in the legacy boundary that repository guidance explicitly prohibits extending.
AGENTS.md reference: AGENTS.md:L73-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f342ce2: openNamedUpload in packages/api/src/files/upload/disk.ts builds the file-backed upload; OpenAI/crud.js only requires and calls it (covered by disk.spec.ts and OpenAI/crud.spec.js).
Adds server-side local document extraction with AnyDoc and pdf-inspector behind the existing document_parser strategy, keeping configured OCR services as a separate fallback layer.
dev landed its own share-snapshot delivery marker (`llmDeliveryPath`) and a share-scoped extracted-text preview, so this branch's parallel `hasTextPreview` marker is gone: snapshots carry the base's marker, the message preview dialog is the base's, and the branch keeps only the snapshot-version rebuild that repairs links snapshotted before text records were captured. A rebuild now also backfills delivery markers in the same view instead of leaving them to the next one. The two upload gates (multer's streaming filter and `filterFile`) asked the same "endpoint allowlist or documentParser allowlist" question in two places; they now call `isAdmissibleUploadType` in packages/api, each naming its own parser scope. A CSV left to tools is read as its own text again: the parser's list names delimited text so a context upload is admissible, which had started routing the fallback-text plan through the parser and storing a markdown table.
…pe as itself The process-wide parser limiter was replaced whenever a caller resolved different bounds. A fresh limiter starts with an active count of zero while the children the old one admitted are still running, so two requests with different `maxConcurrentParsers` could run more parses than either bound allows. The bounds are now fixed at the first parse, a later request naming different ones is logged once instead of honored, and the spec pins that a wider bound still waits behind the limiter already admitting. Admission accepts a type an operator named in `documentParser.supportedMimeTypes`, so dispatch can be handed a document no engine claims (a vendor MIME under a filename with no recognizable extension). AnyDoc threw a bare Error for that, which the upload route reported as a generic 500; it is now `UnsupportedDocumentTypeError` with code `UNSUPPORTED_DOCUMENT_TYPE` and status 415, recognized by `isDocumentParserRefusal` and named in the user-facing message table, like every other parser refusal.
Uploads now stage under a per-request path (`<file_id>-<name>`) so two concurrent uploads of the same filename cannot share one temp file. Three adapters derived the name they send from that staged path instead of the upload, so the id leaked into places a user reads it: - the vector store recorded `<uuid>-report.csv`, which is the name file_search citations show (this is what the `Upload for File Search` e2e provisioning test caught); - the configured RAG `/text` extraction announced the same staged basename; - the OpenAI assistants upload let the SDK infer the name from the stream path, so the provider's own file list showed it. Each now passes `file.originalname` explicitly. The Mistral OCR and code-environment adapters already did.
filterFile scoped parser-only MIME types to tool_resource context, but routing still promotes the legacy ocr resource to context, so a narrowed endpoint refused an ocr upload the parser would have read. Admission now takes the tool resource and treats ocr as the context path.
The fallback never runs the scan classifier, so a page with text beside an image of more text read as complete. The result now carries mayOmitContent, matching the native path when its classifier cannot run.
The server accepts file_search for every type the document parser reads, but the chooser gated it on retrievalMimeTypes only, so legacy Word, EPUB, PowerPoint and ODP showed no destination with context off. The prose formats are split into proseDocumentMimeTypes and the chooser accepts them for file search.
Also truncates the dialog title through an inner block span, since the title element itself does not clip.
The process-wide parser limiter took its bounds from whichever caller parsed first. A code-artifact preview or fallback-text parse passes no options, so it fixed the defaults and a later upload's documentParser bounds were logged and ignored. A caller with no options no longer fixes the bounds; the first caller that names them resizes the one limiter in place, keeping its running count so the swap never admits more than either bound. The limiter gains resize, and release now checks the bound so a narrowed limiter does not start queued work early.
Dev replaced the attach buttons with the Attach and tools palette, so the document scenarios waited for #attach-file-menu-button and #attach-file-button that no longer render. The helpers now open the palette and pick the source row, or the Upload as Text destination under More upload options.
f342ce2 to
374d0ce
Compare
Summary
This PR adds server-side local document extraction with AnyDoc and pdf-inspector while keeping configured OCR services as a separate fallback layer.
AnyDoc and pdf-inspector are internal engines behind LibreChat's existing
document_parserstrategy. They are not independently selectable OCR providers and they do not run in the browser. AnyDoc uses Rust-backed N-API code for its supported formats, and pdf-inspector is also Rust-backed. LibreChat runs both native bindings in operating-system child processes behind one shared concurrency limit.PDFs are routed directly to LibreChat's pdf-inspector module even though AnyDoc can parse PDFs upstream. That keeps page accounting, image-only page handling, native failure recovery, and the PDF.js work limit inside one provider-specific boundary.
Architecture overview
flowchart TD subgraph Browser["Browser"] Pick["Select file"] Infer["Canonicalize generic MIME from filename"] ClientGate{"Known document allowed by local parser<br/>or configured OCR route?"} Preview["Display server-extracted text"] end subgraph Admission["Server admission"] Multer["Multer uses the same MIME inference"] ServerGate{"Allowed by endpoint configuration?"} end subgraph Parser["Local document_parser strategy"] Dispatch{"Effective MIME is PDF?"} PDF["pdf-inspector adapter"] AnyDoc["AnyDoc adapter"] PDFJS["Bounded PDF.js recovery"] Result{"Complete local text?"} end subgraph OCRBoundary["Configured OCR boundary"] OCREligible{"OCR strategy configured,<br/>MIME allowed, capability granted?"} OCR["Mistral, Azure, Vertex,<br/>or custom OCR"] end Store["Persist text, original MIME,<br/>and parser provenance"] Partial["Persist partial text with<br/>bounded omission notice"] Refuse["Hard refusal or extraction error"] Pick --> Infer --> ClientGate ClientGate -->|Yes| Multer --> ServerGate ClientGate -->|No| Refuse ServerGate -->|Yes| Dispatch ServerGate -->|No| Refuse Dispatch -->|PDF| PDF Dispatch -->|Other document| AnyDoc PDF -->|Native parser failure| PDFJS PDF --> Result PDFJS --> Result AnyDoc --> Result Result -->|Complete| Store --> Preview Result -->|Missing text or pages| OCREligible OCREligible -->|Yes| OCR --> Store OCREligible -->|No, partial text exists| Partial --> Preview OCREligible -->|No local text| RefuseNative parser process boundary
Both native engines use the same process-wide limiter. At most two parser children run per API process, so AnyDoc and PDF bursts cannot multiply independently.
This boundary protects the API process from native aborts and segmentation faults. It also makes the 30 second timeout capable of terminating a synchronous native call. AnyDoc's binding is resolved and loaded only inside the child.
Configuration precedence and OCR routing
fileConfig.documentParser.supportedMimeTypesremains authoritative for local parsing. Excluding a known document from that list does not block a deliberately configured OCR route for the same MIME type.flowchart TD Type["Known document MIME"] --> Local{"Allowed by<br/>documentParser allowlist?"} Local -->|Yes| Context["Offer context upload"] Local -->|No| OCRConfigured{"Server reports an OCR<br/>strategy is configured?"} OCRConfigured -->|No| Reject["Do not offer context upload"] OCRConfigured -->|Yes| OCRType{"MIME allowed by<br/>ocr.supportedMimeTypes?"} OCRType -->|No| Reject OCRType -->|Yes| Capability{"Agent has OCR capability?"} Capability -->|No| Reject Capability -->|Yes| ContextThe server adds a derived
ocr.enabledsignal to the file configuration response. The client cannot infer availability from the default OCR MIME catalog, because a catalog alone does not mean that an OCR strategy exists. Upload validation and viable destination routing use this signal together with the agent capability.Shared parsed-text previews
Locally parsed documents have durable extracted text but no durable source binary at their parser marker path. Shared links therefore snapshot a preview-only marker rather than a temp path or the extracted text itself.
Shared-message sanitization exposes only
hasTextPreview, notsource, storage paths, or text. The preview route still requires the file to be present in the shared-link snapshot, checks the live record against the snapshotted revision, and refuses binary streaming for preview-only records.Provider boundaries
document_parseras the only public upload strategy for local document extraction.OCRStrategyto actual OCR services: Mistral OCR, custom OCR, Azure Mistral OCR, and Vertex AI Mistral OCR.anydocandpdf_inspectoronly as persistence provenance values.File-type ownership and admission
fileConfig.documentParser.supportedMimeTypesauthoritative for local parsing of known document types.application/octet-streamandbinary/octet-streamfrom the filename before client or server admission.PDF pipeline
PdfPageLimitError.pagesNeedingOcrin the PDF.js fallback so mixed text and image PDFs do not silently lose pages.AnyDoc pipeline and archive safety
@firecrawl/anydocfor supported non-PDF Office, OpenDocument, ebook, structured text, and markup formats.OCR fallback and partial text
Persistence and preview behavior
Change Type
Testing
@librechat/apicoverage run executed all suites. One unrelated load-sensitive YouTube safety assertion took 1,069 ms against a 1,000 ms limit under the full serial load. Its exact 61-test suite reran independently with all tests passing and the assertion completing in 29 ms.minimatchtree:TypeError: expand is not a function. No source lint finding is produced by that command.@librechat/api, data-provider, data-schemas, the client package, and the API server.git diff --check, the added-line punctuation audit, and the UI accessibility static scan passed.Test Configuration:
@librechat/apitests, including real child-process execution.Checklist