Feat/batch sheet updates - #9
Merged
mike-doctorina merged 4 commits intoJul 14, 2026
Merged
Conversation
mike-doctorina
self-requested a review
July 13, 2026 10:38
mike-doctorina
added a commit
that referenced
this pull request
Jul 14, 2026
… failing batches (#10) * fix(localize): restore sheet writes, disambiguate locale codes, split 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> * fix(localize): support gpt-5 models, never write to non-localization 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> * chore: add sheety:localize and sheety:generate VS Code tasks, regenerate 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> * fix(localize): parse ICU by brace depth, skip reasoning items, plug a 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> * fix(localize): catch a placeholder dropped inside a plural branch 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> * fix(localize): quote A1 ranges, classify sheet-write failures, make the 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> * refactor(localize): put the spreadsheet behind SheetsGateway and cover 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> * chore: fix the ignore patterns of the VS Code tasks --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> --------- Co-authored-by: Mike Matiunin <plugfox@gmail.com> 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.
Что сделано
localize: запись в Google Sheets переведена на batch update по строкам вместо отдельных запросов на каждую ячейку.--workersтеперь реально управляет параллельностью OpenAI-запросов.