Skip to content

feat: add DOC_EXTRACTOR_URL for document text extraction#184

Open
cgmark101 wants to merge 2 commits into
grinev:mainfrom
cgmark101:feat/doc-extractor-url
Open

feat: add DOC_EXTRACTOR_URL for document text extraction#184
cgmark101 wants to merge 2 commits into
grinev:mainfrom
cgmark101:feat/doc-extractor-url

Conversation

@cgmark101

Copy link
Copy Markdown

feat: add DOC_EXTRACTOR_URL for document text extraction

Context

When an LLM does not natively support a document format (PDF, DOCX, PPTX, etc.), the bot currently falls back to a generic "not supported" message and the file content is lost.

This PR adds an optional external extraction mechanism following the same stateless delegate pattern already used by STT_API_URL. If configured, the bot downloads the file and forwards it to an external API that returns the extracted plain text — which is then sent as a prompt to the model.

Why this pattern

Because the bot should not care how text is extracted — it only needs to know where to send the file. The actual extraction (pdftotext, MarkItDown, OCR, LibreOffice, whatever) lives in an external service that users deploy independently. Zero coupling, zero system dependencies added to the bot.

What changed

File Change
src/config.ts + docExtractor config block (apiUrl, apiKey)
src/app/services/document-extractor-service.ts new, mirrors stt-service.ts
src/bot/handlers/document-handler.ts when model lacks PDF support and DOC_EXTRACTOR_URL is set → extract and prompt with text
src/i18n/{en,es,ar,de,fr,ru,zh}.ts + document_extraction_error key
.env.example + DOC_EXTRACTOR_URL / DOC_EXTRACTOR_API_KEY docs

Non-goals

  • Text files (.txt, .py, .cpp) are already handled natively by reading as UTF-8 — no extractor needed.
  • This does not modify existing messages or behaviour when DOC_EXTRACTOR_URL is unset.

Suggested future iteration

The bot.model_no_pdf message could hint at DOC_EXTRACTOR_URL when unset (like stt.not_configured does), but that is left out of this PR to keep the diff minimal.


Authored by Zero — agent node srv-01.

@grinev

grinev commented Jul 11, 2026

Copy link
Copy Markdown
Owner

@cgmark101 thanks for the PR. The delegate pattern mirrors stt-service.ts well, and adding localizations for all languages in one go is appreciated. A few things to address before merging:

  1. Bug: {error} placeholder is never filled. bot.document_extraction_error contains {error} in all 7 locale files, but document-handler.ts:138 calls t("bot.document_extraction_error") without params. errMsg is computed two lines above and only goes to the log, so the user literally sees ... text: {error}. Either pass { error: errMsg } to t(), or drop {error} from every translation and keep the details in the log only (I'd prefer the latter, to avoid leaking the raw API response body into chat).

  2. README is not updated. Add DOC_EXTRACTOR_URL and DOC_EXTRACTOR_API_KEY to the env table (near the STT rows, README.md:237). Also add a short feature section (like "Voice and Audio Transcription") describing the API contract: multipart/form-data with a file field, response JSON { "text": "..." }, Authorization: Bearer ... header.

  3. Missing tests. Please mirror tests/app/services/stt-service.test.ts and add tests/app/services/document-extractor-service.test.ts. Also add cases in tests/bot/handlers/document.test.ts for both branches: successful extraction (prompt with text) and extraction failure (error reply + caption fallback).

  4. Scope mismatch. The PR description says "PDF, DOCX, PPTX, etc.", but the handler only triggers on application/pdf (document-handler.ts:116); other types still fall through to file_type_unsupported. Please either align the wording (and the .env.example comment) to "PDF only", or extend the condition to cover other document MIME types.

  5. Minor: mimeType is unused. extractDocument accepts mimeType but only logs it. Consider new Blob([new Uint8Array(fileBuffer)], { type: mimeType }) so the extractor can detect the format from the part's Content-Type.

  6. Minor: required API key. isDocExtractorConfigured requires both URL and API key. For a local self-hosted extractor, auth is often unnecessary. Consider treating it as configured when only the URL is set, and sending the Authorization header when the key is present.

- Remove {error} placeholder from locale strings (avoids leaking API response body to chat)
- Make DOC_EXTRACTOR_API_KEY optional; only send Authorization header when key is set
- Pass mimeType to Blob constructor so the extractor receives correct Content-Type
- Expand document MIME type support beyond application/pdf (DOCX, PPTX, XLSX, ODF, RTF)
- Add DOC_EXTRACTOR_URL and DOC_EXTRACTOR_API_KEY to README env table
- Add Document Text Extraction feature section to README
- Add tests/app/services/document-extractor-service.test.ts (10 tests)
- Add document extraction coverage to tests/bot/handlers/document.test.ts (5 tests)
- Update .env.example and all 7 locale files to reflect broader document support
@cgmark101

Copy link
Copy Markdown
Author

@grinev all 6 points addressed:

  1. {error} removed from all 7 locale files — error details stay in the log only
  2. ✅ README updated: env vars in table + "Document Text Extraction" feature section
  3. ✅ Tests added: tests/app/services/document-extractor-service.test.ts (10 tests) + 5 new cases in document.test.ts
  4. ✅ MIME types expanded: now matches PDF, DOC, DOCX, PPT, PPTX, XLS, XLSX, ODT, ODP, ODS, RTF
  5. mimeType now passed to Blob constructor as { type: mimeType }
  6. isDocExtractorConfigured only requires URL; Authorization header sent only when key is present

CI: build ✅, lint ✅, all 1103 tests pass ✅

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.

2 participants