Skip to content

camel-docling: return DocTags content and reject html_split_page on docling-serve - #29594

Open
WMK666666 wants to merge 1 commit into
apache:mainfrom
WMK666666:docling-output-format
Open

WMK666666 wants to merge 1 commit into
apache:mainfrom
WMK666666:docling-output-format

Conversation

@WMK666666

@WMK666666 WMK666666 commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

outputFormat documents six values — md, json, html, html_split_page, text, doctags — and the
docling-serve path could not produce two of them: doctags came back as an empty body, and
html_split_page came back as an empty body as well.

What went wrong

DoclingContentExtractor.mapOutputFormat had no doctags case. docling-serve only fills the fields of
the formats that were requested, so a DocTags conversion returns doctags_content and no md_content at
all. The missing case sent it to the markdown branch, which found no markdown and returned "": the
exchange succeeded with an empty body. The same extractor is used by DoclingConsumer when an async task
completes, so async DocTags conversions were empty too.

html_split_page has no field to read. DocumentResponse carries five content fields —
doctagsContent, htmlContent, jsonContent, markdownContent, textContent — and the html one is
filled only when HTML is requested; the split pages are not part of the response at all. The request asked
docling-serve for HTML_SPLIT_PAGE, the response carried no content, and the body came out empty. It now
fails with an IllegalArgumentException that names the reason and the way out, outputFormat=html to get
the HTML of the whole document.

Change

  • DoclingContentExtractor reads getDoctagsContent() for doctags, and mapOutputFormat maps it to
    itself instead of falling through to markdown.
  • mapToOutputFormat rejects html_split_page with the reason, for the same reason the doctags case
    exists: docling-serve was asked for something it cannot return, and the caller got silence. That map is
    where the question "can the service return this format" is answered, and it is where the value used to be
    turned into HTML_SPLIT_PAGE for nothing.

Nothing else moves. In particular the CLI maps are untouched: the CLI operations pass a hardcoded format to
buildDoclingCommand — markdown, html, json or text — so outputFormat never reaches
mapToDoclingFormat or getOutputFileExtension with any other value (#29594 had cases for them; they were
unreachable and are removed).

Tests

DoclingContentExtractorTest builds responses the way docling-serve does — only the requested format's
field filled in — and asserts doctags returns the DocTags content, that DOCTAGS is accepted, that a
response without DocTags gives an empty body rather than the markdown, and that the formats that already
worked still return their own content.

DoclingServeOutputFormatTest asserts the rejection and its message, and that the five values the service
can return still map to the right OutputFormat.

Both halves were checked by reverting them: without the extractor case the three DocTags assertions fail
and the guards stay green; without the rejection html_split_page builds a request instead of throwing.

mvn -pl components/camel-ai/camel-docling test: 93 tests, 2 failures, both
DoclingCustomArgsValidationTest on Windows (/etc/cron.d is not an absolute path there), which fail the
same way without this change. formatter:validate and impsort:check pass.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, focused bug fix that adds doctags and txt case labels to all three switch-based mapping methods plus the extraction switch. API usage verified correct (DocumentResponse.getDoctagsContent() confirmed in docling-serve-api 0.7.0 JAR). Tests are well-structured and cover the new paths including case-insensitivity.

The PR also fixes two pre-existing alias inconsistencies: missing md case in mapToDoclingFormat and missing txt case in getOutputFileExtension. All cases use explicit return statements with no unintended fall-through.

