[codex] Refresh translations when source changes#637
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds source-hash-aware translation caching, incremental-state persistence, and background task orchestration to the translation worker; API surface expanded to accept an optional WorkerExecutionContext for ctx-aware background scheduling. ChangesSource-Hash-Aware Translation Caching and Background Orchestration
Sequence DiagramsequenceDiagram
participant Client
participant Worker as Translation Worker
participant Cache as KV/R2 Cache
participant Queue as Translation Queue
participant BG as Background Context
Client->>Worker: fetch(request, env, ctx?)
Worker->>Worker: readTranslationSourceHash(request)
Worker->>Cache: lookup cached translation
alt cached & sourceHash matches & fresh
Cache-->>Worker: cached translation (with sourceHash)
Worker-->>Client: return cached translation
else cached but stale or sourceHash mismatch
Cache-->>Worker: stale translation
Worker->>BG: ctx.waitUntil(enqueueTranslation(..., sourceHash))
Worker-->>Client: serve stale translation (with sourceHash)
BG->>Queue: enqueue translation task (sourceHash)
else no cached translation
Worker->>Worker: compute translationSourceHash(locale, html)
Worker->>BG: ctx.waitUntil(enqueueTranslation(..., sourceHash))
Worker-->>Client: serve pending marker
BG->>Queue: enqueue translation task (sourceHash)
end
Queue->>Worker: process translation job
Worker->>Cache: putTranslationPendingMarker(..., sourceHash)
Worker->>Cache: store translated response with sourceHash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 860e0c89ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/translation-worker/src/index.ts (1)
2049-2079: 💤 Low valueMinor race condition in source check throttling.
Between checking
alreadyChecked(line 2052) and putting the marker (lines 2055-2063), concurrent requests for the same URL could all pass the check and trigger parallel source fetches. This is benign (just redundant origin requests) since the check has a 5-minute throttle and operations are idempotent.Also, the
Accept-Language: localeheader on line 2069 is overwritten byfetchEnglishOrigininternally. Consider removing it for clarity or adding a comment.🤖 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 `@apps/translation-worker/src/index.ts` around lines 2049 - 2079, In checkTranslatedSourceFreshness, prevent the race between the alreadyChecked check and the caches.default.put by inserting the cache marker immediately after the alreadyChecked guard: call caches.default.put(checkKey, new Response(knownSourceHash ?? '', {...})) before calling loadSourceHtml so concurrent requests see the marker and skip redundant origin fetches; keep the same Cache-Control/X-Capgo headers and allow the marker to be updated later if needed. Also remove (or add a clarifying comment about) the Accept-Language header on the render Request since fetchEnglishOrigin overwrites it—adjust the renderRequest creation in checkTranslatedSourceFreshness accordingly.
🤖 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.
Nitpick comments:
In `@apps/translation-worker/src/index.ts`:
- Around line 2049-2079: In checkTranslatedSourceFreshness, prevent the race
between the alreadyChecked check and the caches.default.put by inserting the
cache marker immediately after the alreadyChecked guard: call
caches.default.put(checkKey, new Response(knownSourceHash ?? '', {...})) before
calling loadSourceHtml so concurrent requests see the marker and skip redundant
origin fetches; keep the same Cache-Control/X-Capgo headers and allow the marker
to be updated later if needed. Also remove (or add a clarifying comment about)
the Accept-Language header on the render Request since fetchEnglishOrigin
overwrites it—adjust the renderRequest creation in
checkTranslatedSourceFreshness accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e26630ab-5a74-4992-8edc-9308f69d3c74
📒 Files selected for processing (1)
apps/translation-worker/src/index.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 614a8083e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 `@apps/translation-worker/src/index.ts`:
- Around line 470-478: The continuation path that sets a pending marker fails to
include the sourceHash, which causes dedupe keys to be lost when
TRANSLATION_COORDINATOR is unset; update the continuation flow to pass the
sourceHash into putTranslationPendingMarker (ensure the variable used to
detect/enqueue translations — the same sourceHash computed earlier in the
continue/refresh branch — is forwarded) so the pending cache entry includes
TRANSLATION_SOURCE_HASH_HEADER; locate the continue/refresh branch that
currently calls putTranslationPendingMarker without sourceHash and add the
sourceHash argument when invoking it.
- Around line 2438-2439: The current matcher allows a hash-less job to match any
record (returning true when job.sourceHash is falsy), which corrupts dedupe
state when the same matcher is reused for both /enqueue and /complete; change
the final check to require a strict sourceHash equality so a hash-less job only
matches hash-less records and a hash-specific job only matches the same hash.
Concretely, in the function that returns using record and job (the block with
"if (!record || record.cacheVersion !== job.cacheVersion || record.locale !==
job.locale || record.url !== job.url) return false"), replace the loose check
"return !job.sourceHash || record.sourceHash === job.sourceHash" with logic that
returns true only when both sourceHash are equal (e.g., treat undefined/empty
the same by comparing record.sourceHash === job.sourceHash or explicitly check
both falsy or exact equality) so both /enqueue and /complete use a symmetric,
strict match.
🪄 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
Run ID: a3b065d4-2cbe-450e-a729-b93c95ceeec6
📒 Files selected for processing (2)
apps/translation-worker/scripts/verify-parser.tsapps/translation-worker/src/index.ts
|
1 participant
Summary
Impact
Localized pages keep returning fast from cache, while source updates are detected much sooner than the previous 24-hour freshness window.
Validation
bun run ci:verify:translationSummary by CodeRabbit