tballison commented on PR #3044:
URL: https://github.com/apache/tika/pull/3044#issuecomment-5432419659
Let me know what your agent thinks of my agent's input.
```
Verdict: still spiking (contract findings). Base 2b70202aae, head
9d22e1a186. Shape: full (6 Opus agents + direction). Direction: proceed — the
cheap alternative
(registry-only + ZipParser) yields zero text because all GeoGebra text
lives in XML attributes and Tika's XML parsers emit character data only;
miscoffice is the right
module. CI green; touched modules build and pass locally; hygiene grep
clean; schema JSON hand-edits byte-match the generator.
Contract (maintainer decisions / interface-shaping)
1. Metadata key casing — geogebra:appName/appVersion/formatVersion are
Tika-coined and must be kebab-case per metadata-changes-4x.adoc:162 ("names
Tika coined are
kebab-cased; verbatim names keep spelling"). toolName, id, date are
verbatim attributes → fine. Public API freezes at 4.1.0; no 4.x→4.x migration
path exists.
2. Component name — @TikaComponent without name yields geo-gebra-parser
(KebabCaseConverter). Set name = "geogebra-parser" now; it's public config
surface.
3. Macro XML clobbers worksheet metadata (GeoGebraParser.java:231-240,
handler :63-75) — both XMLs get the same Metadata; the macro's root <geogebra>
and its nested
<construction> overwrite app/version/format and dc:title/dc:creator.
The fixture hides it (identical versions, blank macro construction attrs).
Reached independently
by 4 lanes. Fix: metadata only from the primary XML; construction only
at depth 1.
4. handleExpression drops the most common text-object form
(GeoGebraXMLHandler.java:106-112) — "Area = "+a is dropped entirely; "a"+"b"
emits a"+"b. Extract each quoted
segment instead of requiring a single literal. The \" unescape is
unverified against the format.
5. Detector at head of the SPI file — first-non-null, no sorting; a
planted root geogebra.xml now outranks JAR/APK/ODF/StarOffice detection. Every
prior detector was
appended. Move to end; also swap the full central-directory walk for
getEntry() lookups (enumerate only when structure.json exists) — this detector
now runs on every
zip Tika sees.
6. No degraded-input handling, unlike ZipParser — (a) unguarded
getInputStream at :273/:296/:333: one AES/unsupported entry aborts the whole
parse (canReadEntryData +
recordEmbeddedStreamException convention); (b) ZipFileHelper.open
failure has no fallback — and since .ggb is now a zip subclass, any bytes named
x.ggb route here and
throw a bare IOException where 4.0.0 gave application/zip; (c) a
malformed slide XML aborts remaining slides + thumbnail + pictures and leaves
<div class="slide">
unclosed (EpubParser's per-part catch/drain is the module convention).
7. .ggs with missing/corrupt structure.json → zero output, no error
(:205-208, :319-325) — slides path is gated on structure.json, then the
worksheet path skips every
_slideN/geogebra.xml by basename. structure.json should supply order
only.
8. geogebra_javascript.js is user code, dropped as "housekeeping" — should
be emitted as EmbeddedResourceType.MACRO. Related: INLINE on every other entry
is wrong for
non-images (VLM/OCR-encode parsers gate on INLINE); use ATTACHMENT or
leave unset. Also basename skip-matching lets dir/geogebra_javascript.js hide
anything.
9. Streaming detection path entirely untested — delete
streamingDetectFinal's body and all tests pass. Add a .ggb to
tika-parsers-standard-integration-tests/test-documents —
TestContainerAwareDetector:117 auto-exercises streaming for every zip
specialization. More broadly all three
fixtures are synthetic; one real GeoGebra file would have caught #4.
10. Upgrade behavior change under-documented — 4.0.0 typed real .ggb as
application/zip (glob lost to magic) with every entry as an attachment; now 1
attachment and no
zip:* metadata. CHANGES should say so, and should drop the false claim
that the detector identifies .ggp.
Edge-case
- readTree on structure.json unbounded (peers use BoundedInputStream);
per-<content> readTree with no shape pre-check builds an exception per non-JSON
value, and blank
output disengages the write limit. Guard on leading [/{; consider
streaming jackson-core (cad-module precedent) instead of databind.
- First slide without a thumbnail → no thumbnail at all (:260-264; others
filtered by basename).
- Slide sort: leading zeros misorder (_slide007 after _slide10); sort runs
before the early return.
- structure.json + _slideN/ + root geogebra.xml → root XML silently never
parsed nor emitted.
- Hard-coded image/png on thumbnail: redundant (glob) and a lie for
non-PNG bytes; the test assertion on it is vacuous.
- Untested: malformed-structure.json fallback, slides-missing-from-JSON
append, unquoted-expression skip, cmdName fallback, nested/multi-line content,
BAG cardinality,
<div class="slide">, first-slide-only metadata (fixture values
identical).
- No PageAnchoring on _slideN/ pictures despite setting N_PAGES
(OpenDocumentParser in the same module does it).
Hygiene (one line each)
Private isBlank duplicates StringUtils.isBlank (used 3× in module) · two
static ObjectMappers · inZip/numericallySorted misnamed ·
handleThumbnail/handleOtherEntries
duplicate ~10 lines · {@code <geogebra>} renders entities literally
· javadoc claims "ink notes" (penstrokes carry no text) and omits macro help ·
3-line sort
comment and test comments that narrate asserts / echo the Copilot exchange
· regen.sh should produce the schema JSONs (result matches anyway) · CHANGES
~2× target length
— proposed: "New GeoGebraParser for *.ggb/*.ggs/*.ggt: geogebra:*
metadata, text, thumbnail as THUMBNAIL embedded doc. Previously typed
application/zip with every entry
as an attachment. *.ggs/*.ggp are new mime types; *.ggp is glob-only
(TIKA-4831)."
```
--
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]
