Skip to content

feat(summary): summarize attached files, and fix document attachments on Mistral - #419

Open
cbcoutinho wants to merge 2 commits into
nextcloud:mainfrom
cbcoutinho:feat/summary-file-attachments
Open

feat(summary): summarize attached files, and fix document attachments on Mistral#419
cbcoutinho wants to merge 2 commits into
nextcloud:mainfrom
cbcoutinho:feat/summary-file-attachments

Conversation

@cbcoutinho

@cbcoutinho cbcoutinho commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Two related changes that let a document be summarized by the model itself rather than by whatever text our caller managed to extract first.

1. fix(multimodal): documents are broken on Mistral

OpenAiFileService::buildDocumentContent() only ever emitted OpenAI's content part:

{"type": "file", "file": {"filename": "", "file_data": "data:application/pdf;base64,…"}}

Mistral rejects that with an HTTP 422 and a misleading message:

{"detail":[{"type":"string_type","loc":["body","messages",0,"user","content","str"],
  "msg":"Input should be a valid string"}]}

Mistral's DocumentURLChunk takes a flat string, not a nested object. Because no variant of its ContentChunk union matches ours, the validator falls back to parsing content as a plain string and reports that failure — which is why the error says nothing about documents. Mistral therefore needs:

{"type": "document_url", "document_url": "data:application/pdf;base64,…", "document_name": ""}

The envelope is now chosen from the configured service URL. The payload is unchanged — every backend that supports documents accepts them inline as a base64 data URI (OpenAI, OpenRouter and Mistral all do), so no separate upload/Files API is involved on any path.

This also fixes document attachments for nextcloud/assistant#602 when running against Mistral.

2. feat(summary): input_attachments on SummaryProvider

SummaryProvider::process() could only work on a string, so a caller had to extract a document's text before asking for a summary. For PDFs that extraction is frequently lossy or empty.

The provider now advertises an optional input_attachments (ListOfFiles) slot. When files are present it skips ChunkService entirely — the whole point is that we never read the file, so there is no text to split — and inlines them in one chat completion.

Deliberate choices:

  • The slot is only advertised when multimodal_document_enabled is on and the chat endpoint is usable. Attachments cannot be carried over the legacy /completions endpoint, so advertising unconditionally would tell callers attachments work and then fail every request.
  • Capped at 10 files, matching MultimodalChatWithToolsProvider.
  • An empty prompt with no usable attachment is now rejected rather than summarized. That state means the attachment never arrived, and a clear error beats a summary of nothing.

Consumer

nextcloud/assistant PR (Files → Summarize sends the file itself when this slot is advertised): cbcoutinho/assistant@feat/summarize-file-attachment. It feature-detects optionalInputShape['input_attachments'] and falls back to today's text extraction when absent, so ordering between the two is not critical.

Testing

Three tests added to OpenAiProviderTest, asserting the exact request body:

  • testSummaryProviderWithPdfAttachment — OpenAI type: file envelope, no chunking
  • testSummaryProviderWithPdfAttachmentOnMistral — flat document_url envelope
  • testSummaryProviderAdvertisesAttachmentsOnlyWhenDocumentsAreEnabled

psalm, cs:check and php -l are clean locally. phpunit was not run locally — the suite is @group DB and extends Test\TestCase, so it needs a full Nextcloud install; the three new tests will first execute in CI. They are modelled on testMultimodalChatWithToolsProvider and testSummaryProvider, but exact-body assertions are brittle, so a first-run adjustment is possible.

Verified against live Mistral and OpenRouter endpoints — see the comment below for the full matrix, including a negative control reproducing the 422. The payloads tested were extracted from buildFileContentFromFile() itself rather than hand-written.


This PR was generated with the help of AI, and reviewed by a Human

buildDocumentContent() only ever emitted OpenAI's content part, so every
document attachment sent to Mistral came back as an HTTP 422 with a
confusing "Input should be a valid string". Mistral takes a flat
document_url string rather than a nested file object; its schema has no
variant matching ours, so the discriminated union falls through to
parsing the content as a plain string and reports that instead.

