tballison commented on PR #2919:
URL: https://github.com/apache/tika/pull/2919#issuecomment-4846044029
Claude makes these two points. They sound reasonable?
```
Two things worth knowing before it merges
1. The ParseHandler change is redundant (harmless). The PR re-adds
parseContext.set(ParseMode.class, parseMode) for "Root cause 1," but that's
already fixed on
main/rc1 — PipesWorker.setupParseContext() (commit 0b38268d4, the earlier
TIKA-4735 fix) already writes the resolved mode back into the same context
object before
EmitHandler reads it. That's why, on current rc1, you see a filtered
[{"X-TIKA:content":"…"}] (filter applied → ParseMode was visible) rather than
full metadata.
So only "Root cause 2" was actually still open. The ParseHandler line is
belt-and-suspenders — fine to keep, just not load-bearing.
2. ⚠️ The tests don't actually lock in this regression. This is the one
thing I'd push on. None of the added/changed tests exercise the
DYNAMIC-passback path that
was the bug:
- AsyncCliParserTest — tests arg parsing only (no emit).
- TikaConfigAsyncWriterTest — tests generated config has
parseMode=CONTENT_ONLY (no emit).
- The existing AsyncProcessorTest#testContentOnlyFromConfigDefault it
cites uses a config with emitStrategy: EMIT_ALL hardcoded — that test passed
before this fix
too, because EMIT_ALL never hits the passback branch.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
