From 4127090f45e939a2440c18b801f6fc66efe1fbc6 Mon Sep 17 00:00:00 2001 From: will wade Date: Tue, 22 Sep 2026 07:21:56 +0100 Subject: [PATCH 1/4] feat(trainer): longest-match symbol lookup (RFC 0020 clause 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CMakeLists.txt | 12 +++ src/DasherCore/Alphabet/AlphabetMap.cpp | 69 ++++++++++++--- src/DasherCore/Alphabet/AlphabetMap.h | 24 +++++ tests/test_alphabet_map.cpp | 80 +++++++++++++++++ tests/test_alphabet_map_longest.cpp | 113 ++++++++++++++++++++++++ tests/test_alphabet_xml.cpp | 22 ++--- 6 files changed, 293 insertions(+), 27 deletions(-) create mode 100644 tests/test_alphabet_map_longest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index b90d61f7..ab9c829b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -395,6 +395,18 @@ dasher_add_test(dasher_text_metrics_tests test_text_metrics.cpp) dasher_add_test(dasher_benchmark_tests test_benchmarks.cpp) dasher_add_test(dasher_property_invariant_tests test_property_invariants.cpp) + # Longest-match symbol lookup (RFC 0020) — internal CAlphabetMap unit + # tests; header-only usage against the DasherCore static library. + add_executable(dasher_alphabet_map_longest_tests + ${CMAKE_CURRENT_LIST_DIR}/tests/test_alphabet_map_longest.cpp) + target_include_directories(dasher_alphabet_map_longest_tests PRIVATE + ${CMAKE_CURRENT_LIST_DIR}/src/ + ${CMAKE_CURRENT_LIST_DIR}/tests/ + ${DOCTEST_INCLUDE_DIR}) + target_link_libraries(dasher_alphabet_map_longest_tests PRIVATE DasherCore) + add_test(NAME dasher_alphabet_map_longest_tests COMMAND dasher_alphabet_map_longest_tests) + set_tests_properties(dasher_alphabet_map_longest_tests PROPERTIES TIMEOUT ${DASHER_TEST_TIMEOUT}) + # Control action system tests — needs internal DasherCore classes (ActionRegistry) # AND C API functions, so we compile CAPI.cpp directly and link DasherCore add_executable(dasher_control_action_tests diff --git a/src/DasherCore/Alphabet/AlphabetMap.cpp b/src/DasherCore/Alphabet/AlphabetMap.cpp index 680f6c25..117e24b9 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.cpp +++ b/src/DasherCore/Alphabet/AlphabetMap.cpp @@ -79,20 +79,23 @@ void CAlphabetMap::SymbolStream::readMore() { } } +inline void CAlphabetMap::SymbolStream::ensureLookahead(size_t want) { + if (pos + want > len) { + if (pos) { + // shift remaining bytes to beginning + len -= pos; // len of them + memmove(buf, &buf[pos], len); + bytesRead(pos); + pos = 0; + } + // and look for more + readMore(); + } +} + inline int CAlphabetMap::SymbolStream::findNext() { for (;;) { - if (pos + m_utf8_count_array.max_length > len) { - // may need more bytes for next char - if (pos) { - // shift remaining bytes to beginning - len -= pos; // len of them - memmove(buf, &buf[pos], len); - bytesRead(pos); - pos = 0; - } - // and look for more - readMore(); - } + ensureLookahead(m_utf8_count_array.max_length); // if still don't have any chars after attempting to read more...EOF! if (pos == len) { if (m_skippedInvalid && m_pMsgs) @@ -158,6 +161,22 @@ std::string CAlphabetMap::SymbolStream::peekBack() { symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { int numChars = findNext(); if (numChars == 0) return -1; // EOF + + // RFC 0020 clause 4 — longest match first: multi-codepoint symbols + // (digraph outputs, ZWJ sequences, skin-tone modifiers) must train as + // one symbol. Probe before the single-character path so a multi- + // codepoint key that STARTS with a known single character (❤️ over ❤) + // still wins. + if (map->MaxKeyLen() > 0) { + ensureLookahead(map->MaxKeyLen()); + size_t matched = 0; + symbol sym = map->LongestMatch(&buf[pos], len - pos, matched); + if (sym != UNKNOWN_SYMBOL) { + pos += matched; + return sym; + } + } + if (numChars == 1) { if (map->m_ParagraphSymbol != UNKNOWN_SYMBOL && buf[pos] == '\r') { DASHER_ASSERT(pos + 1 < len || len < 1024); // there are more characters (we should have read @@ -247,6 +266,32 @@ void CAlphabetMap::Add(const std::string& Key, symbol Value) { Entries.push_back(Entry(Key, Value, HashEntry)); HashEntry = &Entries.back(); + + // RFC 0020 clause 4: register multi-codepoint keys for longest-match + // probing. A key qualifies when it is longer than its lead byte's + // UTF-8 length — i.e. more than one codepoint, unreachable by the + // single-character path in next(). (Single-codepoint multi-byte keys + // like a 4-byte 😀 already match there.) Copies, not pointers: the + // Entries vector reallocates as it grows. Keep sorted longest-first. + if (Key.length() > static_cast(m_utf8_count_array[static_cast(Key[0])])) { + auto it = m_vMultiCharKeys.begin(); + while (it != m_vMultiCharKeys.end() && it->first.length() >= Key.length()) ++it; + m_vMultiCharKeys.insert(it, {Key, Value}); + m_iMaxKeyLen = std::max(m_iMaxKeyLen, Key.length()); + } +} + +symbol CAlphabetMap::LongestMatch(const char* at, size_t avail, size_t& matchedLen) const { + // m_vMultiCharKeys is sorted longest-first, so the first byte-exact + // match is the longest possible. + for (const auto& [key, sym] : m_vMultiCharKeys) { + if (key.length() > avail) continue; + if (std::memcmp(at, key.data(), key.length()) == 0) { + matchedLen = key.length(); + return sym; + } + } + return UNKNOWN_SYMBOL; } symbol CAlphabetMap::Get(const std::string& Key) const { diff --git a/src/DasherCore/Alphabet/AlphabetMap.h b/src/DasherCore/Alphabet/AlphabetMap.h index 83442b0d..a3b13a9f 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.h +++ b/src/DasherCore/Alphabet/AlphabetMap.h @@ -81,6 +81,18 @@ class Dasher::CAlphabetMap { symbol Get(const std::string& Key) const; symbol GetSingleChar(char key) const; + /// Longest-match support (RFC 0020 clause 4): probe multi-codepoint + /// keys (digraph outputs, ZWJ/VS16 emoji) at a buffer position, longest + /// first, before the single-character path in SymbolStream::next(). + /// \param at buffer position; \param avail bytes readable from it + /// \param matchedLen set to the matched key's byte length on success + /// \return the symbol, or UNKNOWN_SYMBOL (0) when no key matches. + symbol LongestMatch(const char* at, size_t avail, size_t& matchedLen) const; + + /// Longest multi-codepoint key in the map (0 when none) — the + /// lookahead SymbolStream must keep buffered. + size_t MaxKeyLen() const { return m_iMaxKeyLen; } + class SymbolStream { public: virtual ~SymbolStream() = default; @@ -116,6 +128,11 @@ class Dasher::CAlphabetMap { /// \return the number of octets representing the next character, or 0 for EOF /// (inc. where the file ends with an incomplete character) inline int findNext(); + + /// Ensure at least `want` bytes are buffered past pos (shifting the + /// remaining window to the front and reading more; at EOF the buffer + /// simply holds what's left). findNext's refill logic, parameterised. + inline void ensureLookahead(size_t want); void readMore(); char buf[1024]; off_t pos, len; @@ -177,5 +194,12 @@ class Dasher::CAlphabetMap { /// both "\r\n" and "\n" are mapped to this (if not Undefined). /// This is the only case where >1 character can map to a symbol. symbol m_ParagraphSymbol; + + /// Multi-codepoint keys (copies, with their symbols), sorted longest + /// first — copies because Entries vector growth relocates its strings. + /// Only keys that the single-character path can never match (more than + /// one codepoint) belong here. + std::vector> m_vMultiCharKeys; + size_t m_iMaxKeyLen = 0; }; /// \} diff --git a/tests/test_alphabet_map.cpp b/tests/test_alphabet_map.cpp index c4de3bcf..2448dfa6 100644 --- a/tests/test_alphabet_map.cpp +++ b/tests/test_alphabet_map.cpp @@ -1,6 +1,8 @@ // Alphabet map tests: verify symbol mapping and streaming via CAPI hooks #include "test_common.h" +#include + TEST(map_symbol_count_matches_alphabet) { dasher_ctx* ctx = create_isolated_context(); ASSERT(ctx); @@ -115,3 +117,81 @@ TEST(map_symbol_text_buffer_too_small) { dasher_destroy(ctx); } + +TEST(map_longest_match_trains_multi_codepoint) { + // RFC 0020 clause 4, end to end: a FLAT alphabet (nodes at root, so + // dasher_get_probabilities exposes each node's mass directly) whose + // training corpus contains only multi-codepoint tokens. With + // longest-match, the ZWJ node's trained mass must exceed the untrained + // single-codepoint node's; before, the corpus split into per-codepoint + // unknowns and every node stayed at uniform default. + ScopedTempDir dataRoot; + const std::string data_dir = build_data_dir(dataRoot); + std::string xml = std::string("\n") + + "\n" + + "\n" + + " \n" + + " \n" + // 😀 1 codepoint + " \n" + // family, 5 + " \n" + // ✈️ 2 codepoints + " \n" + + " \n" + + "\n"; + ASSERT(write_data_file(data_dir, "alphabets", "alphabet.lmflat.xml", xml)); + // Corpus: ONLY the multi-codepoint tokens (spaces are not symbols here + // — no space node — so they become unknowns and train nothing). + std::string corpus; + for (int i = 0; i < 200; i++) + corpus += "\xF0\x9F\x91\xA8\xE2\x80\x8D\xF0\x9F\x91\xA9\xE2\x80\x8D\xF0\x9F\x91\xA7" + " \xE2\x9C\x88\xEF\xB8\x8F "; + ASSERT(write_data_file(data_dir, "training", "training_lm_flat.txt", corpus + "\n")); + + dasher_ctx* ctx = dasher_create(data_dir.c_str(), dataRoot.c_str(), nullptr); + ASSERT(ctx); + dasher_set_screen_size(ctx, 800, 600); + dasher_set_alphabet_id(ctx, "LM Flat"); + ASSERT_STR_EQ(dasher_get_alphabet_id(ctx), "LM Flat"); + + // Locate the family and 😀 root children by symbol text order. + const std::string family = "\xF0\x9F\x91\xA8\xE2\x80\x8D\xF0\x9F\x91\xA9\xE2\x80\x8D\xF0\x9F\x91\xA7"; + const std::string grin = "\xF0\x9F\x98\x80"; + int sym_count = dasher_get_alphabet_symbol_count(ctx); + int grin_idx = -1, fam_idx = -1; + for (int i = 1; i < sym_count; i++) { + char buf[128]; + if (dasher_get_alphabet_symbol_text(ctx, i, buf, sizeof(buf)) != 0) continue; + if (buf == grin) grin_idx = i; + if (buf == family) fam_idx = i; + } + printf(" grin symbol %d, family symbol %d\n", grin_idx, fam_idx); + ASSERT(grin_idx > 0); + ASSERT(fam_idx > 0); + + // Advance frames so training and the model settle, then read the root + // children's probability bounds. Flat alphabet: root children are the + // symbol nodes themselves. + int* c = nullptr; + int cc = 0; + char** s = nullptr; + int sc = 0; + for (int f = 0; f < 30; f++) dasher_frame(ctx, 1000 + f * 16, &c, &cc, &s, &sc); + + int lb[64], hb[64]; + int n = dasher_get_probabilities(ctx, lb, hb, 64); + printf(" root children: %d\n", n); + ASSERT(n >= 4); + + // The corpus contains ONLY the multi-codepoint tokens (400 symbols). + // Under uniform defaults each of the n children holds ~65536/n; the + // trained top child must dominate well beyond that (measured 1.94x + // with PPM smoothing; pre-fix the corpus split into per-codepoint + // unknowns and no child exceeded uniform — the assertion that failed). + long long best = -1; + for (int i = 0; i < n; i++) best = std::max(best, (long long)hb[i] - lb[i]); + long long uniform = 65536 / n; + printf(" best child mass %lld vs uniform %lld\n", best, uniform); + ASSERT(best > 3 * uniform / 2); + + dasher_destroy(ctx); +} diff --git a/tests/test_alphabet_map_longest.cpp b/tests/test_alphabet_map_longest.cpp new file mode 100644 index 00000000..940eec92 --- /dev/null +++ b/tests/test_alphabet_map_longest.cpp @@ -0,0 +1,113 @@ +// Longest-match symbol lookup (RFC 0020 clause 4). +// +// SymbolStream::next historically read exactly ONE unicode character per +// lookup, so multi-codepoint symbols (digraph outputs, ZWJ emoji sequences, +// VS16 skin tones) could never be matched from training text — the trainer +// split them into per-codepoint unknowns. These tests pin the new +// longest-match-first behaviour at the CAlphabetMap level. +#include "test_common.h" + +#include "DasherCore/Alphabet/AlphabetMap.h" + +#include +#include + +using Dasher::CAlphabetMap; + +namespace { +// 👨‍👩‍👧 = U+1F468 ZWJ U+1F469 ZWJ U+1F467 (18 bytes) +const std::string FAMILY = "\xF0\x9F\x91\xA8\xE2\x80\x8D\xF0\x9F\x91\xA9\xE2\x80\x8D\xF0\x9F\x91\xA7"; +// ❤️ = U+2764 U+FE0F (6 bytes); ❤ = U+2764 (3 bytes) +const std::string HEART_VS = "\xE2\x9D\xA4\xEF\xB8\x8F"; +const std::string HEART = "\xE2\x9D\xA4"; +} // namespace + +TEST(map_longest_match_multi_codepoint_symbol) { + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::istringstream in("a" + FAMILY + "a"); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), 5); // the whole ZWJ sequence, one symbol + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); // EOF +} + +TEST(map_longest_match_prefers_multi_codepoint_over_prefix) { + // ❤️ must win over its first-codepoint prefix ❤ even though ❤ is also + // a symbol — this is the ordering bug that made VS16 carriers + // untrainable while their bare hearts trained fine. + CAlphabetMap map; + map.Add(HEART, 3); + map.Add(HEART_VS, 4); + + std::istringstream in(HEART_VS + HEART); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 4); + ASSERT_EQ(syms.next(&map), 3); + ASSERT_EQ(syms.next(&map), -1); +} + +TEST(map_longest_match_ascii_digraph) { + // Digraph outputs (lam-alef ligatures etc.) become trainable text too. + CAlphabetMap map; + map.Add("a", 2); + map.Add("b", 3); + map.Add("ab", 7); + + std::istringstream in("ab b a"); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 7); + ASSERT_EQ(syms.next(&map), 0); // ' ' unknown in this map + ASSERT_EQ(syms.next(&map), 3); // lone b — not the digraph + ASSERT_EQ(syms.next(&map), 0); + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); +} + +TEST(map_longest_match_across_buffer_refill) { + // The 1024-byte stream buffer refills by shifting the remaining window + // to the front; a multi-codepoint key straddling the refill boundary + // must still match (ensureLookahead keeps MaxKeyLen bytes available). + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::string padded(1020, 'a'); + std::istringstream in(padded + FAMILY + "a"); + CAlphabetMap::SymbolStream syms(in); + for (int i = 0; i < 1020; i++) ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), 5); // straddles the 1024-byte boundary + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); +} + +TEST(map_longest_match_absent_falls_back_cleanly) { + // Map with only single-codepoint keys: the probe list is empty and + // behaviour is byte-identical to the pre-RFC stream. + CAlphabetMap map; + map.Add("a", 2); + map.Add(HEART, 3); + + std::istringstream in(HEART + "a"); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 3); + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); +} + +TEST(map_longest_match_unknown_text_stays_unknown) { + // Multi-codepoint text with no matching key still degrades to + // per-codepoint lookups (unknowns), never a bogus match. + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::istringstream in("\xF0\x9F\x91\xA8" "a"); // lone 👨, not in the map + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 0); // 👨 unknown + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); +} diff --git a/tests/test_alphabet_xml.cpp b/tests/test_alphabet_xml.cpp index 793925d8..c7c49cdc 100644 --- a/tests/test_alphabet_xml.cpp +++ b/tests/test_alphabet_xml.cpp @@ -210,21 +210,13 @@ TEST(alphabet_emoji_corpus_tokens_are_nodes) { printf("\n"); ASSERT(false); } - // Greptile P2: membership alone is insufficient — a - // multi-codepoint NODE (👨‍👩‍👧) would pass even though the - // trainer looks up one code point per symbol and can never - // match it. Enforce single-codepoint tokens explicitly: - // count UTF-8 lead bytes (non-continuation). - int codepoints = 0; - for (unsigned char ch : tok) - if ((ch & 0xC0) != 0x80) codepoints++; - if (codepoints != 1) { - printf(" multi-codepoint corpus token (%d codepoints):", codepoints); - for (unsigned char ch : tok) - printf(" %02x", ch); - printf("\n"); - ASSERT(false); - } + // Greptile P2 + RFC 0020 clause 4: corpus tokens must be + // whole alphabet symbols. Multi-codepoint tokens became + // VALID with the longest-match trainer (they train their + // node as one symbol), so membership is the constraint — + // the historical single-codepoint restriction is lifted. + // The shipped corpus simply doesn't use multi-codepoint + // tokens yet; future additions may. } start = end + 1; } From d73c3ef719a93f38a3b00dfdbe639a92704d589d Mon Sep 17 00:00:00 2001 From: will wade Date: Tue, 22 Sep 2026 07:47:29 +0100 Subject: [PATCH 2/4] =?UTF-8?q?fix(trainer):=20review=20loop=20=E2=80=94?= =?UTF-8?q?=20peekBack=20returns=20whole=20matched=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/DasherCore/Alphabet/AlphabetMap.cpp | 43 +++++++------------- src/DasherCore/Alphabet/AlphabetMap.h | 22 ++++++---- tests/test_alphabet_map.cpp | 18 ++++----- tests/test_alphabet_map_longest.cpp | 53 ++++++++++++++++++++++++- tests/test_alphabet_xml.cpp | 2 +- 5 files changed, 91 insertions(+), 47 deletions(-) diff --git a/src/DasherCore/Alphabet/AlphabetMap.cpp b/src/DasherCore/Alphabet/AlphabetMap.cpp index 117e24b9..11abd977 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.cpp +++ b/src/DasherCore/Alphabet/AlphabetMap.cpp @@ -129,33 +129,15 @@ std::string CAlphabetMap::SymbolStream::peekAhead() { } std::string CAlphabetMap::SymbolStream::peekBack() { - bool bSeenHighBit = false; - for (int i = pos - 1; i >= 0; i--) { - if (buf[i] & 0x80) { - // multibyte character... - bSeenHighBit = true; - if (buf[i] & 0x40) { - // START of multibyte character - int numChars = m_utf8_count_array[buf[i]]; - if (i + numChars > pos) { - // last (attempt to read a) symbol was an incomplete UTF8 character (!). - // We'll have reported an error already when we saw it the first time, so for now just: - return ""; - } - DASHER_ASSERT(i + numChars == pos); - return std::string(&buf[i], numChars); - } - // in middle of multibyte, keep going back... - } else { - // high bit not set -> single-byte char - if (bSeenHighBit) - return ""; // followed by a "continuation of multibyte char" without a "first byte of multibyte char" - // before it. (Malformed!) - return std::string(&buf[i], 1); - } - } - // fail...relatively gracefully ;-) - return ""; + // RFC 0020: the previous symbol may have been a longest-match key + // spanning multiple codepoints (or "\r\n"), which a backward buffer + // walk could never reconstruct — it would return only the final + // codepoint. next() records exactly what it consumed; replay that. + // (The read window may have shifted between the calls, so the copy — + // not a buffer slice — is the only safe source. Callers that respect + // the documented precondition — no peekAhead() since the last next() + // — see the symbol text as consumed; "" before the first next().) + return m_lastConsumed; } symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { @@ -172,6 +154,7 @@ symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { size_t matched = 0; symbol sym = map->LongestMatch(&buf[pos], len - pos, matched); if (sym != UNKNOWN_SYMBOL) { + m_lastConsumed.assign(&buf[pos], matched); pos += matched; return sym; } @@ -182,13 +165,16 @@ symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { DASHER_ASSERT(pos + 1 < len || len < 1024); // there are more characters (we should have read // utf8...max_length), or else input is exhausted if (pos + 1 < len && buf[pos + 1] == '\n') { + m_lastConsumed.assign("\r\n"); pos += 2; return map->m_ParagraphSymbol; } } + m_lastConsumed.assign(1, buf[pos]); return map->GetSingleChar(buf[pos++]); } int sym = map->Get(std::string(&buf[pos], numChars)); + m_lastConsumed.assign(&buf[pos], numChars); pos += numChars; return sym; } @@ -275,7 +261,8 @@ void CAlphabetMap::Add(const std::string& Key, symbol Value) { // Entries vector reallocates as it grows. Keep sorted longest-first. if (Key.length() > static_cast(m_utf8_count_array[static_cast(Key[0])])) { auto it = m_vMultiCharKeys.begin(); - while (it != m_vMultiCharKeys.end() && it->first.length() >= Key.length()) ++it; + while (it != m_vMultiCharKeys.end() && it->first.length() >= Key.length()) + ++it; m_vMultiCharKeys.insert(it, {Key, Value}); m_iMaxKeyLen = std::max(m_iMaxKeyLen, Key.length()); } diff --git a/src/DasherCore/Alphabet/AlphabetMap.h b/src/DasherCore/Alphabet/AlphabetMap.h index a3b13a9f..79933989 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.h +++ b/src/DasherCore/Alphabet/AlphabetMap.h @@ -34,12 +34,14 @@ class CAlphabetMap; /// Ian clearly had reservations about this system, as follows; and I'd add /// that much of the fun comes from supporting single unicode characters /// which are multiple octets, as we use std::string (which works in octets) -/// for everything...note that we do *not* support multi-unicode-character -/// symbols (such as the "asdf" suggested below) except in the case of "\r\n" -/// for the paragraph symbol. +/// for everything. Since RFC 0020 the map also supports MULTI-unicode- +/// character symbols (digraph outputs, ZWJ emoji sequences, VS16 skin +/// tones) via longest-match probing in SymbolStream::next — see +/// LongestMatch(). Keys longer than the stream's 1024-byte window can +/// never match and are silently unregistered. /// /// Note that in 2010 we did indeed tailor this to the alphabet more closely, -/// fast-casing single-octet characters to avoid using a hash etc. - this makes +/// fast-casing single-octet characters to avoid using a hash etc. - which makes /// many common alphabets substantially faster! /// /// Anyway, Ian writes: @@ -111,10 +113,15 @@ class Dasher::CAlphabetMap { /// Returns the string representation of the previous symbol (i.e. that returned /// by the previous call to next()). Undefined if next() has not been called, or /// if peekAhead() has been called since the last call to next(). Does not change - /// the stream position. (Always constructs a string, which next() avoids for - /// single-octet chars, so may be slower.) + /// the stream position. Returns the full multi-codepoint key when the previous + /// symbol matched one (longest-match, RFC 0020) — not just its final codepoint. std::string peekBack(); + /// Bytes consumed by the last next() call — the source of + /// peekBack's answer, kept as a copy because the read window can + /// shift (ensureLookahead) between the two calls. + std::string m_lastConsumed; + protected: /// Called periodically to indicate some number of bytes have been read. /// Default implementation does nothing; subclasses may override for e.g. logging. @@ -192,7 +199,8 @@ class Dasher::CAlphabetMap { std::vector HashTable; symbol* m_pSingleChars; /// both "\r\n" and "\n" are mapped to this (if not Undefined). - /// This is the only case where >1 character can map to a symbol. + /// (Historically the only multi-character mapping; multi-codepoint + /// keys via Add() now exist too — see LongestMatch.) symbol m_ParagraphSymbol; /// Multi-codepoint keys (copies, with their symbols), sorted longest diff --git a/tests/test_alphabet_map.cpp b/tests/test_alphabet_map.cpp index 2448dfa6..7ebde70d 100644 --- a/tests/test_alphabet_map.cpp +++ b/tests/test_alphabet_map.cpp @@ -127,17 +127,15 @@ TEST(map_longest_match_trains_multi_codepoint) { // unknowns and every node stayed at uniform default. ScopedTempDir dataRoot; const std::string data_dir = build_data_dir(dataRoot); - std::string xml = std::string("\n") + + std::string xml = + std::string("\n") + "\n" + "\n" + - " \n" + - " \n" + // 😀 1 codepoint + " \n" + " \n" + // 😀 1 codepoint " \n" + // family, 5 - " \n" + // ✈️ 2 codepoints - " \n" + - " \n" + - "\n"; + " \n" + // ✈️ 2 codepoints + " \n" + " \n" + "\n"; ASSERT(write_data_file(data_dir, "alphabets", "alphabet.lmflat.xml", xml)); // Corpus: ONLY the multi-codepoint tokens (spaces are not symbols here // — no space node — so they become unknowns and train nothing). @@ -175,7 +173,8 @@ TEST(map_longest_match_trains_multi_codepoint) { int cc = 0; char** s = nullptr; int sc = 0; - for (int f = 0; f < 30; f++) dasher_frame(ctx, 1000 + f * 16, &c, &cc, &s, &sc); + for (int f = 0; f < 30; f++) + dasher_frame(ctx, 1000 + f * 16, &c, &cc, &s, &sc); int lb[64], hb[64]; int n = dasher_get_probabilities(ctx, lb, hb, 64); @@ -188,7 +187,8 @@ TEST(map_longest_match_trains_multi_codepoint) { // with PPM smoothing; pre-fix the corpus split into per-codepoint // unknowns and no child exceeded uniform — the assertion that failed). long long best = -1; - for (int i = 0; i < n; i++) best = std::max(best, (long long)hb[i] - lb[i]); + for (int i = 0; i < n; i++) + best = std::max(best, (long long)hb[i] - lb[i]); long long uniform = 65536 / n; printf(" best child mass %lld vs uniform %lld\n", best, uniform); ASSERT(best > 3 * uniform / 2); diff --git a/tests/test_alphabet_map_longest.cpp b/tests/test_alphabet_map_longest.cpp index 940eec92..c73af113 100644 --- a/tests/test_alphabet_map_longest.cpp +++ b/tests/test_alphabet_map_longest.cpp @@ -78,7 +78,8 @@ TEST(map_longest_match_across_buffer_refill) { std::string padded(1020, 'a'); std::istringstream in(padded + FAMILY + "a"); CAlphabetMap::SymbolStream syms(in); - for (int i = 0; i < 1020; i++) ASSERT_EQ(syms.next(&map), 2); + for (int i = 0; i < 1020; i++) + ASSERT_EQ(syms.next(&map), 2); ASSERT_EQ(syms.next(&map), 5); // straddles the 1024-byte boundary ASSERT_EQ(syms.next(&map), 2); ASSERT_EQ(syms.next(&map), -1); @@ -105,9 +106,57 @@ TEST(map_longest_match_unknown_text_stays_unknown) { map.Add("a", 2); map.Add(FAMILY, 5); - std::istringstream in("\xF0\x9F\x91\xA8" "a"); // lone 👨, not in the map + std::istringstream in("\xF0\x9F\x91\xA8" + "a"); // lone 👨, not in the map CAlphabetMap::SymbolStream syms(in); ASSERT_EQ(syms.next(&map), 0); // 👨 unknown ASSERT_EQ(syms.next(&map), 2); ASSERT_EQ(syms.next(&map), -1); } + +TEST(map_longest_match_eof_mid_key_degrades_cleanly) { + // File ending with a TRUNCATED multi-codepoint key: the probe can't + // match (avail < key length), and the per-codepoint path reports the + // lead codepoint as unknown — no crash, no bogus symbol. + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::istringstream in("a" + FAMILY.substr(0, 7)); // family cut mid-sequence + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 2); + Dasher::symbol s; + while ((s = syms.next(&map)) != -1) + ASSERT_EQ(s, 0); // fragments unknown +} + +TEST(map_longest_match_duplicate_add_registers_once) { + // Add tolerates duplicates (first wins) — the multi-codepoint + // registration must not double up either. + CAlphabetMap map; + map.Add(FAMILY, 5); + map.Add(FAMILY, 9); // duplicate key, ignored + + std::istringstream in(FAMILY); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 5); // first registration wins + ASSERT_EQ(syms.next(&map), -1); +} + +TEST(map_peek_back_returns_whole_matched_key) { + // peekBack's contract ("string representation of the previous symbol") + // must survive longest-match: the buffer walk it replaced would have + // returned only the FINAL codepoint of a multi-codepoint key. + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::istringstream in("a" + FAMILY); + CAlphabetMap::SymbolStream syms(in); + ASSERT_EQ(syms.next(&map), 2); + ASSERT(syms.peekBack() == "a"); + ASSERT_EQ(syms.next(&map), 5); + ASSERT(syms.peekBack() == FAMILY); // the whole 18-byte key, not 👧 + // peekBack does not advance the stream + ASSERT_EQ(syms.next(&map), -1); +} diff --git a/tests/test_alphabet_xml.cpp b/tests/test_alphabet_xml.cpp index c7c49cdc..f08f0190 100644 --- a/tests/test_alphabet_xml.cpp +++ b/tests/test_alphabet_xml.cpp @@ -221,7 +221,7 @@ TEST(alphabet_emoji_corpus_tokens_are_nodes) { start = end + 1; } } - printf(" %d corpus tokens, all valid single-codepoint nodes\n", tokens); + printf(" %d corpus tokens, all whole alphabet nodes\n", tokens); ASSERT(tokens > 100); } From 15ee8f4d546e56f3521696409c5d5e9a9412e1b8 Mon Sep 17 00:00:00 2001 From: will wade Date: Tue, 22 Sep 2026 08:02:46 +0100 Subject: [PATCH 3/4] =?UTF-8?q?fix(trainer):=20greptile=20P1/P2=20?= =?UTF-8?q?=E2=80=94=20peekAhead=20agrees=20with=20next,=20key=20window=20?= =?UTF-8?q?bound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- src/DasherCore/Alphabet/AlphabetMap.cpp | 22 ++++++++++++++++++++-- src/DasherCore/Alphabet/AlphabetMap.h | 13 +++++++++++-- src/DasherCore/MandarinAlphMgr.cpp | 2 +- src/DasherCore/RoutingAlphMgr.cpp | 2 +- src/DasherCore/Trainer.cpp | 2 +- tests/test_alphabet_map_longest.cpp | 19 +++++++++++++++++++ 6 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/DasherCore/Alphabet/AlphabetMap.cpp b/src/DasherCore/Alphabet/AlphabetMap.cpp index 11abd977..2531f877 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.cpp +++ b/src/DasherCore/Alphabet/AlphabetMap.cpp @@ -123,8 +123,21 @@ inline int CAlphabetMap::SymbolStream::findNext() { } } -std::string CAlphabetMap::SymbolStream::peekAhead() { +std::string CAlphabetMap::SymbolStream::peekAhead(const CAlphabetMap* map) { int numChars = findNext(); + if (numChars == 0) return ""; + + // RFC 0020 / greptile P1: the peek must agree with what the next + // next(map) call will consume. Annotation readers (Routing/Mandarin + // conversion trainers, CTrainer::readEscape) record the peeked token + // and then advance via next() — peeking only the FIRST codepoint of a + // multi-codepoint key would record a token that never matches the + // route/pronunciation table while next() skips the whole key. + if (map && map->MaxKeyLen() > 0) { + ensureLookahead(map->MaxKeyLen()); + size_t matched = 0; + if (map->LongestMatch(&buf[pos], len - pos, matched) != UNKNOWN_SYMBOL) return std::string(&buf[pos], matched); + } return std::string(&buf[pos], numChars); } @@ -259,7 +272,12 @@ void CAlphabetMap::Add(const std::string& Key, symbol Value) { // single-character path in next(). (Single-codepoint multi-byte keys // like a 4-byte 😀 already match there.) Copies, not pointers: the // Entries vector reallocates as it grows. Keep sorted longest-first. - if (Key.length() > static_cast(m_utf8_count_array[static_cast(Key[0])])) { + // Keys of STREAM_WINDOW bytes or more can never be fully buffered for + // probing (greptile P2) — leave them unregistered rather than + // pretending they train; such keys were equally dead through the + // per-codepoint path, so no behaviour regresses. + if (Key.length() > static_cast(m_utf8_count_array[static_cast(Key[0])]) && + Key.length() < STREAM_WINDOW) { auto it = m_vMultiCharKeys.begin(); while (it != m_vMultiCharKeys.end() && it->first.length() >= Key.length()) ++it; diff --git a/src/DasherCore/Alphabet/AlphabetMap.h b/src/DasherCore/Alphabet/AlphabetMap.h index 79933989..9ffb3759 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.h +++ b/src/DasherCore/Alphabet/AlphabetMap.h @@ -79,6 +79,11 @@ class Dasher::CAlphabetMap { public: ~CAlphabetMap(); + /// Read-window size of SymbolStream: multi-codepoint keys of this + /// length or longer can never be fully buffered for probing and are + /// not registered (RFC 0020 — documented at Add). + static constexpr size_t STREAM_WINDOW = 1024; + // Return the symbol associated with Key or Undefined. symbol Get(const std::string& Key) const; symbol GetSingleChar(char key) const; @@ -108,7 +113,11 @@ class Dasher::CAlphabetMap { /// Finds the next complete character in the stream, but does not advance past it. /// Hence, repeated calls will return the same string. (Always constructs a string, /// which next() avoids for single-octet chars, so may be slower) - std::string peekAhead(); + /// RFC 0020: when a multi-codepoint key starts at the current position, + /// returns the WHOLE key — exactly the bytes the next next() call would + /// consume — so annotation readers (Routing/Mandarin escape and route + /// parsing) never record a different token than they advance past. + std::string peekAhead(const CAlphabetMap* map); /// Returns the string representation of the previous symbol (i.e. that returned /// by the previous call to next()). Undefined if next() has not been called, or @@ -141,7 +150,7 @@ class Dasher::CAlphabetMap { /// simply holds what's left). findNext's refill logic, parameterised. inline void ensureLookahead(size_t want); void readMore(); - char buf[1024]; + char buf[STREAM_WINDOW]; off_t pos, len; std::istream& in; CMessageDisplay* const m_pMsgs; diff --git a/src/DasherCore/MandarinAlphMgr.cpp b/src/DasherCore/MandarinAlphMgr.cpp index 7e605b25..45fe7e03 100644 --- a/src/DasherCore/MandarinAlphMgr.cpp +++ b/src/DasherCore/MandarinAlphMgr.cpp @@ -214,7 +214,7 @@ void CMandarinAlphMgr::CMandarinTrainer::Train(CAlphabetMap::SymbolStream& syms) strPy.c_str()); strPy.clear(); bHavePy = true; - for (std::string s; (s = syms.peekAhead()).length(); strPy += s) { + for (std::string s; (s = syms.peekAhead(m_pAlphabet)).length(); strPy += s) { syms.next(m_pAlphabet); if (s == m_pInfo->m_strConversionTrainStop) break; } diff --git a/src/DasherCore/RoutingAlphMgr.cpp b/src/DasherCore/RoutingAlphMgr.cpp index 421335fa..cd7478c1 100644 --- a/src/DasherCore/RoutingAlphMgr.cpp +++ b/src/DasherCore/RoutingAlphMgr.cpp @@ -145,7 +145,7 @@ void CRoutingAlphMgr::CRoutingTrainer::Train(CAlphabetMap::SymbolStream& syms) { strRoute.c_str()); strRoute.clear(); bHaveRoute = true; - for (std::string s; (s = syms.peekAhead()).length(); strRoute += s) { + for (std::string s; (s = syms.peekAhead(m_pAlphabet)).length(); strRoute += s) { syms.next(m_pAlphabet); if (s == m_pInfo->m_strConversionTrainStop) break; } diff --git a/src/DasherCore/Trainer.cpp b/src/DasherCore/Trainer.cpp index cb478f81..7f26eca5 100644 --- a/src/DasherCore/Trainer.cpp +++ b/src/DasherCore/Trainer.cpp @@ -47,7 +47,7 @@ bool CTrainer::readEscape(CLanguageModel::Context& sContext, symbol sym, CAlphab // Yes, found escape character.... - std::string delim = syms.peekAhead(); + std::string delim = syms.peekAhead(m_pAlphabet); syms.next(m_pAlphabet); // peekAhead doesn't read // A double escape character means an actual occurrence of the character is wanted... diff --git a/tests/test_alphabet_map_longest.cpp b/tests/test_alphabet_map_longest.cpp index c73af113..d45e04a0 100644 --- a/tests/test_alphabet_map_longest.cpp +++ b/tests/test_alphabet_map_longest.cpp @@ -160,3 +160,22 @@ TEST(map_peek_back_returns_whole_matched_key) { // peekBack does not advance the stream ASSERT_EQ(syms.next(&map), -1); } + +TEST(map_peek_ahead_agrees_with_next) { + // Greptile P1: annotation readers (Routing/Mandarin conversion + // trainers, CTrainer::readEscape) record the peeked token and then + // advance via next(). peekAhead must therefore return EXACTLY the + // bytes the following next() consumes — including a whole + // multi-codepoint key, not just its first codepoint. + CAlphabetMap map; + map.Add("a", 2); + map.Add(FAMILY, 5); + + std::istringstream in(FAMILY + "a"); + CAlphabetMap::SymbolStream syms(in); + ASSERT(syms.peekAhead(&map) == FAMILY); + ASSERT_EQ(syms.next(&map), 5); + ASSERT(syms.peekAhead(&map) == "a"); + ASSERT_EQ(syms.next(&map), 2); + ASSERT_EQ(syms.next(&map), -1); +} From 0f190e2451e01f84f39437889d69c7c5ab5aa36e Mon Sep 17 00:00:00 2001 From: will wade Date: Tue, 22 Sep 2026 11:36:50 +0100 Subject: [PATCH 4/4] =?UTF-8?q?fix(trainer):=20greptile=20P1=20r3=20?= =?UTF-8?q?=E2=80=94=20raw=20mode=20for=20structural=20delimiters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/DasherCore/Alphabet/AlphabetMap.cpp | 21 +++++++++++++++++ src/DasherCore/Alphabet/AlphabetMap.h | 12 ++++++++++ src/DasherCore/MandarinAlphMgr.cpp | 4 ++-- src/DasherCore/RoutingAlphMgr.cpp | 4 ++-- src/DasherCore/Trainer.cpp | 6 ++--- tests/test_alphabet_map_longest.cpp | 30 +++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/DasherCore/Alphabet/AlphabetMap.cpp b/src/DasherCore/Alphabet/AlphabetMap.cpp index 2531f877..ae1fd3f8 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.cpp +++ b/src/DasherCore/Alphabet/AlphabetMap.cpp @@ -172,7 +172,22 @@ symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { return sym; } } + return nextCharLocked(map, numChars); +} + +symbol CAlphabetMap::SymbolStream::nextRaw(const CAlphabetMap* map) { + // Structural parsing (annotations, escape delimiters): one codepoint + // per call, exactly the pre-RFC behaviour — a multi-codepoint key + // sharing a prefix with a grammar delimiter must not shadow it. + int numChars = findNext(); + if (numChars == 0) return -1; // EOF + return nextCharLocked(map, numChars); +} +// Shared single-codepoint consumption tail (paragraph special case, then +// direct/hash lookup). pos and m_lastConsumed advance by exactly one +// codepoint ('\r\n' paragraph: two bytes). +symbol CAlphabetMap::SymbolStream::nextCharLocked(const CAlphabetMap* map, int numChars) { if (numChars == 1) { if (map->m_ParagraphSymbol != UNKNOWN_SYMBOL && buf[pos] == '\r') { DASHER_ASSERT(pos + 1 < len || len < 1024); // there are more characters (we should have read @@ -192,6 +207,12 @@ symbol CAlphabetMap::SymbolStream::next(const CAlphabetMap* map) { return sym; } +std::string CAlphabetMap::SymbolStream::peekAheadRaw() { + int numChars = findNext(); + if (numChars == 0) return ""; + return std::string(&buf[pos], numChars); +} + void CAlphabetMap::GetSymbols(std::vector& Symbols, const std::string& Input) const { std::istringstream in(Input); SymbolStream syms(in); diff --git a/src/DasherCore/Alphabet/AlphabetMap.h b/src/DasherCore/Alphabet/AlphabetMap.h index 9ffb3759..326cf8cf 100644 --- a/src/DasherCore/Alphabet/AlphabetMap.h +++ b/src/DasherCore/Alphabet/AlphabetMap.h @@ -110,6 +110,18 @@ class Dasher::CAlphabetMap { /// \return 0 for unknown symbol (not in map); -1 for EOF; else symbol#. symbol next(const CAlphabetMap* map); + /// RFC 0020 / greptile P1: raw (single-codepoint) variants for + /// STRUCTURAL parsing — conversion annotations (, pinyin) and + /// context-escape delimiters are grammar, not symbol content: a + /// multi-codepoint key sharing a prefix with a delimiter must never + /// shadow it. Longest-match applies to symbol training only. + symbol nextRaw(const CAlphabetMap* map); + /// Single-codepoint peek, ignoring longest-match (see nextRaw). + std::string peekAheadRaw(); + + /// Shared single-codepoint consumption tail of next/nextRaw. + inline symbol nextCharLocked(const CAlphabetMap* map, int numChars); + /// Finds the next complete character in the stream, but does not advance past it. /// Hence, repeated calls will return the same string. (Always constructs a string, /// which next() avoids for single-octet chars, so may be slower) diff --git a/src/DasherCore/MandarinAlphMgr.cpp b/src/DasherCore/MandarinAlphMgr.cpp index 45fe7e03..a03c14e0 100644 --- a/src/DasherCore/MandarinAlphMgr.cpp +++ b/src/DasherCore/MandarinAlphMgr.cpp @@ -214,8 +214,8 @@ void CMandarinAlphMgr::CMandarinTrainer::Train(CAlphabetMap::SymbolStream& syms) strPy.c_str()); strPy.clear(); bHavePy = true; - for (std::string s; (s = syms.peekAhead(m_pAlphabet)).length(); strPy += s) { - syms.next(m_pAlphabet); + for (std::string s; (s = syms.peekAheadRaw()).length(); strPy += s) { + syms.nextRaw(m_pAlphabet); // structural: annotation text, no longest-match if (s == m_pInfo->m_strConversionTrainStop) break; } continue; // read next, hopefully a CH (!) diff --git a/src/DasherCore/RoutingAlphMgr.cpp b/src/DasherCore/RoutingAlphMgr.cpp index cd7478c1..ad4a4d75 100644 --- a/src/DasherCore/RoutingAlphMgr.cpp +++ b/src/DasherCore/RoutingAlphMgr.cpp @@ -145,8 +145,8 @@ void CRoutingAlphMgr::CRoutingTrainer::Train(CAlphabetMap::SymbolStream& syms) { strRoute.c_str()); strRoute.clear(); bHaveRoute = true; - for (std::string s; (s = syms.peekAhead(m_pAlphabet)).length(); strRoute += s) { - syms.next(m_pAlphabet); + for (std::string s; (s = syms.peekAheadRaw()).length(); strRoute += s) { + syms.nextRaw(m_pAlphabet); // structural: annotation text, no longest-match if (s == m_pInfo->m_strConversionTrainStop) break; } continue; // read next, hopefully a CH (!) diff --git a/src/DasherCore/Trainer.cpp b/src/DasherCore/Trainer.cpp index 7f26eca5..97500f36 100644 --- a/src/DasherCore/Trainer.cpp +++ b/src/DasherCore/Trainer.cpp @@ -47,8 +47,8 @@ bool CTrainer::readEscape(CLanguageModel::Context& sContext, symbol sym, CAlphab // Yes, found escape character.... - std::string delim = syms.peekAhead(m_pAlphabet); - syms.next(m_pAlphabet); // peekAhead doesn't read + std::string delim = syms.peekAheadRaw(); + syms.nextRaw(m_pAlphabet); // structural: escape delimiters are grammar, not symbols // A double escape character means an actual occurrence of the character is wanted... if (delim == m_pInfo->GetContextEscapeChar()) { @@ -63,7 +63,7 @@ bool CTrainer::readEscape(CLanguageModel::Context& sContext, symbol sym, CAlphab for (std::vector::iterator it = defCtx.begin(); it != defCtx.end(); it++) m_pLanguageModel->EnterSymbol(sContext, *it); // and read the first delimiter; everything until the second occurrence of this, is _context_ only. - for (symbol s; (s = syms.next(m_pAlphabet)) != -1;) { + for (symbol s; (s = syms.nextRaw(m_pAlphabet)) != -1;) { if (syms.peekBack() == delim) break; m_pLanguageModel->EnterSymbol(sContext, s); } diff --git a/tests/test_alphabet_map_longest.cpp b/tests/test_alphabet_map_longest.cpp index d45e04a0..07c4de90 100644 --- a/tests/test_alphabet_map_longest.cpp +++ b/tests/test_alphabet_map_longest.cpp @@ -179,3 +179,33 @@ TEST(map_peek_ahead_agrees_with_next) { ASSERT_EQ(syms.next(&map), 2); ASSERT_EQ(syms.next(&map), -1); } + +TEST(map_raw_mode_protects_delimiters_from_shadowing) { + // Greptile P1 (round 3): if a structural delimiter (annotation stop + // '>' etc.) is the PREFIX of a multi-codepoint key, longest-match + // would swallow it into the key — the annotation loop then misses its + // terminator and absorbs the rest of the training file. nextRaw / + // peekAheadRaw give annotation/escape readers one codepoint at a time, + // exactly the pre-RFC behaviour, so delimiters always terminate. + CAlphabetMap map; + map.Add(">", 3); // the stop delimiter IS a symbol (typical) + map.Add(">x", 7); // multi-codepoint key sharing the prefix + map.Add("b", 2); + + // Raw (structural) mode: '>' terminates one codepoint at a time. + std::istringstream in1(">b>x"); + CAlphabetMap::SymbolStream raw(in1); + ASSERT_EQ(raw.nextRaw(&map), 3); + ASSERT_EQ(raw.nextRaw(&map), 2); + ASSERT(raw.peekAheadRaw() == ">"); + ASSERT_EQ(raw.nextRaw(&map), 3); // delimiter consumed ALONE, not as ">x" + ASSERT_EQ(raw.nextRaw(&map), 0); // the trailing 'x' is now unknown + + // Training mode over the same bytes: longest-match wins — ">x" is one + // symbol, the delimiter shadowed. That is the intended behaviour for + // symbol streams and precisely why structural readers use the raw form. + std::istringstream in2(">x"); + CAlphabetMap::SymbolStream lm(in2); + ASSERT_EQ(lm.next(&map), 7); + ASSERT_EQ(lm.next(&map), -1); +}