Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds playback variant catalog models and API mapping, resolves runtime from the selected media version, and applies that runtime and version-specific metadata to Android and TV detail screens. Tests cover multipart durations, fallback behavior, technical labels, and subtitle selection. ChangesSelected media metadata
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant CatalogDetailReadV2
participant ItemDetail
participant selectedMediaRuntimeMinutes
participant MovieDetailContent
participant TvDetailMetadata
CatalogDetailReadV2->>ItemDetail: map playback_variants
MovieDetailContent->>selectedMediaRuntimeMinutes: resolve selected movie runtime
selectedMediaRuntimeMinutes-->>MovieDetailContent: runtime minutes
MovieDetailContent->>ItemDetail: render Android metadata with runtime
TvDetailMetadata->>selectedMediaRuntimeMinutes: resolve selected TV runtime
selectedMediaRuntimeMinutes-->>TvDetailMetadata: runtime minutes
TvDetailMetadata->>ItemDetail: render version-specific TV metadata
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The selected-runtime flow preserves multipart duration resolution for server-generated playback variants, with no actionable merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| val selectedRuntimeMinutes = selectedMediaRuntimeMinutes(detail, selectedVersion) | ||
| val factsLine = HeroMetadata.movieFactsLine(detail, selectedRuntimeMinutes) |
There was a problem hiding this comment.
This changes the visible runtime metadata on phone and TV, but the PR provides no before-and-after images and states that neither interface was manually verified. The repository guide requires before-and-after images for UI changes, so this requirement must be satisfied before merging. Please add evidence covering the affected phone and TV detail states.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
We are not adding screenshots for this PR. This is a small, focused metadata data-flow correction; it does not change the layouts or introduce new UI states. The phone and TV presentation coverage verifies that the selected version drives the displayed runtime and technical metadata, so screenshot evidence is not warranted for this change.
There was a problem hiding this comment.
I agree this is a narrow data-flow correction and that the presentation tests provide meaningful coverage. However, the repository guideline does not limit the evidence requirement to layout or new-state changes; it says to include before-and-after images for UI changes. Because this PR changes the runtime and subtitle metadata visibly rendered on both phone and TV detail screens, it qualifies as a UI change even though the layout is unchanged. The screenshot requirement therefore still applies unless a maintainer grants an explicit exception or the repository guideline is updated.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
1 participant
Problem
Related issue: N/A — narrow fix
Selecting a different movie or episode file could leave Android showing the item-level editorial runtime instead of the selected media duration. Android TV could also show a CC badge inherited from another file because the catalog subtitle list is item-wide. This follows up on the selected-version metadata behavior corrected in silo-server#414.
Approach
playback_variantsfrom the v2 catalog response into the presentation model.playback_variants.The server contract already provides the required file durations and playback-variant totals. No server change is needed. silo-apple#301 carries the coordinated Apple client change.
Validation
Passed:
./scripts/test-check-build-supply-chain.sh./scripts/check-build-supply-chain.sh:android-shared:lintDebug,:androidApp:lintDebug, and:androidTvApp:lintDebug:androidApp:lintVitalReleaseand:androidTvApp:lintVitalRelease:androidApp:assembleDebugand:androidTvApp:assembleDebug./gradlew testDebugUnitTestran 666 Android phone tests and stopped on the existingReflowStyleTest > line height flows from settingsfailure. The same test fails on the cleanmaincheckout at the branch base. All tests added or changed by this PR pass.Manual phone and TV verification against a live server was not run.
Risks
Low. The new response field is optional, and older servers retain the existing editorial-runtime fallback. No playback request or server behavior changes.
Checklist
AI Disclosure
gpt-6-astrawith medium reasoning effortSummary by CodeRabbit
New Features
Bug Fixes