feat(trainer): longest-match symbol lookup (RFC 0020 clause 4) - #98
Merged
Merged
Conversation
SymbolStream::next read exactly one unicode character per lookup (a 2002-era design: 'we do not support multi-unicode-character symbols'), so multi-codepoint symbols — digraph outputs, ZWJ emoji families, VS16 skin tones — could never be matched from training text. The trainer split them into per-codepoint unknowns: 31 of the emoji alphabet's 308 nodes were permanently untrainable. CAlphabetMap now collects multi-codepoint keys at Add time (copies — the Entries vector reallocates) sorted longest-first, and next() probes them before the single-character path, so ❤️ wins over its first-code- point prefix ❤. findNext's refill logic is extracted to ensureLookahead(want), reused to keep MaxKeyLen bytes buffered across the 1024-byte window refills. Tests: 6 unit cases (whole-sequence match, prefix preference, ASCII digraphs, buffer-boundary straddle, empty probe list, unknown degradation) + a CAPI end-to-end case (flat alphabet trained on only multi-codepoint tokens: top child reaches 1.94x uniform mass; the assertion that failed pre-fix). The emoji corpus test relaxes from single-codepoint-only to whole-node tokens. Full suite: 47/47. Signed-off-by: will wade <willwade@gmail.com>
Review loop 1 (7/10) findings: - peekBack's contract broke under longest-match: the backward buffer walk returned only the FINAL codepoint of a multi-codepoint key, so an alphabet combining context-escape delimiters with a multi-char key ending in the delimiter char would falsely terminate a context block (latent — no shipped alphabet triggers it). next() now records the bytes it consumed and peekBack replays them; safe across window shifts by construction. - Stale 'we do not support multi-unicode-character symbols' comments updated; >1024-byte-key limitation documented at Add. - New tests: EOF-mid-key degradation, duplicate Add registers once, peekBack-after-match returns the whole 18-byte key. - clang-format over touched files. Full suite 47/47. Signed-off-by: will wade <willwade@gmail.com>
… bound - P1: annotation readers (Routing/Mandarin conversion trainers, CTrainer::readEscape) record the peeked token then advance via next(); with a multi-codepoint key at the position, peekAhead returned only its first codepoint while next() consumed the whole key — the route/ pronunciation was never learned. peekAhead now takes the map and probes longest-match, returning exactly the bytes the following next() consumes. All three call sites updated. - P2: keys of STREAM_WINDOW (1024) bytes or more can never be fully buffered for probing — now excluded at registration with the limit documented (they were equally dead through the per-codepoint path). - New test: peekAhead/next agreement across a multi-codepoint key. Full suite 47/47. Signed-off-by: will wade <willwade@gmail.com>
If a conversion annotation's stop delimiter ('>' etc.) prefixes a
configured multi-codepoint key, longest-match swallows it into the key:
the annotation loop misses its terminator and absorbs the remainder of
the training file as annotation text.
Structural parsing is grammar, not symbol content: nextRaw /
peekAheadRaw read exactly one codepoint (the pre-RFC behaviour), and
the three structural readers — CTrainer::readEscape (escape delimiters
and the escaped-context loop), Routing and Mandarin annotation
accumulation — use them. Longest-match remains for symbol training,
where shadowing is the intended semantics.
Regression test contrasts both modes over the same '>x' bytes.
Full suite 47/47.
Signed-off-by: will wade <willwade@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes multi-codepoint symbols (digraphs, ZWJ families, VS16 skin tones) trainable:
SymbolStream::nextprobes multi-codepoint map keys longest-first before the single-character path. First PR of RFC 0020.CAlphabetMap::Addcollects multi-codepoint keys (copies, sorted longest-first) +LongestMatch()probe +MaxKeyLen()next()probes before single-char dispatch (❤️ beats its prefix ❤);ensureLookahead(want)extracted from findNext keeps MaxKeyLen bytes buffered across 1024-byte window refillsThe PR is not yet safe to merge because escaped adaptive-training contexts containing multi-codepoint symbols are reconstructed incorrectly.
Findings
Summary
This PR adds longest-match training support for multi-codepoint alphabet symbols and protects structural annotation delimiters with raw, single-codepoint stream operations.
peekBack().Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Training stream] --> B{Context escape?} B -- No --> C[Longest-match next] C --> D[Learn alphabet symbol] B -- Yes --> E[Read delimiter as one raw codepoint] E --> F[Read saved document context] F --> G[Enter resolved symbols into model context] G --> H[Read closing delimiter structurally] H --> CReviews (4) · Last reviewed commit: "fix(trainer): greptile P1 r3 — raw mode ..."