No issues found. ✅

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doctags/txt fixes are correct, and the tests match how docling-serve fills only the requested field. One thing to fix in this PR, since it is the same bug on the same maps:

  1. html_split_page has the same problem as doctags. The description says no map ever selected it, but mapToOutputFormat does (DoclingProducer.java:1682-1683, case "html_split_page": return OutputFormat.HTML_SPLIT_PAGE). So with useDoclingServe=true the request asks docling-serve only for HTML_SPLIT_PAGE, while DoclingContentExtractor.mapOutputFormat falls back to markdown and reads markdown_content, which was not requested: an empty body, exactly the doctags symptom. On the CLI path, mapToDoclingFormat still turns it into --to md. It is one of the six documented outputFormat values, so please either map it through (read the field docling-serve fills for it in the extractor, use --to html_split_page with an html extension in getOutputFileExtension for the CLI, and add the cases to the two new tests), or, if docling-serve does not return it in the body, fail with a clear message instead of an empty body.
  2. Minor: DoclingContentExtractorTest covers the extractor with a hand-built response, which is fine; no producer-level test exercises the serve path end to end, consistent with the existing tests.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • components/camel-ai/camel-docling
  • docs

🔬 Scalpel shadow comparison — Scalpel: 10 of 722 tested, 26 compile-only — current: 10 all tested

Maveniverse Scalpel detected 10 affected modules (current approach: 10).

Skip-tests mode would test 10 modules (2 direct + 9 downstream), skip tests for 26 (generated code, meta-modules)

Modules Scalpel would test (10)
  • camel-docling ← components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingContentExtractor.java, components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingProducer.java, components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingContentExtractorTest.java, components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingServeOutputFormatTest.java
  • camel-jbang-mcp ← downstream of org.apache.camel:camel-catalog
  • camel-jbang-plugin-mcp ← downstream of org.apache.camel:camel-jbang-core
  • camel-jbang-plugin-tui ← downstream of org.apache.camel:camel-catalog
  • camel-jbang-plugin-validate ← downstream of org.apache.camel:camel-yaml-dsl-validator
  • camel-jbang-run ← downstream of org.apache.camel:camel-catalog
  • camel-langchain4j-ingest ← downstream of org.apache.camel:camel-docling
  • camel-launcher-container ← downstream of org.apache.camel:camel-launcher
  • camel-yaml-dsl-validator ← downstream of org.apache.camel:camel-catalog
  • camel-yaml-dsl-validator-maven-plugin ← downstream of org.apache.camel:camel-yaml-dsl-validator
Modules with tests skipped (26)
  • apache-camel
  • camel-allcomponents
  • camel-catalog
  • camel-catalog-console
  • camel-catalog-maven
  • camel-catalog-suggest
  • camel-componentdsl
  • camel-endpointdsl
  • camel-endpointdsl-support
  • camel-itest
  • camel-jbang-core
  • camel-jbang-it
  • camel-jbang-main
  • camel-jbang-plugin-edit
  • camel-jbang-plugin-generate
  • camel-jbang-plugin-kubernetes
  • camel-jbang-plugin-test
  • camel-kamelet-main
  • camel-launcher
  • camel-report-maven-plugin
  • camel-route-parser
  • camel-yaml-dsl
  • camel-yaml-dsl-deserializers
  • camel-yaml-dsl-maven-plugin
  • coverage
  • dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (37 modules, 5m 36s total)

Total reactor time: 5m 36s

