Conversation
There was a problem hiding this comment.
Pull request overview
Implements a richer translation pipeline across the shared Kotlin code and the UI layers to address #1513, adding AI-backed translation with background “pre-translation” + caching, and UI affordances to show/trigger translation state.
Changes:
- Introduces a structured JSON translation format for
UiRichTextand applies translated text while preserving styles/images. - Adds a translation cache table/DAO + pre-translation background service, integrating it into timeline/status/profile loading and deep-link actions.
- Adds settings/UI updates (Android/Desktop/iOS + Compose) for pre-translation and display badges/menu actions.
Reviewed changes
Copilot reviewed 64 out of 66 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/jvmTest/kotlin/dev/dimension/flare/ui/model/DeeplinkEventTest.kt | Adds deeplink round-trip test for translation retry event. |
| shared/src/jvmMain/kotlin/dev/dimension/flare/common/Locale.jvm.kt | Makes locale language read dynamically via getter. |
| shared/src/commonTest/kotlin/dev/dimension/flare/ui/render/TranslationJsonUiRichTextTest.kt | Tests translation JSON export/apply behavior (styles/locked tokens). |
| shared/src/commonTest/kotlin/dev/dimension/flare/data/datasource/microblog/handler/UserHandlerTest.kt | Adds tests for profile pre-translation caching + retry behavior. |
| shared/src/commonTest/kotlin/dev/dimension/flare/data/datasource/microblog/handler/PostHandlerTest.kt | Adds tests for long-text detail pre-translation and wiring helpers. |
| shared/src/commonTest/kotlin/dev/dimension/flare/data/datasource/microblog/MixedRemoteMediatorTest.kt | Adds end-to-end tests for timeline pre-translation scheduling/skip cases and stale cleanup. |
| shared/src/commonTest/kotlin/dev/dimension/flare/data/database/cache/mapper/MicroblogTest.kt | Verifies mapper applies translations, menu actions, display states, and long-text behavior. |
| shared/src/commonTest/kotlin/dev/dimension/flare/data/database/cache/dao/TranslationDaoTest.kt | Adds DAO tests for translation persistence, updates, stale marking, and deletes. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/render/TranslationJson.kt | Introduces JSON translation document/tokenization + apply logic for rich text. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/status/TranslatePresenter.kt | Switches to AiCompletionService, adds translation caching, JSON-based translation application. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/status/StatusContextPresenter.kt | Enables long-text translation display in status context view. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/settings/AiConfigPresenter.kt | Normalizes AI config and updates OpenAI model fetching behavior. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/server/AiTLDRPresenter.kt | Switches TLDR to AiCompletionService and keeps legacy fallback. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/TimelinePresenter.kt | Integrates translation display options + pre-translation into timeline paging flow. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/DeepLinkPresenter.kt | Handles new translation deeplink actions (translate/retry/show original). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/XQT.kt | Captures source language into timeline posts for XQT. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/VVO.kt | Sets source languages for VVO statuses/users. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Rss.kt | Adds optional source language to feed items and persists it. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Misskey.kt | Propagates profile language into UiProfile.sourceLanguages. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Mastodon.kt | Propagates status language into UiTimelineV2.Post.sourceLanguages. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Bluesky.kt | Extracts langs into persistent sourceLanguages for Bluesky posts. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/UiTimelineV2.kt | Adds sourceLanguages + transient translationDisplayState to posts/feeds. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/UiProfile.kt | Adds sourceLanguages + transient translationDisplayState, merges languages on update. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/TranslationDisplayState.kt | Adds display-state enum for translation badge rendering. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/DeeplinkEvent.kt | Adds translation deeplink event payloads + enforces exactly one payload. |
| shared/src/commonMain/kotlin/dev/dimension/flare/di/CommonModule.kt | Registers AiCompletionService + PreTranslationService in DI. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/translation/PreTranslationService.kt | Implements AI-backed pre-translation queueing/batching, caching, skip logic, and retries. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/network/ai/OpenAIService.kt | Adds chatCompletionOrNull helper for config validation. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/network/ai/AiCompletionService.kt | Adds unified AI completion facade for translate/TLDR. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datastore/model/AppSettings.kt | Adds preTranslation flag + iOS default companion. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datastore/model/AiPromptDefaults.kt | Updates default translate prompt to describe JSON translation contract. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/rss/RssTimelineRemoteMediator.kt | Passes source-language into feed render. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/paging/TimelineRemoteMediator.kt | Enqueues statuses/references for pre-translation after saving to DB. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/paging/TimelinePagingMapper.kt | Applies translations when mapping cached DB items to UI models. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/handler/UserHandler.kt | Enqueues profile pre-translation and conditionally applies cached translations. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/handler/PostHandler.kt | Enqueues status pre-translation for detail view and applies cached translations in cache flow. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/ActionMenu.kt | Adds menu item types for translate/show original/retry translation. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/model/TranslationDisplay.kt | Implements translation display logic + menu injection via deeplinks + stable hashing. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/model/DbTranslation.kt | Adds translation entity + enums + converters + entity-key helpers. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/model/DbPagingTimeline.kt | Adds Room relation from DbStatus to DbTranslation list. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/dao/TranslationDao.kt | Adds translation DAO for CRUD + stale in-flight cleanup helpers. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/dao/StatusDao.kt | Adds sync getWithReferencesSync used by retry translation. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/cache/CacaheDatabase.kt | Bumps cache DB version and registers DbTranslation + converters + DAO. |
| shared/src/appleMain/kotlin/dev/dimension/flare/common/Locale.apple.kt | Makes locale language read dynamically via getter. |
| shared/src/androidMain/kotlin/dev/dimension/flare/common/Locale.android.kt | Makes locale language read dynamically via getter. |
| iosApp/flare/UI/Screen/AiConfigScreen.swift | Adds iOS pre-translation toggle; fixes picker binding type. |
| iosApp/flare/UI/FlareTheme.swift | Uses shared AiConfig.default for environment default. |
| iosApp/flare/UI/Component/TranslateStatusComponent.swift | Adds iOS translation badge component. |
| iosApp/flare/UI/Component/Status/StatusView.swift | Shows translation badge for statuses. |
| iosApp/flare/UI/Component/Status/StatusActionView.swift | Adds new action menu labels for translation actions. |
| iosApp/flare/UI/Component/Status/FeedView.swift | Shows translation badge for feed items. |
| iosApp/flare/UI/Component/CommonProfileHeader.swift | Shows translation badge for profiles. |
| iosApp/flare/Assets.xcassets/fa-language.symbolset/fa-language.svg | Adds icon asset for translation badge. |
| iosApp/flare/Assets.xcassets/fa-language.symbolset/Contents.json | Registers translation badge icon asset. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/settings/SettingsScreen.kt | Adds desktop pre-translation toggle in AI config. |
| desktopApp/src/main/composeResources/values/strings.xml | Adds desktop strings for pre-translation toggle. |
| desktopApp/src/main/composeResources/values-zh-rCN/strings.xml | Adds Chinese desktop strings for pre-translation toggle. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/status/StatusTranslatePresenter.kt | Adds cache target info so manual translate can persist in DB. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/status/FeedComponent.kt | Shows translation badge for feed items. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/status/CommonStatusComponent.kt | Adds translation badge + wires translation component with post context; adds menu strings mapping. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/ProfileHeader.kt | Shows translation badge for profiles. |
| compose-ui/src/commonMain/composeResources/values/strings.xml | Adds translation badge/menu strings. |
| compose-ui/src/commonMain/composeResources/values-zh-rCN/strings.xml | Adds Chinese translation badge/menu strings. |
| app/src/main/res/values/strings.xml | Adds Android strings for pre-translation toggle. |
| app/src/main/java/dev/dimension/flare/ui/screen/settings/AiConfigScreen.kt | Adds Android pre-translation toggle UI. |
Comments suppressed due to low confidence (1)
shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/settings/AiConfigPresenter.kt:104
- AiConfigPresenter no longer clears the selected OpenAI
modelwhenserverUrl/apiKeychanges. This can leave an invalid model configured for the new endpoint, causing translation/TLDR calls to fail until the user manually reselects a model. Consider restoring the previous behavior (reset model to empty when serverUrl/apiKey changes) or validating that the selected model is still present in the fetched model list and clearing it if not.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val projection = toProjectionBlocks() | ||
| val projectedBlocksByContent = projection.associateBy { it.content } | ||
| val blocksById = document.blocks.associateBy { it.id } | ||
| if (blocksById.size != projection.size) { | ||
| throw TranslationFormatException( | ||
| "Expected ${projection.size} blocks but found ${blocksById.size}", | ||
| ) | ||
| } | ||
|
|
||
| val translatedContents = | ||
| renderRuns.map { content -> | ||
| when (content) { | ||
| is RenderContent.BlockImage -> content | ||
| is RenderContent.Text -> { | ||
| val projectedBlock = | ||
| projectedBlocksByContent[content] | ||
| ?: return@map content | ||
| val translatedBlock = |
There was a problem hiding this comment.
applyTranslationDocument builds projectedBlocksByContent via projection.associateBy { it.content }, but RenderContent.Text is a data class (value-equality). If the same block content appears multiple times (e.g., two identical paragraphs), the map will collapse entries and the wrong translated block may be applied (or one block never applied). Consider keying projection blocks by their position/index in renderRuns (or carrying a unique block id) instead of using RenderContent.Text as the map key.
| "The input is JSON extracted from a social post.\n" + | ||
| "For each item, either return status=\"Completed\" with a translated payload, " + | ||
| "or status=\"Skipped\" without a payload when the source language already " + | ||
| "matches {target_language}.\n" + | ||
| "Preserve entityKey, block ids, token ids, and token kinds exactly as-is.\n" + | ||
| "Only translate token text where kind is \"Translatable\".\n" + | ||
| "Keep token text where kind is \"Locked\" unchanged.\n" + | ||
| "Return ONLY JSON without markdown code fences or explanations.\n" + |
There was a problem hiding this comment.
The default translate prompt describes returning status="Completed" / status="Skipped", but the actual batch format is JSON with a status field (enum values like "Completed"/"Skipped"). This mismatch can lead the model to emit an incompatible shape. Update the prompt wording to explicitly require a JSON field (e.g., "status": "Completed") matching PreTranslationBatchItem.status.
| case .retryTranslation: return LocalizedStringResource(stringLiteral: "Retry translation") | ||
| case .translate: return LocalizedStringResource(stringLiteral: "Translate") | ||
| case .showOriginal: return LocalizedStringResource(stringLiteral: "Show original") |
There was a problem hiding this comment.
These new action labels are created with LocalizedStringResource(stringLiteral: ...), which bypasses localization and will always display English. The rest of the switch returns localization keys (e.g., "like", "more"). Please add proper localized keys for these actions and return those keys instead of string literals.
| case .retryTranslation: return LocalizedStringResource(stringLiteral: "Retry translation") | |
| case .translate: return LocalizedStringResource(stringLiteral: "Translate") | |
| case .showOriginal: return LocalizedStringResource(stringLiteral: "Show original") | |
| case .retryTranslation: return "retry_translation" | |
| case .translate: return "translate" | |
| case .showOriginal: return "show_original" |
| private fun shouldTranslate( | ||
| existing: DbTranslation?, | ||
| sourceHash: String, | ||
| now: Long, | ||
| ): Boolean { |
There was a problem hiding this comment.
shouldTranslate takes a now parameter but never uses it. This adds noise and can trigger unused-parameter warnings; consider removing the parameter or using it (e.g., for cooldown/backoff decisions).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7220f6609c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val translationEvent: TranslationEvent? = null, | ||
| val postEvent: PostEvent? = null, |
There was a problem hiding this comment.
Because DeeplinkEvent is serialized with ProtoBuf and no explicit @ProtoNumbers are defined, field numbers are derived from declaration order. Inserting translationEvent before postEvent changes the wire tag for postEvent, so previously generated flare-event://... payloads (from older app versions) can no longer decode correctly and will be dropped by parse() after the XOR invariant check. Please preserve existing tag numbers (e.g., via @ProtoNumber) when evolving this schema.
Useful? React with 👍 / 👎.
|
|
||
| internal fun UiRichText.applyTranslationDocument(document: TranslationDocument): UiRichText { | ||
| val projection = toProjectionBlocks() | ||
| val projectedBlocksByContent = projection.associateBy { it.content } |
There was a problem hiding this comment.
Using projection.associateBy { it.content } collapses duplicate RenderContent.Text blocks into a single entry. When a post contains two identical text blocks, both lookups resolve to the same projected block id, so one translated block is effectively reused and the other is ignored, producing incorrect block-to-translation alignment. This should keep a one-to-one mapping (e.g., by index/id) rather than deduplicating by value equality.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
fix #1513