OPENNLP-1930: Regex removal (2/10): Parse Arvores Deitadas markup with cursor scans

[krickert]

Replaces the regular expressions in the Arvores Deitadas (AD) corpus readers with cursor-based character scans built on the StringUtil helpers. This PR also carries the AD reader changes that were split out of #1275.

The hyphen-splitting default change (splitHyphenatedTokens), ADDetokenizer, ADHyphenatedTokenTest and evalPortugueseTokenizerDefaultNaiveBayes are not in this PR. They moved to OPENNLP-1984, a separate stacked PR that will be opened after this one is merged.

Changes

• ADSentenceStream: the markup patterns (<s>, </s>, <p>, <t>, </t>, <caixa>, </caixa>, <ext>, </ext>) are replaced by exact tag-name checks. SentenceParser reads node, leaf, bizarre-leaf and punctuation lines, and the fallback check, with cursor scans. The » + whitespace + ./, cleanup no longer uses a regex.
• ADNameSampleStream: the NER tag, whitespace, underscore, hyphen and alphanumeric patterns are replaced by tagContent, StringUtil.splitOnUnicodeWhitespace, splitOnUnderscores, matchHyphenatedToken and isAlphaNumeric. getTextID reads its ids through ADMetadata.
• ADPOSSampleStream: POS tags that contain whitespace are joined with = by replaceWhitespaceWithEquals instead of \s+.
• ADSentenceSampleStream: the text and paragraph ids come from ADMetadata.parseTextAndParagraph instead of META_1.
• New package-private ADMetadata: reads the text id, paragraph id, text prefix (LIT) and source attribute (CIE). An id that does not fit into an int counts as invalid metadata.
• DetokenizerSampleStreamFactory: closes the detokenizer dictionary input stream after loading. Before this, JUnit @TempDir cleanup failed on Windows.
• corpora.xml: documents the AD reader contract, including the whitespace definition, markup tags, the lemma boundary, tag-less tree lines, metadata ids and punctuation lines.

Behavior changes

The scans follow the format rather than quirks of the old patterns:

• Quoted lemma boundary: a quoted lemma ends at the first matching quote that is followed by whitespace, a secondary tag or a closing bracket. The old greedy patterns backtracked to later quotes, and the new scan does not. Example: =H:n("x" M S)<tab>a') b now gives lemma x, morphology M S and lexeme a') b. Before, it gave lemma x" M S)<tab>a, no morphology and lexeme b. If the rest of the line does not parse after that quote, the line becomes a fallback leaf. Quotes inside a word stay part of the lemma.
• Tree lines without a tag: a tree line that has no tag keeps its full lexeme, and the first lexeme character is no longer dropped.
• Other parsing fixes: the tail of a node line that has parentheses inside a group is now read, and so are tags with quoted brackets.
• Whitespace: whitespace is the Unicode White_Space property (including NBSP, U+0085, U+2028 and U+2029), independent of the toolkit whitespace mode.
• Tag names: markup tags must match by exact name. <sx> and a self-closing <s/> are not sentence tags.
• Punctuation lines: a punctuation line has no letter or digit of any script. An accented or non-Latin word is no longer treated as punctuation.
• Hyphen splitting: when hyphen splitting is enabled, combining marks stay attached to their letters.
• Underscores: leading, trailing or repeated underscores in a lexeme no longer produce empty tokens.

Validation