Module Duration Status
Camel :: JBang :: MCP 46.5s SUCCESS
Camel :: AI :: LangChain4j :: Ingest 35.5s SUCCESS
Camel :: YAML DSL :: Validator 26.0s SUCCESS
Camel :: JBang :: Plugin :: Validate 24.1s SUCCESS
Camel :: YAML DSL 22.5s SUCCESS
Camel :: Component DSL 21.1s SUCCESS
Camel :: AI :: Docling 20.5s SUCCESS
Camel :: Catalog :: Camel Catalog 20.1s SUCCESS
Camel :: JBang :: Run 17.9s SUCCESS
Camel :: JBang :: Plugin :: Testing 12.1s SUCCESS
Camel :: Docs 12.1s SUCCESS
Camel :: JBang :: Plugin :: Kubernetes 12.1s SUCCESS
Camel :: Kamelet Main 11.7s SUCCESS
Camel :: Catalog :: Camel Report Maven Plugin 7.8s SUCCESS
Camel :: Catalog :: Camel Route Parser 6.6s SUCCESS
Camel :: YAML DSL :: Deserializers 6.4s SUCCESS
Camel :: Assembly 4.7s SUCCESS
Camel :: JBang :: Plugin :: Generate 4.7s SUCCESS
Camel :: All Components Sync point 4.6s SUCCESS
Camel :: YAML DSL :: Validator Maven Plugin 3.6s SUCCESS
Camel :: YAML DSL :: Maven Plugins 2.6s SUCCESS
Camel :: Catalog :: Maven 2.3s SUCCESS
Camel :: Catalog :: Suggest (deprecated) 2.1s SUCCESS
Camel :: Catalog :: Dummy Component 1.4s SUCCESS
Camel :: JBang :: Plugin :: MCP 1.4s SUCCESS
Camel :: Coverage 1.0s SUCCESS
Camel :: JBang :: Main 1.0s SUCCESS
Camel :: JBang :: Integration tests 1.0s SUCCESS
Camel :: Catalog :: Console 0.9s SUCCESS
Camel :: JBang :: Plugin :: Edit 0.7s SUCCESS
Camel :: Launcher :: Container 0.7s SUCCESS
Camel :: Endpoint DSL :: Support 0.6s SUCCESS
Camel :: Endpoint DSL n/a
Camel :: Integration Tests n/a
Camel :: JBang :: Core n/a
Camel :: JBang :: Plugin :: TUI n/a
Camel :: Launcher n/a

Top 20 slowest modules:

  • Camel :: JBang :: MCP (46.5s)
  • Camel :: AI :: LangChain4j :: Ingest (35.5s)
  • Camel :: YAML DSL :: Validator (26.0s)
  • Camel :: JBang :: Plugin :: Validate (24.1s)
  • Camel :: YAML DSL (22.5s)
  • Camel :: Component DSL (21.1s)
  • Camel :: AI :: Docling (20.5s)
  • Camel :: Catalog :: Camel Catalog (20.1s)
  • Camel :: JBang :: Run (17.9s)
  • Camel :: JBang :: Plugin :: Testing (12.1s)
  • Camel :: Docs (12.1s)
  • Camel :: JBang :: Plugin :: Kubernetes (12.1s)
  • Camel :: Kamelet Main (11.7s)
  • Camel :: Catalog :: Camel Report Maven Plugin (7.8s)
  • Camel :: Catalog :: Camel Route Parser (6.6s)
  • Camel :: YAML DSL :: Deserializers (6.4s)
  • Camel :: Assembly (4.7s)
  • Camel :: JBang :: Plugin :: Generate (4.7s)
  • Camel :: All Components Sync point (4.6s)
  • Camel :: YAML DSL :: Validator Maven Plugin (3.6s)

⚙️ View full build and test results

@oscerd oscerd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch that the three maps translating outputFormat each knew a different subset and doctags was missing from the ones that matter, so a documented value produced the wrong output. Adding the doctags case to mapOutputFormat and reading document.getDoctagsContent() in the extractor, plus the txt/doctags cases in the format maps, lines the six documented values up with what the engine is actually asked for. The note that docling-serve only fills the fields of the requested formats is the right reason to read the matching field, and doctagsReturnsTheDoctagsContent covers it. LGTM.

Claude Code on behalf of oscerd

@WMK666666

Copy link
Copy Markdown
Contributor Author

html_split_page is in 0f523fc, and I went with the second option after checking what docling-serve returns.

docling-serve cannot return it in the body. DocumentResponse (docling-serve-api 0.7.0) has content fields for doctags, html, json, markdown and text only — there is no split page field. And docling-jobkit fills html_content from OutputFormat.HTML in conversion_options.to_formats (docling_jobkit/convert/results.py); there is no html_split_page counterpart, so asking for it alone leaves every content field null. Mapping it to html_content in the extractor would have read a field the service never fills, so the empty body would stay.

