Skip to content

fix(translate): avoid redundant skip-language LLM detection#1354

Open
frogGuaGuaGuaGua wants to merge 1 commit into
mainfrom
hermes/issue-1353-20260420-114320
Open

fix(translate): avoid redundant skip-language LLM detection#1354
frogGuaGuaGuaGua wants to merge 1 commit into
mainfrom
hermes/issue-1353-20260420-114320

Conversation

@frogGuaGuaGuaGua

Copy link
Copy Markdown
Collaborator

Closes #1353
Related: #1110

Summary

  • disable skip-language LLM detection when the active page translation provider is itself an LLM provider
  • keep skip-language LLM detection enabled for non-LLM translation providers
  • add focused regression coverage for both provider paths and include a patch changeset

Why

Current main still calls shouldSkipByLanguage(preparedText, skipLanguages, config.languageDetection.mode === "llm") inside src/utils/host/translate/translate-variants.ts, so page translation can make an extra LLM language-detection call before the real LLM translation call.

For the behavior reported in #1353, the skip-language check should stay franc-only when page translation itself is using an LLM provider. Non-LLM translation providers should continue using the configured LLM language detection path.

Implementation

  • gate shouldSkipByLanguage(...) with config.languageDetection.mode === "llm" && !isLLMProviderConfig(providerConfig) in src/utils/host/translate/translate-variants.ts
  • add src/utils/host/translate/__tests__/translate-variants.test.ts to assert:
    • openai-default -> shouldSkipByLanguage(..., false)
    • microsoft-translate-default -> shouldSkipByLanguage(..., true)
  • add .changeset/fresh-guests-sit.md

Validation

  • SKIP_FREE_API=true pnpm exec vitest run --configLoader runner src/utils/host/translate/__tests__/skip-language.test.ts src/utils/host/translate/__tests__/translate-variants.test.ts
  • pnpm exec eslint src/utils/host/translate/translate-variants.ts src/utils/host/translate/__tests__/translate-variants.test.ts
  • pnpm run type-check
  • pre-push hook: nx run @read-frog/extension:lint
  • pre-push hook: nx run @read-frog/extension:type-check
  • pre-push hook: nx run @read-frog/extension:test

@changeset-bot

changeset-bot Bot commented Apr 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6e47c6d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@read-frog/extension Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions github-actions Bot added fix contrib-trust:moderate PR author trust score is 30-59. labels Apr 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Contributor trust score

42/100 — Moderate

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 21/35 commits in repo, merged PRs, reviews
Community standing 11/25 account age, followers, repo role
OSS influence 0/20 stars on owned non-fork repositories
PR track record 10/20 merge rate across resolved PRs in this repo

Signals used

  • Repo commits: 12 (author commits reachable from the repository default branch)
  • Repo PR history: merged 14, open 6, closed-unmerged 15
  • Repo reviews: 0
  • PR changed lines: 105 (+104 / -1)
  • Repo permission: write
  • Followers: 0
  • Account age: 1 month
  • Owned non-fork repos considered: max 0, total 0 (none)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contrib-trust:moderate PR author trust score is 30-59. fix on-hold

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] don't allow llm for skip based on language

2 participants