feat(home): rebuild personalization on exact-token learning and candidates/v1 - #941
Closed
ashwkun wants to merge 1 commit into
Closed
feat(home): rebuild personalization on exact-token learning and candidates/v1#941ashwkun wants to merge 1 commit into
ashwkun wants to merge 1 commit into
Conversation
…dates/v1 Replace split Taste/BYL heuristics with one catalog coordinator, cold-start mode machine, learner anchors, greeting missions, and recommendation feedback so Home learning attributes the right card and stays measurable.
ashwkun
added
user-impact-high
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a Home candidates-v1 personalization pipeline with cached retrieval, mode and mission selection, cross-rail allocation, feedback attribution, ranking persistence, and aggregate quality diagnostics. It also wires the pipeline into Home UI and updates related network, analytics, documentation, tooling, and tests. ChangesHome candidate pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant HomeViewModel
participant HomePersonalizationCoordinator
participant PodcastRepository
participant AdaptiveRankingRepository
participant AnalyticsHelper
HomeViewModel->>AdaptiveRankingRepository: resolve anchor and exclusions
HomeViewModel->>HomePersonalizationCoordinator: loadSlate
HomePersonalizationCoordinator->>PodcastRepository: getHomeCandidatesV1
PodcastRepository-->>HomePersonalizationCoordinator: cached or fetched candidates
HomePersonalizationCoordinator-->>HomeViewModel: SlateResult and quality telemetry
HomeViewModel->>AdaptiveRankingRepository: record rail exposures
HomeViewModel->>AnalyticsHelper: trackHomeSlateQualitySnapshot
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt (1)
32-155: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a focused
HomeViewModel.loadData()test
The existing home tests cover DI wiring and pure helpers, but not the reworkedloadData()flow infeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt. Add regression coverage for the fast bootstrap job andpersonalizationSliceupdates.🤖 Prompt for 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. In `@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt` around lines 32 - 155, Add focused tests for HomeViewModel.loadData(), covering the fast bootstrap job’s successful state updates and the personalizationSlice emissions when personalization inputs change. Reuse existing HomeViewModel test fixtures and repository fakes, and verify the resulting observable UI/state values without expanding coverage beyond these two loadData flows.Source: Path instructions
core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackTelemetrySession.kt (1)
430-442: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMissing
core/playback/README.mdupdate for this behavioral change.This PR adds exact-exposure-token threading (
EXPOSURE_ID_KEY,exposureIdinFeedbackTarget) toPlaybackTelemetrySession, a behavioral/contract change to:core:playback. Nocore/playback/README.mdchange accompanies it anywhere in this PR stack.As per path instructions, "If any production Kotlin under core//src/main/ changed, require core//README.md updated in the same PR (unconditional)."
Also applies to: 633-637
🤖 Prompt for 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. In `@core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackTelemetrySession.kt` around lines 430 - 442, Update core/playback/README.md to document the exact exposure-token threading introduced by PlaybackTelemetrySession, including EXPOSURE_ID_KEY and exposureId in FeedbackTarget and its role in resolving feedback against the exact impression. Keep the documentation aligned with the existing playback contract.Source: Path instructions
🤖 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 @.github/workflows/sync-pi-data.yml:
- Line 69: Update the cache step’s actions/cache reference to use the action’s
full immutable commit SHA instead of the mutable v6 tag, while preserving the
existing cache configuration and workflow behavior.
In `@app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt`:
- Around line 174-198: Add hermetic JVM tests under src/test for
reportHomeLearningAttributionHealth, covering successful diagnostics emission
and non-cancellation exception handling while preserving cancellation
propagation and startup stability. Introduce an injectable reporter seam or
equivalent testable abstraction around
AnalyticsHelper.trackHomeLearningAttributionHealth and verify both outcomes
without relying on Android runtime behavior.
In
`@core/analytics/src/test/java/cx/aswin/boxlore/core/analytics/AnalyticsTracksFacadeTest.kt`:
- Around line 462-493: Extend homeSlateQualitySnapshotEvent with a nullable
algorithmVersion case, invoking AnalyticsHelper.trackHomeSlateQualitySnapshot
with algorithmVersion set to null and asserting the emitted algorithm_version
property equals "unknown".
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilder.kt`:
- Around line 103-125: Update HomeCandidatesRequestBuilder.cacheKey to include
request.daypart and request.noveltyPreference, and encode the complete
request.excludedPodcastIds collection instead of truncating it with take(40).
Preserve the existing deterministic ordering for exclusions and all other key
components.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt`:
- Around line 293-333: Update allocateModules so each allocated candidate is
resolved against the episode map for its own module instead of the merged byId
map. Build separate ID maps for taste, becauseYouLike, and mission, then use the
corresponding map when producing each AllocatedEpisodes list; preserve
allocatedCandidates unchanged.
- Around line 70-127: Add coordinator-level JVM coverage in
HomePersonalizationCoordinatorTest for loadSlate and mapResponse using a
hermetic podcastRepository stub. Verify module selection, response mapping, and
cross-module podcast de-duplication, while keeping existing helper tests
unchanged.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeSlateQualityLogic.kt`:
- Around line 45-54: Update the duplicateRatePercent calculation in
HomeSlateQualityLogic to use an explicit duplicate-removal count from the
allocation flow, rather than allocatablePool minus allocated.size, so limit and
eligibility exclusions are not counted as duplicates. Propagate or compute that
count from raw candidate identities before those exclusions, and add a
regression test covering 30 unique candidates producing a 12-item slate with no
duplicates reported.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryHomeCandidates.kt`:
- Around line 14-38: Prevent unbounded growth of homeCandidatesCache by removing
expired entries during the cache write path in getHomeCandidatesV1. Before or
while inserting a new response, sweep entries whose cached timestamp is older
than HomeCandidatesRequestBuilder.CACHE_TTL_MILLIS, while preserving valid
entries and existing cache-read behavior.
In
`@core/designsystem/src/main/java/cx/aswin/boxlore/core/designsystem/theme/ExpressiveMotion.kt`:
- Around line 118-199: Update the core/designsystem module README to document
the stable expressiveClickable modifier overloads, including click behavior,
optional shape/indication configuration, and the long-click overload’s
onLongClickLabel accessibility announcement. Follow the existing module README
template and describe the new expressive press and long-press behavior without
changing the Kotlin implementation.
In
`@core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackTelemetrySession.kt`:
- Around line 633-637: Remove the duplicate EXPOSURE_ID_KEY declaration from the
PlaybackTelemetrySession companion object and update its usages to reference the
existing shared definition across the module boundary, preserving the same
"exposure_id" key behavior.
In `@core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/BoxcastPrefs.kt`:
- Around line 133-151: The preference change introduces
clearRecommendationCaches, but the corresponding core/prefs/README.md
documentation is missing. Update the README to document this method, the
recommendation cache keys it clears, and that it preserves history,
subscriptions, queue, and likes.
In `@core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/UserPreferenceKeys.kt`:
- Around line 40-46: Update core/prefs/README.md to document the newly added
UserPreferenceKeys entries STICKY_MISSION_ID, STICKY_MISSION_SLOT_KEY, and
FIRST_LAUNCH_PERSONALIZATION_RESET_DONE, including their preference keys and
intended usage consistent with the existing README conventions.
In
`@core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/UserPreferencesRepository.kt`:
- Around line 1017-1043: Update core/prefs/README.md to document the new sticky
mission preference streams and setStickyMission function, including their
persisted mission ID and slot-key behavior. Keep the documentation aligned with
the public symbols stickyMissionIdStream, stickyMissionSlotKeyStream, and
setStickyMission.
- Around line 1045-1053: Update
UserPreferencesRepository.hasCompletedFirstLaunchPersonalizationReset() to read
dataStore.data through the same IOException-safe catch-and-emptyPreferences flow
used by other getters in the file before taking first() and accessing
Keys.FIRST_LAUNCH_PERSONALIZATION_RESET_DONE. Preserve propagation of
non-IOException failures while preventing recoverable DataStore I/O errors from
escaping the initial reset check.
In
`@core/ranking/src/main/java/cx/aswin/boxlore/core/ranking/database/AdaptiveRankingDao.kt`:
- Around line 166-167: Wire AdaptiveRankingDao.pruneOutcomesBefore() into the
same ranking retention flow that invokes pruneExposures, using the retention
cutoff so old ranking_outcomes are deleted alongside exposures; otherwise remove
pruneOutcomesBefore() if indefinite outcome retention is intended.
In
`@core/testing/src/test/java/cx/aswin/boxlore/core/testing/architecture/GlossaryCoverageGuardTest.kt`:
- Around line 97-102: Update the phase extraction validation in the test’s
glossary parsing helper to require phaseD.isNotEmpty() alongside phaseAUnionB
and phaseC. Include PHASE_D in the existing parse-failure message so missing or
renamed markers produce a specific error before returning the combined set.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/components/PodcastCard.kt`:
- Around line 52-57: Update the onLongClickLabel in the PodcastCard call to use
the same plus-to-space title transformation as the displayed title, so TalkBack
announces the formatted podcast title consistently.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt`:
- Around line 61-77: Update the exception handling in the coroutine launched by
the home data-loading flow to catch CancellationException before the general
Exception handler and immediately rethrow it. Keep logging non-cancellation
bootstrap failures through the existing BoxCastTiming Log.e call, while
preserving the existing state updates and cleanup behavior for handled
exceptions.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelSlate.kt`:
- Around line 359-376: Replace the locally duplicated EXPOSURE_ID_KEY string in
HomeViewModel.buildExposureSourceContext with the shared playback telemetry
exposure-key symbol or centralized contract, updating the Bundle write
accordingly while preserving the existing exposure token value and entry-point
behavior.
- Around line 220-262: Clear episodeExposureIds at the start of
recordSlateExposures before launching the exposure-recording work, then
repopulate it from the current taste, becauseYouLike, and mission lists.
Preserve the existing recordRailExposures behavior and ensure stale mappings
from prior slate reloads are removed.
In `@scripts/sync/03-sync-episodes.js`:
- Around line 117-120: Update oldestDeferredAgeHours and the related backlog
messages at the identified reporting points to preserve an unknown age when
deferredWithHistory is empty, using a nullable or n/a representation instead of
0. Ensure messages in the deferred backlog reporting flow phrase the age as
unavailable when no deferred show has a prior-check timestamp, while retaining
numeric hour output when history exists.
---
Outside diff comments:
In
`@core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackTelemetrySession.kt`:
- Around line 430-442: Update core/playback/README.md to document the exact
exposure-token threading introduced by PlaybackTelemetrySession, including
EXPOSURE_ID_KEY and exposureId in FeedbackTarget and its role in resolving
feedback against the exact impression. Keep the documentation aligned with the
existing playback contract.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt`:
- Around line 32-155: Add focused tests for HomeViewModel.loadData(), covering
the fast bootstrap job’s successful state updates and the personalizationSlice
emissions when personalization inputs change. Reuse existing HomeViewModel test
fixtures and repository fakes, and verify the resulting observable UI/state
values without expanding coverage beyond these two loadData flows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 050f793b-8719-460f-86c8-0078742d6972
⛔ Files ignored due to path filters (2)
docs/analytics/event_glossary.csvis excluded by!**/*.csvdocs/analytics/glossary_emission_coverage.csvis excluded by!**/*.csv
📒 Files selected for processing (102)
.github/workflows/sync-pi-data.ymlapp/README.mdapp/src/main/java/cx/aswin/boxlore/AppContainer.ktapp/src/main/java/cx/aswin/boxlore/BoxLoreApplication.ktapp/src/main/java/cx/aswin/boxlore/navigation/NavGraphTabDestinations.ktconfig/detekt/baseline.xmlconfig/ktlint/app-baseline.xmlconfig/ktlint/core-catalog-baseline.xmlconfig/ktlint/feature-home-baseline.xmlcore/analytics/README.mdcore/analytics/src/main/java/cx/aswin/boxlore/core/analytics/Analytics.ktcore/analytics/src/main/java/cx/aswin/boxlore/core/analytics/AnalyticsGlossary.ktcore/analytics/src/main/java/cx/aswin/boxlore/core/analytics/AnalyticsHelper.ktcore/analytics/src/main/java/cx/aswin/boxlore/core/analytics/HomeQualityAnalyticsTracks.ktcore/analytics/src/main/java/cx/aswin/boxlore/core/analytics/RecordingAnalytics.ktcore/analytics/src/test/java/cx/aswin/boxlore/core/analytics/AnalyticsTracksFacadeTest.ktcore/analytics/src/test/java/cx/aswin/boxlore/core/analytics/GlossaryAllEventsEmissionTest.ktcore/catalog/README.mdcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryHomeCandidates.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryRecommendations.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeAnchorSelectionLogic.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesMapper.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilder.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeDiscoveryMission.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeMeaningfulPlayLogic.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationMode.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeSlateAllocationLogic.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeSlateQualityLogic.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/RecommendationLanguages.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryRecommendationsLanguageTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilderTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationLogicTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeSlateQualityLogicTest.ktcore/designsystem/src/main/java/cx/aswin/boxlore/core/designsystem/theme/ExpressiveMotion.ktcore/model/README.mdcore/model/src/main/java/cx/aswin/boxlore/core/model/HomeSlateQualityTelemetry.ktcore/model/src/main/java/cx/aswin/boxlore/core/model/RankingExposureHealthTelemetry.ktcore/network/README.mdcore/network/src/main/java/cx/aswin/boxlore/core/network/BoxLoreApi.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/NetworkModule.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/AutoTranscriptResponse.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/CuratedCuriosityModels.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/HomeCandidatesModels.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/ITunesResponse.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/OnboardingModels.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/PodcastIndexModels.ktcore/network/src/main/java/cx/aswin/boxlore/core/network/model/SyncModels.ktcore/network/src/test/java/cx/aswin/boxlore/core/network/BoxLoreApiContractTest.ktcore/network/src/test/java/cx/aswin/boxlore/core/network/model/HomeCandidatesV1SerializationTest.ktcore/network/src/test/resources/fixtures/home_candidates_v1.jsoncore/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackTelemetrySession.ktcore/prefs/src/main/java/cx/aswin/boxlore/core/prefs/BoxcastPrefs.ktcore/prefs/src/main/java/cx/aswin/boxlore/core/prefs/UserPreferenceKeys.ktcore/prefs/src/main/java/cx/aswin/boxlore/core/prefs/UserPreferencesRepository.ktcore/ranking/README.mdcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/AdaptiveCandidateScorer.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/AdaptiveLinearModel.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/AdaptiveRankingRepository.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/LearningEventLog.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingExposureHealthLogic.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingFeedbackRepository.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingModels.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingReward.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingRuntimeControls.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/RankingSerialization.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/database/AdaptiveRankingDao.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/database/AdaptiveRankingDatabase.ktcore/ranking/src/main/java/cx/aswin/boxlore/core/ranking/database/AdaptiveRankingEntities.ktcore/ranking/src/test/java/cx/aswin/boxlore/core/ranking/AdaptiveRankingRepositoryTest.ktcore/ranking/src/test/java/cx/aswin/boxlore/core/ranking/AdaptiveRankingTest.ktcore/ranking/src/test/java/cx/aswin/boxlore/core/ranking/RankingExposureHealthLogicTest.ktcore/ranking/src/test/java/cx/aswin/boxlore/core/ranking/RankingFeedbackRepositoryTest.ktcore/testing/src/test/java/cx/aswin/boxlore/core/testing/architecture/GlossaryCoverageGuardTest.ktdocs/ANALYTICS_EVENT_GLOSSARY.mddocs/recommendation-system.mdfeature/home/README.mdfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeDataModels.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeed.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedRecommendations.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeScreen.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeUiModels.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModel.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelAdaptive.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelAssembler.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelBecauseYouLike.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelSlate.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/BecauseYouLikeSection.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/CuratedEpisodeCard.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/ForYouSection.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/PodcastCard.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/RecommendationFeedbackMenu.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/logic/HomeAnchorConfidenceLogic.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/logic/HomeFeedbackLogic.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/logic/HomeMissionContextLogic.ktfeature/home/src/test/java/cx/aswin/boxlore/feature/home/HomeViewModelConnectivityDepsTest.ktfeature/home/src/test/java/cx/aswin/boxlore/feature/home/logic/HomeAnchorConfidenceLogicTest.ktfeature/home/src/test/java/cx/aswin/boxlore/feature/home/logic/HomeFeedbackLogicTest.ktfeature/home/src/test/java/cx/aswin/boxlore/feature/home/logic/HomeMissionContextLogicTest.ktscripts/sync/03-sync-episodes.jsscripts/sync/lib/config.js
💤 Files with no reviewable changes (1)
- feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelBecauseYouLike.kt
| # English-only) so a model change actually busts this cache instead of | ||
| # silently reusing stale weights under a mismatched name. | ||
| - name: Cache embedding model | ||
| uses: actions/cache@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the cache action to a commit SHA.
Line 69 uses a mutable tag while this job has credentials that can push to master. Pin actions/cache to its full commit SHA.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 69-69: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for 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.
In @.github/workflows/sync-pi-data.yml at line 69, Update the cache step’s
actions/cache reference to use the action’s full immutable commit SHA instead of
the mutable v6 tag, while preserving the existing cache configuration and
workflow behavior.
Source: Linters/SAST tools
Comment on lines
+174
to
+198
| /** | ||
| * Observational-only exact-exposure-token attribution health for the HOME surface | ||
| * (quality-observability). Fired once per app start, mirroring | ||
| * [reportAdaptiveRankingStatus] — this is a low-volume diagnostic aggregate, not a | ||
| * per-session or per-slate signal, so a periodic/repeating scheduler would be overkill. | ||
| */ | ||
| @Suppress("TooGenericExceptionCaught") | ||
| private fun reportHomeLearningAttributionHealth() { | ||
| applicationScope.launch { | ||
| try { | ||
| val diagnostics = container.adaptiveRankingRepository.exposureResolutionDiagnostics() | ||
| cx.aswin.boxlore.core.analytics.AnalyticsHelper | ||
| .trackHomeLearningAttributionHealth(diagnostics) | ||
| } catch (error: kotlinx.coroutines.CancellationException) { | ||
| throw error | ||
| } catch (error: Exception) { | ||
| // Attribution health is optional and must never destabilize app startup. | ||
| android.util.Log.w( | ||
| "BoxLoreApplication", | ||
| "Failed to report Home learning attribution health", | ||
| error, | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add hermetic JVM coverage for the new startup diagnostic.
Cover successful emission and non-cancellation failure handling without destabilizing startup. Extract an injectable reporter seam if needed.
As per coding guidelines, “When a PR adds or substantially changes production logic… it must add or extend hermetic JVM unit tests.” As per path instructions, “For each substantially changed main source, check the PR also adds or extends a related *Test.kt under src/test for that behavior.”
🤖 Prompt for 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.
In `@app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt` around lines 174 -
198, Add hermetic JVM tests under src/test for
reportHomeLearningAttributionHealth, covering successful diagnostics emission
and non-cancellation exception handling while preserving cancellation
propagation and startup stability. Introduce an injectable reporter seam or
equivalent testable abstraction around
AnalyticsHelper.trackHomeLearningAttributionHealth and verify both outcomes
without relying on Android runtime behavior.
Sources: Coding guidelines, Path instructions
Comment on lines
+462
to
+493
| @Test | ||
| fun homeSlateQualitySnapshotEvent() { | ||
| AnalyticsHelper.trackHomeSlateQualitySnapshot( | ||
| HomeSlateQualityTelemetry( | ||
| mode = "PERSONALIZED", | ||
| algorithmVersion = "v3", | ||
| fromCache = true, | ||
| isFallback = false, | ||
| requestedModuleCount = 4, | ||
| nonEmptyModuleCount = 3, | ||
| allocatedCandidateCount = 12, | ||
| duplicateRatePercent = 10, | ||
| noveltyRatePercent = 80, | ||
| cacheAgeBucket = "under_15m", | ||
| responseLatencyBucket = "300_999ms", | ||
| ), | ||
| ) | ||
|
|
||
| assertTrue("home_slate_quality_snapshot" in names()) | ||
| val props = firstProps("home_slate_quality_snapshot") | ||
| assertEquals("PERSONALIZED", props["mode"]) | ||
| assertEquals("v3", props["algorithm_version"]) | ||
| assertEquals(true, props["from_cache"]) | ||
| assertEquals(false, props["is_fallback"]) | ||
| assertEquals(4, props["requested_module_count"]) | ||
| assertEquals(3, props["non_empty_module_count"]) | ||
| assertEquals(12, props["allocated_candidate_count"]) | ||
| assertEquals(10, props["duplicate_rate_percent"]) | ||
| assertEquals(80, props["novelty_rate_percent"]) | ||
| assertEquals("under_15m", props["cache_age_bucket"]) | ||
| assertEquals("300_999ms", props["response_latency_bucket"]) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the nullable algorithm-version fallback.
This only tests a populated version; add a null case asserting algorithm_version == "unknown".
As per coding guidelines, “must add or extend hermetic JVM unit tests … covering new happy, empty, and error behavior as applicable.”
🤖 Prompt for 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.
In
`@core/analytics/src/test/java/cx/aswin/boxlore/core/analytics/AnalyticsTracksFacadeTest.kt`
around lines 462 - 493, Extend homeSlateQualitySnapshotEvent with a nullable
algorithmVersion case, invoking AnalyticsHelper.trackHomeSlateQualitySnapshot
with algorithmVersion set to null and asserting the emitted algorithm_version
property equals "unknown".
Source: Coding guidelines
Comment on lines
+103
to
+125
| fun cacheKey(request: HomeCandidatesV1Request): String = | ||
| buildString { | ||
| append(request.requestedModules.sorted().joinToString(",")) | ||
| append('|') | ||
| append(request.country) | ||
| append('|') | ||
| append(request.languages.sorted().joinToString(",")) | ||
| append('|') | ||
| append(request.anchorPodcastId.orEmpty()) | ||
| append('|') | ||
| append(request.missionId.orEmpty()) | ||
| append('|') | ||
| append(request.revision.orEmpty()) | ||
| append('|') | ||
| append(request.seeds.joinToString(",") { "${it.episodeId}:${it.podcastId}:${it.weight}" }) | ||
| append('|') | ||
| append( | ||
| request.excludedPodcastIds | ||
| .sorted() | ||
| .take(40) | ||
| .joinToString(","), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cache key omits fields that affect the request/response, risking stale/mismatched personalized results.
cacheKey() ignores daypart and noveltyPreference entirely, and truncates excludedPodcastIds to the first 40 sorted ids even though the request itself can carry up to 250 (Line 50). Two logically different requests (e.g., different daypart, or exclusions differing beyond the 40-id window) can collapse to the same key, causing a stale cached response — including one that doesn't reflect newly hard-excluded shows — to be served for up to the 4h TTL.
🛠️ Proposed fix: include full exclusion set and remaining fields in the key
fun cacheKey(request: HomeCandidatesV1Request): String =
buildString {
append(request.requestedModules.sorted().joinToString(","))
append('|')
append(request.country)
append('|')
append(request.languages.sorted().joinToString(","))
append('|')
append(request.anchorPodcastId.orEmpty())
append('|')
append(request.missionId.orEmpty())
append('|')
append(request.revision.orEmpty())
append('|')
+ append(request.daypart.orEmpty())
+ append('|')
+ append(request.noveltyPreference?.toString().orEmpty())
+ append('|')
append(request.seeds.joinToString(",") { "${it.episodeId}:${it.podcastId}:${it.weight}" })
append('|')
- append(
- request.excludedPodcastIds
- .sorted()
- .take(40)
- .joinToString(","),
- )
+ append(request.excludedPodcastIds.sorted().joinToString(",").hashCode())
+ append('|')
+ append(request.excludedEpisodeIds.sorted().joinToString(",").hashCode())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun cacheKey(request: HomeCandidatesV1Request): String = | |
| buildString { | |
| append(request.requestedModules.sorted().joinToString(",")) | |
| append('|') | |
| append(request.country) | |
| append('|') | |
| append(request.languages.sorted().joinToString(",")) | |
| append('|') | |
| append(request.anchorPodcastId.orEmpty()) | |
| append('|') | |
| append(request.missionId.orEmpty()) | |
| append('|') | |
| append(request.revision.orEmpty()) | |
| append('|') | |
| append(request.seeds.joinToString(",") { "${it.episodeId}:${it.podcastId}:${it.weight}" }) | |
| append('|') | |
| append( | |
| request.excludedPodcastIds | |
| .sorted() | |
| .take(40) | |
| .joinToString(","), | |
| ) | |
| } | |
| fun cacheKey(request: HomeCandidatesV1Request): String = | |
| buildString { | |
| append(request.requestedModules.sorted().joinToString(",")) | |
| append('|') | |
| append(request.country) | |
| append('|') | |
| append(request.languages.sorted().joinToString(",")) | |
| append('|') | |
| append(request.anchorPodcastId.orEmpty()) | |
| append('|') | |
| append(request.missionId.orEmpty()) | |
| append('|') | |
| append(request.revision.orEmpty()) | |
| append('|') | |
| append(request.daypart.orEmpty()) | |
| append('|') | |
| append(request.noveltyPreference?.toString().orEmpty()) | |
| append('|') | |
| append(request.seeds.joinToString(",") { "${it.episodeId}:${it.podcastId}:${it.weight}" }) | |
| append('|') | |
| append(request.excludedPodcastIds.sorted().joinToString(",").hashCode()) | |
| append('|') | |
| append(request.excludedEpisodeIds.sorted().joinToString(",").hashCode()) | |
| } |
🤖 Prompt for 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.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilder.kt`
around lines 103 - 125, Update HomeCandidatesRequestBuilder.cacheKey to include
request.daypart and request.noveltyPreference, and encode the complete
request.excludedPodcastIds collection instead of truncating it with take(40).
Preserve the existing deterministic ordering for exclusions and all other key
components.
Comment on lines
+70
to
+127
| suspend fun loadSlate(request: SlateRequest): SlateResult { | ||
| val meaningfulPlays = HomeMeaningfulPlayLogic.countMeaningfulPlays(request.history) | ||
| val hasEligibleAnchor = !request.anchorPodcastId.isNullOrBlank() | ||
| val modeBeforeLoad = | ||
| HomePersonalizationModeLogic.derive( | ||
| meaningfulPlayCount = meaningfulPlays, | ||
| hasEligiblePositiveShow = hasEligibleAnchor, | ||
| personalizedCandidatesLoaded = false, | ||
| personalizedRequestFailed = false, | ||
| ) | ||
|
|
||
| val modules = | ||
| when { | ||
| request.becauseYouLikeOnly -> listOf("because_you_like") | ||
| modeBeforeLoad == HomePersonalizationMode.REGIONAL -> listOf("regional") | ||
| request.modules.isNotEmpty() -> request.modules | ||
| else -> | ||
| buildList { | ||
| add("taste") | ||
| if (hasEligibleAnchor) add("because_you_like") | ||
| add("mission") | ||
| add("regional") | ||
| } | ||
| } | ||
|
|
||
| val body = | ||
| HomeCandidatesRequestBuilder.build( | ||
| HomeCandidatesRequestBuilder.BuildRequest( | ||
| modules = modules, | ||
| country = request.country, | ||
| languages = request.languages, | ||
| history = request.history, | ||
| anchorPodcastId = request.anchorPodcastId, | ||
| missionId = request.missionId, | ||
| excludedPodcastIds = request.excludedPodcastIds, | ||
| excludedEpisodeIds = request.excludedEpisodeIds, | ||
| noveltyPreference = request.noveltyPreference, | ||
| daypart = request.daypart, | ||
| revision = request.revision, | ||
| ), | ||
| ) | ||
| val fetchStartedAt = System.currentTimeMillis() | ||
| val fetch = podcastRepository.getHomeCandidatesV1(body) | ||
| val fetchMeta = | ||
| HomeCandidatesFetchMeta( | ||
| fromCache = fetch.fromCache, | ||
| cacheAgeMillis = fetch.cacheAgeMillis, | ||
| responseLatencyMillis = System.currentTimeMillis() - fetchStartedAt, | ||
| requestedModules = modules, | ||
| ) | ||
| return mapResponse( | ||
| response = fetch.response, | ||
| fetchMeta = fetchMeta, | ||
| request = request, | ||
| meaningfulPlays = meaningfulPlays, | ||
| hasEligibleAnchor = hasEligibleAnchor, | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for tests exercising HomePersonalizationCoordinator's allocation/mapping path.
rg -n "HomePersonalizationCoordinator" --type=kt -g '*Test.kt' -C2
rg -n "allocateModules|buildPersonalizedResult|loadSlate" --type=kt -g '*Test.kt' -C2Repository: boxcreate/boxlore
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## candidate files\n'
git ls-files 'core/catalog/src/main/java/**HomePersonalizationCoordinator.kt' \
'core/catalog/src/test/**' \
'core/catalog/**README.md' \
'ARCHITECTURE.md' | sed 's#^`#-` #'
printf '\n## test references\n'
rg -n --glob '*Test.kt' 'HomePersonalizationCoordinator|allocateModules|buildPersonalizedResult|loadSlate|mapResponse' core/catalog/src/test core/catalog/src/main/java || true
printf '\n## coordinator outline\n'
ast-grep outline core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt --view expanded || trueRepository: boxcreate/boxlore
Length of output: 3678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## coordinator excerpt\n'
nl -ba core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt | sed -n '1,260p'
printf '\n## test files mentioning related logic\n'
fd -a --type f 'core/catalog/src/test' | sed 's#^`#-` #'
printf '\n## candidate test excerpts\n'
for f in \
core/catalog/src/test/**/HomeSlateQualityLogicTest.kt \
core/catalog/src/test/**/HomeCandidatesRequestBuilderTest.kt \
core/catalog/src/test/**/HomePersonalizationLogicTest.kt
do
if [ -f "$f" ]; then
echo "----- $f"
nl -ba "$f" | sed -n '1,260p'
fi
doneRepository: boxcreate/boxlore
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show any direct tests or indirect call sites for the coordinator.
rg -n --glob '*Test.kt' \
'HomePersonalizationCoordinator|HomeCandidatesRequestBuilder|HomePersonalizationLogic|HomeSlateQualityLogic|allocateModules|buildPersonalizedResult|mapResponse|loadSlate' \
core/catalog/src/test core/catalog/src/main/java || true
# Focus on the coordinator implementation and any nearby test utilities.
nl -ba core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt | sed -n '1,220p'Repository: boxcreate/boxlore
Length of output: 3190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List catalog test files to see what coverage exists around the coordinator.
find core/catalog/src/test -type f | sort
# Show module README presence required by policy.
find core/catalog -maxdepth 2 -name README.md | sortRepository: boxcreate/boxlore
Length of output: 2657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow search with a valid file filter.
rg -n --glob '*Test.kt' \
'HomePersonalizationCoordinator|allocateModules|buildPersonalizedResult|mapResponse|loadSlate|HomeCandidatesRequestBuilder|HomePersonalizationLogic|HomeSlateQualityLogic' \
core/catalog/src/test core/catalog/src/main/java || trueRepository: boxcreate/boxlore
Length of output: 3149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt | sed -n '1,220p'Repository: boxcreate/boxlore
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show only test file names to decide whether there is a direct coordinator test.
find core/catalog/src/test -type f | sort | sed -n '1,200p'Repository: boxcreate/boxlore
Length of output: 2634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## coordinator\n'
sed -n '1,260p' core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt
printf '\n## HomePersonalizationLogicTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationLogicTest.kt
printf '\n## HomeCandidatesRequestBuilderTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilderTest.kt
printf '\n## HomeSlateQualityLogicTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeSlateQualityLogicTest.ktRepository: boxcreate/boxlore
Length of output: 27985
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show exact declarations in the coordinator around module allocation and response mapping.
sed -n '1,380p' core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.ktRepository: boxcreate/boxlore
Length of output: 14621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## coordinator\n'
sed -n '1,380p' core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt
printf '\n## HomePersonalizationLogicTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationLogicTest.kt
printf '\n## HomeCandidatesRequestBuilderTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeCandidatesRequestBuilderTest.kt
printf '\n## HomeSlateQualityLogicTest\n'
sed -n '1,260p' core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/home/HomeSlateQualityLogicTest.ktRepository: boxcreate/boxlore
Length of output: 32076
Add coordinator-level JVM coverage HomePersonalizationLogicTest, HomeCandidatesRequestBuilderTest, and HomeSlateQualityLogicTest cover the extracted helpers, but not HomePersonalizationCoordinator.loadSlate/mapResponse itself. Add a hermetic test for the module-selection + response-mapping path, including the cross-module de-dup case.
🤖 Prompt for 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.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/home/HomePersonalizationCoordinator.kt`
around lines 70 - 127, Add coordinator-level JVM coverage in
HomePersonalizationCoordinatorTest for loadSlate and mapResponse using a
hermetic podcastRepository stub. Verify module selection, response mapping, and
cross-module podcast de-duplication, while keeping existing helper tests
unchanged.
Source: Path instructions
Comment on lines
52
to
+57
| title = podcast.title.replace("+", " "), | ||
| subtitle = podcast.artist.replace("+", " "), | ||
| onClick = onClick, | ||
| modifier = modifier, | ||
| onLongClick = onFeedback?.let { { feedbackMenuExpanded = true } }, | ||
| onLongClickLabel = "More options for ${podcast.title}", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the long-press label consistent with the displayed title.
The visual title replaces + with spaces, but TalkBack receives the raw title.
Proposed fix
- onLongClickLabel = "More options for ${podcast.title}",
+ onLongClickLabel = "More options for ${podcast.title.replace("+", " ")}",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| title = podcast.title.replace("+", " "), | |
| subtitle = podcast.artist.replace("+", " "), | |
| onClick = onClick, | |
| modifier = modifier, | |
| onLongClick = onFeedback?.let { { feedbackMenuExpanded = true } }, | |
| onLongClickLabel = "More options for ${podcast.title}", | |
| title = podcast.title.replace("+", " "), | |
| subtitle = podcast.artist.replace("+", " "), | |
| onClick = onClick, | |
| modifier = modifier, | |
| onLongClick = onFeedback?.let { { feedbackMenuExpanded = true } }, | |
| onLongClickLabel = "More options for ${podcast.title.replace("+", " ")}", |
🤖 Prompt for 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.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/components/PodcastCard.kt`
around lines 52 - 57, Update the onLongClickLabel in the PodcastCard call to use
the same plus-to-space title transformation as the displayed title, so TalkBack
announces the formatted podcast title consistently.
Comment on lines
61
to
77
| launch { | ||
| fastJob.join() | ||
| _isRecommendationsLoaded.value = false | ||
| _isTrendingLoaded.value = false | ||
| try { | ||
| android.util.Log.d("BoxCastTiming", "VM: Background personalized Home screen load for region=$region") | ||
|
|
||
| val interests = boxcastPrefs.getUserGenres().toList() | ||
|
|
||
| val historyDeferred = async { playbackRepository.getHistoryForRecommendations(15) } | ||
| val subscribedIdsDeferred = async { subscriptionRepository.subscribedPodcastIds.first().toList() } | ||
| val subscribedPodcastsDeferred = async { subscriptionRepository.subscribedPodcasts.first() } | ||
|
|
||
| val history = historyDeferred.await() | ||
| val subscribedIds = subscribedIdsDeferred.await() | ||
| val subscribedPodcasts = subscribedPodcastsDeferred.await() | ||
| val subscribedGenres = subscribedPodcasts.mapNotNull { it.genre }.distinct() | ||
|
|
||
| val bootstrapData = | ||
| podcastRepository.getHomeBootstrapData( | ||
| podcastRepository.getHomeBootstrapDataFast( | ||
| country = region, | ||
| vibeIds = emptyList(), | ||
| history = history, | ||
| interests = interests, | ||
| subscribedPodcastIds = subscribedIds, | ||
| subscribedGenres = subscribedGenres, | ||
| ) | ||
|
|
||
| val distinctRecs = | ||
| bootstrapData.recommendations | ||
| .distinctBy { it.id } | ||
| .distinctBy { it.title.lowercase().trim() } | ||
| _recommendations.value = distinctRecs | ||
| _isRecommendationsFallback.value = bootstrapData.isRecommendationsFallback | ||
| try { | ||
| val json = Json { ignoreUnknownKeys = true } | ||
| val serialized = json.encodeToString(distinctRecs) | ||
| boxcastPrefs.saveRecommendationsCache( | ||
| serialized, | ||
| bootstrapData.isRecommendationsFallback, | ||
| ) | ||
| } catch (ce: Exception) { | ||
| android.util.Log.e("HomeViewModel", "Failed to cache recommendations", ce) | ||
| } | ||
| _briefingState.value = bootstrapData.briefing | ||
| _briefingChaptersState.value = bootstrapData.briefingChapters | ||
| trendingState.value = bootstrapData.trending | ||
| } catch (e: Exception) { | ||
| android.util.Log.e("BoxCastTiming", "VM: Recommendations load failed", e) | ||
| android.util.Log.e("BoxCastTiming", "VM: Fast Bootstrap API load failed", e) | ||
| } finally { | ||
| _isRecommendationsLoaded.value = true | ||
| _isTrendingLoaded.value = true | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the reported lines.
FILE="feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt"
echo "== $FILE =="
nl -ba "$FILE" | sed -n '1,220p'
echo
echo "== Nearby tests in feature/home =="
git ls-files 'feature/home/src/test/**/*.kt' 'feature/home/src/androidTest/**/*.kt' | sed 's#^`#-` #'
echo
echo "== Search for CancellationException handling patterns =="
rg -n "CancellationException|catch \(e: Exception\)|catch \(.*Exception" feature/home core app -g '*.kt'Repository: boxcreate/boxlore
Length of output: 283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the file in full context around the reactive wiring and state updates.
FILE="feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt"
wc -l "$FILE"
sed -n '1,260p' "$FILE"Repository: boxcreate/boxlore
Length of output: 14381
Rethrow CancellationException here.
catch (e: Exception) also traps coroutine cancellation, so a cancelled viewModelScope job will be logged as a bootstrap failure and still reach finally. Catch CancellationException separately and rethrow it before handling other exceptions.
🤖 Prompt for 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.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt`
around lines 61 - 77, Update the exception handling in the coroutine launched by
the home data-loading flow to catch CancellationException before the general
Exception handler and immediately rethrow it. Keep logging non-cancellation
bootstrap failures through the existing BoxCastTiming Log.e call, while
preserving the existing state updates and cleanup behavior for handled
exceptions.
Comment on lines
+220
to
+262
| private fun HomeViewModel.recordSlateExposures( | ||
| taste: List<Episode>, | ||
| becauseYouLike: List<Episode>, | ||
| mission: List<Episode>, | ||
| ) { | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| recordRailExposures(taste, exposureEntryPointBySurface.getValue("taste")) | ||
| recordRailExposures(becauseYouLike, exposureEntryPointBySurface.getValue("because_you_like")) | ||
| recordRailExposures(mission, exposureEntryPointBySurface.getValue("mission")) | ||
| } | ||
| } | ||
|
|
||
| private suspend fun HomeViewModel.recordRailExposures( | ||
| episodes: List<Episode>, | ||
| entryPoint: String, | ||
| ) { | ||
| episodes.forEach { episode -> | ||
| val exposureId = | ||
| rankingFeedback.recordExposure( | ||
| RankingExposure( | ||
| episodeId = episode.id, | ||
| podcastId = episode.podcastId.orEmpty(), | ||
| objective = RankingObjective.SLATE, | ||
| surface = RankingSurface.HOME, | ||
| source = CandidateSource.SERVER_RECOMMENDATION, | ||
| features = | ||
| CandidateFeatureBuilder.build( | ||
| CandidateSignals( | ||
| serverRelevance = (episode.retrievalScore ?: 0.0).coerceIn(0.0, 1.0), | ||
| isUnseenShow = true, | ||
| isUnplayed = true, | ||
| ), | ||
| ), | ||
| entryPoint = entryPoint, | ||
| online = true, | ||
| retrievalReason = episode.recommendationReason, | ||
| ), | ||
| ) | ||
| if (exposureId.isNotBlank()) { | ||
| episodeExposureIds[episode.id] = exposureId | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
episodeExposureIds grows unbounded across slate reloads.
recordRailExposures only inserts into episodeExposureIds; entries for episodes that fall out of the slate on the next reload (region/daypart/subscription changes, feedback-triggered reallocation, mission rotation) are never evicted. Over a long session this map keeps accumulating stale episode→exposure mappings for the life of the HomeViewModel.
Since the slate arrays are fully replaced on every reload anyway, clearing the map before repopulating (or at the top of recordSlateExposures) is safe and bounds it to the current slate's size.
🧹 Proposed fix
private fun HomeViewModel.recordSlateExposures(
taste: List<Episode>,
becauseYouLike: List<Episode>,
mission: List<Episode>,
) {
viewModelScope.launch(Dispatchers.IO) {
+ episodeExposureIds.clear()
recordRailExposures(taste, exposureEntryPointBySurface.getValue("taste"))
recordRailExposures(becauseYouLike, exposureEntryPointBySurface.getValue("because_you_like"))
recordRailExposures(mission, exposureEntryPointBySurface.getValue("mission"))
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private fun HomeViewModel.recordSlateExposures( | |
| taste: List<Episode>, | |
| becauseYouLike: List<Episode>, | |
| mission: List<Episode>, | |
| ) { | |
| viewModelScope.launch(Dispatchers.IO) { | |
| recordRailExposures(taste, exposureEntryPointBySurface.getValue("taste")) | |
| recordRailExposures(becauseYouLike, exposureEntryPointBySurface.getValue("because_you_like")) | |
| recordRailExposures(mission, exposureEntryPointBySurface.getValue("mission")) | |
| } | |
| } | |
| private suspend fun HomeViewModel.recordRailExposures( | |
| episodes: List<Episode>, | |
| entryPoint: String, | |
| ) { | |
| episodes.forEach { episode -> | |
| val exposureId = | |
| rankingFeedback.recordExposure( | |
| RankingExposure( | |
| episodeId = episode.id, | |
| podcastId = episode.podcastId.orEmpty(), | |
| objective = RankingObjective.SLATE, | |
| surface = RankingSurface.HOME, | |
| source = CandidateSource.SERVER_RECOMMENDATION, | |
| features = | |
| CandidateFeatureBuilder.build( | |
| CandidateSignals( | |
| serverRelevance = (episode.retrievalScore ?: 0.0).coerceIn(0.0, 1.0), | |
| isUnseenShow = true, | |
| isUnplayed = true, | |
| ), | |
| ), | |
| entryPoint = entryPoint, | |
| online = true, | |
| retrievalReason = episode.recommendationReason, | |
| ), | |
| ) | |
| if (exposureId.isNotBlank()) { | |
| episodeExposureIds[episode.id] = exposureId | |
| } | |
| } | |
| } | |
| private fun HomeViewModel.recordSlateExposures( | |
| taste: List<Episode>, | |
| becauseYouLike: List<Episode>, | |
| mission: List<Episode>, | |
| ) { | |
| viewModelScope.launch(Dispatchers.IO) { | |
| episodeExposureIds.clear() | |
| recordRailExposures(taste, exposureEntryPointBySurface.getValue("taste")) | |
| recordRailExposures(becauseYouLike, exposureEntryPointBySurface.getValue("because_you_like")) | |
| recordRailExposures(mission, exposureEntryPointBySurface.getValue("mission")) | |
| } | |
| } | |
| private suspend fun HomeViewModel.recordRailExposures( | |
| episodes: List<Episode>, | |
| entryPoint: String, | |
| ) { | |
| episodes.forEach { episode -> | |
| val exposureId = | |
| rankingFeedback.recordExposure( | |
| RankingExposure( | |
| episodeId = episode.id, | |
| podcastId = episode.podcastId.orEmpty(), | |
| objective = RankingObjective.SLATE, | |
| surface = RankingSurface.HOME, | |
| source = CandidateSource.SERVER_RECOMMENDATION, | |
| features = | |
| CandidateFeatureBuilder.build( | |
| CandidateSignals( | |
| serverRelevance = (episode.retrievalScore ?: 0.0).coerceIn(0.0, 1.0), | |
| isUnseenShow = true, | |
| isUnplayed = true, | |
| ), | |
| ), | |
| entryPoint = entryPoint, | |
| online = true, | |
| retrievalReason = episode.recommendationReason, | |
| ), | |
| ) | |
| if (exposureId.isNotBlank()) { | |
| episodeExposureIds[episode.id] = exposureId | |
| } | |
| } | |
| } |
🤖 Prompt for 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.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelSlate.kt`
around lines 220 - 262, Clear episodeExposureIds at the start of
recordSlateExposures before launching the exposure-recording work, then
repopulate it from the current taste, becauseYouLike, and mission lists.
Preserve the existing recordRailExposures behavior and ensure stale mappings
from prior slate reloads are removed.
Comment on lines
+359
to
+376
| /** | ||
| * Builds the playback `sourceContext` extras carrying the Home slate's exact exposure token. | ||
| * [EXPOSURE_ID_KEY] must match `PlaybackTelemetrySession.EXPOSURE_ID_KEY` (internal to | ||
| * `:core:playback`, so feature modules mirror the plain string key like `"entry_point"` above). | ||
| */ | ||
| internal fun HomeViewModel.buildExposureSourceContext( | ||
| episodeId: String, | ||
| entryPoint: cx.aswin.boxlore.core.model.PlaybackEntryPoint, | ||
| ): android.os.Bundle? { | ||
| val exposureId = episodeExposureIds[episodeId] ?: return null | ||
| return android.os.Bundle().apply { | ||
| putString("entry_point", entryPoint.name.lowercase()) | ||
| putString(EXPOSURE_ID_KEY, exposureId) | ||
| } | ||
| } | ||
|
|
||
| private const val EXPOSURE_ID_KEY = "exposure_id" | ||
| private const val HOME_SLATE_HISTORY_LIMIT = 15 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EXPOSURE_ID_KEY is a duplicated magic string across module boundaries — see consolidated comment.
🤖 Prompt for 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.
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelSlate.kt`
around lines 359 - 376, Replace the locally duplicated EXPOSURE_ID_KEY string in
HomeViewModel.buildExposureSourceContext with the shared playback telemetry
exposure-key symbol or centralized contract, updating the Bundle write
accordingly while preserving the existing exposure token value and entry-point
behavior.
Comment on lines
+117
to
+120
| const oldestDeferredAgeHours = deferredWithHistory.length > 0 | ||
| ? Math.round((now - Math.min(...deferredWithHistory.map((id) => st.shows[id].c))) / 3600000) | ||
| : 0; | ||
| const runsToClearBacklog = deferred > 0 ? Math.ceil(deferred / cfg.MAX_CHECKS_PER_RUN) : 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep unknown backlog age distinct from zero.
When every deferred show is never checked, oldestDeferredAgeHours becomes 0; Lines 130, 218, 223, and 230 then report a real-looking 0h age despite there being no prior-check timestamp. Use a nullable/n/a age and phrase these details as unavailable when no deferred show has history.
Suggested fix
- const oldestDeferredAgeHours = deferredWithHistory.length > 0
+ const oldestDeferredAgeHours = deferredWithHistory.length > 0
? Math.round((now - Math.min(...deferredWithHistory.map((id) => st.shows[id].c))) / 3600000)
- : 0;
+ : null;
+ const backlogAgeDetail = oldestDeferredAgeHours === null
+ ? 'age unavailable: no deferred show has a prior check'
+ : `oldest previously checked show waiting ${log.fmt(oldestDeferredAgeHours)}h`;Also applies to: 130-130, 218-218, 223-223, 230-230
🤖 Prompt for 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.
In `@scripts/sync/03-sync-episodes.js` around lines 117 - 120, Update
oldestDeferredAgeHours and the related backlog messages at the identified
reporting points to preserve an unknown age when deferredWithHistory is empty,
using a nullable or n/a representation instead of 0. Ensure messages in the
deferred backlog reporting flow phrase the age as unavailable when no deferred
show has a prior-check timestamp, while retaining numeric hour output when
history exists.
This was referenced Jul 22, 2026
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
/home/candidates/v1, with a durable cold-start mode machine and learner-selected BYL anchors.Motivation
Home discovery felt poorly personalized because learning often attributed the wrong impression and rails used split heuristics. This rebuild makes attribution exact, unifies slate allocation, and makes quality measurable before expanding personalization elsewhere.
What changed
/home/candidates/v1DTOs + Retrofit;HomePersonalizationCoordinator(seeds, mode, allocation, ≥4h cache); region→language mapping.home_slate_quality_snapshot+home_learning_attribution_health(aggregate/privacy-safe).Behavior & compatibility
/home/candidates/v1consumption.PERSONALIZED.RankingRolloutPolicy); quality gates are observational dashboards, not kill switches.Impact (required)
User impact — pick exactly one
user-impact-highuser-impact-mediumuser-impact-lowno-user-impactListener impact
What changes in the user’s life:
Home recommendations should feel more like their taste: clearer “Based on Your Taste” vs regional popular, Because You Like anchored to a real favorite (with a change-anchor option), a single rotating discovery mission under the greeting, and long-press ways to say more like this / not for me / hide this show so the feed improves from their feedback.
Backend
backend-change(sync health logging / workflow notes; consumes already-deployed candidates API)Test plan
./gradlew testDebugUnitTest detekt ktlintCheck --continuegreen locally./gradlew installDebugon connected device