Conversation
…y alphabet (RFC 0020)
Implements the RFC 0020 core: when BP_EMOJI_GROUP is on (default
true), CAlphIO merges the groups-only extension (Data/emoji/
extension.xml, 336 nodes incl. 30 skin-tone variants) into every
loaded alphabet — emoji become real symbols: committable, renderable,
and PPM-learnable in context, in any language. No corpus changes:
uniform priors + adaptation personalise.
- CAlphIO::LoadEmojiExtension caches the extension XML; MakeExtendedInfo
re-parses the alphabet file into a private copy (derived infos own
their ControlActions — sharing them with the base would double-free)
and appends the extension groups via ParseGroupRecursive
- SP_EMOJI_SKIN_TONE filters tone variants at merge (base + preferred)
- DasherInterfaceBase: extension load at realize ({dataDir, dataDir/
Data} resolution), GetActiveAlphabet serves the derived info (cached
per alphabet+tone), BP/SP changes rebuild like an alphabet switch;
derived infos kept alive while node trees may reference them, freed
in the destructor after the model
- Tests: merged-by-default across English/German/Arabic (62→398/403/
424 symbols), toggle-off restores the base set, tone filter keeps
base+preferred only; lm test scoped to base alphabet
Depends on #98 (longest-match) for training the multi-codepoint
variants. Full suite 47/47.
Signed-off-by: will wade <willwade@gmail.com>
| const CAlphInfo* base = m_AlphIO->GetInfo(alphId); | ||
| const CAlphInfo* derived = m_AlphIO->MakeExtendedInfo(alphId, tone); |
There was a problem hiding this comment.
When emoji support is enabled, this builds the extended alphabet only for the C API symbol-inspection functions. CNodeCreationManager still builds the actual map, language model, and selectable node tree from the unextended alphabet returned by GetInfo(). The tests can therefore see emoji through symbol enumeration, but users cannot select them from the Dasher node tree, so the core feature does not work.
Knowledge Base Used:
| BP_SIMULATE_TRANSPARENCY, | ||
| BP_CONTROL_MODE, | ||
| BP_SLOW_CONTROL_BOX, | ||
| BP_EMOJI_GROUP, |
There was a problem hiding this comment.
Adding BP_EMOJI_GROUP before END_OF_BPS shifts every existing long and string parameter value. These numeric keys are passed directly through the public C API, whose contract states that renumbering is an ABI break. A host compiled against the previous values can therefore read or update the wrong setting after loading this library.
Knowledge Base Used:
| <node label="👍🏾" tone="medium-dark"><textCharAction /></node> | ||
| <node label="👍🏿" tone="dark"><textCharAction /></node> | ||
| <node label="👎"><textCharAction /></node> | ||
| <node label="👍🏽"><textCharAction /></node> |
There was a problem hiding this comment.
Medium tone bypasses filtering
This unmarked 👍🏽 entry is retained for every skin-tone preference. Selecting light or dark therefore still exposes the medium-tone thumbs-up, while selecting medium creates two identical nodes. That violates the advertised base-plus-preferred filtering behavior.
| <node label="👍🏽"><textCharAction /></node> |
Knowledge Base Used: Alphabet configuration and managers
| const std::string key = alphId + '\x1f' + tone; | ||
| if (m_pExtendedAlphInfo && m_strExtendedKey == key) return m_pExtendedAlphInfo; | ||
|
|
||
| const CAlphInfo* base = m_AlphIO->GetInfo(alphId); | ||
| const CAlphInfo* derived = m_AlphIO->MakeExtendedInfo(alphId, tone); | ||
| m_pExtendedAlphInfo = derived; | ||
| m_strExtendedKey = key; | ||
| if (derived != base) // MakeExtendedInfo falls back to the shared base | ||
| m_vExtendedAlphInfos.push_back(derived); // info when nothing merged |
There was a problem hiding this comment.
Alphabet cache grows indefinitely
The cache remembers only the latest alphabet-and-tone key. After switching to another value and back, it reparses and retains another complete derived alphabet instead of reusing the earlier entry. Repeated runtime setting changes followed by alphabet queries therefore make session memory grow continuously until the interface is destroyed.
Knowledge Base Used:
- F1: CNodeCreationManager built the node tree from the REGISTERED base info — the extension existed only in four CAPI introspection getters (split-brain: symbol count said 398, the tree rendered 62). Build from GetActiveAlphabet(). - F2: MakeExtendedInfo started the extension chain at nullptr, orphaning the base group chain (leak + loss of all base group coverage). Start at the base chain's tail. - F4: dead CloneGroupTree declaration removed. - F5: re-parse guarded — divergent AlphID falls back to the base. - Node-tree regression tests: root children grow with the extension, base coverage intact (symbol 1 still 'a', full-range integrity). - IterateChildGroups scales children against the LM's ACTUAL cumulative total instead of assuming 65536 — PPM's escape mass (large while the extension's symbols are untrained) left the tree not filling the screen. No-op for fully-trained alphabets. - LM/property tests scoped to the base alphabet (raw-cumulative contracts); the raw-surface shortfall + get_probabilities/ root_child_bounds transient disagreement recorded for follow-up. Signed-off-by: will wade <willwade@gmail.com>
| const unsigned int iActualRange((*pCProb)[iMax - 1] - (*pCProb)[iMin - 1]); | ||
| DASHER_ASSERT(iActualRange > 0); | ||
| unsigned int iRange(iActualRange); |
There was a problem hiding this comment.
Control branch loses its range
When control mode is enabled, probability generation reserves one twentieth of the normalized range for the control node. This calculation expands the remaining alphabet probability mass back across the entire range, so the last alphabet child ends at 65536. AddExtras then gives the control node bounds of [65536, 65536], leaving it with no selectable area. The base group must retain the range reserved for the control branch.
Knowledge Base Used:
RFC 0020 core. Emoji merge into every alphabet when
BP_EMOJI_GROUPis on (default on): 336-node groups-only extension atData/emoji/extension.xml(30 skin-tone variants), merged at load into a private re-parse of the alphabet file — derived infos own their ControlActions.SP_EMOJI_SKIN_TONEfilters variants (base + preferred). No corpus changes: uniform priors + PPM adaptation personalise in-context, in any language.Tests: English 62→398 / German 403 / Arabic (RTL) 424 symbols with the extension; toggle-off restores base; tone filter verified. Depends on #98 (merged) for training multi-codepoint variants. Full suite 47/47.
Follow-ups (separate PRs): Android settings pickers + toggle removal (RFC clause 6), dark-mode strip fix, crash-handler release ride-along.
The PR is not safe to merge because control mode loses its selectable control branch, and two earlier blocking defects remain unresolved.
Findings
Summary
This PR adds a settings-controlled emoji extension to every loaded alphabet, including skin-tone filtering, derived alphabet ownership, runtime node-tree integration, and regression coverage. The latest revision connects the extension to the actual node manager and repairs merged group ordering, but its probability rescaling consumes the space reserved for control mode.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR XML[Base alphabet XML] --> Merge[Derived alphabet merge] Emoji[Emoji extension XML] --> Merge Settings[Emoji and tone settings] --> Merge Merge --> Manager[Node creation manager] Manager --> LM[Language-model probabilities] LM --> Scale[Alphabet child-range scaling] Scale --> Alphabet[Alphabet children] Scale -->|currently consumes full range| Control[Reserved control branch] Manager --> Tree[Selectable node tree]Reviews (2) · Last reviewed commit: "fix(emoji): review loop 1 (2/10) — model..."