• AD tests at the head: ADSentenceStreamTest 248, ADMetadataTest 87, ADNameSampleStreamTest 84 test runs; all 472 tests in opennlp.tools.formats.ad pass.
• ArvoresDeitadasEval on Bosque_CF_8.0 and FlorestaVirgem_CF_3.0: the verifyTrainingData hashes of the sentence samples are unchanged. The tree, name, POS and chunk streams differ in 2 Bosque and 86 Floresta samples. All of these differences come from the reader fixes above: tag fragments such as .vp and <ARGOpp no longer appear as lexemes, and tag-less tree lines keep their text (P: reads DP, CJT: reads CJT, and the =H==( and =H===( lines contribute ( again).
• The tokenizer eval constants in ArvoresDeitadasEval are unchanged in this PR.

OPENNLP-1930

[rzo1]

Here are some additional comments. We ran all AD streams (sentence, name, POS, chunk, raw tree, detokenized tokens) over the real Bosque_CF_8.0 and FlorestaVirgem_CF_3.0 corpora with main, #1275 and this PR. This PR changes nothing there: every stream is byte-identical to #1275, and the verifyTrainingData hashes in ArvoresDeitadasEval stay the same. The tokenizer eval input does change, but that comes from #1275 (see below). The divergences are all in markup and whitespace the corpora don't contain, and the PR doesn't document them. Def. still needs the eval build before merge.

Blocking

1. PR description. It says the parsers "accept the same lines as the patterns they replace", with ~911k fuzzed lines and "500,000 for the metadata, with zero differences", and that "the line-terminator rule of the original patterns … is restored". None of that holds at the head: commit 5795ab2 added hasLineTerminator, beac8cd removed it again, and 7a8ebb4/beac8cd67 change behavior on purpose. OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275 vs this PR, ASCII-only fuzz: 3,968 differences in 5 kinds:

   • A. A whitespace-only lexeme (=a=a() ) is no longer a Leaf. Documented and tested.
   • B. \w became Character.isLetterOrDigit in the fallback check, so =-é. is now null and the word is dropped. Tested, not documented.
   • C. Whitespace follows the global mode (see 3).
   • D. The fallback check now reads past line terminators. Tested and documented.
   • E. Tag matching: <sx>, <tx>, <title>, <pp>, <caixa2>, <exta> and <situacao id="2"> now parse differently (<tx>/<title> shift sentence samples 143–150 times in 30k mutated documents; <situacao> in place of <s ...> changes all streams). <s/> and <p/> matched <s[^>]*> before and are rejected now, untested and undocumented.

   Also, lines that threw SIOOBE and lines that parsed have partly swapped. Rewrite the description to list these changes and the corpus results above. The test counts are off too ("ADMetadataTest adds 61": there are 63, and 22 cases removed from ADSentenceSampleStreamTest go unmentioned).

2. ADMetadata.java:98-122, ADMetadataTest.java:55/:129. Metadata containing U+0085/U+2028/U+2029 used to throw "Invalid metadata" and is now read (3M fuzz inputs: 0 differences without a line terminator; with one, 86,262 / 24,227 / 52,206, all old reject → new accept, e.g. 12 p=1<U+2028>). The test comment "the reader does not produce one inside a line" is wrong: BufferedReader.readLine only splits on \n/\r, and an ISO-8859-1 byte 0x85 in a SOURCE: line reaches meta. Pick one behavior on purpose. Either restore the rejection, or keep the lenient version, fix the comment, and add a stream-level test with U+0085 in SOURCE:.

3. ADSentenceStream.java:528, :564, :657, :695, :932. Tree and markup parsing depend on opennlp.whitespace.mode (OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275 blocking 5, still open and now wider):

   • An ISO-8859-1 cp1252 ellipsis byte 0x85 becomes U+0085: ==PU:pu("<U+0085>" PU)\t<U+0085> is a Node under UNICODE (token lost) and a Leaf under LEGACY.
   • ==H:n("casa" F S)<U+0085>casa parses under UNICODE and becomes a junk fallback leaf under LEGACY; …)<U+001C>casa and =S:np<U+001C> do the reverse.
   • NBSP in a functional tag: =H:n<NBSP>("o" M S)\tx was a leaf with func n<NBSP>; now a fallback leaf with lexeme :n<NBSP>("o" M S)\tx.
   • =S:n<U+2028>p was a node and is now a fallback leaf whose junk token reaches the name samples (X:p<U+2028>ar <START:organization> Editora …).
   • Trailing U+2028 or NBSP is stripped from node tags.

   corpora.xml:458-459 ("A line separator or next line character inside a line is read as an ordinary character") is therefore wrong, and so is the test comment at :103. Exclude line terminators in scanTag, isTagGroupRun and skipWhitespace, or use a fixed predicate. Document the mode dependency (corpora.xml:443 says "Unicode whitespace"), and add tests in both modes for U+0085 as lexeme, NBSP in a tag, U+2028 and U+001C.

4. Eval build. Run ArvoresDeitadasEval before merge and list it as done:

   • The verifyTrainingData hashes stay the same for main, OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275 and this PR (Bosque 140568367548727787313497336739085858596, Floresta 2614161133949079191933514776652602918), so the sentence-detector and chunker cross-evals get identical input.
   • The tokenizer eval input changes because of OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275: FlorestaVirgem lines 495095/503989/503995/504001 (=H==(, =H===() give an empty fallback lexeme, which main emitted as "" and splitOnUnderscores now drops. Perceptron 10-fold: main 0.9994887308380267, this PR 0.9994907289173516. That is within the 0.0001 delta but makes the constant stale.
   • GIS, QN and NB were not run.

Minor

• ADSentenceStream.java:925-935, ADSentenceStreamTest.java:309-338. Add rows for <s/>, <p/>, <t/> and <s<U+0085>id>, and say in corpora.xml:454-457 that a self-closing tag is not a tag.
• corpora.xml:448-450. Kind B (a fallback lexeme starting with a letter of any script is ignored) is tested but not documented. Add one sentence. :447-448 still says "the first run of digits" (OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275); the text id follows the leading ASCII letters and hyphens directly. :455 is overlong.
• ADSentenceStream.java:446-461. The bizarre-leaf lemma loop runs indexOf(')') again for every quote and hyphen candidate, so it is cubic: "=" + "-"*4000 + "=y(\"" + "\""*4000 + ")" takes 2.0 s. The old regex took 163 s, so it's not a regression, but compute the first ) once, as parseLeaf does with noRestAt.
• ADSentenceStream.java:370. new boolean[line.length() + 1] is allocated for every non-node line before the (+quote check at :391. Allocate it lazily.
• ADSentenceStream.java:349/371/427/492-505, :591, :625, :644; ADMetadata.java:42-49, :98-122; ADSentenceSampleStream.java:138-143. Arrays used as tuples again (tag[0], rest[0..4], int[]{text, para}, a 4-slot span array, two arrays per sentence). Use private records. scanLevelAndTag recomputes the prefix run on every call, and :403 searches again for the separator scanTag already found.
• ADSentenceStream.java:391, :447, :527, :543-544, :610-612, :639, :678, :708. '(', ')', '<', '>' and quotes are literals while TAG_SEPARATOR/TAG_GROUP_OPEN/TAG_GROUP_CLOSE (:66-69) are constants. Either drop the constants or declare all of them.
• ADSentenceStream.java:381-388, :478-491, :580-604. The @return texts describe array slots, and "indexes after which no rest was found" is unexplained jargon. Make them concrete once records replace the arrays.
• ADSentenceStream.java:313-314 (pre-existing). line.substring(level + 1) throws SIOOBE for a fallback line ending in =; parse() swallows it and returns null, which ends read() early. This PR changes which lines reach it (====N<:adj'a'b <<a> )<U+2028>= was a Node and now throws). Guard it while touching this code.
• ADSentenceStream.java:905-910 (pre-existing, all versions). read() loops forever when an opening <s …> has no content before EOF. Worth a JIRA.
• ADMetadata.java:39-41. parseTextAndParagraph throws NumberFormatException for a digit run over Integer.MAX_VALUE (so does getTextID:557). Add @throws, or map it to "Invalid metadata".
• ADNameSampleStream.java:538-550 (pre-existing). textIdMeta2/textMeta2 are locals, so the LIT/CIE branch always returns 0 (or -1 for source="") and clearAdaptiveData never fires for LIT/CIE. The rewrite keeps this dead compare-and-increment. Either fix it with fields (separate JIRA, eval build) or reduce it to validation plus return 0 with a comment.
• ADNameSampleStream.java:541-542, :552, :536. meta.startsWith("LIT") is evaluated twice, and "LIT"/"CIE" are bare literals; declare constants and branch once. // Amazonia explains nothing; add Javadoc to getTextID. Remove the blank line after the brace at :536.
• Still open from OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275, please address here:
  • NER_PREFIX/magic 5 (ADNameSampleStream.java:501)
  • AIOOBE on splitOnUnderscores("_") (:265-269)
  • static helpers (ADNameSampleStream.java:375/401/425/458/478/497/511, ADPOSSampleStream.java:160, ADSentenceStream.java:925/:945)
  • duplicated anonymous ObjectStream helpers in ADSentenceSampleStreamTest.java:77-85 and ADNameSampleStreamTest.java:210-218 (use ObjectStreamUtils.createObjectStream)
  • bare RuntimeException asserts
• ADMetadataTest.java. The move dropped reject cases "CF_1001 p=2", "CF 1001 p=2", "1001 p =2" and "1001 p= 2". Put them back.
• Title. Implementation wording. Suggestion: "OPENNLP-1930: Read Arvores Deitadas tags by exact name and ignore whitespace-only lexemes".
• Stacking. OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275 blocking 1 still applies: this PR sits on ccea670, which contains the first round of AD changes. Move those here.
• Pre-existing, for a separate JIRA. ADChunkSampleStream throws NPE on FlorestaVirgem after 4,641 samples (the eval uses Bosque, so it is unaffected).

Verified:

• ADMetadata is package-private, so it adds no frozen API.
• No java.util.regex is left in the ad package, and the per-paragraph Pattern.compile and dead commented-out copies from OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275 are gone.
• The metadata scan matches the old regexes exactly on input without line terminators, including overflow and non-ASCII digits.

[rzo1]

CI is red.

[rzo1]

#1275 is split up and closed. The AD reader changes it carried (ADNameSampleStream, ADPOSSampleStream, ADSentenceStream, ADSentenceSampleStream, their tests and the AD paragraph of the corpora chapter) now belong to this PR, which already contains them. Please rebase onto #1311 (part 1) instead of the old #1275 branch.

[krickert]

I'm running eval locally, if that's green I'll push

[rzo1]

I'm running eval locally, if that's green I'll push

Pleasae rebase this one on latest main. This will be next in the Regex queue.

[rzo1]

Thanks for the rebase. The branch is on current main, and the Windows fix (closing the detokenizer dictionary stream) looks right.

Blocking

1. Hyphen-splitting default. splitHyphenatedTokens changes from true to false in ADNameSampleStreamFactory (and in all 8 cli.xml entries), and ADTokenSampleStreamFactory now wraps the detokenizer in the new ADDetokenizer. Anyone using -format ad for name finder or tokenizer training silently gets different training data. Nobody asked for this in OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275, OPENNLP-1930: Regex removal (2/10): Parse Arvores Deitadas markup with cursor scans #1276, or OPENNLP-1928: Regex removal (1/10): Replace trivial patterns with character scans #1311, and it isn't regex removal. Please move it, together with ADDetokenizer, ADHyphenatedTokenTest, and evalPortugueseTokenizerDefaultNaiveBayes, into its own JIRA and PR with a release note.

2. PR description and commit message.

   • The body still says "Based on OPENNLP-1928: Regex removal (1/8): Replace trivial patterns with character scans in StringUtil #1275". It doesn't mention the hyphen default, ADDetokenizer, the detokenizer stream fix, or the lemma boundary rule from 2def6bf (the first matching quote at a field boundary ends the lemma, and later quotes are no longer tried).
   • The test counts are wrong. At the head they are: ADSentenceStreamTest 248, ADMetadataTest 87, ADNameSampleStreamTest 84.
   • Commit 3d48242 says "the output is unchanged", but the hyphen default changes CLI output.
   • It also mentions an ADChunkSampleStream change that this PR doesn't contain; that change is already on main via OPENNLP-1950 (OPENNLP-1950: Fix ADChunkSampleStream fails with NPE on the FlorestaVirgem corpus #1299).

   Please update both.

3. Eval build. Please post the ArvoresDeitadasEval results for all tokenizer variants: Perceptron, GIS, QN, and NB. The description says the Perceptron value moves and that GIS, QN, and NB were not run, but the constants in the diff are unchanged. If the new NB test stays in some PR, its expected value (0.9951936406770416) needs a run that someone other than the author has checked.

4. Please update the description as well.

Minor

• The lemma scan now stops at the first quote that closes the field and no longer backtracks to later quotes like the old greedy pattern did. If the rest of the line doesn't parse after that quote, the line becomes a fallback leaf. That matches the documented intent and is fine, but please list it under the behavior changes in the description.

[krickert]

Moved this to draft. The split would be a stack, so I just need more time to think about it - it's not a big scope either - but I'll let it remain in draft for this reason.

It won't take long to do. I think this is mergable now but I won't flip it back until I test the new PR/JIRA. It's just a quick methodical way to do it. (it might only take an hour to fix though, so likely when everyone sees this message it'll probably be green anyway)

[krickert]

Separation done. It's stacked, so I'll open a PR once this is merged - it's also ready to go https://github.com/ai-pipestream/opennlp/pull/new/OPENNLP-1984-ad-hyphen-default

I'll open the new one as soon as this is merged.

I'll do another pass of the remaining PRs. Please let me know if you catch any, let me know and I'll do the same surgery.

Thank you @rzo1 and @mawiesne for your patience on this - greatly appreciate it.

[rzo1]

Thanks for the split. The hyphen default, ADDetokenizer, ADHyphenatedTokenTest and the NB eval are out of this PR now, and I updated the description to match the code.

A few things are still open:

1. Commit message. d556ac4 still says it adds evalPortugueseTokenizerDefaultNaiveBayes (F1 0.9951936406770416), but that test isn't in this PR anymore; it belongs with OPENNLP-1984. Please reword that commit since it goes into the squashed commit message.
2. Unrelated docs change. corpora.xml adds a new "CoNLL-U" section (tools.corpora.conll.u) that has nothing to do with AD. It looks like it slipped in during the rebase. Please drop it or say where it belongs.
3. Eval run. The eval tests are running on this head: https://ci-builds.apache.org/job/opennlp/job/eval-tests-configurable/81/

Once these are sorted, it's good from my side. @mawiesne, could you take a look as well?

[rzo1]

Thanks!

[rzo1]

https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/81/ failed. Needs an value update. Improvment:

ArvoresDeitadasEval.evalPortugueseChunkerPerceptron:190->chunkerCrossEval:138 expected: <0.9631066789979492> but was: <0.963736197429554>

[krickert]

Rerunning..

https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/83/console

[rzo1]

EVAL is FINE now.

[krickert]

It's green again