What the PR does now:

  • On the serve path, buildConvertRequest rejects it before the request is built, with the reason and both ways out: useDoclingServe=false to run the CLI, or outputFormat=html to get the whole document's HTML from docling-serve. No more silent empty body.
  • On the CLI path it now maps through: --to html_split_page, written with an html extension. docling writes the split pages as one .html file (docling/cli/main.py), so the extension follows html.
  • DoclingServeOutputFormatTest (new) pins the rejection, including in upper case, and that the five values the service can return still reach toFormats. DoclingOutputFormatTest gained the --to case and the extension case, so getOutputFileExtension is now covered — it was not asserted before, which is why the missing html_split_page case went unnoticed.

Reverse-checked: dropping the guard fails only the two serve-path tests, dropping the --to case fails only htmlSplitPageIsRequestedFromTheCli, dropping the extension case fails only htmlSplitPageIsWrittenAsHtml.

On point 2: agreed, no producer-level serve test exists for the other formats either — DoclingServeProducerTest needs a real service and is skipped without one. I kept the new test at the request level, which is where the bug was.

Regression: the component's 100 unit tests pass apart from DoclingCustomArgsValidationTest, which fails the same way on an unmodified tree here because /etc/cron.d and /var/www/html/uploads are not absolute paths on Windows.

@WMK666666

Copy link
Copy Markdown
Contributor Author

Added one thing to the same branch: an entry in camel-4x-upgrade-guide-4_24.adoc, because the value is documented and a route that used it now gets an error instead of an empty body on the serve path, and the CLI now gets the format it asked for. Both are worth a line for anyone upgrading.

@Croway

Croway commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

The docling-serve side looks right: doctags_content is only filled when DocTags is requested, and an in-body response never carries split-page HTML (docling-jobkit _export_document_as_content). So the extractor case and the html_split_page rejection fix real empty bodies.

  1. The CLI changes can't be reached. outputFormat and CamelDoclingOutputFormat are only read by SUBMIT_ASYNC_CONVERSION (DoclingProducer.java:371), which only works against docling-serve. Every CLI call passes a fixed format: "markdown", "html", "json" or "text" (lines 309, 321, 331, 343, 353, 717).
    • I ran every CLI operation with outputFormat=doctags, as the option and as the header, against a fake docling script. On main and on this branch it always got --to md/html/json/text.
    • So the new cases in mapToDoclingFormat / getOutputFileExtension, DoclingOutputFormatTest, and the second paragraph of the upgrade-guide entry describe a change no route can see. (The earlier review asked for the CLI html_split_page mapping, but the CLI never receives that value either.)
    • Please drop the CLI half, or keep it as symmetry but remove the CLI claims from the upgrade guide and the description.
  2. Formatting: mvn formatter:format rewraps the class Javadoc of DoclingServeOutputFormatTest, and CI fails on the uncommitted change.
  3. Description: it still says html_split_page is "deliberately left as it is".
  4. Conflict with camel-docling: honour the OCR headers in the CLI command #29595: both PRs change the buildConvertRequest signature. Whichever merges second has to adapt DoclingServeOutputFormatTest to buildConvertRequest(source, format, exchange).

Claude Code on behalf of Croway

@WMK666666

WMK666666 commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author

Both points are taken in 7d32190.

The CLI half is gone (@Croway). The CLI operations pass a hardcoded format to buildDoclingCommand —
markdown, html, json or text — so outputFormat never reaches mapToDoclingFormat or
getOutputFileExtension with anything else. The md/txt/doctags/html_split_page cases were
unreachable, and so was DoclingOutputFormatTest; both are removed and the two maps are back to what main
has.

