feat: enable chunking in remote (Docling Serve) mode
Hybrid approach: reuse LocalChunker to chunk the DoclingDocument JSON returned by Serve, so chunking works identically in both local and remote modes without calling Serve's chunk endpoint. Backend: - _build_chunker() always returns LocalChunker (remove engine guard) - Use docling-core[chunking] extra for required dependencies - Skip client-side batching in remote mode (Serve manages its own resources, and batching discards document_json needed for chunking) - Fix Serve form fields: remove generate_page_images (not a Serve field), use repeated form keys for to_formats and page_range - Log Serve error response body on 4xx/5xx for diagnosis - Fix FastAPI 204 DELETE routes missing response_model=None Frontend: - Update chunking feature flag to enable Prepare UI in remote mode Closes #51
This commit is contained in:
parent
0bffe6e7d4
commit
3bdc4cec50
12 changed files with 140 additions and 23 deletions
|
|
@ -155,7 +155,7 @@ async def delete_chunk(job_id: str, chunk_index: int, service: ServiceDep) -> li
|
|||
]
|
||||
|
||||
|
||||
@router.delete("/{job_id}", status_code=204)
|
||||
@router.delete("/{job_id}", status_code=204, response_model=None)
|
||||
async def delete_analysis(job_id: str, service: ServiceDep) -> None:
|
||||
"""Delete an analysis job."""
|
||||
deleted = await service.delete(job_id)
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ async def get_document(doc_id: str, service: ServiceDep) -> DocumentResponse:
|
|||
return _to_response(doc)
|
||||
|
||||
|
||||
@router.delete("/{doc_id}", status_code=204)
|
||||
@router.delete("/{doc_id}", status_code=204, response_model=None)
|
||||
async def delete_document(doc_id: str, service: ServiceDep) -> None:
|
||||
"""Delete a document and its file."""
|
||||
deleted = await service.delete(doc_id)
|
||||
|
|
|
|||
|
|
@ -74,7 +74,7 @@ async def ingest_analysis(
|
|||
)
|
||||
|
||||
|
||||
@router.delete("/{doc_id}", status_code=204)
|
||||
@router.delete("/{doc_id}", status_code=204, response_model=None)
|
||||
async def delete_ingested_document(doc_id: str, ingestion: IngestionDep) -> None:
|
||||
"""Delete all indexed chunks for a document."""
|
||||
await ingestion.delete_document(doc_id)
|
||||
|
|
|
|||
|
|
@ -96,6 +96,13 @@ class ServeConverter:
|
|||
headers=self._headers(),
|
||||
)
|
||||
|
||||
if response.status_code >= 400:
|
||||
logger.error(
|
||||
"Docling Serve error %d: %s (form_data=%s)",
|
||||
response.status_code,
|
||||
response.text[:500],
|
||||
{k: v for k, v in form_data.items()},
|
||||
)
|
||||
response.raise_for_status()
|
||||
result_data = response.json()
|
||||
|
||||
|
|
@ -122,8 +129,12 @@ def _build_form_data(
|
|||
) -> dict[str, str | list[str]]:
|
||||
"""Build form fields matching Docling Serve's multipart form contract.
|
||||
|
||||
Array fields (to_formats) are sent as lists — httpx encodes them as
|
||||
repeated form keys (to_formats=md&to_formats=html&to_formats=json).
|
||||
Serve uses FastAPI's ``Form()`` parsing — list/tuple fields are sent
|
||||
as **repeated form keys** (httpx encodes Python lists this way
|
||||
automatically: ``to_formats=md&to_formats=html&to_formats=json``).
|
||||
|
||||
Note: ``generate_page_images`` is a PdfPipelineOptions field, NOT a
|
||||
ConvertDocumentsOptions field — sending it causes a 422.
|
||||
"""
|
||||
data: dict[str, str | list[str]] = {
|
||||
"to_formats": ["md", "html", "json"],
|
||||
|
|
@ -135,11 +146,12 @@ def _build_form_data(
|
|||
"do_picture_classification": str(options.do_picture_classification).lower(),
|
||||
"do_picture_description": str(options.do_picture_description).lower(),
|
||||
"include_images": str(options.generate_picture_images).lower(),
|
||||
"generate_page_images": str(options.generate_page_images).lower(),
|
||||
"images_scale": str(options.images_scale),
|
||||
}
|
||||
if page_range is not None:
|
||||
data["page_range"] = f"{page_range[0]}-{page_range[1]}"
|
||||
# Serve expects page_range as two repeated form fields:
|
||||
# page_range=1&page_range=10
|
||||
data["page_range"] = [str(page_range[0]), str(page_range[1])]
|
||||
return data
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -57,12 +57,15 @@ def _build_converter():
|
|||
|
||||
|
||||
def _build_chunker():
|
||||
"""Build the chunker adapter — only available in local mode."""
|
||||
if settings.conversion_engine == "local":
|
||||
from infra.local_chunker import LocalChunker
|
||||
"""Build the chunker adapter.
|
||||
|
||||
return LocalChunker()
|
||||
return None
|
||||
Uses LocalChunker in all modes — in remote mode it chunks the
|
||||
DoclingDocument JSON returned by Docling Serve, so docling-core
|
||||
(lightweight) is the only local dependency needed.
|
||||
"""
|
||||
from infra.local_chunker import LocalChunker
|
||||
|
||||
return LocalChunker()
|
||||
|
||||
|
||||
def _build_repos() -> tuple[SqliteDocumentRepository, SqliteAnalysisRepository]:
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
docling-core>=2.0.0,<3.0.0
|
||||
docling-core[chunking]>=2.0.0,<3.0.0
|
||||
fastapi>=0.115.0,<1.0.0
|
||||
uvicorn[standard]>=0.32.0,<1.0.0
|
||||
python-multipart>=0.0.12
|
||||
|
|
|
|||
|
|
@ -324,11 +324,18 @@ class AnalysisService:
|
|||
file_path: str,
|
||||
options: ConversionOptions,
|
||||
) -> ConversionResult | None:
|
||||
"""Run batched or single conversion. Returns None if the job was deleted mid-batch."""
|
||||
"""Run batched or single conversion. Returns None if the job was deleted mid-batch.
|
||||
|
||||
Batching is only used for local mode — it limits memory usage when
|
||||
Docling runs in-process. In remote mode the Serve instance manages
|
||||
its own resources, and batching would discard document_json (needed
|
||||
for chunking).
|
||||
"""
|
||||
total_pages = _count_pdf_pages(file_path)
|
||||
batch_size = self._config.batch_page_size
|
||||
is_remote = self._is_remote_converter()
|
||||
|
||||
if batch_size > 0 and total_pages > batch_size:
|
||||
if batch_size > 0 and total_pages > batch_size and not is_remote:
|
||||
return await self._run_batched_conversion(
|
||||
job_id, file_path, options, total_pages, batch_size
|
||||
)
|
||||
|
|
@ -337,6 +344,15 @@ class AnalysisService:
|
|||
timeout=self._conversion_timeout,
|
||||
)
|
||||
|
||||
def _is_remote_converter(self) -> bool:
|
||||
"""Check if the converter is a remote (Serve) adapter."""
|
||||
try:
|
||||
from infra.serve_converter import ServeConverter
|
||||
|
||||
return isinstance(self._converter, ServeConverter)
|
||||
except ImportError:
|
||||
return False
|
||||
|
||||
async def _finalize_analysis(
|
||||
self,
|
||||
job_id: str,
|
||||
|
|
|
|||
|
|
@ -465,3 +465,68 @@ class TestRechunkEndpoint:
|
|||
},
|
||||
)
|
||||
assert resp.status_code == 422
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Remote chunking path — hybrid local chunking from Serve's document_json
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestRemoteChunkingPath:
|
||||
"""Verify that chunking works on document_json produced by Serve (remote mode)."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rechunk_with_serve_document_json(self):
|
||||
"""AnalysisService.rechunk() works with a LocalChunker even in remote mode."""
|
||||
from infra.local_chunker import LocalChunker
|
||||
from services.analysis_service import AnalysisService
|
||||
|
||||
chunker = LocalChunker()
|
||||
analysis_repo = AsyncMock()
|
||||
document_repo = AsyncMock()
|
||||
converter = AsyncMock() # ServeConverter mock — not used for rechunking
|
||||
|
||||
service = AnalysisService(
|
||||
converter=converter,
|
||||
analysis_repo=analysis_repo,
|
||||
document_repo=document_repo,
|
||||
chunker=chunker,
|
||||
)
|
||||
|
||||
# Simulate a completed job with document_json from Serve
|
||||
job = AnalysisJob(id="j-remote", document_id="d1")
|
||||
job.mark_running()
|
||||
job.mark_completed(
|
||||
markdown="# Title\nParagraph text here.",
|
||||
html="<h1>Title</h1><p>Paragraph text here.</p>",
|
||||
pages_json="[]",
|
||||
document_json=json.dumps({
|
||||
"schema_name": "DoclingDocument",
|
||||
"version": "1.0.0",
|
||||
"name": "test",
|
||||
"origin": {
|
||||
"mimetype": "application/pdf",
|
||||
"filename": "test.pdf",
|
||||
"binary_hash": 0,
|
||||
},
|
||||
"furniture": {"self_ref": "#/furniture", "children": [], "content_layer": "furniture"},
|
||||
"body": {"self_ref": "#/body", "children": [], "content_layer": "body"},
|
||||
"groups": [],
|
||||
"texts": [],
|
||||
"pictures": [],
|
||||
"tables": [],
|
||||
"key_value_items": [],
|
||||
"form_items": [],
|
||||
"pages": {},
|
||||
}),
|
||||
)
|
||||
analysis_repo.find_by_id = AsyncMock(return_value=job)
|
||||
analysis_repo.update_chunks = AsyncMock(return_value=True)
|
||||
|
||||
chunks = await service.rechunk(
|
||||
"j-remote",
|
||||
{"chunker_type": "hybrid", "max_tokens": 512},
|
||||
)
|
||||
|
||||
assert isinstance(chunks, list)
|
||||
analysis_repo.update_chunks.assert_called_once()
|
||||
|
|
|
|||
|
|
@ -39,7 +39,6 @@ class TestBuildFormData:
|
|||
assert data["do_picture_classification"] == "false"
|
||||
assert data["do_picture_description"] == "false"
|
||||
assert data["include_images"] == "false"
|
||||
assert data["generate_page_images"] == "false"
|
||||
assert data["images_scale"] == "1.0"
|
||||
assert set(data["to_formats"]) == {"md", "html", "json"}
|
||||
|
||||
|
|
@ -56,9 +55,14 @@ class TestBuildFormData:
|
|||
assert data["images_scale"] == "2.0"
|
||||
assert data["include_images"] == "true"
|
||||
|
||||
def test_page_range_included_when_set(self):
|
||||
def test_no_generate_page_images_field(self):
|
||||
"""generate_page_images is a PdfPipelineOptions field, not a Serve field."""
|
||||
data = _build_form_data(ConversionOptions())
|
||||
assert "generate_page_images" not in data
|
||||
|
||||
def test_page_range_as_repeated_fields(self):
|
||||
data = _build_form_data(ConversionOptions(), page_range=(11, 20))
|
||||
assert data["page_range"] == "11-20"
|
||||
assert data["page_range"] == ["11", "20"]
|
||||
|
||||
def test_page_range_absent_when_none(self):
|
||||
data = _build_form_data(ConversionOptions())
|
||||
|
|
@ -402,11 +406,12 @@ class TestServeConverterConvert:
|
|||
assert len(result.pages[0].elements) == 1
|
||||
assert result.pages[0].elements[0].type == "title"
|
||||
|
||||
# Verify form fields sent as dict with list for repeated keys
|
||||
# Verify form fields sent correctly
|
||||
call_kwargs = mock_client.post.call_args
|
||||
sent_data = call_kwargs.kwargs.get("data", {})
|
||||
assert sent_data["do_ocr"] == "true"
|
||||
assert set(sent_data["to_formats"]) == {"md", "html", "json"}
|
||||
assert "generate_page_images" not in sent_data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_http_error_raises(self, tmp_path):
|
||||
|
|
@ -414,6 +419,8 @@ class TestServeConverterConvert:
|
|||
test_file.write_bytes(b"%PDF-1.4 fake content")
|
||||
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 500
|
||||
mock_response.text = "Internal Server Error"
|
||||
mock_response.raise_for_status.side_effect = httpx.HTTPStatusError(
|
||||
"Server Error",
|
||||
request=MagicMock(),
|
||||
|
|
@ -510,3 +517,17 @@ class TestConverterWiring:
|
|||
converter = _build_converter()
|
||||
assert isinstance(converter, ServeConverter)
|
||||
assert converter._api_key == "my-key"
|
||||
|
||||
def test_remote_engine_builds_chunker(self):
|
||||
"""Chunker must be available in remote mode (hybrid local chunking)."""
|
||||
from infra.local_chunker import LocalChunker
|
||||
from infra.settings import Settings
|
||||
|
||||
with patch(
|
||||
"main.settings",
|
||||
Settings(conversion_engine="remote", docling_serve_url="http://serve:5001"),
|
||||
):
|
||||
from main import _build_chunker
|
||||
|
||||
chunker = _build_chunker()
|
||||
assert isinstance(chunker, LocalChunker)
|
||||
|
|
|
|||
|
|
@ -29,12 +29,12 @@ describe('useFeatureFlagStore', () => {
|
|||
expect(store.isEnabled('chunking')).toBe(true)
|
||||
})
|
||||
|
||||
it('disables chunking when engine is remote', async () => {
|
||||
it('enables chunking when engine is remote', async () => {
|
||||
mockApiFetch.mockResolvedValue({ status: 'ok', engine: 'remote' })
|
||||
const store = useFeatureFlagStore()
|
||||
await store.load()
|
||||
expect(store.engine).toBe('remote')
|
||||
expect(store.isEnabled('chunking')).toBe(false)
|
||||
expect(store.isEnabled('chunking')).toBe(true)
|
||||
})
|
||||
|
||||
it('enables disclaimer when deploymentMode is huggingface', async () => {
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ interface FeatureFlagContext {
|
|||
const featureRegistry: Record<FeatureFlag, FeatureFlagDef> = {
|
||||
chunking: {
|
||||
description: 'Document chunking for RAG preparation',
|
||||
isEnabled: (ctx) => ctx.engine === 'local',
|
||||
isEnabled: (ctx) => ctx.engine !== null,
|
||||
},
|
||||
disclaimer: {
|
||||
description: 'Show shared-instance disclaimer banner',
|
||||
|
|
|
|||
|
|
@ -23,6 +23,6 @@ describe('useFeatureFlag', () => {
|
|||
expect(flag.value).toBe(true)
|
||||
|
||||
store.$patch({ engine: 'remote' })
|
||||
expect(flag.value).toBe(false)
|
||||
expect(flag.value).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue