Stop the nightly translation re-translating everything, most days - #685
Open
chhhee10 wants to merge 4 commits into
Open
Stop the nightly translation re-translating everything, most days#685chhhee10 wants to merge 4 commits into
chhhee10 wants to merge 4 commits into
Conversation
`rust-quality` cached `~/.cargo` AND `target/` under a combined `actions/cache@v6`, which writes a ref-scoped copy from every branch that misses the exact key. Because the entry carries build output, each copy is 1.5-2.3 GiB, and five were live at once — PR refs 677, 679, 680, 681 and main — putting the repository at 11.56 GiB against GitHub's 10 GiB cap and therefore permanently in LRU eviction. The thing being evicted was not another cargo build. It was the 13 KB doc translation cache, read once every 24 hours by the nightly `translate-docs` run and so always the least-recently-used entry in the store. Losing it re-translated 48 pages into 14 languages the next morning: ~125 runner-minutes and a full LLM pass per language, against a 4-minute baseline when it survives. Six consecutive days of that, Aug 6-11, cost ~750 runner-minutes and six full-corpus passes through the gateway. Restore on every run, save only on a push to main — the split `build-daemon.yml` already uses, whose comment gives the other reason to want it (a PR branch can otherwise write a poisoned `target/` that a later release run restores straight into a published binary). `cache-hit != 'true'` keeps a run that changed nothing from re-uploading 2 GiB. What a PR gives up: one whose `Cargo.lock` moved rebuilds from a stale-but-close main cache. That is already what `restore-keys` hands it today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The only save sat in `consolidate`, downstream of both the matrix gate
(`if: needs.translate.result == 'success'`) and `mintlify validate`. So the
day's cache was contingent on fourteen languages and a nav check all succeeding:
Aug 6 discarded ~110 minutes of completed translation because one `ko` page
failed validation, and Aug 12 discarded a full run because consolidate's
validation failed. In both cases every language had finished its work and
uploaded its fragment; the cache was thrown away anyway.
Each fragment is already authoritative for its own language, so nothing has to
be merged before it can be stored. Each language now saves its own, in the job
that produced it, immediately after the step that proved it good. Consolidate's
merged save stays as the cross-language fallback.
The restore key changes for a related reason. It read
`translation-cache-${{ hashFiles('scripts/translate-docs/.translation-cache.json') }}`,
which ALWAYS evaluated to the bare literal `translation-cache-`: the file is
gitignored, so it is absent at checkout and `hashFiles` returns "" for a path
that matches nothing. Every restore that ever worked was a `restore-keys` prefix
match. That is not a bug on its own — but it means a total miss and a hit are
indistinguishable, so the expensive case was silent. It is now a per-language
key with the merged entry as fallback, and a miss emits a `::warning` naming
what it is about to cost.
Artifact retention 1 → 7 days, so a run that dies mid-pipeline leaves a human a
recovery path rather than expiring overnight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
chhhee10
force-pushed
the
fix/translate-cache-eviction
branch
from
August 12, 2026 12:27
58a0a80 to
06833d8
Compare
`isCached` is a pure function of the ENGLISH source hash. It records that a page
was translated once — never that the translation is on disk now — and those two
facts came apart in production.
Translations land on an auto-translate PR branch. While that branch sits
unmerged, `main` lacks the files and the cache still reports them done, so they
are never regenerated. Meanwhile `--update-nav` reads the ENGLISH tree and emits
nav entries for them, and `mintlify validate` fails on entries pointing at files
that are not there. Verified on the live repo: `docs/cli/{update,migrate}.mdx`
exist on main, `docs/zh/cli/` has neither, and PR #682 carrying them is still
open — 28 missing files across 14 locales.
That is non-convergent, which is what makes it worth a code change rather than a
merge. A cache HIT writes nothing, so validation fails and the cache is never
saved; a full cache MISS spends 120 runner-minutes and goes green. The pipeline
had no path to a cheap success while #682 stayed open, and Aug 12 is exactly
that: all 14 languages finished in ~20 seconds each, and consolidate failed.
Statting the output makes the cache self-healing against any "translated once,
never landed" gap, whatever opened it — an unmerged PR, a hand-reverted file, a
locale added to the matrix after the fact.
Guarded at all four sites rather than one. `cli.ts` is load-bearing: the batch
path sorts pages into cached/uncached itself and never calls `translateMdxPage`
for a cached one, so guarding only the translator would have fixed nothing. The
two single-page paths are guarded too, or they and the batch path disagree about
what "cached" means.
The tests pin both directions — a missing output re-translates, a present one is
still skipped. The second matters as much as the first: without it a later
refactor could satisfy this commit by making the guard a cache bypass, and every
run would be a full re-translation with the suite still green. Confirmed the
first test fails on the pre-fix code rather than assuming it would.
NOTE: this changes translation OUTPUT, not just caching. The first run after it
lands re-sends those 28 pages to the model, so the text will not be byte-identical
to what sits on #682 — expect a noisy diff there once. ~10 minutes, once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTranslation cache hits now require existing output files. CI and translation workflows use explicit cache restore and save steps. Release notes document these changes, and tests cover missing and present cached outputs. ChangesTranslation cache reliability
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
chhhee10
force-pushed
the
fix/translate-cache-eviction
branch
from
August 12, 2026 12:28
06833d8 to
b20ebad
Compare
Contributor
Hermes
No actionable correctness, security, data-safety, or compatibility defects found in the PR diff. The relevant cache, workflow, batch, and single-page paths were inspected. What this changesflowchart LR
n0RustCIcache["~ Rust CI cache"]
n1Translationworkflow["~ Translation workflow"]
n2Translationcache["Translation cache"]
n3TranslationbatchCLI["~ Translation batch CLI"]
n4MDXtranslator["~ MDX translator"]
n5READMEtranslator["~ README translator"]
n6Translationtests["~ Translation tests"]
n1Translationworkflow -- "restores and saves cache fragments" --> n2Translationcache
n1Translationworkflow -- "starts per-language translation" --> n3TranslationbatchCLI
n3TranslationbatchCLI -- "dispatches uncached MDX tasks" --> n4MDXtranslator
n3TranslationbatchCLI -- "dispatches uncached README tasks" --> n5READMEtranslator
n3TranslationbatchCLI -- "checks hashes and output existence" --> n2Translationcache
n4MDXtranslator -- "validates cached output existence" --> n2Translationcache
n5READMEtranslator -- "validates cached output existence" --> n2Translationcache
n6Translationtests -- "exercises cache-hit behavior" --> n4MDXtranslator
Rounds
FindingsNothing raised yet.
|
The save key embeds `github.run_id`, and GitHub REUSES that id when someone re-runs a failed job. On the second attempt the primary key already exists, the restore scores an exact hit, and the save collides with itself. Found by running it rather than reasoning about it. A throwaway workflow exercising the same key shapes showed all four cases: - first run: cache-matched-key='', save proceeds - next run: restored from probe-zh-<prev_id>, cache-hit='false' - re-run: cache-hit='true' <- the collision - languages stayed isolated: ja restored ja's payload, zh restored zh's The same `cache-hit != 'true'` guard `build-daemon.yml:137` carries. With it the re-run skips the save and stays green. That probe also confirmed the two things the rest of this branch assumes and could not otherwise check: `cache-matched-key` really is empty on a total miss — so the new warning fires exactly when a language is about to re-translate everything, and stays silent on the prefix hits that are the normal case — and the `restore-keys` prefix genuinely carries the previous run's file across, which is the whole mechanism by which tomorrow's run inherits today's cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/translate-docs.yml:
- Around line 105-108: Update the workflow’s prepare logic to validate every
language from inputs.languages against the supported language allowlist and
reject any invalid value before shell execution. Pass only the validated
language value to translation steps through an environment variable, and quote
that variable wherever the translation command uses it so shell metacharacters
cannot execute commands.
In `@scripts/translate-docs/readme-translator.ts`:
- Around line 223-225: Add direct tests for translateReadme covering both
cache-condition outcomes: regenerate the README translation when
docs/i18n/README.<lang>.md is missing, and skip generation when that file
exists. Add these cases to
__tests__/scripts/translate-docs/readme-translator.test.ts, using the existing
test setup and mocks to verify translation is invoked only for the missing-file
case.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b204146e-eeac-40f8-8d0c-3ec775e03b57
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/translate-docs.ymlCHANGELOG.md__tests__/scripts/translate-docs/mdx-translator.test.tsscripts/translate-docs/cli.tsscripts/translate-docs/mdx-translator.tsscripts/translate-docs/readme-translator.ts
Comment on lines
+105
to
+108
| - name: Warn on translation cache miss | ||
| if: steps.restore-cache.outputs.cache-matched-key == '' | ||
| run: | | ||
| echo "::warning title=Translation cache MISS::${{ matrix.lang }} will re-translate every page (~9 runner-minutes and one full LLM pass)" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,125p' .github/workflows/translate-docs.yml
rg -n -C 4 'translate-docs\.yml|uses:.*translate-docs|inputs:\s*$|languages:' .github/workflowsRepository: FailproofAI/failproofai
Length of output: 9116
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- workflow remainder ---'
sed -n '120,260p' .github/workflows/translate-docs.yml
printf '%s\n' '--- language-related implementation ---'
rg -n -C 5 'languages|language|lang' scripts/translate-docs package.json .github/workflows/translate-docs.yml \
-g '*.ts' -g '*.js' -g '*.json' -g '*.yml' -g '*.yaml' -g 'package.json' \
| head -n 500
printf '%s\n' '--- workflow references and permissions ---'
rg -n -C 5 'translate-docs|workflow_call|workflow_dispatch|permissions:|ANTHROPIC_API_KEY|ANTHROPIC_AUTH_TOKEN' .github README.md scripts \
-g '*.yml' -g '*.yaml' -g '*.md' -g '*.ts' -g '*.js' -g 'package.json' \
| head -n 500
printf '%s\n' '--- candidate files ---'
git ls-files scripts/translate-docs .github/workflows/translate-docs.ymlRepository: FailproofAI/failproofai
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
workflow = Path(".github/workflows/translate-docs.yml").read_text()
cases = [
'en',
'en; echo INJECTED',
"en' ; echo INJECTED; #",
'en", "ja"]',
]
needles = [
"if [ -n \"${{ inputs.languages }}\" ]; then",
"echo '${{ inputs.languages }}' | jq -Rc",
"echo \"::warning title=Translation cache MISS::${{ matrix.lang }}",
"bun run translate --languages ${{ matrix.lang }}",
]
for value in cases:
print(f"=== input: {value!r} ===")
expanded = workflow.replace("${{ inputs.languages }}", value).replace("${{ matrix.lang }}", value)
for needle in needles:
pos = expanded.find(needle.split("${{")[0])
if pos >= 0:
print(expanded[pos:expanded.find("\n", pos)])
PYRepository: FailproofAI/failproofai
Length of output: 1652
Validate and quote inputs.languages before shell use
A workflow_dispatch user can set inputs.languages. The value reaches prepare shell source and the unquoted translation command. For example, en;id runs id as a separate command while ANTHROPIC_API_KEY is set. Validate each language against the supported allowlist in prepare, then pass the validated value through a quoted environment variable.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 108-108: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/translate-docs.yml around lines 105 - 108, Update the
workflow’s prepare logic to validate every language from inputs.languages
against the supported language allowlist and reject any invalid value before
shell execution. Pass only the validated language value to translation steps
through an environment variable, and quote that variable wherever the
translation command uses it so shell metacharacters cannot execute commands.
Source: Linters/SAST tools
Comment on lines
+223
to
+225
| // `&& existsSync(outputPath)` — see the MDX path. Cached records that a | ||
| // translation was produced, not that the file is there now. | ||
| if (isCached(cache, "README.md", lang, sourceContent) && existsSync(outputPath)) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add direct tests for the README cache condition.
The supplied tests invoke translateMdxPage only. They do not exercise translateReadme.
Add tests in __tests__/scripts/translate-docs/readme-translator.test.ts for both cases: regenerate when docs/i18n/README.<lang>.md is absent, and skip generation when it exists. As per coding guidelines: “When you add or change logic, add a corresponding test in __tests__/.”
🤖 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 `@scripts/translate-docs/readme-translator.ts` around lines 223 - 225, Add
direct tests for translateReadme covering both cache-condition outcomes:
regenerate the README translation when docs/i18n/README.<lang>.md is missing,
and skip generation when that file exists. Add these cases to
__tests__/scripts/translate-docs/readme-translator.test.ts, using the existing
test setup and mocks to verify translation is invoked only for the missing-file
case.
Source: Coding guidelines
hermes-exosphere
approved these changes
Aug 12, 2026
hermes-exosphere
left a comment
hermes-exosphere
left a comment
Contributor
There was a problem hiding this comment.
Hermes found no blocking issues in this revision.
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.
The
Translate Docscron cost 4 minutes on Aug 3-5 and 118-136 minutesevery day from Aug 6-11 — ~750 wasted runner-minutes and six full-corpus passes
through the LLM gateway in six days.
Three causes compound. None of them is the translation cache's own logic, which
is sound —
isCachedvalidates per entry against the English source hash, andthe top-level
sourceHashfield is vestigial (written in three places, read innone).
1. The cache was evicted between runs — dominant, ~105 min/day
ci.ymlcachedtarget/alongside the cargo registry under a combinedactions/cache@v6, which writes a ref-scoped copy from every branch thatmisses the exact key. With build output included each copy is 1.5-2.3 GiB, and
five were live at once — PR refs 677, 679, 680, 681 and main — putting the repo
at 11.56 GiB against GitHub's 10 GiB cap, so the store sits permanently in
LRU eviction.
What that evicted was the 13 KB translation cache, read once every 24 hours
and therefore always the least-recently-used entry in the store. Aug 9's jobs
say so directly:
Cache not found for input keys: translation-cache-, translation-cache-, the morning after Aug 8 loggedCache saved with key: translation-cache-1adebf3f….Fixed with the restore/save split
build-daemon.yml:117-144already uses, whosecomment gives the other reason to want it (a PR branch can otherwise write a
poisoned
target/that a release run restores into a published binary).2. The cache was saved once, at the end of a serial pipeline
The only save sat in
consolidate, downstream of both the matrix gate andmintlify validate. One page failing in one language discarded all fourteen —Aug 6 lost ~110 minutes of finished translation to a single
kopage. Eachlanguage now saves its own fragment in the job that produced it, right after the
step that proved it good. Each fragment is already authoritative for its own
language, so nothing needs merging first; consolidate's merged save stays as the
cross-language fallback.
A miss is also visible now. The old restore key always evaluated to the bare
literal
translation-cache-— the file is gitignored, so it is absent atcheckout and
hashFilesreturns""for a path matching nothing. Every restorethat ever worked was a
restore-keysprefix match, and a total miss lookedexactly like a hit: nothing failed, nothing warned, the job just spent nine
minutes and a full LLM pass.
3. A cache HIT never checked the file exists — this is Aug 12, and it deadlocks
isCachedrecords that a page was translated once, not that it is on disknow. Translations land on an auto-translate PR branch; while that sits
unmerged,
mainlacks the files and the cache still says done, so they are neverregenerated — while
--update-navreads the English tree and emits naventries for them.
mintlify validatethen fails on 28 missing files.Verified live rather than inferred:
docs/cli/update.mdxanddocs/cli/migrate.mdxare onmain,docs/zh/cli/has neither, and #682carrying them is still open.
That is non-convergent: a cache hit writes nothing → validation fails → the
cache is never saved; a full cache miss spends 120 minutes → goes green. There
was no path to a cheap success while #682 stayed open.
re-sends those 28 pages to the model, so the text will not be byte-identical to
what is on #682 — expect one noisy diff there. ~10 minutes, once.
Guarded at all four call sites.
cli.tsis the load-bearing one: the batchpath sorts pages into cached/uncached itself and never calls
translateMdxPagefor a cached page, so guarding only the translator would have fixed nothing.
Testing
Beyond the suite, the deadlock was reproduced against the real repo with a
realistically seeded cache (every English page marked translated for
zhat itscurrent hash, as a successful Aug 11 run would have left it):
The new unit tests pin both directions — a missing output re-translates, a
present one is still skipped. The second matters as much as the first: without
it a later refactor could satisfy this PR by turning the guard into a cache
bypass, and every run would be a full re-translation with the suite still green.
I also confirmed the first test fails on the pre-fix code rather than
assuming it would.
bun run test:run— 3458 pass, 10 skippedtsc --noEmit— cleaneslinton every changed file — cleanDeliberately not included
already makes the cache survive that failure, so this is reorder risk for
little return.
Expected effect
Steady state returns to the ~4-minute cache-hit day. The first run after merge
re-translates the 28 pages
mainis missing (~10 minutes, once), and the repocache should fall back under the 10 GiB cap as the four PR-scoped cargo copies
age out.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HEwcerc9jE7ZBkfBiYRbep
Hermes review
d960a1d2532a0b648cfb12ce0fdfc7da5718e9cc1d8f31d926828f3bae215c58f5b35baa44acbff0gpt-5.6-terraSummary
No actionable correctness, security, data-safety, or compatibility defects found in the PR diff. The relevant cache, workflow, batch, and single-page paths were inspected.
Changes
Validation
None configured.
Findings
None.
Open questions
None.
Policy overrides
None.
Summary by CodeRabbit
Bug Fixes
Chores
Documentation
Tests