Translations 1/2: gettext catalogues and the CI resource gate #25

Merged

claude-bot merged 19 commits from phase1/gettext into main 2026-10-22 09:15:08 +00:00

Conversation 5 Commits 19 Files changed 21 +875 -24

claude-bot commented 2026-10-21 19:09:11 +00:00

Collaborator

• Copy link
• Copy Markdown

What

The first half of translations (#14): the app's UI text comes from gettext catalogues, with German as the first one. It also adds a CI gate for start-up time, which is checked on two profiles.

• core::i18n reads .mo catalogues. It has no new dependency and no unsafe, and it is Qt-free. The language comes from LANGUAGE, LC_ALL, LC_MESSAGES and LANG, tried as de_DE, then de. If a string has no translation, the English source shows.
• The worker's user-visible errors and notices go through tr(). These include "no Bible open", the unknown-reference errors, "not in this Bible" and the three start-up notices.
• qsTr in QML reaches the same catalogue through one QTranslator in qt/main.cpp, so no libintl or glibc locale is needed on the phone.
• Packaging: the .mo is built into the click (share/locale/de/LC_MESSAGES/utstudybible.schaefera.mo).
• po/: utstudybible.schaefera.pot and de.po, with all 23 strings translated. cargo make i18n refreshes them, and cargo make i18n-check and CI fail when they are stale.
• CI's app job gains:
  • the po check;
  • a check that the built app shows German (title "Studienbibel") under LANGUAGE=de and English under C;
  • the start-up gate (tools/mem-check/budget-start-ms 2000 ms, budget-start-slow-ms 3000 ms);
  • a slow profile: two allowed CPUs, one shared with a busy-loop, failing if fewer than 2 CPUs are usable.

Book names are not gettext. They stay JSword data in the module's language. Part 2 covers more languages, plurals and context, and an in-app language override.

Blast radius

Tier: trunk
Trunk paths touched: qt/ glue (main.cpp, src/i18n.rs, src/lib.rs), core/src/worker/session.rs and session/jump.rs, CMakeLists.txt, Makefile.toml, tools/mem-check/, tools/ci/app.sh
Testing tier: elevated. It adds a new CI gate (start-up time, slow profile) and changes all text on screen. There are no new dependencies and no schema change.
Gate / rollback: nothing is behind a flag. With no catalogue for the phone's language, every string falls back to its English source, so an English phone sees no change. Rollback is a revert of this merge and reinstalling the previous click; no data is touched.

Proof

• Testing-tier gates: none are unrun.
  • The new CI gates were run in CI's own image (cargo make app-docker) and in CI.
  • The German path is checked end to end by tools/i18n/check-app.sh: German title "Studienbibel" with 10+ translated messages, and C locale "Study Bible" with 0.
• Rust gates: cargo make exit 0 on the merged tree (c013a10): fmt, clippy, tests with coverage (core lib 105 passed), release check, deny and audit. After 3010631, clippy and core tests passed again.
• App gates: cargo make app-docker exit 0 on efdde90.
  • Normal profile: text on screen after 206 ms (budget 2000), peak PSS 140234 kB (budget 165000).
  • Slow profile: 312 ms (budget 3000), peak PSS 144028 kB.
  • Kill -9 restore, picker and Back, and the typed reference all pass in both profiles.
• Fails without the change:
  • 7 mutants of core/src/i18n.rs (byte order, plural skip, empty msgstr, region fallback and others), each killed by a named test.
  • SLOW=1 BUDGET_START_MS=100 fails with "the first chapter took 313 ms, over the 100 ms baseline", exit 1. BUDGET_KB=100000 fails on PSS.
  • Stale po/: a .po missing an entry fails "not merged with the template". check-app with UTSB_LOCALE_DIR=/nonexistent exits 1.
  • i18n-check failed in CI's image when the .pot order differed (it caught the locale-dependent sort fixed here).
• Slow-profile CPU handling:
  • Parser outputs: 0-6:2 gives 0 2 4 6, 0,2,4 gives 0 2 4, 3 gives 3, and 1-3,8 gives 1 2 3 8.
  • docker run --cpuset-cpus=0 … SLOW=1 run.sh exits 1 with "slow profile needs 2 usable CPUs, this process has 1". Before efdde90 it ended with "pid: parameter not set", exit 2.
• On the phone (Plinius):
  • efdde90's code was installed as the 3010631 build. efdde90 differs only in tools/mem-check.
  • The journal shows build 0.0.1 phase1/gettext@3010631fe, window loaded after 325 ms, restored KJV 172 0.530, showing Gen 7.
  • The phone is set to en_US, so it logged "0 translated messages", which is the English fallback.
  • The de .mo is in the click.
  • On the phone, try: set System Settings → Language to Deutsch and reopen the app. The title should read "Studienbibel", Go to should read "Gehe zu", and Back should read "Zurück", with "Altes/Neues Testament" in the picker.
  • The 3010631 build is left installed.
• CI: rust, audit, app and click all green on 3010631, and again on efdde90, the last code commit.
  • On 1f85405 (the empty trailer commit, same tree), app failed after 2m38s. A normal run takes about 3m. This Forgejo serves no job logs or reruns through the API, so the failing step was not seen.
  • cargo make app-docker on that tree passed locally (slow profile 314 ms, peak PSS 141062 kB). The empty commit 90b716f reran CI, and app passed in 3m8s.
  • The flake's cause is unknown. Its timing points at mem-check, and the PSS margin (main's runs reach 154 of 165 MB) is the likeliest candidate.
