Skip to content

Fix #1110: [BUG] improve the speed when skip language#1285

Closed
JiwaniZakir wants to merge 1 commit intomengxi-ream:mainfrom
JiwaniZakir:fix/1110-bug-improve-the-speed-when-skip-languag
Closed

Fix #1110: [BUG] improve the speed when skip language#1285
JiwaniZakir wants to merge 1 commit intomengxi-ream:mainfrom
JiwaniZakir:fix/1110-bug-improve-the-speed-when-skip-languag

Conversation

@JiwaniZakir
Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir commented Apr 6, 2026

Closes #1110

Type of Changes

  • ✨ New feature (feat)
  • 🐛 Bug fix (fix)
  • 📝 Documentation change (docs)
  • 💄 UI/style change (style)
  • ♻️ Code refactoring (refactor)
  • ⚡ Performance improvement (perf)
  • ✅ Test related (test)
  • 🔧 Build or dependencies update (build)
  • 🔄 CI/CD related (ci)
  • 🌐 Internationalization (i18n)
  • 🧠 AI model related (ai)
  • 🔄 Revert a previous commit (revert)
  • 📦 Other changes that do not modify src or test files (chore)

Description

When the "skip language" feature was enabled with LLM language detection and an LLM translation provider, shouldSkipByLanguage was making a separate LLM API call to detect the language of every paragraph before translation. This was redundant because the LLM translation prompt already handles language detection internally, resulting in two LLM API calls per paragraph.

The fix is in src/utils/host/translate/translate-variants.ts inside translateTextUsingPageConfig. The enableLLM flag passed to shouldSkipByLanguage (and ultimately to detectLanguage) is now conditioned on whether the active translation provider is non-LLM:

// Before
config.languageDetection.mode === "llm"

// After
config.languageDetection.mode === "llm" && !isLLMProviderConfig(providerConfig)

This implements the exact behavior described in the issue:

  • LLM detection + LLM translation: skip language check uses franc only (enableLLM: false) — no redundant API call
  • LLM detection + non-LLM translation (e.g. microsoft-translate-default): LLM detection is still used (enableLLM: true)
  • franc detection: unchanged, always uses franc
  • Skip languages off: unchanged, detection is not called at all

Related Issue

Closes #

How Has This Been Tested?

  • Added unit tests
  • Verified through manual testing

Two new unit tests added in src/utils/host/__tests__/translate-text.test.tsx under "skip language detection with LLM translation provider":

  1. LLM provider (openai-default) + LLM detection mode + skipLanguages: ["jpn"]: asserts detectLanguage is called with { enableLLM: false }.
  2. Non-LLM provider (microsoft-translate-default) + LLM detection mode + skipLanguages: ["jpn"]: asserts detectLanguage is called with { enableLLM: true }.

Both tests use a 14-character Japanese string that falls in the range >= MIN_LENGTH_FOR_SKIP_LLM_DETECTION (10) and < MIN_LENGTH_FOR_TARGET_LANG_DETECTION (50), ensuring only shouldSkipByLanguage triggers detectLanguage, not isTextAlreadyInTargetLanguage.

Screenshots

N/A

Checklist

  • I have tested these changes locally
  • I have updated the documentation accordingly if necessary
  • My code follows the code style of this project
  • My changes do not break existing functionality
  • If my code was generated by AI, I have proofread and improved it as necessary.

Additional Information

isLLMProviderConfig (imported from @/types/config/provider) is used to determine whether the resolved providerConfig is LLM-based, keeping the logic consistent with how provider types are already distinguished elsewhere in the codebase.


This PR was created with AI assistance (Claude). The changes were reviewed by quality gates and a critic model before submission.


Summary by cubic

Speeds up page translation when “skip languages” is enabled by avoiding redundant LLM language detection with LLM translation providers. Fixes #1110 and removes one LLM API call per paragraph.

  • Bug Fixes
    • LLM detection + LLM translation (e.g., openai-default): skip-language check uses franc only (no extra LLM call).
    • LLM detection + non-LLM translation (e.g., microsoft-translate-default): still uses LLM detection.
    • franc-only detection and “skip languages” off remain unchanged.
    • Added unit tests for both provider types to verify enableLLM behavior.

Written for commit e430f9b. Summary will update on new commits.

When skip-language is on with LLM detection mode, avoid an extra LLM
API call by using franc (enableLLM: false) for the language skip check
whenever the translation provider itself is LLM-based, since the
translation prompt already handles source language detection.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 6, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 6, 2026

⚠️ No Changeset found

Latest commit: e430f9b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Contributor trust score

15/100 — New contributor

This score estimates contributor familiarity with mengxi-ream/read-frog using public GitHub signals. It is advisory only and does not block merges automatically.

Outcome

Score breakdown

Dimension Score Signals
Repo familiarity 0/35 commits in repo, merged PRs, reviews
Community standing 7/25 account age, followers, repo role
OSS influence 3/20 stars on owned non-fork repositories
PR track record 5/20 merge rate across resolved PRs in this repo

Signals used

  • Repo commits: 0 (author commits reachable from the repository default branch)
  • Repo PR history: merged 0, open 1, closed-unmerged 0
  • Repo reviews: 0
  • PR changed lines: 53 (+52 / -1)
  • Repo permission: read
  • Followers: 12
  • Account age: 45 months
  • Owned non-fork repos considered: max 1, total 1 (JiwaniZakir/aegis (1), JiwaniZakir/osbot-v4 (0), JiwaniZakir/claudeshell (0), JiwaniZakir/npx-card (0), JiwaniZakir/sbi-dataloader-gsoc (0), JiwaniZakir/gsoc-2026-strategy_part2 (0), JiwaniZakir/gsoc-2026-strategy (0), JiwaniZakir/lifemanagement-kit (0), JiwaniZakir/meridian (0), JiwaniZakir/spectra (0), JiwaniZakir/lattice (0), JiwaniZakir/Partnerships_OS (0), JiwaniZakir/sentinel (0), JiwaniZakir/imagegeneration (0), JiwaniZakir/erisdronesite (0), JiwaniZakir/JiwaniZakir (0))

Policy

  • Low-score review threshold: < 30
  • Auto-close: score < 20 and changed lines > 1000
  • Policy version: v1.1

Updated automatically when the PR changes or when a maintainer reruns the workflow.

@github-actions github-actions bot added contrib-trust:new PR author trust score is 0-29. needs-maintainer-review Contributor trust automation recommends maintainer review. labels Apr 6, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

@JiwaniZakir
Copy link
Copy Markdown
Author

The optimization makes sense — if the translation LLM already determines language as part of its prompt, the pre-flight shouldSkipByLanguage call is pure overhead. Worth double-checking that the skip behavior still triggers correctly when the translation response comes back and the detected language matches the skip list, since you're now relying on that single call to handle both detection and the skip decision. Also curious whether this changes behavior for edge cases where the translation provider's language detection disagrees with what the dedicated detection call would have returned.

Copy link
Copy Markdown
Owner

@mengxi-ream mengxi-ream left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix makes sense to me.

Please add changeset record and also correct the PR title

@mengxi-ream
Copy link
Copy Markdown
Owner

wait I found you are trying to solve #1110. This actually is not the best solution. will create another PR for that

@mengxi-ream mengxi-ream closed this Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contrib-trust:new PR author trust score is 0-29. needs-maintainer-review Contributor trust automation recommends maintainer review. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] improve the speed when skip language

2 participants