html_split_page (@davsclaus) is the fail-with-a-message branch, because there is no field to map it to:
DocumentResponse has five content fields — doctagsContent, htmlContent, jsonContent,
markdownContent, textContent — and the html one is only filled when HTML is requested. The split pages
are not in the response at all, which is why the body was empty. The message no longer points at the CLI
either, since that is not a way to get them; it names outputFormat=html as the way to get the HTML of the
whole document.

What is left is the doctags half you both reviewed: the extractor reads doctags_content, and
html_split_page no longer returns an empty body on the serve path.

Formatter: mvn formatter:format did rewrap the Javadoc of DoclingServeOutputFormatTest; the formatted
result is committed, and formatter:validate plus impsort:check pass.

Interaction with #29595: that PR replaces buildConvertRequest(String, String) with a three-argument
overload that takes the exchange, so the two used to touch the same method. The rejection now sits in
mapToOutputFormat — where the format is decided, and which #29595 does not touch — and the test asks that
map instead of building a request. The two PRs no longer overlap, so either can land first.

Tests: DoclingServeOutputFormatTest (3) and DoclingContentExtractorTest (4) green.
mvn -pl components/camel-ai/camel-docling test: 93 tests, 2 failures, both
DoclingCustomArgsValidationTest on Windows (/etc/cron.d is not an absolute path there), the same two that
fail without this change.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 1617c89. Since fc27b5b there are four commits: html_split_page now fails, there is an upgrade-guide entry, the CLI half is gone, and the newest commit moves the rejection into mapToOutputFormat.

My point 1 is fixed with the second option I gave: fail with a clear message. That is the right choice, because DocumentResponse has no field for the split pages, so there is nothing to map them to. Croway's comment is handled too:

  • CLI half dropped: I confirmed that every CLI operation passes a fixed format (DoclingProducer.java:309-353, :717), so outputFormat never reached mapToDoclingFormat/getOutputFileExtension. Those two maps are back to what main has, and DoclingOutputFormatTest is removed.
  • Overlap with #29595: the rejection now sits in mapToOutputFormat (DoclingProducer.java:1673-1697), not in buildConvertRequest. DoclingProducer.java now merges cleanly with #29595.
  • Description: it no longer says html_split_page is left as it is.

What is left is small:

  1. Nit: the title still says "doctags and txt", but this PR no longer changes txt (DoclingContentExtractor.mapOutputFormat already handled it). Something like "camel-docling: return DocTags content and reject html_split_page on docling-serve" would fit. Since 1617c89, the description's "buildConvertRequest rejects html_split_page" should say mapToOutputFormat.
  2. Suggestion: the outputFormat option still lists html_split_page in its enums (DoclingConfiguration.java:55-56). outputFormat is only read by SUBMIT_ASYNC_CONVERSION, which needs docling-serve, so that value now always fails. Adding "html_split_page is not supported by docling-serve" to the option description (and regenerating) would make the catalog and the docs agree with the code. A follow-up is fine for this one.
  3. #29591 and #29595 also append a camel-docling entry at the end of camel-4x-upgrade-guide-4_24.adoc (here at line 7938). Whichever PR merges second needs a trivial rebase.

The doctags half is unchanged from what I reviewed, and the upgrade-guide entry is accurate. CI has not run on 1617c89 yet (it is waiting for approval).

Claude Code on behalf of Claus Ibsen. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

@WMK666666 WMK666666 changed the title camel-docling: produce the requested format for doctags and txt camel-docling: return DocTags content and reject html_split_page on docling-serve Nov 12, 2026
@WMK666666

Copy link
Copy Markdown
Contributor Author

Point 1 is in: the title is now camel-docling: return DocTags content and reject html_split_page on
docling-serve
.

The description point was already handled when the body was rewritten at 16:23Z — it says
mapToOutputFormat rejects the value (that map is where the format is decided), and buildConvertRequest is
not mentioned in it any more. The sentence you quoted reads "mapToOutputFormat rejects html_split_page
with the reason, for the same reason the doctags case exists".

