Repository navigation
feat: build senses from DSL cards - #25
Merged
Merged
Conversation
Layer four: turning a card's token stream into structured senses. This is the layer the rebuild exists for. The predecessor produced **one sense per card** — 3,434,222 rows over 3,434,222 headwords, exactly one each — because it treated a line as a sense and a card's whole sense run arrives on a single physical line. It then tried to recover the structure downstream by splitting that blob on semicolons. Over the same input this produces 3,691,468 senses, of which 115,825 cards carry more than one; 了 has 30 and 一 and 打 have 57 apiece. `[mN]` is an **indent level, not a container**, and getting that wrong is the one mistake that looks right in a unit test. A card writes `[m1]…[/m][m2]…[/m]`, closing each span before opening the next, so by the time the `[m2]` arrives its parent is already closed. Hierarchy comes from the levels in document order, not from which spans happen to be open — my first version used an open/close stack and gave every nested sense a nil parent. A level-indexed map of the most recent sense at each depth is what makes `[m4]` find the right `[m2]`, and what makes a later `[m1]` start a new branch instead of joining the old one. Markup is not reliably balanced either: the set has two more `[mN]` opens than `[/m]` closes, so anything still open at the end of a card is closed there. One entry per reading, rather than one entry whose reading is the whole comma-separated string. It is what the schema says, the duplication is nothing (9,069 cards of 3,434,224), and the alternative puts `le, liǎo, liào` above the word on the page — which is precisely the prototype behaviour this replaces. `_` is not a reading. A naive split yields `[""]`, an entry claiming its pronunciation is the empty string, for 77% of the dictionary. Examples split at the end of the leading Han run, keeping `raw` always because the split is a heuristic. The CJK blocks alone are not enough: the quotation marks Chinese uses live in General Punctuation, and a block test stops at the first opening quote and hands half the sentence to the translation. Senses gained level, parent, label, part of speech, registers and examples; glosses gained style runs. Both encode only what they use — writing the empty defaults for every CC-CEDICT sense added 14 MB to a 35 MB bundled dictionary, and omitting them takes it to 32 MB, below where it started. Both decode field by field so a container compiled before these fields existed still opens, rather than a model addition forcing a re-import. Validated over the whole set: 3,434,224 cards to 3,442,623 entries and 3,691,468 senses in 70 seconds, with zero diagnostics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoBuild structured senses from DSL cards
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
widgetii
enabled auto-merge (squash)
October 4, 2026 19:59
Eight correctness fixes to the sense builder, seven of them raised in review on this branch and all eight measured against the full 3,434,224-card set. **Gloss ranges moved out from under the text they describe.** A style run or a cross-reference records a UTF-16 range into the sense's text, and the text was then trimmed in one function while the offsets were adjusted in another. Lifting `1)` out of a gloss also removes the space after it, which the offset arithmetic did not account for, so every range in a numbered sense pointed one character to the right: 16,957 senses with a style run and 3,974 with a cross-reference, the latter not merely shifted but dropped whenever it reached the end of its gloss. A further 85,395 unnumbered senses had the same fault from the final trim alone — anything whose prose follows a `[p]…[/p]`, which leaves a leading space, and there are 395,878 of those tags. Now the prefix is measured once, where it is removed, and every range moves by that one number and is clipped to the result. Clipped, not discarded: a run reaching into whitespace the trim took is still the run the dictionary asked for. Over the whole set, 0 of 380,683 style runs and 0 of 96,165 references now point at anything other than what they claim to; the tests assert the substring a range selects rather than its offsets, because an offset that is wrong by one still reads as a plausible number in a failure message. **Markup left open was discarded rather than settled.** `[/m]` cleared the pending style and reference offsets, so 241 sense spans that close with a style tag still open kept their text and lost their emphasis. An unclosed `[p]` or `[ex]` was worse: those divert text out of the gloss, so its contents left the sense altogether and the buffer went on collecting the *next* sense's prose. None occur in this set, but losing text is the one thing this importer must never do. Everything still open is now settled against the sense it was opened in — at `[/m]`, when the next sense opens, and at the end of the card. **A decoration opening before any `[mN]`** — 479 cards — lost its offset, because the fallback sense was created later by the first text token and cleared what the `[i]` had already recorded. Settling on sense boundaries makes the clearing unnecessary, so it is gone rather than guarded. **A `[ref]` span including the space before its closing tag** — 21 of them, `[ref]尽管 [/ref]` — recorded a headword with a trailing space, which matches no entry in any dictionary, so the link pointed nowhere. The span is trimmed along with the string, so the range and the word it names cannot disagree. **A card's article is not always one physical line.** 乐芙兰 is the one card in 3,434,224 that breaks a line inside an open `[m1]`, and most other DSL dictionaries write one sense per line. A break inside a sense is worth a space: concatenating gave `Ле Блан(чемпион из Лиги Легенд)`. **Several headword lines are several spellings**, each now its own entry over the same senses, so every spelling can be looked up. Reading the second line as the traditional form of the first was an invention — the format says consecutive headword lines are alternative headwords and says nothing about script — and it silently dropped every spelling after the second. No card in this set has more than one headword, so this changes nothing about what BKRS imports; it changes what the next dictionary imports. Counts over the whole set are unchanged — 3,434,224 cards to 3,442,623 entries and 3,691,468 senses, zero diagnostics, 17.8 s — which is the point: these fix what the senses contain, not how many there are. Each fix was mutation-tested; two of the first attempts were killed by no test at all and the tests were written until they were. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
1 participant
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.
Layer four of the BKRS importer: a card's token stream becomes structured senses. This is the layer the rebuild exists for.
The bug this replaces
The predecessor produced one sense per card — 3,434,222 rows over 3,434,222 headwords, exactly one each — because it treated a physical line as a sense, and a card's entire sense run arrives on one line. It then tried to recover the structure downstream by splitting that blob on semicolons.
Over the same input this produces 3,691,468 senses, of which 115,825 cards carry more than one. 了 has 30; 一 and 打 have 57 apiece, rendered as numbered lists under Roman-numeral divisions with parts of speech and split examples.
[mN]is an indent level, not a containerGetting this wrong is the one mistake that still passes a unit test. A card writes
[m1]…[/m][m2]…[/m]— each span closed before the next opens — so by the time the[m2]arrives its parent is already closed. Hierarchy comes from the levels in document order, not from which spans happen to be open.My first version used an open/close stack and gave every nested sense a
nilparent.SenseAssemblynow keepslastByLevel: [Int: Int], the most recent sense at each depth: that is what lets an[m4]find the right[m2], and what makes a later[m1]start a new branch instead of joining the old one. Opening a level forgets everything deeper.Markup is not reliably balanced either — the set has two more
[mN]opens than[/m]closes — so anything still open at the end of a card is closed there.Other decisions worth the review
le, liǎo, liào. It is what the schema says, the duplication is negligible (9,069 of 3,434,224 cards), and the alternative puts all three readings above the word on the page, which is the prototype behaviour this replaces._is not a reading. A naive split yields[""]— an entry claiming its pronunciation is the empty string — for 77% of the dictionary.rawunconditionally, because the Chinese/translation split is a heuristic. CJK blocks alone are not enough: the quotation marks Chinese uses live in General Punctuation, so a block test stops at the first“and hands half the sentence to the translation.[p]markers go toregistersverbatim rather than being dropped. A field marker the list has not heard of is still something the dictionary chose to print.Validation
Against the full local BKRS set (never committed — see CONTRIBUTING):
30 new tests (24 card builder, 6 sense coding). Core suite at 223 tests, lint clean, no warnings under
-warnings-as-errors, both app bundles build, bundled dictionary 32 MB.🤖 Generated with Claude Code