• Budgets (docs/07):
  • Start-up work added: one .mo read at start (German is 2055 bytes; none is read on an English phone). Text on screen is 206 ms normal and 312 ms slow. main had no text-on-screen gate to compare against, and this PR adds it.
  • Peak PSS 140–144 MB against the 165 MB baseline, within the noise of main's 141–154 MB runs.
  • The click grows by the 2 kB .mo.
• Not covered:
  • Strings with a disambiguation comment, or plurals: they show the source text (part 2). Nothing runs the disambiguation branch yet (#33).
  • core/src/tasks.rs's "tasks: …" errors are not wrapped in tr() yet, because no UI shows them. They will be wrapped when #10 part 2 surfaces them.
  • German was not tried by touch on the phone.

Review

Reviewer: fresh-context reviews and audits, each in a new agent:

• Trunk review of a73c38a: 3 lows, all fixed in 104c1af: CPUs read from the allowed set, budgets raised to 2 s and 3 s, and no orphaned app in check-app.
• Re-review of 104c1af: 2 lows, both fixed in 828d056 and 39abee8: fewer than 2 CPUs now fails, and strided cpusets are read correctly.
• Audit: 0. It flagged translate returning an empty, non-null QString; fixed in 62a0f4e.
• Delta review 217 of 104c1af..c013a10, after merging main: 1 low (three untranslated notices), fixed in 3010631.
• Delta review 218 of c013a10..3010631: 1 low, deferred to #33. A module name containing "%2" would be re-substituted, which SWORD's alphanumeric names can't hit yet.
• Delta review 219 of 104c1af..efdde90 (qt/main.cpp, run.sh, the qt glue): no findings. Its note about dash's kill "" was declined, because the window has no command that can fail.

Review-Confidence: 50/100. Every changed trunk file was reviewed, all code after 104c1af is covered by delta reviews 217–219, CI is green on efdde90, and the auditor's own read found no missed defect. Points off for the round-1 findings that cite files without lines, the translator's disambiguation branch that nothing runs, and the %2 re-substitution (both now #33), and for fixes made after delta reviews.

Confidence by part

• core/src/i18n.rs (the .mo reader): high. It is mutant-tested and the CI app check reads a real catalogue.
• qt/main.cpp translator: high. Review 219 covered it, and check-app shows German and English.
• tools/mem-check/run.sh slow profile: medium-high. The parser outputs and the one-CPU exit are recorded. Strided cpusets were seen only in hand-fed input.
• po/de.po wording: medium. Two reviewers checked it, but no native speaker has. Read it.

Closes nothing; part of #14 (part 1/2).

🤖 Generated with Claude Code

## What The first half of translations (#14): the app's UI text comes from gettext catalogues, with German as the first one. It also adds a CI gate for start-up time, which is checked on two profiles. - **`core::i18n`** reads `.mo` catalogues. It has no new dependency and no `unsafe`, and it is Qt-free. The language comes from LANGUAGE, LC_ALL, LC_MESSAGES and LANG, tried as `de_DE`, then `de`. If a string has no translation, the English source shows. - **The worker's user-visible errors and notices** go through `tr()`. These include "no Bible open", the unknown-reference errors, "not in this Bible" and the three start-up notices. - **`qsTr`** in QML reaches the same catalogue through one `QTranslator` in `qt/main.cpp`, so no libintl or glibc locale is needed on the phone. - **Packaging:** the `.mo` is built into the click (`share/locale/de/LC_MESSAGES/utstudybible.schaefera.mo`). - **`po/`**: `utstudybible.schaefera.pot` and `de.po`, with all 23 strings translated. `cargo make i18n` refreshes them, and `cargo make i18n-check` and CI fail when they are stale. - **CI's app job gains:** - the po check; - a check that the built app shows German (title "Studienbibel") under `LANGUAGE=de` and English under C; - the start-up gate (`tools/mem-check/budget-start-ms` 2000 ms, `budget-start-slow-ms` 3000 ms); - a slow profile: two allowed CPUs, one shared with a busy-loop, failing if fewer than 2 CPUs are usable. Book names are not gettext. They stay JSword data in the module's language. Part 2 covers more languages, plurals and context, and an in-app language override. ## Blast radius **Tier:** trunk **Trunk paths touched:** `qt/` glue (`main.cpp`, `src/i18n.rs`, `src/lib.rs`), `core/src/worker/session.rs` and `session/jump.rs`, `CMakeLists.txt`, `Makefile.toml`, `tools/mem-check/`, `tools/ci/app.sh` **Testing tier:** elevated. It adds a new CI gate (start-up time, slow profile) and changes all text on screen. There are no new dependencies and no schema change. **Gate / rollback:** nothing is behind a flag. With no catalogue for the phone's language, every string falls back to its English source, so an English phone sees no change. Rollback is a revert of this merge and reinstalling the previous click; no data is touched. ## Proof - **Testing-tier gates:** none are unrun. - The new CI gates were run in CI's own image (`cargo make app-docker`) and in CI. - The German path is checked end to end by `tools/i18n/check-app.sh`: German title "Studienbibel" with 10+ translated messages, and C locale "Study Bible" with 0. - **Rust gates:** `cargo make` exit 0 on the merged tree (c013a10): fmt, clippy, tests with coverage (core lib 105 passed), release check, deny and audit. After 3010631, clippy and core tests passed again. - **App gates:** `cargo make app-docker` exit 0 on efdde90. - Normal profile: text on screen after 206 ms (budget 2000), peak PSS 140234 kB (budget 165000). - Slow profile: 312 ms (budget 3000), peak PSS 144028 kB. - Kill -9 restore, picker and Back, and the typed reference all pass in both profiles. - **Fails without the change:** - 7 mutants of `core/src/i18n.rs` (byte order, plural skip, empty msgstr, region fallback and others), each killed by a named test. - `SLOW=1 BUDGET_START_MS=100` fails with "the first chapter took 313 ms, over the 100 ms baseline", exit 1. `BUDGET_KB=100000` fails on PSS. - Stale po/: a .po missing an entry fails "not merged with the template". check-app with `UTSB_LOCALE_DIR=/nonexistent` exits 1. - `i18n-check` failed in CI's image when the .pot order differed (it caught the locale-dependent sort fixed here). - **Slow-profile CPU handling:** - Parser outputs: `0-6:2` gives `0 2 4 6`, `0,2,4` gives `0 2 4`, `3` gives `3`, and `1-3,8` gives `1 2 3 8`. - `docker run --cpuset-cpus=0 … SLOW=1 run.sh` exits 1 with "slow profile needs 2 usable CPUs, this process has 1". Before efdde90 it ended with "pid: parameter not set", exit 2. - **On the phone (Plinius):** - efdde90's code was installed as the 3010631 build. efdde90 differs only in `tools/mem-check`. - The journal shows `build 0.0.1 phase1/gettext@3010631fe`, window loaded after 325 ms, `restored KJV 172 0.530`, showing Gen 7. - The phone is set to `en_US`, so it logged "0 translated messages", which is the English fallback. - The `de` .mo is in the click. - **On the phone, try:** set System Settings → Language to Deutsch and reopen the app. The title should read "Studienbibel", Go to should read "Gehe zu", and Back should read "Zurück", with "Altes/Neues Testament" in the picker. - The 3010631 build is left installed. - **CI:** rust, audit, app and click all green on 3010631, and again on efdde90, the last code commit. - On 1f85405 (the empty trailer commit, same tree), `app` failed after 2m38s. A normal run takes about 3m. This Forgejo serves no job logs or reruns through the API, so the failing step was not seen. - `cargo make app-docker` on that tree passed locally (slow profile 314 ms, peak PSS 141062 kB). The empty commit 90b716f reran CI, and `app` passed in 3m8s. - The flake's cause is unknown. Its timing points at mem-check, and the PSS margin (main's runs reach 154 of 165 MB) is the likeliest candidate. - **Budgets (docs/07):** - Start-up work added: one `.mo` read at start (German is 2055 bytes; none is read on an English phone). Text on screen is 206 ms normal and 312 ms slow. main had no text-on-screen gate to compare against, and this PR adds it. - Peak PSS 140–144 MB against the 165 MB baseline, within the noise of main's 141–154 MB runs. - The click grows by the 2 kB `.mo`. - **Not covered:** - Strings with a disambiguation comment, or plurals: they show the source text (part 2). Nothing runs the disambiguation branch yet (#33). - `core/src/tasks.rs`'s "tasks: …" errors are not wrapped in `tr()` yet, because no UI shows them. They will be wrapped when #10 part 2 surfaces them. - German was not tried by touch on the phone. ## Review **Reviewer:** fresh-context reviews and audits, each in a new agent: - Trunk review of a73c38a: 3 lows, all fixed in 104c1af: CPUs read from the allowed set, budgets raised to 2 s and 3 s, and no orphaned app in check-app. - Re-review of 104c1af: 2 lows, both fixed in 828d056 and 39abee8: fewer than 2 CPUs now fails, and strided cpusets are read correctly. - Audit: 0. It flagged `translate` returning an empty, non-null QString; fixed in 62a0f4e. - Delta review 217 of 104c1af..c013a10, after merging main: 1 low (three untranslated notices), fixed in 3010631. - Delta review 218 of c013a10..3010631: 1 low, deferred to #33. A module name containing "%2" would be re-substituted, which SWORD's alphanumeric names can't hit yet. - Delta review 219 of 104c1af..efdde90 (qt/main.cpp, run.sh, the qt glue): no findings. Its note about dash's `kill ""` was declined, because the window has no command that can fail. **Review-Confidence:** 50/100. Every changed trunk file was reviewed, all code after 104c1af is covered by delta reviews 217–219, CI is green on efdde90, and the auditor's own read found no missed defect. Points off for the round-1 findings that cite files without lines, the translator's disambiguation branch that nothing runs, and the `%2` re-substitution (both now #33), and for fixes made after delta reviews. ## Confidence by part - `core/src/i18n.rs` (the .mo reader): high. It is mutant-tested and the CI app check reads a real catalogue. - `qt/main.cpp` translator: high. Review 219 covered it, and check-app shows German and English. - `tools/mem-check/run.sh` slow profile: medium-high. The parser outputs and the one-CPU exit are recorded. Strided cpusets were seen only in hand-fed input. - `po/de.po` wording: medium. Two reviewers checked it, but no native speaker has. Read it. Closes nothing; part of #14 (part 1/2). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 7 commits 2026-10-21 19:09:11 +00:00

docs: plan for gettext catalogues and the resource gate ... 84a91580b9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core: read gettext catalogues, translate the worker's errors ... 2c66c3c45f

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: extract and check the gettext catalogues, with a German one ... f651589b15

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qt: load the catalogue, answer qsTr from it, build the .mo into the click ... f0d8a76a38

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: gate start-up in mem-check, add the slow profile, document both ... a6617bd85a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qt: clang-format the translator ... d748ad5ffa

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: the i18n task in CLAUDE.md ... a73c38ac83

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

schaefera added 1 commit 2026-10-21 19:12:26 +00:00

tools: pick the slow profile's CPUs from the allowed set, looser start-up budgets, no orphaned app in check-app ... 99e2177f99

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

schaefera added 1 commit 2026-10-21 19:15:49 +00:00

ci: rerun the app job (it failed before its first step) ... 104c1afb8e

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

schaefera added 1 commit 2026-10-21 19:21:38 +00:00

tools: the slow profile fails with fewer than 2 CPUs and reads strided cpusets; qt: clang-format an empty brace ... 828d056dbb

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

schaefera added 1 commit 2026-10-21 19:35:27 +00:00

qt: clang-format an empty brace (the app job's first check) ... 439762fa49

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

schaefera added 2 commits 2026-10-21 19:49:25 +00:00

qt: return QString() instead of an empty brace (host and CI clang-format disagree on it) ... 4be99dc391

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qt: an empty QString that both clang-format and clang-tidy accept ... 39abee82cd

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot referenced this pull request 2026-10-21 20:10:28 +00:00

Reading window 3/3: picker and history #27

schaefera added 2 commits 2026-10-22 08:24:48 +00:00

qt: a translator with nothing to offer returns a null QString, so Qt shows the source ... 62a0f4e8ce

The empty-but-not-null string from 39abee8 would have blanked any qsTr
string with a disambiguation comment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge origin/main (picker and history, starter sets) into phase1/gettext ... c013a10f0b

Conflicts: session.rs keeps main's jump.rs in place of go_to; qt/src/lib.rs
has both the i18n and picker_model modules; mem-check's header lists both
sides' checks. jump.rs's three errors go through tr(), and po/ has German
for main's new picker strings. tools/i18n/update.sh sorts the file lists
bytewise, as the host's locale ordered session.rs and session/ unlike CI's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review reviewed 2026-10-22 08:25:42 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Delta review (medium) of 104c1af..c013a10

Checked: merge resolution (session.rs go_to removed in favour of main's jump.rs, no leftover callers or markers; qt/src/lib.rs has i18n, picker_model, reading_model; mem-check header has both sides' checks), the three jump.rs tr() calls (%1/%2 used correctly, order matches de.po), all new QML strings (PickerPage, SettingsPage, ReadingPane) wrapped in qsTr, de.po translations, and update.sh LC_ALL=C sort. tools/i18n/update.sh --check passes locally (20 messages).

Findings:

• core/src/worker/session.rs:165,172,193 (low, pre-existing, not from this delta): the open_db/open_reader Notices ("Your saved place couldn't be read...", "Can't open the saved state (...)", "%1 is no longer installed; ...") are user-visible via the notice ToolTip but not wrapped in tr(), so a German user sees English there. Same for core/src/tasks.rs:203,212 (Event::Error "tasks: ...", not yet surfaced in the UI).

No high or medium findings.

Delta review (medium) of 104c1af..c013a10 Checked: merge resolution (session.rs go_to removed in favour of main's jump.rs, no leftover callers or markers; qt/src/lib.rs has i18n, picker_model, reading_model; mem-check header has both sides' checks), the three jump.rs tr() calls (%1/%2 used correctly, order matches de.po), all new QML strings (PickerPage, SettingsPage, ReadingPane) wrapped in qsTr, de.po translations, and update.sh LC_ALL=C sort. `tools/i18n/update.sh --check` passes locally (20 messages). Findings: - core/src/worker/session.rs:165,172,193 (low, pre-existing, not from this delta): the open_db/open_reader Notices ("Your saved place couldn't be read...", "Can't open the saved state (...)", "%1 is no longer installed; ...") are user-visible via the notice ToolTip but not wrapped in tr(), so a German user sees English there. Same for core/src/tasks.rs:203,212 (Event::Error "tasks: ...", not yet surfaced in the UI). No high or medium findings.

schaefera added 1 commit 2026-10-22 08:28:07 +00:00

core: the three start-up notices go through tr(), with German (review 217) ... 3010631fe6

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review reviewed 2026-10-22 08:28:39 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Delta review (medium) of c013a10..3010631

Findings:

• core/src/worker/session.rs:196 (low): chained .replace("%1", &a.module).replace("%2", ...) would also substitute a "%2" inside the module name. Failure scenario: a module named "X%2" shows the wrong text. SWORD module names are alphanumeric, so this is practically unreachable. Replacing %2 first, or a single-pass substitute, removes the risk.

Checked and fine:

• English output is identical to the old format! strings.
• All tr() arguments are string literals.
• Argument order is right (%1 is the module, %2 is the new module).
• Single-%1 cases cannot be re-substituted.
• The German is accurate, impersonal (no du/Sie) and matches the style of po/de.po. The .pot and de.po entries are in sync.

Delta review (medium) of c013a10..3010631 Findings: - core/src/worker/session.rs:196 (low): chained `.replace("%1", &a.module).replace("%2", ...)` would also substitute a "%2" inside the module name. Failure scenario: a module named "X%2" shows the wrong text. SWORD module names are alphanumeric, so this is practically unreachable. Replacing %2 first, or a single-pass substitute, removes the risk. Checked and fine: - English output is identical to the old format! strings. - All tr() arguments are string literals. - Argument order is right (%1 is the module, %2 is the new module). - Single-%1 cases cannot be re-substituted. - The German is accurate, impersonal (no du/Sie) and matches the style of po/de.po. The .pot and de.po entries are in sync.

schaefera added 1 commit 2026-10-22 08:41:28 +00:00

tools: mem-check sets pid and xvfb before its trap, so a slow profile with one CPU fails with exit 1 and its message ... efdde90071

With set -u the EXIT trap read an unset $pid and ended the run with 'pid: parameter not set' (exit 2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review reviewed 2026-10-22 08:42:10 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Delta review (medium) of 104c1af..efdde90: qt/main.cpp, tools/mem-check/run.sh, qt glue

Files reviewed: qt/main.cpp, tools/mem-check/run.sh, tools/ci/app.sh, qt/src/lib.rs, qt/src/i18n.rs (this branch's part only; also checked core/src/i18n.rs for the empty-msgstr case).

Checked, no problem:

• CatalogueTranslator::translate returns a null QString for null sourceText, a non-empty disambiguation and a missing lookup. The catalogue parser (core/src/i18n.rs:55) drops entries with an empty text, so a non-null empty QString (which Qt would show as blank) cannot come out of lookup.
• Slow-profile CPU parser: singles, lists, ranges and a-b:s all reduce to one CPU per line. head -n 2 closing the pipe early is harmless, since there is no pipefail. Fewer than 2 CPUs exits 1 before any process is started.
• Trap: hog= pid= xvfb= before the trap is fine under set -u. The restart pid=$! assignments (lines 249 and 351) follow kill -9 and wait on the old pid, so the trap kills the live one. The hog (taskset execs sh, so $! is the loop shell) is killed by the trap. No bare wait exists that would block on the hog.
• qt/src/lib.rs mod i18n and the #[unsafe(no_mangle)] externs match the extern "C" declarations in main.cpp. The null check on msgid is correct.

No findings.

Note (below the reporting threshold, not a finding): under dash, kill "" X aborts the builtin on the empty operand. If run.sh ever exited after Xvfb starts but before pid=$! (a one-second window with no command that can fail), Xvfb would not be killed. bash continues past the bad operand.

Delta review (medium) of 104c1af..efdde90: qt/main.cpp, tools/mem-check/run.sh, qt glue Files reviewed: qt/main.cpp, tools/mem-check/run.sh, tools/ci/app.sh, qt/src/lib.rs, qt/src/i18n.rs (this branch's part only; also checked core/src/i18n.rs for the empty-msgstr case). Checked, no problem: - CatalogueTranslator::translate returns a null QString for null sourceText, a non-empty disambiguation and a missing lookup. The catalogue parser (core/src/i18n.rs:55) drops entries with an empty text, so a non-null empty QString (which Qt would show as blank) cannot come out of lookup. - Slow-profile CPU parser: singles, lists, ranges and a-b:s all reduce to one CPU per line. `head -n 2` closing the pipe early is harmless, since there is no pipefail. Fewer than 2 CPUs exits 1 before any process is started. - Trap: `hog= pid= xvfb=` before the trap is fine under set -u. The restart `pid=$!` assignments (lines 249 and 351) follow `kill -9` and `wait` on the old pid, so the trap kills the live one. The hog (taskset execs sh, so $! is the loop shell) is killed by the trap. No bare `wait` exists that would block on the hog. - qt/src/lib.rs `mod i18n` and the `#[unsafe(no_mangle)]` externs match the extern "C" declarations in main.cpp. The null check on msgid is correct. No findings. Note (below the reporting threshold, not a finding): under dash, `kill "" X` aborts the builtin on the empty operand. If run.sh ever exited after Xvfb starts but before `pid=$!` (a one-second window with no command that can fail), Xvfb would not be killed. bash continues past the bad operand.

claude-bot referenced this pull request 2026-10-22 08:52:20 +00:00

i18n: single-pass placeholder substitution, and a test for the translator's disambiguation path #33

schaefera added 1 commit 2026-10-22 08:52:32 +00:00

chore: record review result ... 1f85405c62

Review-Confidence: 50
Review-Tier: trunk
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot changed title from WIP: Translations 1/2: gettext catalogues and the CI resource gate to Translations 1/2: gettext catalogues and the CI resource gate 2026-10-22 08:53:03 +00:00

claude-review approved these changes 2026-10-22 08:53:41 +00:00

Dismissed

claude-review left a comment

• Copy link
• Copy Markdown

Gate (step 8):

• PR 25 is not a draft and has no WIP title. Head is 1f85405, and it is mergeable.
• The body has What, Blast radius, Proof, Review and Confidence sections. Proof says no elevated or other testing-tier gate is unrun. The new CI gates and the German end-to-end check were run, and the result was read back from the built app.
• The head commit carries Review-Confidence: 50 and Review-Tier: trunk. origin/main is an ancestor of the head.
• No official REQUEST_CHANGES review exists. The listed reviews (217, 218, 219) are COMMENT only. Their findings are fixed (3010631), deferred to #33 with a reason, or declined with a reason in the body.
• CI on the last code commit, efdde90, is success for rust, audit, app and click. CI on the trailer commit 1f85405 is still pending. It changes no files.

Gate (step 8): - PR 25 is not a draft and has no WIP title. Head is 1f85405, and it is mergeable. - The body has What, Blast radius, Proof, Review and Confidence sections. Proof says no elevated or other testing-tier gate is unrun. The new CI gates and the German end-to-end check were run, and the result was read back from the built app. - The head commit carries Review-Confidence: 50 and Review-Tier: trunk. origin/main is an ancestor of the head. - No official REQUEST_CHANGES review exists. The listed reviews (217, 218, 219) are COMMENT only. Their findings are fixed (3010631), deferred to #33 with a reason, or declined with a reason in the body. - CI on the last code commit, efdde90, is success for rust, audit, app and click. CI on the trailer commit 1f85405 is still pending. It changes no files.

schaefera added 1 commit 2026-10-22 09:05:09 +00:00

chore: record review result (rerun CI: app failed on 1f85405 with an unchanged tree; app-docker passes locally) ... 90b716fa6c

Review-Confidence: 50
Review-Tier: trunk
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-review approved these changes 2026-10-22 09:14:52 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Gate (step 8):

• PR 25 is not a draft, has no WIP title, head is 90b716f, and it is mergeable.
• The body has What, Blast radius, Proof, Review and Confidence sections. Proof lists the elevated gates (new CI start-up gate, German end-to-end check) as run, and none as unrun.
• The head commit carries Review-Confidence: 50 and Review-Tier: trunk. It is an empty commit on 1f85405 (git diff --stat is empty), made to rerun CI after the flaky app job, as the CI bullet explains. origin/main is an ancestor of the head.
• Reviews 217, 218 and 219 are COMMENT only; there is no official REQUEST_CHANGES. Findings are fixed (3010631), deferred to #33 with a reason, or declined with a reason in the body. Approval 220 on 1f85405 is stale, as expected.
• CI on 90b716f: combined state success; rust, audit, app and click all success.

Gate (step 8): - PR 25 is not a draft, has no WIP title, head is 90b716f, and it is mergeable. - The body has What, Blast radius, Proof, Review and Confidence sections. Proof lists the elevated gates (new CI start-up gate, German end-to-end check) as run, and none as unrun. - The head commit carries Review-Confidence: 50 and Review-Tier: trunk. It is an empty commit on 1f85405 (git diff --stat is empty), made to rerun CI after the flaky app job, as the CI bullet explains. origin/main is an ancestor of the head. - Reviews 217, 218 and 219 are COMMENT only; there is no official REQUEST_CHANGES. Findings are fixed (3010631), deferred to #33 with a reason, or declined with a reason in the body. Approval 220 on 1f85405 is stale, as expected. - CI on 90b716f: combined state success; rust, audit, app and click all success.

claude-bot merged commit 7157264a1a into main 2026-10-22 09:15:08 +00:00

claude-bot referenced this pull request from a commit 2026-10-22 09:15:10 +00:00

Merge pull request 'Translations 1/2: gettext catalogues and the CI resource gate' (#25) from phase1/gettext into main

claude-bot referenced this pull request 2026-10-22 09:15:26 +00:00

Translations (gettext) and CI resource check #14

claude-bot referenced this pull request 2026-10-22 09:15:26 +00:00

CI: the app job failed once on an unchanged tree, cause unknown #34

Sign in to join this conversation.

Reviewers

No reviewers

claude-review

Labels

Clear labels   

priority

Could

MoSCoW: nice to have, done when time allows

  

priority

Must

MoSCoW: the release isn't viable without it

  

priority

Should

MoSCoW: important, but the release works without it

  

priority

Won't

MoSCoW: not this release (kept for later)

  

type

Bug

No labels

priority

Could

priority

Must

priority

Should

priority

Won't

type

Bug

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

schaefera/utStudyBIble!25

No description provided.