Point 2 (outputFormat still lists html_split_page in its enums): I left it out of this PR, as you
suggested, because it is not just a description change — the option description lives in the
@UriParam of DoclingConfiguration, so the generated metadata (src/generated/java/.../docling.json and
the catalog copies) has to be regenerated and committed with it, which would add a second concern to a PR
that is otherwise two lines of behaviour. It is worth doing: after this PR that value can only produce the
error, since outputFormat is read only by SUBMIT_ASYNC_CONVERSION, which needs docling-serve. Say the
word and I will do it here instead of in a follow-up — I would rather you decide than open another PR you
have to review.

Point 3 (upgrade-guide conflict with #29591/#29595): understood, whichever lands second rebases. #29594's
entry is the last one in the camel-docling section of camel-4x-upgrade-guide-4_24.adoc, and
DoclingProducer.java no longer overlaps either PR, so only the docs file would need the rebase.

CI on 1617c89 is still waiting for approval, as you noted.

@WMK666666

Copy link
Copy Markdown
Contributor Author

A note on the Build and test check (it is red on 0e90f55 and again on 80d7804 once approved): the failure
is not in this module.

  • Failing test: camel-jbang-core ExportMainApplicationPropertiesTest.shouldExportUserPropertyOverride —
    expected: <test> but was: <world>, i.e. --property hello=test not overriding the sample
    application.properties. Three attempts (surefire.rerunFailingTestsCount=2), all fail.
  • Camel :: AI :: Docling ... SUCCESS [23.192 s] in the very same reactor run, so the component builds and
    its tests pass on the PR.
  • It is not specific to this branch: the same test fails on unrelated PRs in the same window —
    fix/camel-26653-minio-max-mesages-per-poll (run 43088298315) and feature/CAMEL-23832-cxf-descriptions
    (run 43073244596). camel-jbang-core has no dependency on camel-docling.

So I read it as a main-branch problem, not something this PR introduces. Nothing to change here for it, but
the check will show red until main is fixed or the run is retried.

@WMK666666
WMK666666 force-pushed the docling-output-format branch from 1617c89 to 55f1def Compare November 12, 2026 08:24
@WMK666666

Copy link
Copy Markdown
Contributor Author

Rebased on main (55f1def) as well, since #29591 and #29593 are in and this branch had the same conflict at
the end of the camel-docling section of the 4.24 upgrade guide. Both entries are kept: the apiKeyHeader
one from main first, then the html_split_page one from this PR.

DoclingProducer.java again merged cleanly with #29593's BearerTokenDoclingServeClient selection; the
html_split_page rejection in mapToOutputFormat is untouched by it. DoclingContentExtractor.java did
not change on main.

Diff is unchanged by the rebase: +188/-2 over the same five files, one commit on top of 009cff12.

Two open items from your last review, for the record:

  • The title is now camel-docling: return DocTags content and reject html_split_page on docling-serve, and
    the description already says the rejection is in mapToOutputFormat (it was rewritten with the 16:23Z
    update; buildConvertRequest no longer appears).
  • The outputFormat enums note is still not in this PR, as you allowed. It needs
    DoclingConfiguration's @UriParam description plus the regenerated metadata, so it is a second concern;
    if you would rather have it here than in a follow-up, say so and I will push it on this branch.

CI on the new commit will need approval.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR

Approve: a squash and rebase of 1617c89 with identical content, and the title and description nits are fixed. Blockers: None. Other findings: 2 (optional / heads-up)

Thanks @WMK666666. The changed lines of the squashed commit are identical to 1617c89, the version I last reviewed.

  1. Title and description: fixed; the description now says mapToOutputFormat rejects html_split_page.
  2. [Suggestion, follow-up is fine] outputFormat still lists html_split_page in its enums (DoclingConfiguration.java:55-56), and that value now always fails on docling-serve. One sentence in the option description, plus a regen, would make the catalog agree with the code.
  3. Heads-up: this PR and #29595 both append a camel-docling section after the apiKeyHeader one in camel-4x-upgrade-guide-4_24.adoc, so whichever merges second needs a trivial rebase.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

@davsclaus davsclaus added this to the 4.24.0 milestone Nov 12, 2026
@davsclaus davsclaus added the bug Something isn't working label Nov 12, 2026
@davsclaus

Copy link
Copy Markdown
Contributor

Thanks @WMK666666, this is approved, but #29595 was just merged and touches the same camel-docling files, so this PR now has a merge conflict. Could you rebase it on main? We can merge it once CI is green again.

Claude Code on behalf of davsclaus

@gnodet

gnodet commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

🔄 Backport Bot

This bugfix targets main and may need porting to:

  • camel-4.22.x

Add a port/<branch> label to automatically create a backport PR for that branch.

Backport PRs will be created automatically on merge (once a port label is added). Comment /port to create them immediately.

ℹ️ If you push additional commits after /port, use /port again to update the port PRs.

…ocling-serve

Rebased on main now that apache#29595 is in, as asked.

The only conflict was the 4.24 upgrade guide, where apache#29595 appended its
OCR-headers entry after apache#29591's apiKeyHeader one: the file now carries
all three camel-docling sections, apiKeyHeader first, then the OCR
headers, then html_split_page.

DoclingProducer.java merged cleanly with the OcrSettings refactor from
apache#29595: the html_split_page rejection and the package-private
mapToOutputFormat sit below resolveOcrSettings and are untouched by it.
DoclingContentExtractor.java is unchanged on main.

Content is otherwise the version approved in 55f1def: DoclingContentExtractor
reads doctags_content when DocTags is requested, and mapToOutputFormat
rejects html_split_page with the reason and the way out, because
DocumentResponse has no field for the split pages.
@WMK666666
WMK666666 force-pushed the docling-output-format branch from 55f1def to 14e39d4 Compare November 12, 2026 12:32
@WMK666666

Copy link
Copy Markdown
Contributor Author

Rebased on main (14e39d4), on top of fe2f3ce.

DoclingProducer.java merged cleanly with #29595's OcrSettings refactor: the html_split_page rejection
and the package-private mapToOutputFormat sit below resolveOcrSettings and were not touched by it.
DoclingContentExtractor.java is unchanged on main.

The only conflict was, as you predicted, the 4.24 upgrade guide. It now carries three camel-docling
sections in merge order — apiKeyHeader (from #29591), the OCR headers (from #29595), then
html_split_page (this PR) — with a blank line between them.

The diff is unchanged by the rebase, which is how I checked nothing was lost: +188/-2 over the same five
files (DoclingProducer.java +15/−2, DoclingContentExtractor.java +5, the two tests +81/+80, the guide
+7), one commit on top of fe2f3ce, mergeable=True.

CI will need another approval for 14e39d4; nothing else has changed.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on my earlier review (SHA 55f1def) — the new commits add the html_split_page rejection that davsclaus flagged, plus two new test classes. All previous findings are addressed.

Checked the new code:

  • mapToOutputFormat throws IllegalArgumentException for html_split_page during request-building (before the API call) — correct fail-fast behavior.
  • Case-insensitivity works: .toLowerCase() is called before the switch, so HTML_SPLIT_PAGE hits the throw case.
  • DoclingContentExtractorTest and DoclingServeOutputFormatTest cover the new code paths. doctagsReturnsTheDoctagsContent would fail without the new extractor case; htmlSplitPageFailsWithTheReasonInsteadOfAnEmptyBody would fail without the throw. Tests are valid.
  • Upgrade guide entry is accurate and actionable.

LGTM. ✅

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@WMK666666

Copy link
Copy Markdown
Contributor Author

Opened the outputFormat follow-up from your review as a separate PR: #29720 (the option description now says html_split_page is rejected on the docling-serve path, plus the matching docling.json regen). It should merge after this one, since it only documents the behaviour this PR introduces.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants