Repository navigation
Widen the braille specs from Danish to 102 upstream specs - #22
Conversation
4642f3b to
0d605e7
Compare
| private static void RunSpec(string specFile) | ||
| { | ||
| BrailleSpec spec = BrailleSpecReader.Read(Path.Combine(SpecDirectory, specFile)); | ||
|
|
||
| IndexUpstreamTables(); | ||
|
|
||
| var mismatches = new List<string>(); | ||
| var unexpectedPasses = new List<string>(); | ||
| int checkedCount = 0; | ||
|
|
||
| foreach (BrailleSpecCase testCase in cases) | ||
| foreach (BrailleSpecCase testCase in spec.Cases) |
There was a problem hiding this comment.
- P2 — Skipped spec coverage is not reported. The reader records unsupported constructs in
BrailleSpec.SkippedConstructs, butBrailleSpecTests.csruns onlyspec.Casesand never surfaces that tally. Consequently, the test output does not indicate how many cases were omitted for options such astypeform,mode, orcursorPos. Please report the skipped counts (or assert expected counts) so omissions remain visible as the PR intends.
🤖 AI-assisted review: This PR has been reviewed with AI assistance.
There was a problem hiding this comment.
Fixed in de2da03.
- Each spec's test writes
<spec>: N cases checked, M entries skipped (construct: count, ...)to the test output and includes the same in its failure message. - Passing-test output is easy to miss, so
SkippedEntriesMatchTheRecordedTallyalso pins the totals across all specs: currently typeform 391, outputPos 5067, testmode hyphenate 1198, mode 14, testmode display 5. Any coverage gained or lost fails that test with the new tally.
The tally now counts dropped entries rather than constructs. Before, a flags-level testmode counted once per block, and an entry with two unsupported options counted twice. Fixing that also exposed a bug: {typeform: ..., testmode: forward} let the later testmode undo the skip, so the case ran without its typeform. An unsupported option now always drops its entry.
There was a problem hiding this comment.
Correction to my reply above: de2da03 was pushed to the wrong repository and never reached this PR. The fix is now here as 3b89c7d.
The recorded totals have also changed since that reply. main moved to liblouis 3.38.0, so ac4b889 re-copies the specs from 3.38.0. That release adds a new expected_typeform option (17 entries skipped), drops th-g1.yaml, and changes en-ueb.yaml in a way YamlDotNet can't parse, so that spec moves to the README's "do not parse" group. The typeform count goes from 391 to 287, mostly because en-ueb's 107 entries are gone. The commit message has the details.
Review feedback on Notalib#22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; Notalib#18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback on Notalib#22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; Notalib#18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0d605e7 to
9ca40b0
Compare
Review feedback on #22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; #18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9ca40b0 to
ac4b889
Compare
Review feedback on #22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; #18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ac4b889 to
6a710d3
Compare
Review feedback on #22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; #18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6a710d3 to
3c8354c
Compare
Runs liblouis's own expectations for roughly 60 languages through the wrapper instead of only Danish. 102 specs, all passing. Five things the reader did not model, each found by specs failing rather than by reading the format: - a test entry can lead with a description, [label, input, expected] - a translation table can be written inline as a block scalar, not just named. nemeth.yaml failed 133 of 133 on this alone - so can a display table, for the same reason - a table can be named by file rather than by query, in which case it must not go through lou_findTable - typeform, mode, inputPos, outputPos and cursorPos change what the expected output means, so those cases are counted and dropped rather than run against the wrong expectation The counting is the point: BrailleSpec.SkippedConstructs records what was recognised but not driven, so coverage that is not happening stays visible. Anything outside that list still throws. Each spec runs on a thread with a large stack. Compiling a table can recurse deeply - ancient-languages-borger.utb needs between 640KB and 768KB, more than the test host gives a test - and a stack overflow kills the process rather than failing a test. It is compilation, not translation: once a table list is compiled, translating through it runs in 128KB. Nothing about the input matters, and liblouis caches compiled tables process wide, so without a large stack somewhere the outcome depends on which test compiled a table first. 40 specs are held back with their reasons written down in the README. 29 of them fail on table resolution and are probably one root cause rather than 29. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback on #22: the reader tallied unsupported constructs in BrailleSpec.SkippedConstructs, but nothing ever reported the tally, so the test output never said how much of a spec went unchecked. Each spec's test now writes how many cases it checked and how many entries it dropped per construct, and puts the same in its failure message. Test output only shows on a passing run at detailed verbosity, so SkippedEntriesMatchTheRecordedTally also pins the totals across all specs: coverage gained or lost now fails a test with the new tally. The tally did not count what the review asked about, so it is now one per dropped entry, under the construct that dropped it. Before, a flags-level testmode counted once per block however many entries it dropped, and an entry with two unsupported options counted twice. Making that change also showed that an unsupported option could be undone: a testmode later in the same options replaced the Unsupported mode, so an entry like {typeform: ..., testmode: forward} ran without its typeform and compared against an expectation that needs it. An unsupported option now always drops its entry. The TableCache comment blamed up-front resolution on lou_findTable's result being freed with the wrong allocator; #18 frees it correctly now, and the cache is just there to share the lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main moved to liblouis 3.38.0 (#25), and the specs say what that version's tables should produce. Against the 3.33.0 specs, hbo.yaml and sv.yaml failed on table resolution: 3.38.0 adds hbo-cantillated.utb and renews the Swedish tables, and the specs' __assert-match lines moved with them. Thirteen of the specs here changed upstream and are re-copied. Two need more than a copy: * th-g1.yaml no longer exists upstream; Thai grade 1 is covered by th.yaml. Removed. * en-ueb.yaml no longer parses. 3.38.0 continues a flow sequence on an unindented line, which libyaml accepts and YamlDotNet 18.1.0 throws on. Moved to the README's "do not parse" group with the other spec that has that problem, since the specs are kept verbatim rather than edited to suit the parser. The en-ueb backward specs use a new test option, expected_typeform, which checks the typeform back-translation reports. BackTranslate does not return one, so those entries are dropped and counted like the other options the harness does not drive. The skip tally changes accordingly: 17 entries for expected_typeform, and 287 rather than 391 for typeform, most of the difference being en-ueb.yaml's 107. The README now gives the 3.38.0 counts and lists the ten specs upstream added since the selection was made. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main moved to liblouis 3.39.0 (#31). Four of the specs here changed upstream: bg.yaml, de-blista-dictionary.yaml, en-ueb-symbols_harness.yaml and hbo.yaml, in line with the release's Bulgarian and Biblical Hebrew back-translation work. None of the Danish specs changed. en-ueb.yaml still does not parse with YamlDotNet. The README now gives the 3.39.0 count of upstream specs, 160, and adds the three it introduced (en-nz.yaml, ht.yaml, mi.yaml) to the list not yet tried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3c8354c to
7802435
Compare
Runs liblouis's own expectations for roughly 60 languages through the wrapper instead of only Danish. 102 specs, all passing.
Five things the reader did not model
Every one was found by specs failing, not by reading the format:
[label, input, expected]nemeth.yamlfailed 133 of 133 on this alonelou_findTabletypeform,mode,inputPos,outputPosandcursorPoschange what the expected output means, so those cases are counted and dropped rather than run against the wrong expectationThat last one is the design point.
BrailleSpec.SkippedConstructsrecords what was recognised but not driven, so coverage that is not happening stays visible instead of quietly disappearing:inputPosoutputPostypeformTypeFormcursorPosmodeTranslationModeAnything outside that list still throws. Driving these would widen the corpus and cover wrapper surface that has no tests today — the same job twice over.
Why each spec runs on its own large-stack thread
Compiling a table can recurse deeply.
ancient-languages-borger.utbneeds between 640 KB and 768 KB, which is more than the test host gives a test, and a stack overflow kills the process rather than failing a test.It is compilation, not translation — once a table list is compiled, translating through it runs in 128 KB. Nothing about the input matters; ASCII overflows the same as non-BMP. And liblouis caches compiled tables process-wide, keyed by table-list string and never evicted, so without a large stack somewhere the outcome depends on which test happened to compile a given table first.
Worth knowing beyond the tests: the same failure is reachable in production. An ASP.NET request thread has about 1 MB. A service that compiles a table list for the first time on a request thread can overflow the same way, uncatchably. Compiling table lists at startup avoids it. Diagnosed jointly with the P/Invoke audit session, after we both drew wrong conclusions from experiments that were really measuring the compile cache.
40 specs held back
Not because they are wrong — because nobody has established yet whether the disagreement is the harness or the wrapper, and a suite that is expected to be red is worse than a smaller green one. Listed with reasons in
braille-specs/README.md:__assert-matchnames. The queries look well formed (language:bn grade:1), so the likely cause is which tables reachlou_indexTablesor which liblouis manages to analyse. Cheapest place to start.no.yaml167/868,ru.yaml39/140. The interesting group: either an unmodelled per-case option or a real difference.🤖 Generated with Claude Code
Recreated from #16 to move the head branch onto
Notalib/LibLouis.NET, which GitHub stacked PRs require (stacks cannot span forks).