feat: add run --estimate for pre-run cost estimates(cli,sdk)#2111
Open
cherkanovart wants to merge 1 commit into
Open
feat: add run --estimate for pre-run cost estimates(cli,sdk)#2111cherkanovart wants to merge 1 commit into
cherkanovart wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new ChangesEstimate Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/cli/src/cli/cmd/run/estimate.ts (1)
10-22: 💤 Low valueConsider clarifying "leaf" terminology or verifying flat data assumption.
The comment describes counting "leaf string-value lengths," but the implementation only checks top-level values in the
processableDataobject. IfprocessableDatacan contain nested objects, this function would miss strings in nested structures.However, in typical i18n workflows, translation data is flattened into dot-notation keys (e.g.,
{"auth.login.title": "Login"}), so top-level iteration is likely correct.Recommendation: Either:
- Update the comment to clarify that data is expected to be flat (e.g., "the sum of string values — processableData keys are already flattened"), or
- Add a test case with a nested object to document the expected behavior (should return 0 for nested values)
🤖 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 `@packages/cli/src/cli/cmd/run/estimate.ts` around lines 10 - 22, The comment for countTranslatableChars claims it sums "leaf string-value lengths" but the implementation only inspects top-level values, so either clarify the assumption or make behavior explicit: update the comment to state that processableData is expected to be flattened (e.g., "processableData keys are already flattened; only top-level string values are counted"), or if nested objects may appear, modify tests to assert current behavior (add a test with a nested object expecting nested strings are ignored) or change the implementation to recursively traverse nested objects; reference the countTranslatableChars function when applying the change.packages/sdk/src/index.ts (1)
904-930: ⚡ Quick winConsider adding retry logic and event tracking for consistency.
The
estimate()method uses plainfetchinstead offetchWithRetry, which means transient 5xx errors won't be retried. Since cost estimates are user-facing operations that users request before committing to expensive translations, retrying on transient failures would improve reliability.Additionally,
estimate()doesn't track events (ESTIMATE_START/SUCCESS/ERROR), unlike other public methods (localizeObject,localizeText, etc.). Adding tracking would improve observability and usage analytics.♻️ Suggested improvements
- Replace
fetchwithfetchWithRetryfor resilience:- const res = await fetch(url, { + const res = await this.fetchWithRetry( + url, + { method: "POST", headers: this.headers, body: JSON.stringify({ items: parsedItems }), signal, - }); + }, + signal, + );
- Add event tracking for observability:
async estimate( items: { targetLocale: string; sourceChars: number }[], signal?: AbortSignal, ): Promise<CostEstimate> { + const trackProps = { method: "estimate" }; + trackEvent( + this.config.apiKey, + this.config.apiUrl, + TRACKING_EVENTS.ESTIMATE_START, + trackProps, + ); + try { const parsedItems = estimateItemsSchema.parse(items); const url = `${this.config.apiUrl}/process/estimate`; const res = await this.fetchWithRetry( url, { method: "POST", headers: this.headers, body: JSON.stringify({ items: parsedItems }), signal, }, signal, ); await LingoDotDevEngine.throwOnHttpError(res, "Error estimating cost"); - return res.json(); + const result = await res.json(); + trackEvent( + this.config.apiKey, + this.config.apiUrl, + TRACKING_EVENTS.ESTIMATE_SUCCESS, + trackProps, + ); + return result; + } catch (error) { + trackEvent( + this.config.apiKey, + this.config.apiUrl, + TRACKING_EVENTS.ESTIMATE_ERROR, + { + ...trackProps, + errorMessage: error instanceof Error ? error.message : String(error), + }, + ); + throw error; + } }Note: You'll need to add
ESTIMATE_START,ESTIMATE_SUCCESS, andESTIMATE_ERRORtoTRACKING_EVENTSinutils/tracking-events.ts.🤖 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 `@packages/sdk/src/index.ts` around lines 904 - 930, The estimate() method should be made resilient and observable: replace the plain fetch call with fetchWithRetry (preserving method, headers, body and signal) and keep the existing LingoDotDevEngine.throwOnHttpError(res, ...) behavior; surround the operation with Tracking.track calls (emit ESTIMATE_START before the request, ESTIMATE_SUCCESS on successful JSON return, and ESTIMATE_ERROR in the catch path) and ensure the ESTIMATE_START/ESTIMATE_SUCCESS/ESTIMATE_ERROR constants are added to TRACKING_EVENTS so the events are recognized.
🤖 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 `@packages/cli/src/cli/cmd/run/estimate.ts`:
- Around line 10-22: The comment for countTranslatableChars claims it sums "leaf
string-value lengths" but the implementation only inspects top-level values, so
either clarify the assumption or make behavior explicit: update the comment to
state that processableData is expected to be flattened (e.g., "processableData
keys are already flattened; only top-level string values are counted"), or if
nested objects may appear, modify tests to assert current behavior (add a test
with a nested object expecting nested strings are ignored) or change the
implementation to recursively traverse nested objects; reference the
countTranslatableChars function when applying the change.
In `@packages/sdk/src/index.ts`:
- Around line 904-930: The estimate() method should be made resilient and
observable: replace the plain fetch call with fetchWithRetry (preserving method,
headers, body and signal) and keep the existing
LingoDotDevEngine.throwOnHttpError(res, ...) behavior; surround the operation
with Tracking.track calls (emit ESTIMATE_START before the request,
ESTIMATE_SUCCESS on successful JSON return, and ESTIMATE_ERROR in the catch
path) and ensure the ESTIMATE_START/ESTIMATE_SUCCESS/ESTIMATE_ERROR constants
are added to TRACKING_EVENTS so the events are recognized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a4d5682-3422-4047-aa5c-5ecece52ed10
📒 Files selected for processing (11)
.changeset/run-estimate-flag.mdpackages/cli/src/cli/cmd/run/_types.tspackages/cli/src/cli/cmd/run/_utils.tspackages/cli/src/cli/cmd/run/estimate.spec.tspackages/cli/src/cli/cmd/run/estimate.tspackages/cli/src/cli/cmd/run/execute.tspackages/cli/src/cli/cmd/run/index.tspackages/cli/src/cli/localizer/_types.tspackages/cli/src/cli/localizer/lingodotdev.tspackages/sdk/src/index.spec.tspackages/sdk/src/index.ts
AndreyHirsa
approved these changes
Jun 12, 2026
cherkanovart
changed the title
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.
Adds a pre-run cost estimate to the sync CLI, mirroring the platform CLI's
push --estimate(ENG-1067).What was done
LingoDotDevEngine.estimate(items)method — POSTs per-target-locale character counts to the new/process/estimateendpoint and returns the approximate cost with a per-locale breakdown. Items are zod-validated and locale codes normalized to canonical BCP 47 client-side.lingo.dev run --estimateflag. Computes the same change delta as a regular run (checksums → delta → processable data, same--force/--key/--file/--bucketsemantics), counts translatable characters (leaf string values only), fetches the estimate, prints a per-locale breakdown, and exits. Nothing is translated, written, or billed; lockfile and target files stay untouched.computeProcessableData) and bucket-loader construction extracted fromexecute.tsinto shared_utils.tshelpers so estimate and execute can never disagree on what is "pending".--estimateis rejected when combined with--watchor--frozen, and errors with a clear message for providers without server-side pricing (pseudo, BYOK).Notes for review/testing
/process/estimateendpoint is not deployed yet. Code here is inert until then (the flag fails with a 404 from the API).lingo.dev run --estimate→ per-locale breakdown matchespush --estimatesemantics; re-run without changes → "$0.00 — nothing needs translation";--estimate --watch→ clear error.Tests
estimate.spec.tscoveringcountTranslatableCharsandcomputeProcessableData(delta scoping,--force, key patterns) — 6/6 pass.turbo build typecheck testfor both packages green.Summary by CodeRabbit
New Features
--estimateflag to theruncommand to display estimated translation costs without processingestimate()method to the SDK for cost estimationTests