[
https://issues.apache.org/jira/browse/TIKA-4889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18115689#comment-18115689
]
ASF GitHub Bot commented on TIKA-4889:
--------------------------------------
Copilot commented on code in PR #3174:
URL: https://github.com/apache/tika/pull/3174#discussion_r4018303305
##########
docs/modules/ROOT/pages/configuration/parsers/vlm-parsers.adoc:
##########
@@ -29,8 +29,9 @@ for earlier shapes.
Three implementations are provided out of the box. None is auto-loaded: each
must be
named explicitly in your configuration. (Changed in 4.1.0: `openai-vlm-parser`
previously
auto-registered via SPI.) To use a VLM as the OCR engine for embedded images
and rendered
-PDF pages, name it in the `text-recognizers` list — see
-xref:configuration/index.adoc[Configuration].
+PDF pages, configure it under `engines` and name it in the `text-recognizers`
list — see
+xref:configuration/index.adoc[Configuration]. Naming it under `parsers` is
deprecated since
+4.1.0 and unsupported in 4.2.0.
Review Comment:
The `engines` form shown here cannot load these VLM classes: `EngineLoader`
requires `Engine`, while the VLM implementations are parser-based
`Parser`/`TextRecognizer` components. The page now directs users to an invalid
configuration; document the inline `text-recognizers` form (or add an adapter)
instead.
##########
tika-parsers/tika-parsers-ml/tika-inference/src/main/java/org/apache/tika/inference/OpenAIImageEmbeddingParser.java:
##########
@@ -185,9 +190,10 @@ public void parse(TikaInputStream tis, ContentHandler
handler,
@Override
public void initialize() throws TikaConfigException {
- LOG.info("openai-image-embedding-parser runs one request per image;
the \"engines\" + "
- + "\"inference\" shape (openai-embedding-engine, input IMAGES,
task embed) batches "
- + "a document's images into one request");
+ LOG.warn("openai-image-embedding-parser is deprecated since 4.1.0 and
will be removed in "
+ + "4.2.0: configure the endpoint as an openai-embedding-engine
under \"engines\" "
+ + "and bind it with an IMAGES \"inference\" binding (task
embed), which batches a "
+ + "document's images into one request");
Review Comment:
For a legacy `openai-image-embedding-parser` entry, `ComponentInstantiator`
invokes this initializer and `ParserLoader.finish()` now emits another
deprecation WARN for the same entry. That contradicts the one-WARN-per-entry
behavior documented in the release notes; centralize the warning or suppress
one of these paths.
##########
tika-serialization/src/main/java/org/apache/tika/config/loader/ParserLoader.java:
##########
@@ -205,66 +205,71 @@ private static String names(List<Parser> parsers) {
return names.toString();
}
- /**
- * Enrichers named directly under {@code "parsers"} that the composite
never dispatches
- * to: every type they advertise is claimed by another parser there, or
they advertise
- * none (engine unavailable, or told to skip). Both shapes look configured
and do
- * nothing as parsers.
- */
- static List<Parser> undispatchedEnrichers(Parser root) {
- List<Parser> inert = new ArrayList<>();
+ /** Engines (enrichers) named directly under {@code "parsers"}: the
deprecated 4.0 shape. */
+ static List<Parser> enginesUnderParsers(Parser root) {
+ List<Parser> engines = new ArrayList<>();
if (!(root instanceof CompositeParser composite) || root instanceof
DefaultParser) {
- return inert;
+ return engines;
+ }
+ for (Parser member : composite.getAllComponentParsers()) {
+ if (ContentEnrichers.isEnricher(member)) {
+ engines.add(member);
+ }
+ }
+ return engines;
+ }
+
+ /** The types the composite dispatches to this member as their parser. */
+ static Set<MediaType> parsedTypes(Parser root, Parser member) {
+ Set<MediaType> parsed = new TreeSet<>();
+ if (!(root instanceof CompositeParser composite)) {
+ return parsed;
}
ParseContext empty = new ParseContext();
Map<MediaType, Parser> dispatch = composite.getParsers(empty);
MediaTypeRegistry registry = composite.getMediaTypeRegistry();
- for (Parser member : composite.getAllComponentParsers()) {
- if (!ContentEnrichers.isEnricher(member)) {
- continue;
- }
- boolean dispatched = false;
- for (MediaType type : member.getSupportedTypes(empty)) {
- if (dispatch.get(registry.normalize(type)) == member) {
- dispatched = true;
- break;
- }
- }
- if (!dispatched) {
- inert.add(member);
+ for (MediaType type : member.getSupportedTypes(empty)) {
+ if (dispatch.get(registry.normalize(type)) == member) {
+ parsed.add(type);
}
}
- return inert;
+ return parsed;
}
- // the 4.0 shape still works, so it is INFO; an entry that never runs at
all is a WARN
- private static void logUndispatched(Parser inert, CompositeContentEnricher
enrichers,
- boolean listConfigured) {
- String name = ParserUtils.getParserClassname(inert);
- Set<MediaType> advertised = inert.getSupportedTypes(new
ParseContext());
+ /** One WARN per engine under "parsers": the deprecation, then what the
entry does today. */
+ private static void warnEngineUnderParsers(Parser engine, Parser root,
+ CompositeContentEnricher
enrichers,
+ boolean listConfigured) {
+ String name = ParserUtils.getParserClassname(engine);
+ String lead = name + " is named under \"parsers\", which is deprecated
for engines since "
+ + "4.1.0 and unsupported in 4.2.0: configure it under
\"engines\" and name it in "
+ + "\"text-recognizers\". ";
Review Comment:
This migration advice is invalid for the OCR/VLM parser classes that this
warning handles: `engines` accepts only `Engine` implementations, while those
classes are `Parser`/`ContentEnricher` implementations. Following the warning
makes the replacement fail config loading; direct parser-based recognizers need
the inline `text-recognizers` form unless an Engine adapter is added.
##########
tika-parsers/tika-parsers-ml/tika-inference/src/main/java/org/apache/tika/inference/OpenAIImageEmbeddingParser.java:
##########
@@ -80,7 +80,12 @@
* {@link #parse}.
*
* @since Apache Tika 4.0
+ * @deprecated since 4.1.0, removed in 4.2.0. Configure the endpoint as an
+ * {@code openai-embedding-engine} under {@code "engines"} and bind it with an
{@code IMAGES}
+ * {@code "inference"} binding and the {@code embed} task: one request per
document tree
+ * instead of one per image, and the vectors land where {@link ChunkTarget}
puts them.
Review Comment:
The replacement `IMAGES` binding batches by
`EmbeddingEngine.getMaxBatchSize()`, so documents with more images than that
limit still make multiple requests. This new Javadoc promises one request per
document tree; describe it as bounded batching instead.
This issue also appears on line 195 of the same file.
> Simplify inference configuration
> --------------------------------
>
> Key: TIKA-4889
> URL: https://issues.apache.org/jira/browse/TIKA-4889
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
