feat(details): auto-translate README + release notes on open#635
Open
rainxchzed wants to merge 4 commits into
Open
feat(details): auto-translate README + release notes on open#635rainxchzed wants to merge 4 commits into
rainxchzed wants to merge 4 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds persistent auto-translate preferences with UI controls and ViewModel wiring, and triggers conditional automatic translation of Details README and release notes during initial load. ChangesAuto-translate Settings and Details Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 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
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 2898-2903: The explicit auto-translate target can be an empty
string and currently wins the fallback; change how `explicit` (and optionally
`app`) are derived so blank strings are treated as unset before computing
`target` — for example, in `DetailsViewModel` when calling
`tweaksRepository.getAutoTranslateTargetLang().first()` wrap the result with
`.takeIf { it.isNotBlank() }` (or use `ifBlank { null }`) so `explicit ?: app ?:
translationRepository.getDeviceLanguageCode()` only sees non-blank values,
ensuring app/device fallbacks occur as intended.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Translation.kt`:
- Around line 72-75: The AutoTranslate UI only exposes the enable switch; update
the AutoTranslateCard invocation to also pass the current target language and
selection callback so users can pick a repository auto-translate target;
specifically, add props for the selected language
(state.autoTranslateTargetLang) and a selection handler that calls
onAction(TweaksAction.OnAutoTranslateTargetSelected(it)), matching the
action/field names, and ensure the AutoTranslateCard component accepts and
forwards these props so the target-language path becomes reachable from this
screen.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0d66b41-410a-4f20-aa08-b1d699399073
📒 Files selected for processing (8)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.ktcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Translation.kt
Greptile SummaryThis PR wires up automatic translation of READMEs and release notes in the Details screen: two new
Confidence Score: 4/5The auto-translate toggle and DetailsViewModel logic are safe to merge; the explicit target picker silently omits 20 of 33 translation-supported languages, leaving those targets unreachable through the UI. The repository layer and DetailsViewModel are correct. The language picker in AutoTranslateCard reuses LanguageDropdown, which is backed by AppLanguages.ALL (13 entries) instead of SupportedLanguages.all (33 entries). Users wanting to explicitly target German, Dutch, Portuguese, Vietnamese, or any of the other 14 languages not in AppLanguages cannot do so — the dropdown simply won't list them. That functional gap is a real, present defect in the translation-target selection path. feature/tweaks/presentation/.../sections/Translation.kt — the LanguageDropdown reuse restricts the auto-translate target picker to app-UI languages only. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant DVM as DetailsViewModel
participant TR as TweaksRepository
participant TLR as TranslationRepository
participant S as DetailsState
U->>DVM: Open repository details
DVM->>S: _state.update(readmeMarkdown, readmeLanguage, selectedRelease, ...)
DVM->>DVM: maybeAutoTranslate(readmeBody, releaseDescription)
DVM->>TR: getAutoTranslateEnabled().first()
alt "enabled = false"
DVM-->>DVM: return (no-op)
else "enabled = true"
DVM->>TR: getAutoTranslateTargetLang().first()
DVM->>TR: getAppLanguage().first()
DVM->>TLR: getDeviceLanguageCode()
note over DVM: target = explicit ?: app ?: deviceLocale
alt "readmeBody not blank AND translatedText == null AND sourceLang != target"
DVM->>TLR: translate(readmeBody, target)
TLR-->>DVM: TranslationResult
DVM->>S: "copy(aboutTranslation = TranslationState(isShowingTranslation=true, ...))"
end
alt "releaseDescription not blank AND translatedText == null AND readmeLanguage != target"
DVM->>TLR: translate(releaseDescription, target)
TLR-->>DVM: TranslationResult
DVM->>S: "copy(whatsNewTranslation = TranslationState(isShowingTranslation=true, ...))"
end
end
Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…s guard, target reset UI
# Conflicts: # core/presentation/src/commonMain/composeResources/values/strings.xml
Comment on lines
+136
to
+141
| selectedTag = targetLanguageTag, | ||
| onLanguageSelected = onTargetSelected, | ||
| ) | ||
| if (targetLanguageTag == null && !appLanguageTag.isNullOrBlank()) { | ||
| Spacer(Modifier.height(6.dp)) | ||
| Text( |
There was a problem hiding this comment.
LanguageDropdown is built over AppLanguages.ALL, which contains only the 13 languages the app ships UI translations for. The translation service, however, supports 33 languages (SupportedLanguages.all). As a result, a user who wants to auto-translate READMEs into German (de), Dutch (nl), Portuguese (pt), Ukrainian (uk), Vietnamese (vi), or any of the other 14 missing languages cannot select those targets through this picker — the dropdown simply won't offer them. The device-locale fallback path can still reach these languages silently, but explicit selection is broken for roughly 60% of the supported translation targets.
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
TweaksRepositoryprefs (auto_translate_enabled,auto_translate_target_lang) with KSafe migration entries.Auto-translate repositoriestoggle in Tweaks → Translation section.DetailsViewModel.maybeAutoTranslatekicks offTranslateAbout+TranslateWhatsNewonce when readme and release description first load and the user's app language differs from the source.Closes #423.
Test plan
Summary by CodeRabbit
New Features
Chores