Pick the envelope from the configured service URL. The payload itself is
unchanged: every backend that supports documents accepts them inline as
a base64 data URI, so no separate upload API is involved either way.

Signed-off-by: Chris Coutinho <chris@coutinho.io>
The summary provider could only ever work on a string, so callers had to
extract the text of a document before asking for a summary. That loses
whatever the extractor cannot read, which for PDFs is often everything.

Advertise an optional input_attachments slot and, when files are given,
inline them in a single chat completion instead. Chunking is skipped
entirely on that path: the point is that we never read the file, so
there is no text to split.

The slot is only advertised when document support is enabled and the
chat completion endpoint is usable, since attachments cannot be carried
over the legacy completions endpoint. Advertising it unconditionally
would tell callers attachments work and then fail every request.

An empty prompt with no usable attachment is now rejected rather than
summarized, so a dropped attachment surfaces as an error instead of a
summary of nothing.

Signed-off-by: Chris Coutinho <chris@coutinho.io>
@cbcoutinho

Copy link
Copy Markdown
Author

Verified against live endpoints ✅

Tested with a real Mistral API key and a real OpenRouter key. To avoid testing a hand-written payload that merely resembles the code, I extracted the exact content part OpenAiFileService::buildFileContentFromFile() emits for each dialect and POSTed those bytes:

OpenAI  : {"type":"file","file":{"filename":"ruritania.pdf","file_data":"data:application/pdf;base64,<PDF>"}}
Mistral : {"type":"document_url","document_url":"data:application/pdf;base64,<PDF>","document_name":"ruritania.pdf"}

The fixture is a 659-byte PDF containing one invented fact — "The annual rainfall in Zenda is 812 millimetres" — so a correct 812 proves the model actually read the document rather than recalling anything from training.

Dialect Endpoint Result
Mistral document_url api.mistral.ai, mistral-small-latest 200 → 812
Mistral document_url api.mistral.ai, mistral-medium-latest 200 → 812
OpenAI type: file OpenRouter, pinned to OpenAI 200 → 812
OpenAI type: file api.mistral.ai (the bug) 422

The negative control reproduces the exact error quoted in the description:

{"detail":[{"type":"string_type","loc":["body","messages",0,"user","content","str"],
  "msg":"Input should be a valid string", ...}]}

Also confirmed document_name is optional — Mistral returns 812 with or without it.

Unrelated finding worth knowing about: OpenRouter silently drops PDFs on some upstreams

While testing I hit non-deterministic answers on OpenRouter. It turns out to depend on which upstream OpenRouter routes to, and it is reproducible when pinned:

provider engine Answer (correct = 812)
{"only":["OpenAI"]} default 812, 812
{"only":["Azure"]} default 600, 500hallucinated
{"only":["Azure"]} file-parser / mistral-ocr 812, 812

Unpinned, the same request alternated between the two: 812 (OpenAI), 830 / 610 / 600 (Azure).

So OpenRouter's type: file passthrough is per-upstream, and when the upstream cannot handle it the document is dropped without any error — the model then answers confidently from nothing. This is not caused by this PR (the OpenAI dialect is unchanged) and it contradicts OpenRouter's docs, which say PDF support "works on any model on OpenRouter". But it matters for anyone summarizing documents through OpenRouter, since a fabricated summary looks exactly like a real one.

There is an existing workaround needing no code change — admins can force pre-parsing through the llm_extra_params admin setting, since plugins is not a key the request builder sets itself:

{"plugins": [{"id": "file-parser", "pdf": {"engine": "mistral-ocr"}}]}

Happy to add explicit handling (e.g. defaulting that plugin when the service URL is OpenRouter) if maintainers want it — I left it out here because mistral-ocr is billed per page and that is a product decision, not a bug fix.


This comment was generated with the help of AI, and reviewed by a Human

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant