Repository navigation
test: Index every table file, not a list of extensions - #35
Open
henrikottesorensen wants to merge 1 commit into
Open
henrikottesorensen wants to merge 1 commit into
henrikottesorensen wants to merge 1 commit into
Conversation
BrailleSpecTests indexed tables/ through an extension allow-list that
predated upstream's .tbl files, so none of the 55 reached
lou_indexTables - and .tbl is exactly where upstream keeps the
#+language: / #+grade: / #+type: metadata that lou_findTable matches on.
liblouis applies no filter of its own: indexTablePath in metadata.c
hands lou_indexTables every file on LOUIS_TABLEPATH and lou_indexTables
drops the ones analyzeTable finds no metadata in. So a filter in the
harness could only ever be wrong in one direction, and it was.
That one omission is the whole of the group the specs README called "27
fail on table resolution, and are probably one root cause rather than
27". 20 queries matched nothing; 7 settled for a worse match from
another language - language:ar grade:1 resolved to he-IL.utb,
language:en region:en-US grade:2 to en-ueb-g2.ctb. Every one of the 27
now resolves to exactly the table its __assert-match names, and no
query from the 100 specs already here resolves differently than before.
25 of the 27 pass and are restored, taking the suite to 125 specs over
roughly 70 languages. The recorded skipped tally moves with them:
test option: mode 14 -> 19, test option: typeform 287 -> 339.
The remaining two resolve their tables correctly and fail on something
else, documented in the specs README rather than chased here:
en-us-g2.yaml this harness carries flags across a bare tests: block,
upstream resets to forward (lou_checkyaml.c declares
testmode inside the loop over the flags/tests keys)
es-g0-g1.yaml BrailleSpec.Unescape handles \xNNNN and \\ but not the
\n, \r and \s that liblouis's own parser takes
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
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.
The one root cause behind the 27
BrailleSpecTests.TableCacheindexedtables/through an extension allow-list:Upstream 3.39.0 ships 55
.tblfiles, and.tblis not on that list. So none of them reachedlou_indexTables— and a.tblis precisely a metadata header (#+language:,#+grade:,#+type:) plusincludelines, i.e. exactly the filelou_findTableis meant to match on.liblouis applies no filter of its own.
indexTablePath()inmetadata.cwalksLOUIS_TABLEPATHwith
listDir()and handslou_indexTablesevery non-directory entry;lou_indexTablesthendrops any file
analyzeTablefinds no metadata in. A filter in the harness could therefore onlyever be wrong in one direction — dropping tables upstream would have indexed. It was.
That single omission is the whole of the group the specs README described as "27 fail on table
resolution, and are probably one root cause rather than 27": 20 queries matched nothing, 7 settled
for a lower-scoring table from another language or code.
Measured
A C probe linked against the built osx-arm64
liblouis.dylib3.39.0, indexingLibLouis.NET.Tables/tablestwice — once with the filter, once with every file — then running eachexcluded spec's query:
language:bn grade:1bn.tblbn.tbllanguage:hr type:computer dots:8hr-comp8.tblhr-comp8.tbllocale:pa grade:1pa.tblpa.tbllanguage:es type:literary grade:1es-no.utbes.tbles.tbllanguage:ar grade:1he-IL.utbar.tblar.tbllanguage:en region:en-GB grade:2en-ueb-g2.ctben_GB.tblen_GB.tbllanguage:en region:en-US grade:2en-ueb-g2.ctben_US.tblen_US.tbllanguage:en region:en-US type:computer dots:8en-nabcc.utben_US-comp8-ext.tblen_US-comp8-ext.tbllanguage:cmn-Hans region:cmn-CN system:cmn-traditional variant:no-tonezhcn-g1.ctbzh_CHN.tblzh_CHN.tblEvery query from the 27 resolves to exactly the table its
__assert-matchnames. Run over all 128distinct queries in the suite, the only resolutions that change are those 27's — nothing the 100
specs already here depend on moves.
What this restores
25 of the 27, taking the suite from 100 specs to 125, over roughly 70 languages. The
recorded skipped tally moves with them:
test option: mode14 → 19,test option: typeform287 → 339.
Green on net8.0 and net10.0 locally (208 tests each).
The two that stay out, and why
Both now resolve their tables correctly and fail on something else. Diagnosed and written into the
specs README rather than fixed here, since they are not this root cause:
en-us-g2.yaml(6/20) — the harness carriesflags:forward into a following baretests:block; upstream does not.
lou_checkyaml.cdeclaresint testmode = MODE_DEFAULT;inside theloop over the
flags/testskeys, so atests:block with noflags:of its own runs forward.The six failures are cases the harness runs backward and upstream runs forward — back-translating
five identical cells and expecting five different bullet characters cannot pass either way.
es-g0-g1.yaml(8/992) —BrailleSpec.Unescapehandles\xNNNN,\yNNNN,\uNNNN,\\and
\", but not the\n,\rand\sthis spec uses, so they reach liblouis as a literalbackslash plus a letter. liblouis's own
parseCharsInternalalso takes\e \f \n \r \s \t \v \wand\ZNNNNNNNN. Worth completing carefully:\sappears across thespecs, so it changes the input of cases that pass today —
spaces.yamlis the obvious one tore-check.
Also recorded in the README: the 7 specs the old list did not name (
ar-ar-g1,ar-ar-g1_harness,en-GB-g2,en-us-comp8-ext-back_harness,en-us-comp8-ext-for_harness,en-us-g2,zh-chn),and the fact that
CopyToOutputDirectorynever deletes — so a stalebin/keeps tables upstreamhas since removed, and with the filter gone those get indexed too.
🤖 Generated with Claude Code