Conversation
![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: 34faa50219
ℹ️ 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".
| xQt, | ||
|
|
||
| VVo, | ||
| Nostr, |
There was a problem hiding this comment.
PlatformType is @Serializable and is embedded in protobuf-serialized models (for example UiTimelineV2.Post, persisted via AccountTypeConverter.fromUiTimelineV2). Moving an existing enum member changes its protobuf numeric value, so cached rows written before this commit can decode to the wrong platform after upgrade (or fail), which corrupts timeline rendering until cache is rebuilt. Keep existing enum entries in their previous order and only append new values.
Useful? React with 👍 / 👎.
| id = 0, | ||
| url = it.host, | ||
| title = "${it.instanceName ?: it.host} - $typeName", |
There was a problem hiding this comment.
In the Mastodon save path, edited items are always created with id = 0 and inserted, so editing an existing subscription (id != null) creates new rows instead of updating the original one. Since edit actions pass existing IDs from the RSS list, this leaves stale rows behind and duplicates subscriptions in the database/UI. The save path should preserve or replace the current record when editing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR enhances “guest mode” and RSS handling by generalizing RSS sources into broader “subscriptions” (including Mastodon instance timelines) and introducing host-aware guest account handling to avoid cross-instance ambiguity.
Changes:
- Add
SubscriptionTypeand a newSubscriptionTimelineTabItemto support Mastodon “trends/public/local” timelines alongside RSS. - Extend RSS/subscription UI flows (Android/Compose/Desktop/iOS) to discover, display, pin, and save Mastodon instance subscriptions.
- Update persistence: add a
typecolumn toDbRssSources(DB version 8) and addAccountType.GuestHost(host)to represent guest context per instance; remove the guest settings datastore/screen.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/commonTest/kotlin/dev/dimension/flare/data/datasource/microblog/MixedRemoteMediatorTest.kt | Stabilizes async test behavior by using test backgroundScope and advanceUntilIdle(). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/screen/settings/AllTabsPresenter.kt | Builds tabs for RSS vs non-RSS subscriptions (new tab item type). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/screen/rss/RssListWithTabsPresenter.kt | Updates pin/unpin keying + tab item creation for RSS vs subscription timelines. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/settings/GuestConfigPresenter.kt | Removes guest host configuration presenter. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/profile/ProfilePresenter.kt | Makes profile menu composition resilient when no active account key exists (guest). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/rss/RssTimelinePresenter.kt | Adds SubscriptionTimelinePresenter and updates “All RSS” to load mixed subscription loaders. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/rss/ExportOPMLPresenter.kt | Exports OPML for RSS-only sources (excludes Mastodon subscriptions). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/rss/EditRssSourcePresenter.kt | Adds Mastodon-instance save flow that creates multiple subscription rows per selected timeline type. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/presenter/home/rss/CheckRssSourcePresenter.kt | Adds Mastodon instance detection and available-timeline probing when RSS detection fails. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/UiRssSource.kt | Adds type and adjusts host derivation for Mastodon subscriptions. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Render.kt | Maps DbRssSources.type into UiRssSource. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Mastodon.kt | Switches guest rendering to AccountType.GuestHost(host) and threads host into note parsing/link rewriting. |
| shared/src/commonMain/kotlin/dev/dimension/flare/model/PlatformType.kt | Reorders enum values (name-based serialization should remain stable). |
| shared/src/commonMain/kotlin/dev/dimension/flare/model/AccountType.kt | Adds GuestHost(host) and an overload to map null account keys to host-specific guest type. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/repository/AccountRepository.kt | Treats GuestHost as “no active account” for UI account provider; adds guest service flow for GuestHost. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/model/TabSettings.kt | Adds SubscriptionTimelineTabItem and updates presenter wiring for subscription timelines. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datastore/model/GuestData.kt | Removes guest datastore model/serializer. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datastore/AppDataStore.kt | Removes guestDataStore from app datastore. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/rss/RssDataSource.kt | Adds loader creation for non-RSS subscription types (guest Mastodon timelines). |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/guest/mastodon/GuestUserTimelinePagingSource.kt | Converts to CacheableRemoteLoader with a stable pagingKey. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/guest/mastodon/GuestTrendsRemoteMediator.kt | New guest loader for Mastodon trending statuses with offset-based pagination. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/guest/mastodon/GuestPublicTimelineRemoteMediator.kt | New guest loader for Mastodon federated/local timelines with max_id pagination. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/guest/mastodon/GuestMastodonDataSource.kt | Uses AccountType.GuestHost(host) for guest handlers to avoid cross-host mixing. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/app/model/DbRssSources.kt | Adds SubscriptionType and persists type column on DbRssSources. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/app/dao/RssSourceDao.kt | Adds DAO query by (url, type) for subscriptions. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/app/AppDatabase.kt | Bumps DB version to 8, adds auto-migration 7→8, registers SubscriptionTypeConverter. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/database/adapter/SubscriptionTypeConverter.kt | New Room type converter for SubscriptionType. |
| shared/schemas/dev.dimension.flare.data.database.app.AppDatabase/8.json | Adds schema snapshot for DB v8 including DbRssSources.type. |
| iosApp/flare/UI/Screen/RssScreen.swift | Routes to RSS vs subscription timeline tabs; adds delete action; adds Mastodon timeline selection UI. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/rss/EditRssSourceScreen.kt | Adds Mastodon subscription selection + saving; updates pinned-tab matching for subscription types. |
| desktopApp/src/main/composeResources/values/strings.xml | Renames RSS strings to “Subscriptions” and adds Mastodon timeline labels + hint text. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/screen/rss/RssListWithTabs.kt | Displays subscription-type-specific labels and updates pin keying to include subscription type. |
| compose-ui/src/commonMain/composeResources/values/strings.xml | Renames RSS UI text to “Subscriptions” and adds Mastodon timeline labels. |
| app/src/main/res/values/strings.xml | Renames RSS UI text to “Subscriptions” and adds Mastodon timeline labels + hint text. |
| app/src/main/java/dev/dimension/flare/ui/screen/settings/SettingsSelectEntryBuilder.kt | Removes navigation entry for guest settings. |
| app/src/main/java/dev/dimension/flare/ui/screen/settings/SettingsScreen.kt | Removes guest settings item from settings UI. |
| app/src/main/java/dev/dimension/flare/ui/screen/settings/GuestSettingScreen.kt | Removes guest settings dialog screen. |
| app/src/main/java/dev/dimension/flare/ui/screen/rss/RssSourceEditSheet.kt | Adds Mastodon instance timeline selection UI and updates list styling to segmented items. |
| app/src/main/java/dev/dimension/flare/ui/route/Route.kt | Removes Route.Settings.GuestSetting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @ColumnInfo(defaultValue = "0") | ||
| val openInBrowser: Boolean = false, | ||
| val lastUpdate: Long, | ||
| @ColumnInfo(defaultValue = "RSS") |
There was a problem hiding this comment.
@ColumnInfo(defaultValue = "RSS") is emitted into the migration SQL verbatim; for a TEXT column this needs to be a quoted SQL literal (e.g. 'RSS'). As written, the auto-migration 7→8 can fail at runtime because DEFAULT RSS is not valid SQL in SQLite.
| @ColumnInfo(defaultValue = "RSS") | |
| @ColumnInfo(defaultValue = "'RSS'") |
| KoinComponent { | ||
| private val appDatabase by inject<AppDatabase>() | ||
|
|
There was a problem hiding this comment.
SubscriptionTimelinePresenter injects appDatabase but never uses it. The repo sets allWarningsAsErrors, so this unused property will fail the build; please remove the injection or use it (e.g., by fetching the stored DbRssSources instead of constructing a placeholder).
| val linkSources = | ||
| runCatching { | ||
| coroutineScope { | ||
| RssService | ||
| .detectLinkSources(actualUrl) |
There was a problem hiding this comment.
The new linkSources detection wraps the entire coroutineScope { ... awaitAll() } in runCatching { ... }.getOrNull(). This means a single failure while fetching one discovered source (e.g. RssService.fetch(it) throwing) will drop all otherwise-valid sources and incorrectly fall back to Mastodon detection, changing behavior from “show error” to “mis-detect”. Consider handling failures per-source (returning null and filtering) and only falling back when no sources successfully resolve.
fix #1420