fix(localize): restore sheet writes, disambiguate locale codes, split failing batches - #10
Merged
Merged
Conversation
… failing batches PR #9 rewrote localizeRows() from an async* generator into a StreamController but dropped the `yield row` without adding a `controller.add(row)`, so the stream closed without ever emitting: updateSheet() was never called and no translation reached the spreadsheet. On top of that fix, harden the pipeline against the model itself: - Locale codes are spelled out for the model in both the prompt and the JSON schema — English name, native endonym, and an explicit note for codes that are routinely misread ("uk" is Ukrainian, not United Kingdom). - A failed batch of languages is split into single-language requests instead of being re-sent as a whole, so a rare language the model chokes on cannot break its neighbours. Unusable payloads are never retried with the same prompt; only transient failures (network, timeout, 429, 5xx) are. - Every translation is validated before it is written: ICU placeholders and markup tags must survive, no empty values, no markdown fences, no runaway output. A rejected translation is retried alone. - Requests get a hard timeout (--timeout, default 120s) and a token budget that scales with the batch size, so the model can neither hang a worker nor truncate its answer into invalid JSON. - A row that cannot be written to the sheet is skipped instead of aborting the whole run; partially localized rows are still saved. The pipeline moved from bin/ to lib/ and is now covered by 58 unit tests, including HTTP-level tests against a local stand-in for the OpenAI API. The test step in CI is enabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sheets
Two failures found by running the pipeline against the real spreadsheet.
1. gpt-5-mini rejects the sampling parameters outright:
`400 Unsupported parameter: 'temperature' is not supported with this
model`. Every request failed this way, so localize had been writing
nothing at all since gpt-5-mini became the default in 0.4.2. Reasoning
models (gpt-5*, o*) now get `reasoning: {effort: low}` instead of
temperature/top_p, and their token budget reserves headroom for the
reasoning tokens, which are billed against the same max_output_tokens
ceiling and would otherwise truncate the answer into invalid JSON.
2. A sheet was localized regardless of its shape. A spreadsheet also holds
reference tables whose columns are ordinary data, and localize happily
"translated" them, overwriting the data with model output. A sheet is now
localized only when its header matches the documented layout
`label | description | meta | en | <locale> ...`; anything else is skipped
with a warning, independently of the --ignore flag.
Also regenerates example/ from the current spreadsheet.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ate example Both tasks carry the flags the project actually runs with, in particular an --ignore list that keeps the reference sheets (locales, help, backend-*, telegram-*) out of the pipeline. example/ is regenerated with the same flags: the ARBs move to src/l10n, use the "app" prefix and drop @@last_modified, so a rerun no longer rewrites every file with a new timestamp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… semaphore leak
Findings of an adversarial review of the branch.
- Placeholder validation rejected every correctly translated plural. The
regex `\{[^{}]*\}` cannot cross nested braces, so on
`{count, plural, one {# message} other {# messages}}` it matched the branch
bodies — the very text meant to be translated — instead of the argument.
A correct German translation therefore "lost" the placeholders, was retried
alone, was rejected again and the cell was left empty forever. The same hole
silently accepted a translation that had flattened the directive away. The
scanner now matches braces by depth: a plural is ONE argument whose branches
are ordinary text, argument names are compared as a set (plural categories
legitimately differ per language: two in English, four in Russian, one in
Japanese), and a lost `plural`/`select` directive is caught explicitly.
The sheets of this project do ship such strings.
- parseResponseBody accepted any output item with a `content` list. Reasoning
models may put their chain of thought in exactly that shape, ahead of the
message, so the thinking text was jsonDecode'd as the payload and every
request failed non-retryably. Only `message` items and their `output_text`
parts are read now; a refusal is reported as a refusal.
- The semaphore slot was acquired outside the try, and released after
`client.close()`. A throw from either would leak the slot; `workers` such
leaks starve every worker and hang the run with no output. Release now
happens in an outer finally that nothing can bypass.
- `gpt-5-chat-latest` is not a reasoning model — it accepts temperature and
rejects `reasoning.effort` — but the `gpt-5` prefix swept it up, so every
request would 400. Chat variants and the pre-reasoning o1 models are
excluded. The reasoning token reserve is raised to 16k: it is a ceiling, not
a charge, and an exhausted one truncates the answer into invalid JSON.
- Two columns whose headers sanitize to the same locale (`pt-BR` / `pt_BR`,
or a trailing space) made the pipeline ask for the language twice and write
the answer into the first column only, leaving the second empty forever. The
duplicate column is now skipped with a warning.
- An error delivered inside a 200 body is no longer retried as if it were a
transient network failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comparing argument names as a set kept the directive check honest across
languages with different plural categories, but it could not see a translation
that keeps `{value, plural, ...}` and drops `{value}` from the branch bodies —
the number simply never renders ("лет" instead of "5 лет"). Branch arguments
are now collected separately and compared on their own.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he concurrency test bite
Findings of a review of the tests, the CLI and the CI.
- The concurrency test was vacuous: the fake OpenAI server awaited each handler
inside its accept loop, so it served requests one at a time and could never
observe a client exceeding its limit. Deleting the semaphore entirely kept
the suite green. The fake now serves concurrently, and the test asserts the
peak is exactly `workers` — an upper bound alone would also be satisfied by a
client stuck at one request. A second test covers slot release after a failed
request, the leak that would otherwise hang the whole run.
- A sheet title with a space or an apostrophe ("App Strings") built an invalid
bare A1 range, so the API rejected the write with 400 and the row was lost.
Titles are quoted now, with inner quotes doubled.
- The sheet writer retried every failure 3x with a fixed 30s sleep, including
the permanent ones (bad range, missing scope, deleted sheet). Since writes are
sequential, one doomed row stalled every row behind it for a minute. Failures
are classified now — only 429/5xx/network are retried, with exponential
backoff.
- --sheet was declared mandatory, so args threw before the friendly "Missing
required argument" check could ever run. A bad numeric option (--workers=abc,
--batch=99) was silently swallowed into the default; it warns now.
- The system prompt and the in-body error path had no coverage: both could be
deleted with the suite still green.
- CHANGELOG did not mention the most disruptive change of 0.5.0 — that a sheet
is now localized only if its header matches the documented layout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r it with tests The sheet-write path — the exact code where the PR#9 regression lived — was bound to SheetsApi and a global rate limiter, so it was reachable only through a live Google account and had no tests at all. - SheetsGateway is the interface the pipeline talks to; GoogleSheetsGateway is its only Sheets-aware implementation and the only place that knows about googleapis. Its DetailedApiRequestError is mapped onto SheetsException, whose status code carries the retry decision. - updateRow (range building, batching, failure classification, retries) and RateLimiter (injectable clock and sleep, serialized so the check and the reservation cannot interleave) moved to lib/ as ordinary testable code. - bin/localize.dart keeps only argument parsing, authentication and wiring. 17 new tests cover what could not be tested before: A1 quoting for titles with spaces and apostrophes, one batch per row, empty cells skipped, retry on 429/5xx/transport, immediate give-up on 400/403/404, the run surviving an unwritable row, and the rate-limiter window. Mutating the failure classification kills a test; mutating the range quoting kills three. Also completes the 0.5.0 changelog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--ignore takes regular expressions, not globs. "telegram-*" reads as "telegram followed by any number of dashes" and only matched telegram-monetization by accident, as a substring; "backend-*" was redundant next to "backend", which already matches unanchored. The tasks now use anchored regexes, and the README says outright that these are RegExps and that a glob-looking pattern will not do what it looks like it does. Verified: the new patterns skip the same six sheets and regenerate example/ byte for byte. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Критичный баг из #9
localizeRowsбыл переписан сasync*наStreamController, ноyield rowвыкинули, аcontroller.add(row)не добавили. Стрим закрывался, ни разу ничего не выдав →updateSheetне вызывался никогда. С момента мержа #9localizeпереводил всё и не писал в таблицу ничего. Починено, покрыто тестом.Проблема 1 —
ukвоспринимается как United KingdomХэш-таблица была (0.4.1), но она добавляла
uk (Ukrainian)в одну строку промта, и модель это игнорировала. Теперь код языка расшифровывается в трёх местах сразу:uk — Ukrainian (українська) — Ukrainian (Cyrillic script). NOT English and NOT "United Kingdom" — British English would be "en_GB".descriptionполяtext, где модель физически не может это пропустить.Добавлены эндонимы (~70 языков) и заметки-предупреждения для ~25 реально путаемых кодов (cs/ce, sl/sk, ja/jv, ka/kn, ms/ml, se/sv, …).
Проблема 2 — редкие языки возвращают билиберду и ломают соседей
Упавший батч теперь разбивается, а не ретраится: каждый язык из него уходит отдельным запросом. Языки, которые уже перевелись в батче, сохраняются и повторно не запрашиваются; язык, упавший в одиночку, оставляет свою ячейку пустой (подхватится на следующем прогоне), а соседи всё равно пишутся в таблицу.
Скорее всего, у билиберды была конкретная причина:
max_output_tokensбыл захардкожен в 2048 вне зависимости от размера батча, и батч из редких языков (которые плохо токенизируются) обрезался посреди JSON — ответ приходит сstatus: incompleteи парсится как мусор. Теперь бюджет токенов масштабируется от числа языков, а обрезанный ответ детектируется явно, а не скармливается вjsonDecode.Что ещё улучшено
--timeout, по умолчанию 120с) — раньше зависший запрос держал воркер вечно.{name}) и теги разметки должны сохраниться, пустые значения запрещены, никаких markdown-фенсов и разбегающегося вывода. Отклонённый перевод ретраится в одиночку — страдает только он.Структура и тесты
Пайплайн переехал из
bin/localize.dartвlib/src/localize/(вbin/остались CLI и работа с Google Sheets) — именно это и делает его тестируемым.58 тестов в семи файлах: таблица языков, сборка промта, правила валидации, извлечение ячеек, логика разбиения батчей на фейковом клиенте и HTTP-тесты OpenAI-клиента против локального стенда (ретрай на 500, отсутствие ретрая на 401 и на битом payload, таймаут, лимит параллелизма).
Закомментированный шаг тестов в CI включён; format/analyze теперь покрывают
lib/иtest/, а не толькоbin/.Dockerfileкопируетlib/.🤖 Generated with Claude Code