Use real Wrangler translation runtime probe#619
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a real-model verification flow and JSON-mode translation to the translation worker: new verification script runs wrangler dev against a test endpoint that exercises real AI/Cache/R2 bindings; CI/workflow and package scripts call this verifier; worker defaults, batch JSON handling, and a runtime test route are updated. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Script as verify-real-ai.ts
participant R2 as R2 Bucket
participant Wrangler as wrangler dev
participant Worker as Translation Worker
participant AI as AI Binding/API
participant Cache as Cache API
participant Probe as /__translation-test__/real-runtime
GHA->>Script: run verify:real-translation
Script->>R2: wrangler r2 bucket create
R2-->>Script: bucket ready
Script->>Wrangler: start wrangler dev (free localhost port)
Wrangler-->>Script: dev server running + logs
Script->>Probe: GET /__translation-test__/real-runtime
Probe->>Worker: invoke test handler
Worker->>AI: request with response_format (JSON-mode)
AI-->>Worker: JSON { translations: [...] }
Worker->>Cache: write diagnostics / results
Worker->>R2: store translations
Worker-->>Probe: return diagnostic JSON (model, flags, translations)
Probe-->>Script: payload received
Script->>Script: validate model, counts, tokens, flags
Script->>Wrangler: terminate dev
Script-->>GHA: exit (success/failure)
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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 16 minutes and 40 seconds. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/translation-worker/scripts/verify-real-ai.ts`:
- Around line 105-110: The fetchProbe function currently calls fetch(url)
without a per-request timeout, so a single request can hang beyond the outer
TIMEOUT_MS retry loop; update fetchProbe to create an AbortController for each
fetch, start a per-request timer (e.g., requestTimeoutMs < TIMEOUT_MS or
configurable), pass controller.signal to fetch, and clear the timer on
completion, and handle the abort/timeout error path so the retry loop can
continue; reference the fetchProbe function and TIMEOUT_MS constant when making
the change.
- Around line 91-102: The probe currently only fails when both samples remain
untranslated and misses protected tokens; update the untranslated check in the
translations mapping block to fail if any translation equals its corresponding
SOURCE_TEXTS entry (i.e., change the && to a per-item comparison or use
Array.some comparing translations[i].trim() === SOURCE_TEXTS[i]) so a partially
untranslated batch is rejected, and extend the protected-token loop (the array
used when building joined from translations) to include "npm" and "bun"
alongside the existing tokens to ensure those tokens aren't dropped or
translated.
In `@apps/translation-worker/src/index.ts`:
- Around line 1833-1857: The probeRuntimeStorage function currently may leave
temporary cache entries and R2 objects when an exception occurs; wrap the probe
logic in a try/finally (or use nested try/finally blocks) so that both the cache
probeKey and the R2 key are always deleted regardless of errors. Specifically,
in probeRuntimeStorage generate nonce and construct cacheProbeKey and r2Key as
before, perform the cache put/match and R2 put/get checks inside try, and then
in the finally block call caches.default.delete(cacheProbeKey) and
env.TRANSLATION_STORE.delete(r2Key) (guarding against undefined variables if
needed) so cleanup always runs even when the cache or R2 probe throws.
🪄 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: 147788f1-d765-4887-8cdf-cf91b187e831
📒 Files selected for processing (8)
.github/workflows/deploy-translation.ymlapps/translation-worker/package.jsonapps/translation-worker/scripts/verify-real-ai.tsapps/translation-worker/src/index.tsapps/translation-worker/test/index.test.tsapps/translation-worker/wrangler.jsoncapps/translation-worker/wrangler.real-test.jsoncpackage.json
💤 Files with no reviewable changes (1)
- apps/translation-worker/test/index.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/translation-worker/scripts/verify-real-ai.ts`:
- Around line 61-81: runWrangler() can hang because it awaits child.exited with
no timeout; modify runWrangler to race the process completion against a timeout
using Promise.race() (the existing sleep() and REQUEST_TIMEOUT_MS should be
reused) so the function returns/throws when the timeout elapses. Specifically,
keep spawning the child and collecting stdout/stderr as before, but wrap
awaiting child.exited (and the stdout/stderr reads) in a Promise.race with a
timeout promise that rejects or returns a timed-out result consistent with how
the main probe loop/cleanup handles timeouts; ensure ensureDevelopmentBucket()
receives a non-hanging failure when wrangler exceeds REQUEST_TIMEOUT_MS so the
outer deadline can handle it.
In `@apps/translation-worker/src/index.ts`:
- Around line 992-1019: The protectTranslationTokens function currently replaces
protected tokens with placeholders but restore(translated) blindly reinserts
only exact placeholders, allowing the model to drop or rewrite tokens and the
worker to accept bad translations; update restore (in protectTranslationTokens)
to validate that every original protected token occurrence is present in the
final restored string and if any are missing or altered, treat the translation
as invalid (e.g., throw an error or return a failure signal) so callers can
retry/fail the translation; implement the check by comparing counts or positions
of each original token versus occurrences in the restored output after
re-insertion and fail when any token’s expected occurrences are not preserved.
🪄 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: 6022f0b8-b481-48d3-baaf-8b1a2c75a2ec
📒 Files selected for processing (2)
apps/translation-worker/scripts/verify-real-ai.tsapps/translation-worker/src/index.ts
|
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
1 participant
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.
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores