Feature/irish g2p - #8
Merged
Merged
Conversation
Adds Irish as the 17th language in the IPA transcriber, powered by a
17-pass phonological rule pipeline (ga-irish_engine + ga-passes/) with
generated per-dialect lexical exception tables (ga-passes/lex_subs_*.lua).
Irish is the first language in the app without a Wiktionary pron module
(it doesn't exist — Irish IPA on Wiktionary is hand-entered per word).
The engine provides automatic G2P for all three dialects (Connacht,
Munster, Ulster), making this the first browser-based Irish IPA generator.
Architecture:
- lua_modules/ga-irish_engine.lua — token-pipeline orchestrator (copied from
the dedicated irish repo, require paths adjusted → ga-passes.*)
- lua_modules/ga-passes/*.lua — 17 passes + init + _shared + lex_subs_*.lua
- lua_modules/ga-pron_wasm.lua — adapter; pre-warms mw.ustring and loads
lex_subs tables at init time to avoid wasmoon yield-across-C-boundary errors
- scripts/languages.js — added Irish with Connacht/Munster/Ulster styles
- scripts/utils.js — added Irish handler (window.ga_ipa.transcribe)
- index.html — added "Irish (experimental)" to description
- golden.json — 10 golden test entries passing all three dialects
Benchmark accuracy (rule pipeline + hybrid lex_subs layer):
Connacht: 96.92% exact / 99.22 NormLev
Munster: 93.76% exact / 98.13 NormLev
Ulster: 94.84% exact / 98.46 NormLev
The yield-across-C-boundary issue in wasmoon: ustring.toNFC and mw.ustring
both lazily require sub-modules (normalization-data, upper, charsets) via
the async :await()-based require shim, which fails from JS→Lua callbacks.
Fixed by pre-touching mw.ustring and running a dummy toNFC("á") at init
time, inside the doString coroutine where awaiting is legal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verifies in real Playwright (Chromium) that: - Irish appears in the language dropdown - Connacht: 'caisleán' → contains ʃlʲ (slender cluster) - Munster: 'cailín' → contains lʲiːnʲ (stress attraction + long vowel) - Ulster: 'baile' → contains bˠælʲə (a-fronting before slender cons) No regressions in any of the 18 active e2e tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When selecting Irish, the textarea pre-fills with a sample sentence (caisleán cailín baile Gaeltacht uisce) if no saved input or ?text= URL param is present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1300+ line help file covering all three dialects (Connacht, Munster, Ulster) with full IPA symbol tables, broad/slender distinction guide, lenition/eclipsis mutation tables, stress patterns, vowel digraphs, sonorant system, epenthesis rules, and dialect-specific feature docs. Based on the engine's 17-pass rule pipeline code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Irish transcription support for Connacht, Munster, and Ulster through a Lua token pipeline, integrates it into the web UI, supplies dialect lexical data and help documentation, and adds golden plus Playwright coverage. ChangesIrish transcription pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant IrishAdapter
participant IrishEngine
participant PassPipeline
participant LexicalTables
Browser->>IrishAdapter: transcribe(text, dialect)
IrishAdapter->>IrishEngine: transcribe(word, dialect)
IrishEngine->>PassPipeline: run_all(tokens, context)
PassPipeline-->>IrishEngine: resolved token stream
IrishEngine->>LexicalTables: apply dialect substitution
LexicalTables-->>IrishEngine: final IPA
IrishEngine-->>Browser: rendered transcription
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (13)
wiktionary_pron/e2e/transcription.spec.js (1)
50-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertions read a possibly stale
#result .ipanode. Eachtranscribe()call adds output to#resultwithout the test clearing prior results, so.first()may point at the previous dialect's node. Today the expected substrings are disjoint across the three cases, so this can only cause a timeout rather than a false pass — but it makes future failures hard to read. Consider clearing#result(or reloading) between dialects, or asserting on.last().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/e2e/transcription.spec.js` around lines 50 - 73, Update the Irish dialect test around each transcribe call so assertions target the newly generated result rather than a potentially stale node. Prefer asserting on the last `#result .ipa` element after each `transcribe()` call, or clear `#result` between dialects while preserving the existing dialect sequence and expected IPA checks.wiktionary_pron/help/irish.html (1)
863-863: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
s → tsis t-prefixation, not eclipsis. The doc itself classifies it correctly at line 900 ("T-prefix mutation"). Listing it in the Eclipsis (Urú) table is misleading for learners; either move it out or annotate it as t-prefixation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/help/irish.html` at line 863, Correct the Irish mutation table containing the s → ts entry so it is no longer presented as eclipsis (Urú). Move the entry to the existing “T-prefix mutation” section, or annotate it explicitly as t-prefixation while preserving its pronunciation and example.wiktionary_pron/scripts/tests/golden/golden.json (1)
345-362: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding
baileto the golden set. The e2e test asserts Ulsterbaile→bˠælʲə, but there is no fast golden case for it; a-fronting regressions would only surface in the slower Playwright run. Munster/Ulster rows foranam,Gaeilge, anduiscewould likewise widen dialect coverage cheaply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/scripts/tests/golden/golden.json` around lines 345 - 362, Extend the golden test cases with Irish dialect coverage for `baile`, including the Ulster pronunciation `bˠælʲə`, and add the suggested Munster/Ulster variants for `anam`, `Gaeilge`, and `uisce` using their expected pronunciations. Keep each entry marked as a successful phonemic case with the appropriate dialect arguments.wiktionary_pron/scripts/main.js (2)
683-690: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
constand a data-driven default map instead of an inline special case.
var tais inconsistent with theconststyle used throughout this function, and hardcoding one language's example text inline won't scale when the next language needs a default.♻️ Suggested change
- // Language-specific default example text - if (selectedLanguage === "Irish") { - var ta = document.getElementById("text_to_transcribe"); - if (ta && !localStorage.getItem("inputText") && !urlParams.get("text")) { - ta.value = "cleachtann mic léinn an aibítir fhoghraíochta idirnáisiúnta go cúramach i gcónaí"; - } - } + // Language-specific default example text + const defaultText = DEFAULT_EXAMPLE_TEXT[selectedLanguage]; + if (defaultText) { + const textArea = document.getElementById("text_to_transcribe"); + if ( + textArea && + !localStorage.getItem("inputText") && + !urlParams.get("text") + ) { + textArea.value = defaultText; + } + }Declared near the top of the module:
const DEFAULT_EXAMPLE_TEXT = { Irish: "cleachtann mic léinn an aibítir fhoghraíochta idirnáisiúnta go cúramach i gcónaí", };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/scripts/main.js` around lines 683 - 690, Replace the inline Irish-only default block with a module-level DEFAULT_EXAMPLE_TEXT map, then use the selected language to retrieve its default text. In the existing default-example logic, declare the textarea reference with const and apply the mapped value only when the textarea exists and no stored or URL input is present, preserving the current Irish text.
845-854: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the dangling comments describing a non-existent
elsebranch.Lines 851-853 sit between the assignment and the closing brace and describe behavior implemented elsewhere ("the language-change handler below" is actually above and already handles the default). Misleading for the next reader.
♻️ Suggested cleanup
- // Retrieve text from local storage on page load, or set default per language. - // Check for URL ?text= param inside the lang handler instead (it runs later). + // Retrieve text from local storage on page load. Language-specific defaults and + // the ?text= URL param are applied by updateOptionsUponLanguageSelection. console.log("DOMContentLoaded", localStorage.getItem("inputText")); const savedText = localStorage.getItem("inputText"); if (savedText) { textArea.value = savedText; - // If no saved text and the HTML placeholder is still showing, set a language- - // specific default after the user selects a language. The language-change - // handler below calls enableAll() and can overwrite at that point. }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/scripts/main.js` around lines 845 - 854, Remove the dangling comments inside the savedText conditional in the DOMContentLoaded handler, leaving the existing localStorage retrieval and textArea assignment logic unchanged.wiktionary_pron/lua_modules/ga-irish_engine.lua (2)
177-183: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant duplicate branches.
if token.stress and token.type == "cons"and the followingelseif token.stressboth just insertS.STRESS_MARK— they're functionally identical and can be merged into a singleif token.stress then.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-irish_engine.lua` around lines 177 - 183, Merge the redundant stress-handling branches in the token-processing logic into a single if token.stress condition that inserts S.STRESS_MARK. Remove the unnecessary token.type == "cons" check while preserving the existing behavior for all stressed tokens.
224-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLex-subs modules are pre-loaded twice.
ga-irish_engine.luaalreadypcall(require, ...)slex_subs_connacht/munster/ulsterat module load time; sincega-pron_wasm.luarequiresga-irish_enginefirst (line 16), Lua's module cache means the identicalpcall(require, ...)calls inga-pron_wasm.luaare dead code that only ever return the cached result.
wiktionary_pron/lua_modules/ga-irish_engine.lua#L224-L229: keep as the single source of truth for lex_subs pre-loading.wiktionary_pron/lua_modules/ga-pron_wasm.lua#L19-L21: remove the redundantpcall(require, "ga-passes.lex_subs_*")calls, or add a comment clarifying why belt-and-suspenders loading here is intentionally kept.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-irish_engine.lua` around lines 224 - 229, Keep the lex_subs pre-loading in ga-irish_engine.lua as the single source of truth, preserving its existing connacht, munster, and ulster cache initialization. In ga-pron_wasm.lua lines 19-21, remove the redundant pcall(require, ...) calls for those modules; no direct change is needed at the ga-irish_engine.lua anchor.wiktionary_pron/lua_modules/ga-passes/02_stress.lua (1)
31-68: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLarge lookup tables are rebuilt on every pass invocation / every segment.
UNSTRESSEDandMONOSYLLABIC_STRESSare declared insiderun(), so they're reconstructed once per word processed.MONO_STRESS,MONO_NO_STRESS,MONO_STRESS_DIA/MONO_NO_STRESS_DIA, andA_PREFIX_SECOND_STRESSare declared inside thefor _, seg in ipairs(segments) doloop, so they're rebuilt once per word segment — for a large lexicon (this repo ships lex_subs tables with 1000+ entries per dialect, suggesting bulk/batch transcription is a real workload) this is meaningful repeated allocation. Hoisting these table literals to module scope (top-level locals, built once) would remove the per-call/per-segment rebuild cost with no behavior change.Also applies to: 74-115, 143-164, 232-254, 258-281, 306-314
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua` around lines 31 - 68, Hoist the immutable lookup tables UNSTRESSED, MONOSYLLABIC_STRESS, MONO_STRESS, MONO_NO_STRESS, MONO_STRESS_DIA, MONO_NO_STRESS_DIA, and A_PREFIX_SECOND_STRESS from run() and the per-segment loop to module-level locals. Keep their contents and lookup behavior unchanged, ensuring they are constructed once when the module loads rather than per invocation or segment.wiktionary_pron/lua_modules/ga-passes/13_sonorants.lua (2)
24-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment describes a
suffixparameter that doesn't exist.
insert_combining(phon, combining)takes two arguments; the comment documentsbase_char/combining/suffix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/13_sonorants.lua` around lines 24 - 35, The documentation above insert_combining must match its actual two-argument interface: remove the nonexistent base_char and suffix parameter descriptions, and describe phon and combining while preserving the documented return behavior.
99-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate keys in
NON_TENSOR_SLENDER.
caintim,guiochtaintandpeinteailteare declared twice in the same table constructor.🧹 Proposed cleanup
-- Additional n+t over-application exceptions caintim=true, guiochtaint=true, peinteailte=true, - -- Loanwords and verbal suffix -t(-e) forms: n+t is non-tensor - caintim=true, guiochtaint=true, peinteailte=true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/13_sonorants.lua` around lines 99 - 102, Remove the duplicate caintim, guiochtaint, and peinteailte entries from NON_TENSOR_SLENDER, keeping each key declared only once while preserving the intended exception entries.wiktionary_pron/lua_modules/ga-passes/07_nasalization.lua (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHeader comment contradicts the implementation.
The file header says
o/u/ó/ú -> [uː], but the pass only raises shorto/uand toʊ(line 46) — the inline comment at 32-34 documents the opposite of the header.📝 Suggested comment fix
-- Pass `#7`: Vowel nasal raising. --- o/u/ó/ú -> [uː] before geminate nasals (nn, ng, doubled n n). +-- Short o/u -> [ʊ] before geminate/velar nasals (nn, ng, doubled n n). +-- Long ó/ú keep their length and are not raised.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/07_nasalization.lua` around lines 1 - 4, Update the header comment for Pass `#7` to match the implementation: describe raising only short o and u before geminate nasals, with the resulting vowel quality matching the ʊ behavior documented by the inline comment near the transformation. Remove the inaccurate ó/ú and [uː] claims while preserving the remaining pass-order and reference information.wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua (1)
276-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStep labels are duplicated/out of order.
There are two "Step 4n" (289, 312), two "Step 4l" (276, 346) and two "Step 4f" (349, 367), plus placeholder-only steps (4o at 407 documents a change that was folded elsewhere). In a 1100-line pass these labels are the only navigation aid — renumber or drop them.
Also applies to: 346-349, 407-411
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua` around lines 276 - 289, Renumber or remove the duplicated and out-of-order step labels throughout the cleanup pass, including the sections around the iəw glide, later Step 4l/4f blocks, and the placeholder-only Step 4o. Preserve the existing transformation logic while making all retained labels unique, sequential, and accurate navigation markers.wiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.lua (1)
644-659: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePermanently dead branch (
stress_seen = true+elseif false).The rationale note is valuable, but keeping an unreachable body around invites accidental re-activation and confuses
stress_seenbookkeeping. Consider reducing this to the comment alone (git history retains the body), same treatment as06d_anticipatory_raising.lua.usub_firstat lines 13-18 is likewise unused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.lua` around lines 644 - 659, Remove the unreachable stress-processing block around the local stress bookkeeping, including the unused stress_seen logic and its dead elseif false branch, while preserving its rationale as a comment if needed. Also remove the unused usub_first declaration near the top of the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiktionary_pron/help/irish.html`:
- Around line 19-20: Add an entry for the Irish help page to the language-link
list in help/index.html, using the existing naming and ordering conventions so
the “All Languages” navigation includes irish.html.
- Around line 836-837: Correct the headword in the “After vowels” example within
the pronunciation guidance from “scíobh” to “scríobh,” preserving the existing
pronunciation text and formatting.
In `@wiktionary_pron/index.html`:
- Line 24: Update the meta description text in the HTML document to change
“Portuguese require” to “Portuguese requires,” leaving the rest of the
supported-language description unchanged.
In `@wiktionary_pron/lua_modules/ga-irish_engine.lua`:
- Around line 188-205: Extract the onset-start computation used by the
preprocess loop into a shared helper, preserving its phonotactic legality rules
and transparent handling of apostrophe and hyphen boundaries. Update the
secondary-stress branch around the `token.secondary` check to call that helper
instead of walking backward through arbitrary consonants, so its `onset_start`
decision matches preprocessing and emits the mark consistently.
In `@wiktionary_pron/lua_modules/ga-passes/01_polarity.lua`:
- Around line 117-129: The forward scan in 01_polarity.lua’s sonorant handling
must stop at non-apostrophe boundary tokens and preserve the requested polarity
guard correction around the next_cons lookup; apply this at
wiktionary_pron/lua_modules/ga-passes/01_polarity.lua lines 117-129. Apply the
same boundary-stop check before the vowel match in the next_vowel scan at
wiktionary_pron/lua_modules/ga-passes/03_eclipsis.lua lines 72-84, allowing
apostrophe boundaries to be crossed.
- Around line 117-129: Update the sonorant fallback in the visible
polarity-processing block to run only when polarity is nil, preserving false
values derived from vowel context. Bound the forward scan at a boundary token,
so it stops before entering another word while retaining the existing vowel stop
and same-word consonant lookup.
In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua`:
- Around line 431-452: Write the computed seg_root_vowel_count back to
context.root_vowel_count before returning from the pass, alongside the existing
context.is_monosyllabic assignment. Preserve the current local counting and
monosyllabic logic.
In `@wiktionary_pron/lua_modules/ga-passes/08_slender_coda.lua`:
- Around line 24-28: Update the slender-coda condition in the pass so it only
rewrites vowels before a palatal/slender `ng`, preserving broad-`ng` vowels.
Implement the documented `nn` case if this pass is intended to support it, and
replace the byte-length check on `token.ortho` with the project’s Unicode-aware
single-character check so fada vowels are included.
In `@wiktionary_pron/lua_modules/ga-passes/09_consonants.lua`:
- Around line 135-157: Update the word-final checks in the th branch and the
dh/gh branch to treat a following boundary token or nil as word-final, matching
the existing bhf handling. Preserve the current silent pronunciation for final
th and dh/gh, while allowing phrase-internal and trailing-boundary cases to use
the final path.
In `@wiktionary_pron/lua_modules/ga-passes/10_vowels.lua`:
- Around line 534-566: The OI_TO_OE override is incorrectly nested under the
preceding token.phon == "ɛ" condition. De-nest the OI_TO_OE block by closing the
EPS_TO_I handling before it, while preserving its existing ortho == "oi" and
token.phon == "ɛ" checks so the override evaluates independently of the EPS_TO_I
tables.
- Around line 211-236: Update the vowel-handling branches around ortho == "ai",
"ái", and "aí" to use the existing word-final boundary-aware helper
is_word_final(next) or is_word_final(next_t) instead of testing for nil next
tokens. Remove the unreachable duplicated “ai and not next” condition from the
combined aí branch, while preserving the medial and word-final phonetic outputs.
In `@wiktionary_pron/lua_modules/ga-passes/11_unstressed_reduction.lua`:
- Around line 56-58: Update the `writes_context` metadata in the
`unstressed_reduction` pass definition to `true`, because the pass assigns
`context.ulster_keep_long`, which is consumed by `15_dialect_finalize.lua`;
leave the pass logic unchanged.
- Around line 46-54: Remove the unused is_short_vowel helper, including its
undefined ustring reference and redundant long-vowel guard; preserve the
existing inline phon:match("ː") and SHORT_VOWELS logic in run.
In `@wiktionary_pron/lua_modules/ga-passes/12_epenthesis.lua`:
- Around line 124-165: The generic epenthesis branch handles l + bh/mh before
the dedicated branch, so the required final-position gating and bh/mh-to-v
rewrite are skipped. Update the first branch to exclude tokens with
tokens[i].ortho == "l", allowing the existing l + bh/mh logic to run; preserve
generic handling for other sonorants.
In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua`:
- Around line 759-763: Update the trailing apostrophe cleanup in the
function-word segment iteration to use the existing seg_ranges boundary index
instead of seg[`#seg`].ortho_indices[2]. Preserve the current behavior of clearing
next_boundary.phon for a trailing apostrophe, while avoiding access to
ortho_indices on tokens created by make_token().
In `@wiktionary_pron/scripts/tests/golden/golden.json`:
- Around line 327-344: Remove the duplicate golden test entry for “cailín” with
args “Irish;Munster;Phonemic” from the golden cases, retaining one instance with
the expected value “kɑˈlʲiːnʲ” and preserving the distinct Ulster case.
---
Nitpick comments:
In `@wiktionary_pron/e2e/transcription.spec.js`:
- Around line 50-73: Update the Irish dialect test around each transcribe call
so assertions target the newly generated result rather than a potentially stale
node. Prefer asserting on the last `#result .ipa` element after each
`transcribe()` call, or clear `#result` between dialects while preserving the
existing dialect sequence and expected IPA checks.
In `@wiktionary_pron/help/irish.html`:
- Line 863: Correct the Irish mutation table containing the s → ts entry so it
is no longer presented as eclipsis (Urú). Move the entry to the existing
“T-prefix mutation” section, or annotate it explicitly as t-prefixation while
preserving its pronunciation and example.
In `@wiktionary_pron/lua_modules/ga-irish_engine.lua`:
- Around line 177-183: Merge the redundant stress-handling branches in the
token-processing logic into a single if token.stress condition that inserts
S.STRESS_MARK. Remove the unnecessary token.type == "cons" check while
preserving the existing behavior for all stressed tokens.
- Around line 224-229: Keep the lex_subs pre-loading in ga-irish_engine.lua as
the single source of truth, preserving its existing connacht, munster, and
ulster cache initialization. In ga-pron_wasm.lua lines 19-21, remove the
redundant pcall(require, ...) calls for those modules; no direct change is
needed at the ga-irish_engine.lua anchor.
In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua`:
- Around line 31-68: Hoist the immutable lookup tables UNSTRESSED,
MONOSYLLABIC_STRESS, MONO_STRESS, MONO_NO_STRESS, MONO_STRESS_DIA,
MONO_NO_STRESS_DIA, and A_PREFIX_SECOND_STRESS from run() and the per-segment
loop to module-level locals. Keep their contents and lookup behavior unchanged,
ensuring they are constructed once when the module loads rather than per
invocation or segment.
In `@wiktionary_pron/lua_modules/ga-passes/07_nasalization.lua`:
- Around line 1-4: Update the header comment for Pass `#7` to match the
implementation: describe raising only short o and u before geminate nasals, with
the resulting vowel quality matching the ʊ behavior documented by the inline
comment near the transformation. Remove the inaccurate ó/ú and [uː] claims while
preserving the remaining pass-order and reference information.
In `@wiktionary_pron/lua_modules/ga-passes/13_sonorants.lua`:
- Around line 24-35: The documentation above insert_combining must match its
actual two-argument interface: remove the nonexistent base_char and suffix
parameter descriptions, and describe phon and combining while preserving the
documented return behavior.
- Around line 99-102: Remove the duplicate caintim, guiochtaint, and peinteailte
entries from NON_TENSOR_SLENDER, keeping each key declared only once while
preserving the intended exception entries.
In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua`:
- Around line 276-289: Renumber or remove the duplicated and out-of-order step
labels throughout the cleanup pass, including the sections around the iəw glide,
later Step 4l/4f blocks, and the placeholder-only Step 4o. Preserve the existing
transformation logic while making all retained labels unique, sequential, and
accurate navigation markers.
In `@wiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.lua`:
- Around line 644-659: Remove the unreachable stress-processing block around the
local stress bookkeeping, including the unused stress_seen logic and its dead
elseif false branch, while preserving its rationale as a comment if needed. Also
remove the unused usub_first declaration near the top of the module.
In `@wiktionary_pron/scripts/main.js`:
- Around line 683-690: Replace the inline Irish-only default block with a
module-level DEFAULT_EXAMPLE_TEXT map, then use the selected language to
retrieve its default text. In the existing default-example logic, declare the
textarea reference with const and apply the mapped value only when the textarea
exists and no stored or URL input is present, preserving the current Irish text.
- Around line 845-854: Remove the dangling comments inside the savedText
conditional in the DOMContentLoaded handler, leaving the existing localStorage
retrieval and textArea assignment logic unchanged.
In `@wiktionary_pron/scripts/tests/golden/golden.json`:
- Around line 345-362: Extend the golden test cases with Irish dialect coverage
for `baile`, including the Ulster pronunciation `bˠælʲə`, and add the suggested
Munster/Ulster variants for `anam`, `Gaeilge`, and `uisce` using their expected
pronunciations. Keep each entry marked as a successful phonemic case with the
appropriate dialect arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 739e5533-6a9b-47ba-9d24-95397c2de0cc
📒 Files selected for processing (32)
wiktionary_pron/e2e/transcription.spec.jswiktionary_pron/help/irish.htmlwiktionary_pron/index.htmlwiktionary_pron/lua_modules/ga-irish_engine.luawiktionary_pron/lua_modules/ga-passes/01_polarity.luawiktionary_pron/lua_modules/ga-passes/02_stress.luawiktionary_pron/lua_modules/ga-passes/03_eclipsis.luawiktionary_pron/lua_modules/ga-passes/04_cluster_simplify.luawiktionary_pron/lua_modules/ga-passes/05_mutated_fricatives.luawiktionary_pron/lua_modules/ga-passes/06_vocalization.luawiktionary_pron/lua_modules/ga-passes/06d_anticipatory_raising.luawiktionary_pron/lua_modules/ga-passes/07_nasalization.luawiktionary_pron/lua_modules/ga-passes/08_slender_coda.luawiktionary_pron/lua_modules/ga-passes/09_consonants.luawiktionary_pron/lua_modules/ga-passes/09b_vowel_adjunct.luawiktionary_pron/lua_modules/ga-passes/10_vowels.luawiktionary_pron/lua_modules/ga-passes/11_unstressed_reduction.luawiktionary_pron/lua_modules/ga-passes/12_epenthesis.luawiktionary_pron/lua_modules/ga-passes/13_sonorants.luawiktionary_pron/lua_modules/ga-passes/14_final_cleanup.luawiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.luawiktionary_pron/lua_modules/ga-passes/_shared.luawiktionary_pron/lua_modules/ga-passes/init.luawiktionary_pron/lua_modules/ga-passes/lex_subs_connacht.luawiktionary_pron/lua_modules/ga-passes/lex_subs_munster.luawiktionary_pron/lua_modules/ga-passes/lex_subs_ulster.luawiktionary_pron/lua_modules/ga-pron_wasm.luawiktionary_pron/scripts/languages.jswiktionary_pron/scripts/main.jswiktionary_pron/scripts/tests/golden.test.jswiktionary_pron/scripts/tests/golden/golden.jsonwiktionary_pron/scripts/utils.js
Comment on lines
+117
to
+129
| -- Sonorant polarity: when no vowel context, check next consonant | ||
| -- Hickey II.1.8: sonorants (l/n/r/m) assimilate to following consonant's polarity | ||
| local sonorants = { l = true, n = true, r = true, m = true } | ||
| if sonorants[token.ortho] and not polarity then | ||
| local next_cons = nil | ||
| for k = i + 1, #tokens do | ||
| if tokens[k].type == "cons" then next_cons = tokens[k]; break end | ||
| if tokens[k].type == "vowel" then break end | ||
| end | ||
| if next_cons and next_cons.palatal ~= nil then | ||
| polarity = next_cons.palatal | ||
| end | ||
| end |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Forward token scans that skip word-boundary checks (shared pattern). Both scans walk tokens forward looking for a vowel/consonant without checking for a boundary token, unlike the other scans in 01_polarity.lua that correctly guard on tokens[j].type == "boundary". Since tokens can represent multi-word phrases (confirmed by 02_stress.lua's segment splitting and multi-word lex_subs entries such as "ar aghaidh"), both can leak context across word boundaries.
wiktionary_pron/lua_modules/ga-passes/01_polarity.lua#L117-L129: addif tokens[k].type == "boundary" and tokens[k].source ~= "apostrophe" then break endto thenext_consscan (see per-site fix, which also corrects thenot polarityguard).wiktionary_pron/lua_modules/ga-passes/03_eclipsis.lua#L72-L84: add the same boundary check to thenext_vowelscan before theif tokens[j].type == "vowel"check.
📍 Affects 2 files
wiktionary_pron/lua_modules/ga-passes/01_polarity.lua#L117-L129(this comment)wiktionary_pron/lua_modules/ga-passes/03_eclipsis.lua#L72-L84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/lua_modules/ga-passes/01_polarity.lua` around lines 117 -
129, The forward scan in 01_polarity.lua’s sonorant handling must stop at
non-apostrophe boundary tokens and preserve the requested polarity guard
correction around the next_cons lookup; apply this at
wiktionary_pron/lua_modules/ga-passes/01_polarity.lua lines 117-129. Apply the
same boundary-stop check before the vowel match in the next_vowel scan at
wiktionary_pron/lua_modules/ga-passes/03_eclipsis.lua lines 72-84, allowing
apostrophe boundaries to be crossed.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Sonorant polarity fallback has a wrong guard and an unbounded cross-word scan.
Two issues in this block:
not polarityfires for bothnilandfalse, but the comment says this fallback is meant only for "when no vowel context" (polarity == nil). As written, a sonorant whose polarity was already correctly derived as broad (false) from real vowel context can be overwritten by an unrelated next-consonant's polarity.- The
for k = i + 1,#tokensdoscan has notokens[k].type == "boundary"check, unlike the sibling scans elsewhere in this file. For a multi-wordtokensarray (confirmed elsewhere in the pipeline), a trailing sonorant at the end of one word with no following consonant in-word can pick up polarity from the next word's first consonant.
🐛 Suggested fix
- if sonorants[token.ortho] and not polarity then
+ if sonorants[token.ortho] and polarity == nil then
local next_cons = nil
for k = i + 1, `#tokens` do
+ if tokens[k].type == "boundary" and tokens[k].source ~= "apostrophe" then break end
if tokens[k].type == "cons" then next_cons = tokens[k]; break end
if tokens[k].type == "vowel" then break end
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- Sonorant polarity: when no vowel context, check next consonant | |
| -- Hickey II.1.8: sonorants (l/n/r/m) assimilate to following consonant's polarity | |
| local sonorants = { l = true, n = true, r = true, m = true } | |
| if sonorants[token.ortho] and not polarity then | |
| local next_cons = nil | |
| for k = i + 1, #tokens do | |
| if tokens[k].type == "cons" then next_cons = tokens[k]; break end | |
| if tokens[k].type == "vowel" then break end | |
| end | |
| if next_cons and next_cons.palatal ~= nil then | |
| polarity = next_cons.palatal | |
| end | |
| end | |
| -- Sonorant polarity: when no vowel context, check next consonant | |
| -- Hickey II.1.8: sonorants (l/n/r/m) assimilate to following consonant's polarity | |
| local sonorants = { l = true, n = true, r = true, m = true } | |
| if sonorants[token.ortho] and polarity == nil then | |
| local next_cons = nil | |
| for k = i + 1, `#tokens` do | |
| if tokens[k].type == "boundary" and tokens[k].source ~= "apostrophe" then break end | |
| if tokens[k].type == "cons" then next_cons = tokens[k]; break end | |
| if tokens[k].type == "vowel" then break end | |
| end | |
| if next_cons and next_cons.palatal ~= nil then | |
| polarity = next_cons.palatal | |
| end | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/lua_modules/ga-passes/01_polarity.lua` around lines 117 -
129, Update the sonorant fallback in the visible polarity-processing block to
run only when polarity is nil, preserving false values derived from vowel
context. Bound the forward scan at a boundary token, so it stops before entering
another word while retaining the existing vowel stop and same-word consonant
lookup.
Comment on lines
+759
to
+763
| -- Also silence any trailing apostrophe boundary (e.g., "a'" -> ə not ə') | ||
| local next_boundary = tokens[seg[#seg].ortho_indices[2] + 1] or {} | ||
| if next_boundary.type == "boundary" and next_boundary.ortho == "'" then | ||
| next_boundary.phon = "" | ||
| end |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Does the tokenizer set ortho_indices on tokens?
rg -n 'ortho_indices' wiktionary_pron/lua_modules
fd -a 'ga-irish_engine.lua' -x rg -n -C5 'tokenize_word|ortho' {}Repository: hellpanderrr/hellpanderrr.github.io
Length of output: 4011
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect only the tokenizer's table constructor and the affected cleanup code path.
fd -a 'ga-irish_engine.lua' -x sed -n '12,174p' {}
printf '\n--- 14_final_cleanup relevant ---\n'
sed -n '720,775p' wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua
printf '\n--- token table construction exact matches ---\n'
rg -n -C2 'type = "|phon = "|ortho = "|ortho_indices|range' wiktionary_pron/lua_modules/ga-irish_engine.lua wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua | sed -n '1,220p'Repository: hellpanderrr/hellpanderrr.github.io
Length of output: 28452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the shared token constructor and make_token implementation.
rg -n -C8 'function make_token|make_token = ' wiktionary_pron/lua_modules/ga-irish_engine.lua wiktionary_pron/lua_modules/ga-passes/ga-common.lua wiktionary_pron/lua_modules || true
fd -a 'ga-common.lua' -x sed -n '1,80p' {}Repository: hellpanderrr/hellpanderrr.github.io
Length of output: 1695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the helper module used by the Irish passes to identify make_token implementation.
sed -n '1,220p' wiktionary_pron/lua_modules/ga-irish_engine.lua
# Inspect the exact function-word cleanup loop context around fw_segments construction and access.
sed -n '700,770p' wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.luaRepository: hellpanderrr/hellpanderrr.github.io
Length of output: 12186
Use the existing segment range for the trailing apostrophe cleanup.
make_token() does not populate ortho_indices, so seg[#seg].ortho_indices[2] throws for function-word overrides such as a'. Use seg_ranges/its boundary index when iterating fw segments instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua` around lines 759
- 763, Update the trailing apostrophe cleanup in the function-word segment
iteration to use the existing seg_ranges boundary index instead of
seg[`#seg`].ortho_indices[2]. Preserve the current behavior of clearing
next_boundary.phon for a trailing apostrophe, while avoiding access to
ortho_indices on tokens created by make_token().
Comment on lines
+327
to
+344
| { | ||
| "text": "cailín", | ||
| "args": "Irish;Munster;Phonemic", | ||
| "status": "success", | ||
| "value": "kɑˈlʲiːnʲ" | ||
| }, | ||
| { | ||
| "text": "cailín", | ||
| "args": "Irish;Ulster;Phonemic", | ||
| "status": "success", | ||
| "value": "ˈkælʲinʲ" | ||
| }, | ||
| { | ||
| "text": "cailín", | ||
| "args": "Irish;Munster;Phonemic", | ||
| "status": "success", | ||
| "value": "kɑˈlʲiːnʲ" | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Duplicate golden case. cailín / Irish;Munster;Phonemic is listed twice with the identical expected value, producing two identically-named mocha tests.
🧹 Proposed fix
{
"text": "cailín",
"args": "Irish;Ulster;Phonemic",
"status": "success",
"value": "ˈkælʲinʲ"
},
- {
- "text": "cailín",
- "args": "Irish;Munster;Phonemic",
- "status": "success",
- "value": "kɑˈlʲiːnʲ"
- },
{
"text": "anam",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "text": "cailín", | |
| "args": "Irish;Munster;Phonemic", | |
| "status": "success", | |
| "value": "kɑˈlʲiːnʲ" | |
| }, | |
| { | |
| "text": "cailín", | |
| "args": "Irish;Ulster;Phonemic", | |
| "status": "success", | |
| "value": "ˈkælʲinʲ" | |
| }, | |
| { | |
| "text": "cailín", | |
| "args": "Irish;Munster;Phonemic", | |
| "status": "success", | |
| "value": "kɑˈlʲiːnʲ" | |
| }, | |
| { | |
| "text": "cailín", | |
| "args": "Irish;Munster;Phonemic", | |
| "status": "success", | |
| "value": "kɑˈlʲiːnʲ" | |
| }, | |
| { | |
| "text": "cailín", | |
| "args": "Irish;Ulster;Phonemic", | |
| "status": "success", | |
| "value": "ˈkælʲinʲ" | |
| }, | |
| { | |
| "text": "anam", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/scripts/tests/golden/golden.json` around lines 327 - 344,
Remove the duplicate golden test entry for “cailín” with args
“Irish;Munster;Phonemic” from the golden cases, retaining one instance with the
expected value “kɑˈlʲiːnʲ” and preserving the distinct Ulster case.
hellpanderrr
added a commit
that referenced
this pull request
Jul 30, 2026
BUG fixes:
- B1: Guard ortho_indices nil access in 14_final_cleanup function-word override
- B2: Add word-boundary check in 01_polarity forward sonorant scan
- B3: Remove bh/mh from generic epenthesis branch (dedicated l+bh/mh handles it)
- B4: Fix insert_combining doc comment to match 2-param signature
Improvements:
- I1: Extract shared find_onset_start helper from render_output() secondary stress walk
- I2: golden.json: Remove duplicate cailin Munster entry; add anam/Gaeilge/uisce/baile for all dialects
- I3: Hoist MONOSYLLABIC_STRESS table to module scope in 02_stress.lua
- I4: Data-driven DEFAULT_EXAMPLE_TEXT map in main.js
- I5: Replace dead elseif false block in 15_dialect_finalize with comment
- I6: Fix writes_context=false to true in 11_unstressed_reduction
- I7: Fix 07_nasalization header to match implementation (short o/u only)
- I8: Remove dangling comments in main.js
- I9: Fix scribh->scríb, scíobh->scríobh headword typos in irish.html
- I10: Move s->ts from eclipsis table to T-prefix annotation in irish.html
- I11: Add Irish help page entry to help/index.html
- I12: Fix subject-verb agreement in index.html ('require' -> 'requires')
- I13: Remove duplicate keys in NON_TENSOR_SLENDER table in 13_sonorants.lua
…w + golden.json corrected
BUG fixes:
- B1: Guard ortho_indices nil access in 14_final_cleanup function-word override
- B2: Add word-boundary check in 01_polarity forward sonorant scan
- B3: Remove bh/mh from generic epenthesis branch (dedicated l+bh/mh handles it)
- B4: Fix insert_combining doc comment to match 2-param signature
Improvements:
- I1: Extract shared find_onset_start helper from render_output() secondary stress walk
- I2: golden.json: Remove duplicate cailin Munster entry; add anam/Gaeilge/uisce/baile for all dialects (with corrected values)
- I3: Hoist MONOSYLLABIC_STRESS table to module scope in 02_stress.lua
- I4: Data-driven DEFAULT_EXAMPLE_TEXT map in main.js
- I5: Replace dead elseif false block in 15_dialect_finalize with comment
- I6: Fix writes_context=false to true in 11_unstressed_reduction
- I7: Fix 07_nasalization header to match implementation (short o/u only)
- I8: Remove dangling comments in main.js
- I9: Fix scribh->scríb, scíobh->scríobh headword typos in irish.html
- I10: Move s->ts from eclipsis table to T-prefix annotation in irish.html
- I11: Add Irish help page entry to help/index.html
- I12: Fix subject-verb agreement in index.html ('require' -> 'requires')
- I13: Remove duplicate keys in NON_TENSOR_SLENDER table in 13_sonorants.lua
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua (1)
760-767: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve apostrophe-boundary cleanup for synthetic segments.
The guard avoids the nil-index crash, but when
last_segis a clone withoutortho_indices, this branch silently skips clearing the following'boundary. That leaves function-word overrides such asa'incorrectly pronounced. Use the existingseg_rangesboundary index instead of token metadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua` around lines 760 - 767, Update the apostrophe-boundary cleanup around last_seg and next_boundary to derive the following boundary token from the existing seg_ranges index rather than last_seg.ortho_indices, while retaining the boundary type and apostrophe checks and clearing next_boundary.phon for both ordinary and synthetic segments.
🧹 Nitpick comments (1)
wiktionary_pron/lua_modules/ga-passes/02_stress.lua (1)
31-64: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the dead duplicate
UNSTRESSEDtable.The pass constructs one lookup table, then shadows it with an identical table before processing any segment. This wastes per-call allocations and creates two sources of truth. Keep one module-scope table and reference it from
run(). (raw.githubusercontent.com)Also applies to: 115-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua` around lines 31 - 64, Remove the duplicate local UNSTRESSED table in the later run flow, retain a single module-scope UNSTRESSED definition, and update run() to reference that shared table without rebuilding it per call. Ensure all existing entries and lookup behavior remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiktionary_pron/help/irish.html`:
- Around line 78-80: Update the dialectal vowel-quality example in the
introductory paragraph to match the mappings documented in this page: describe
long á as /ɑː/ in Connacht and Munster versus /aː/ in Ulster, and do not use the
unsupported Connacht /aː/ versus Munster /æː/ distinction.
---
Duplicate comments:
In `@wiktionary_pron/lua_modules/ga-passes/14_final_cleanup.lua`:
- Around line 760-767: Update the apostrophe-boundary cleanup around last_seg
and next_boundary to derive the following boundary token from the existing
seg_ranges index rather than last_seg.ortho_indices, while retaining the
boundary type and apostrophe checks and clearing next_boundary.phon for both
ordinary and synthetic segments.
---
Nitpick comments:
In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua`:
- Around line 31-64: Remove the duplicate local UNSTRESSED table in the later
run flow, retain a single module-scope UNSTRESSED definition, and update run()
to reference that shared table without rebuilding it per call. Ensure all
existing entries and lookup behavior remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b502d4b6-b125-4ad9-9b48-bbd37e304add
📒 Files selected for processing (14)
wiktionary_pron/help/index.htmlwiktionary_pron/help/irish.htmlwiktionary_pron/index.htmlwiktionary_pron/lua_modules/ga-irish_engine.luawiktionary_pron/lua_modules/ga-passes/01_polarity.luawiktionary_pron/lua_modules/ga-passes/02_stress.luawiktionary_pron/lua_modules/ga-passes/07_nasalization.luawiktionary_pron/lua_modules/ga-passes/11_unstressed_reduction.luawiktionary_pron/lua_modules/ga-passes/12_epenthesis.luawiktionary_pron/lua_modules/ga-passes/13_sonorants.luawiktionary_pron/lua_modules/ga-passes/14_final_cleanup.luawiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.luawiktionary_pron/scripts/main.jswiktionary_pron/scripts/tests/golden/golden.json
🚧 Files skipped from review as they are similar to previous changes (2)
- wiktionary_pron/index.html
- wiktionary_pron/scripts/main.js
Comment on lines
+78
to
+80
| <p>Irish (Gaeilge) has three major dialect groups — Connacht, Munster, and Ulster — each with distinct phonological | ||
| patterns. This tool supports all three dialects. Key dialectal differences include vowel quality (e.g., | ||
| Connacht /aː/ vs Munster /æː/), consonant realization (e.g., word-final broad bh/mh → vˠ/vʲ in Connacht, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a dialect distinction supported by this page.
The new example says Connacht /aː/ versus Munster /æː/, but the page’s own mappings describe long á as /ɑː/ in Connacht/Munster and /aː/ in Ulster; /æ/ is the Ulster pre-slender realization. Align this sentence with the tables.
Proposed wording
- patterns. This tool supports all three dialects. Key dialectal differences include vowel quality (e.g.,
- Connacht /aː/ vs Munster /æː/), consonant realization (e.g. word-final broad bh/mh → vˠ/vʲ in Connacht,
+ patterns. This tool supports all three dialects. Key dialectal differences include vowel quality (e.g.,
+ Connacht/Munster /ɑː/ vs Ulster /aː/ for long á, and Ulster /æ/ before slender consonants),
+ consonant realization (e.g. word-final broad bh/mh → vˠ/vʲ in Connacht,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p>Irish (Gaeilge) has three major dialect groups — Connacht, Munster, and Ulster — each with distinct phonological | |
| patterns. This tool supports all three dialects. Key dialectal differences include vowel quality (e.g., | |
| Connacht /aː/ vs Munster /æː/), consonant realization (e.g., word-final broad bh/mh → vˠ/vʲ in Connacht, | |
| <p>Irish (Gaeilge) has three major dialect groups — Connacht, Munster, and Ulster — each with distinct phonological | |
| patterns. This tool supports all three dialects. Key dialectal differences include vowel quality (e.g., | |
| Connacht/Munster /ɑː/ vs Ulster /aː/ for long á, and Ulster /æ/ before slender consonants), | |
| consonant realization (e.g., word-final broad bh/mh → vˠ/vʲ in Connacht, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/help/irish.html` around lines 78 - 80, Update the dialectal
vowel-quality example in the introductory paragraph to match the mappings
documented in this page: describe long á as /ɑː/ in Connacht and Munster versus
/aː/ in Ulster, and do not use the unsupported Connacht /aː/ versus Munster /æː/
distinction.
hellpanderrr
force-pushed
the
feature/irish-g2p
branch
from
July 30, 2026 10:44
9a802f8 to
cbbb402
Compare
…-final boundary check - 02_stress: write context.root_vowel_count back so pass 11 can use it - 08_slender_coda: guard next.palatal==true so broad ng is not affected - 09_consonants: use boundary-aware word-final test (not nxt type==boundary or nil) for th/dh/gh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiktionary_pron/lua_modules/ga-irish_engine.lua`:
- Around line 169-187: Update find_onset_start and the secondary-stress path to
reuse the same complete onset-selection logic as preprocessing, including
phonotactic legality and transparent apostrophe/hyphen markers. Ensure illegal
medial clusters resolve to the preprocessing-selected final consonant, while
preserving appropriate word or morpheme boundary handling so valid secondary
stress marks are not suppressed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 095d51dd-5bfe-4dd0-9280-3b6031c9c31f
📒 Files selected for processing (16)
wiktionary_pron/help/index.htmlwiktionary_pron/help/irish.htmlwiktionary_pron/index.htmlwiktionary_pron/lua_modules/ga-irish_engine.luawiktionary_pron/lua_modules/ga-passes/01_polarity.luawiktionary_pron/lua_modules/ga-passes/02_stress.luawiktionary_pron/lua_modules/ga-passes/07_nasalization.luawiktionary_pron/lua_modules/ga-passes/08_slender_coda.luawiktionary_pron/lua_modules/ga-passes/09_consonants.luawiktionary_pron/lua_modules/ga-passes/11_unstressed_reduction.luawiktionary_pron/lua_modules/ga-passes/12_epenthesis.luawiktionary_pron/lua_modules/ga-passes/13_sonorants.luawiktionary_pron/lua_modules/ga-passes/14_final_cleanup.luawiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.luawiktionary_pron/scripts/main.jswiktionary_pron/scripts/tests/golden/golden.json
🚧 Files skipped from review as they are similar to previous changes (4)
- wiktionary_pron/index.html
- wiktionary_pron/lua_modules/ga-passes/07_nasalization.lua
- wiktionary_pron/help/irish.html
- wiktionary_pron/lua_modules/ga-passes/15_dialect_finalize.lua
Comment on lines
+169
to
+187
| -- Shared onset-start helper: walks backward from vowel_idx to find | ||
| -- the phonotactically legal onset start. Stops at word boundaries; | ||
| -- optionally stops at boundary tokens (use true for secondary stress, | ||
| -- which must not cross word/morpheme boundaries). | ||
| local function find_onset_start(tokens, vowel_idx, stop_at_boundaries) | ||
| local onset = vowel_idx | ||
| for j = vowel_idx - 1, 1, -1 do | ||
| local t = tokens[j] | ||
| if t.type == "cons" and t.phon and t.phon ~= "" then | ||
| onset = j | ||
| elseif t.type == "boundary" and stop_at_boundaries then | ||
| break | ||
| elseif t.phon == nil or t.phon == "" then | ||
| -- skip silent/ghost consonants (fh, th, etc.) | ||
| else | ||
| break | ||
| end | ||
| end | ||
| return onset |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
The secondary-stress helper still diverges from preprocessing.
find_onset_start(..., true) stops at every boundary and accepts any preceding consonant cluster, while preprocessing treats apostrophe/hyphen markers as transparent and applies phonotactic legality. For an illegal medial cluster, preprocessing can move secondary to the final consonant, then this helper walks back to the previous consonant and suppresses the stress mark because onset_start ~= i. Reuse the complete onset-selection algorithm for both paths, including boundary and legal-cluster handling.
Also applies to: 209-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/lua_modules/ga-irish_engine.lua` around lines 169 - 187,
Update find_onset_start and the secondary-stress path to reuse the same complete
onset-selection logic as preprocessing, including phonotactic legality and
transparent apostrophe/hyphen markers. Ensure illegal medial clusters resolve to
the preprocessing-selected final consonant, while preserving appropriate word or
morpheme boundary handling so valid secondary stress marks are not suppressed.
- Remove unreachable arm; intent is already covered by the / not-next branch below it. - Use boundary-aware word-final test in the aí branch instead of bare , matching the pattern used by 09_consonants and bhf. - De-nest OI_TO_OE from inside the block so it is not gated on EPS tables having rewritten the vowel.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiktionary_pron/lua_modules/ga-passes/10_vowels.lua`:
- Around line 228-234: Update the is_final calculation in the ortho == "aí"
branch to treat only stream end or actual space-boundary tokens as word-final;
exclude apostrophe and hyphen elision boundaries identified by the token
metadata used in ga-irish_engine.lua. Preserve the existing medial consonant
handling and final iː mapping for genuine word boundaries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ee33861-0e7d-4375-b862-a2c5540fdb06
📒 Files selected for processing (1)
wiktionary_pron/lua_modules/ga-passes/10_vowels.lua
Comment on lines
+228
to
+234
| elseif ortho == "aí" then | ||
| -- aí: unstressed variant → word-final iː, medial short a | ||
| -- Word-final detection uses boundary-aware check (not just nil next) | ||
| -- per Hickey II.1.9: boundary tokens exist in multi-word phrases. | ||
| local next_t = tokens[i + 1] | ||
| local is_final = not next_t or next_t.type == "boundary" | ||
| if not is_final and next_t.type == "cons" then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude elision boundaries from the word-final check.
next_t.type == "boundary" also matches apostrophe/hyphen tokens, which are elision markers rather than true word boundaries in ga-irish_engine.lua. An aí before one of these markers can therefore receive the incorrect final iː mapping; restrict this check to the stream end or actual space boundaries.
🐛 Suggested fix
- local is_final = not next_t or next_t.type == "boundary"
+ local is_final = not next_t or
+ (next_t.type == "boundary" and
+ next_t.ortho ~= "'" and next_t.ortho ~= "-")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elseif ortho == "aí" then | |
| -- aí: unstressed variant → word-final iː, medial short a | |
| -- Word-final detection uses boundary-aware check (not just nil next) | |
| -- per Hickey II.1.9: boundary tokens exist in multi-word phrases. | |
| local next_t = tokens[i + 1] | |
| local is_final = not next_t or next_t.type == "boundary" | |
| if not is_final and next_t.type == "cons" then | |
| elseif ortho == "aí" then | |
| -- aí: unstressed variant → word-final iː, medial short a | |
| -- Word-final detection uses boundary-aware check (not just nil next) | |
| -- per Hickey II.1.9: boundary tokens exist in multi-word phrases. | |
| local next_t = tokens[i + 1] | |
| local is_final = not next_t or | |
| (next_t.type == "boundary" and | |
| next_t.ortho ~= "'" and next_t.ortho ~= "-") | |
| if not is_final and next_t.type == "cons" then |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiktionary_pron/lua_modules/ga-passes/10_vowels.lua` around lines 228 - 234,
Update the is_final calculation in the ortho == "aí" branch to treat only stream
end or actual space-boundary tokens as word-final; exclude apostrophe and hyphen
elision boundaries identified by the token metadata used in ga-irish_engine.lua.
Preserve the existing medial consonant handling and final iː mapping for genuine
word boundaries.
Every other language gets its example text from the sampleTexts map in dynamic_meta.js, applied by updateMeta(). Irish was never added there, so updateMeta() assigned sampleTexts["Irish"] === undefined to the textarea. The previous workaround added a separate DEFAULT_EXAMPLE_TEXT map in main.js, but it ran before updateMeta() in the same handler, so updateMeta() overwrote it moments later. It also gated on localStorage rather than the textarea, so it could not repopulate an emptied form. Fix the data instead of working around it: - Add Irish to sampleTexts (an Irish passage about the IPA, matching the theme of the other languages' samples). - Add Irish to translatedMeta so the meta description no longer renders "undefined" for Irish. - Remove the DEFAULT_EXAMPLE_TEXT block from main.js. Verified in a real browser (Playwright): Irish shows its sample on first load and again after clearing the form and reselecting the language; German is unaffected. Full e2e suite: 21 passed, 5 skipped. Golden: 81 pass.
ga-irish_engine.lua already requires all three lex_subs tables at module scope, so the identical pcall(require, ...) calls in the adapter were no-ops against Lua's module cache. Keep the engine as the single source of truth and note why the timing still holds: requiring the engine pulls the tables in from inside the doString coroutine, which is what keeps the lazy loads off the JS->Lua callback path. Verified: golden 81 pass; transcription e2e 13/13 pass (adapter init is exactly what these exercise).
The ga-passes copy had drifted from the irish repo's passes/ after the CodeRabbit fixes were applied here but never benchmarked. Three of them turned out to be net-negative once measured, so they are reverted here and the two repos are now byte-identical (modulo the ga-passes require prefix). Reverted (measured against the 6598/4102/4785-word benchmark): - 12_epenthesis: bh/mh restored to the generic branch (-31 Connacht / -37 Munster without them — the dedicated l+bh/mh branch is word-final-gated, so excluding them dropped legitimate medial svarabhakti) - 08_slender_coda: next.palatal guard removed (-5 Connacht) - 09_consonants: boundary-aware word-final th/dh/gh removed (-3 C / -3 U) Also here: - 02_stress: MONOSYLLABIC_STRESS was hoisted via a bare global `_MONOSYLLABIC_STRESS`, leaking into _G. Now a module-scope local. - 13_sonorants: the NON_TENSOR_SLENDER dedup had dropped BOTH copies of caintim/guiochtaint/peinteailte rather than one, breaking those 3 words. - lex_subs_munster/ulster: replaced with the benchmark-verified tables from the irish repo (the deployed ones were generated against a since-changed engine). - golden.json: Gaeilge/Munster expected value was recorded from the drifted engine; corrected to the canonical output. Verified: golden 81 pass; full e2e 21 passed / 5 skipped (pre-existing stashes). Benchmark in the source repo is unchanged from baseline — Connacht 96.92%, Munster 93.76%, Ulster 94.84%.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
wiktionary_pron/lua_modules/ga-passes/02_stress.lua (1)
451-453: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRegression:
context.root_vowel_countpersistence removed again.Per the change details, this diff deletes the end-of-pass
context.root_vowel_count = seg_root_vowel_countwrite, leaving onlycontext.is_monosyllabicassigned beforereturn tokens. This is the exact issue previously flagged and marked "✅ Addressed" — it appears to have regressed.context.root_vowel_countdefaults to0inga-irish_engine.lua's context table, so any downstream pass reading it will again always see0.🐛 Suggested fix
context.is_monosyllabic = seg_is_monosyllabic + context.root_vowel_count = seg_root_vowel_count return tokens🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua` around lines 451 - 453, Restore the end-of-pass assignment of seg_root_vowel_count to context.root_vowel_count before returning tokens in the affected pass, alongside the existing context.is_monosyllabic assignment. Preserve the return behavior and ensure downstream passes receive the computed root vowel count.wiktionary_pron/lua_modules/ga-passes/09_consonants.lua (1)
135-157: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRegression: word-final
th/dh/ghagain ignore boundary tokens.This is the same issue previously flagged and marked "✅ Addressed":
th(line 136) anddh/gh(line 150) still testi ==#tokens`` instead of treating a followingboundarytoken (or `nil`) as word-final, unlike the correctly-handled `bhf` branch a few lines below (162-176). The leftover unused `local next = tokens[i + 1]` at line 149 (never referenced in the dh/gh block) suggests the boundary-aware check was present at some point and has since been reverted.Impact: in multi-word phrase input, a word ending in
th/dh/ghfollowed by aboundarytoken (not the very last array element) takes the non-final path (h/j/ɣ) instead of being silenced.🐛 Suggested fix
elseif token.ortho == "th" then - if i == `#tokens` then + local after = tokens[i + 1] + if after == nil or after.type == "boundary" then token.phon = "" else local word_initial = (prev == nil) or (prev.type == "boundary") local nxt = tokens[i + 1] if word_initial and token.palatal == true and nxt and nxt.type == "vowel" and nxt.ortho == "eo" then token.phon = "\xc3\xa7" -- ç else token.phon = "h" end end elseif token.ortho == "dh" or token.ortho == "gh" then - local next = tokens[i + 1] - if i == `#tokens` then + local nxt = tokens[i + 1] + if nxt == nil or nxt.type == "boundary" then -- Word-final dh/gh: silent token.phon = "" elseif token.palatal == true then token.phon = "j" else token.phon = "ɣ" end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiktionary_pron/lua_modules/ga-passes/09_consonants.lua` around lines 135 - 157, Update the th handling and the dh/gh handling to treat a following boundary token or nil as word-final, matching the boundary-aware behavior of the bhf branch. Use the existing next-token lookup to apply silent pronunciation in those cases, and remove the unused local next declaration if it is replaced by the shared check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@wiktionary_pron/lua_modules/ga-passes/02_stress.lua`:
- Around line 451-453: Restore the end-of-pass assignment of
seg_root_vowel_count to context.root_vowel_count before returning tokens in the
affected pass, alongside the existing context.is_monosyllabic assignment.
Preserve the return behavior and ensure downstream passes receive the computed
root vowel count.
In `@wiktionary_pron/lua_modules/ga-passes/09_consonants.lua`:
- Around line 135-157: Update the th handling and the dh/gh handling to treat a
following boundary token or nil as word-final, matching the boundary-aware
behavior of the bhf branch. Use the existing next-token lookup to apply silent
pronunciation in those cases, and remove the unused local next declaration if it
is replaced by the shared check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d9e5e95-edb1-4d92-b5f9-9bb177c43bc9
📒 Files selected for processing (11)
wiktionary_pron/lua_modules/ga-passes/02_stress.luawiktionary_pron/lua_modules/ga-passes/08_slender_coda.luawiktionary_pron/lua_modules/ga-passes/09_consonants.luawiktionary_pron/lua_modules/ga-passes/12_epenthesis.luawiktionary_pron/lua_modules/ga-passes/13_sonorants.luawiktionary_pron/lua_modules/ga-passes/lex_subs_munster.luawiktionary_pron/lua_modules/ga-passes/lex_subs_ulster.luawiktionary_pron/lua_modules/ga-pron_wasm.luawiktionary_pron/scripts/dynamic_meta.jswiktionary_pron/scripts/main.jswiktionary_pron/scripts/tests/golden/golden.json
Two CodeRabbit findings that were still valid on the merge candidate: - 02_stress: seg_root_vowel_count was computed but never written to context, so context.root_vowel_count (which the pass header advertises and downstream code reads) was always nil. - 11_unstressed_reduction: a dead local is_short_vowel() referenced an undefined `ustring`; it would have raised if called. run() already inlines the equivalent check. Ported from the benchmarked source repo, so the two copies stay byte-identical. Verified: benchmark unchanged (C 96.92 / M 93.76 / U 94.84), golden 81 pass, transcription e2e 13/13 pass.
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.
Summary by CodeRabbit