Skip to content

feat(alphabet): emoji extension — settings-gated emoji group for every alphabet (RFC 0020) - #99

Open
willwade wants to merge 2 commits into
mainfrom
feat/emoji-extension
Open

willwade wants to merge 2 commits into
mainfrom
feat/emoji-extension

Conversation

@willwade

@willwade willwade commented Sep 22, 2026 •

Copy link
Copy Markdown

RFC 0020 core. Emoji merge into every alphabet when BP_EMOJI_GROUP is on (default on): 336-node groups-only extension at Data/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_TONE filters 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.

RetriggerConfidence Score: 2/5

The PR is not safe to merge because control mode loses its selectable control branch, and two earlier blocking defects remain unresolved.

Findings

  1. P1 Control branch loses its range ▶
  2. P1 Emoji bypasses runtime tree ▶
  3. P1 Parameter keys are renumbered ▶
  4. P1 Medium tone bypasses filtering ▶
  5. P2 Alphabet cache grows indefinitely ▶

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.

  • Loads and merges a groups-only emoji resource into active alphabets.
  • Rebuilds the alphabet manager when emoji or skin-tone settings change.
  • Uses the derived alphabet consistently for mapping, prediction, training, and node construction.
  • Adds tests for extension toggling, filtering, symbol coverage, and tree integration.
  • Introduces a control-mode regression by expanding alphabet children over the reserved control interval.
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]
Loading

Reviews (2) · Last reviewed commit: "fix(emoji): review loop 1 (2/10) — model..."

…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>
Comment on lines +681 to +682
const CAlphInfo* base = m_AlphIO->GetInfo(alphId);
const CAlphInfo* derived = m_AlphIO->MakeExtendedInfo(alphId, tone);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Emoji bypasses runtime tree

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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Parameter keys are renumbered

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:

Comment thread Data/emoji/extension.xml
<node label="👍🏾" tone="medium-dark"><textCharAction /></node>
<node label="👍🏿" tone="dark"><textCharAction /></node>
<node label="👎"><textCharAction /></node>
<node label="👍🏽"><textCharAction /></node>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Suggested change
<node label="👍🏽"><textCharAction /></node>

Knowledge Base Used: Alphabet configuration and managers

Comment on lines +678 to +686
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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>
Comment on lines +612 to +614
const unsigned int iActualRange((*pCProb)[iMax - 1] - (*pCProb)[iMin - 1]);
DASHER_ASSERT(iActualRange > 0);
unsigned int iRange(iActualRange);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant