feat: integrate project-based navigation and details view#26
Conversation
* Refactored `LyricsActivity` to use `projectId` for data retrieval instead of passing raw song and lyrics data via intents. * Introduced `ProjectDetailsScreen` in Compose for immersive video playback and project sharing. * Updated `AppNavHost` to include a new `ProjectDetails` route and handle project creation/persistence before starting `LyricsActivity`. * Modified `LyricsMotionWorker` and `getLyricsVideoProducer` to initialize using `MotionProject` from the database. * Added support for project metadata (start time, image, lyrics) stored as JSON in `MotionProject`. * Improved `ProjectsScreenCompose` with empty state handling for project thumbnails. * Updated `LyricsViewModel` to track `selectedStartTimeInSeconds`. * Updated `LyricsActivityTest` to align with the new project-based initialization.
📝 WalkthroughWalkthroughThis PR migrates the lyrics video workflow from a stateless, parameter-based architecture to a project-centric, database-backed model. Activities and workers now accept a Changes
Sequence DiagramsequenceDiagram
actor User
participant LyricsActivity
participant MotionStoreDao
participant LyricsVideoProducer
participant MotionVideoPlayer
participant LyricsMotionWorker
User->>LyricsActivity: start(context, projectId)
LyricsActivity->>MotionStoreDao: query(projectId)
MotionStoreDao-->>LyricsActivity: MotionProject{metadata: {lyrics, image, startTime}}
LyricsActivity->>LyricsVideoProducer: getLyricsVideoProducer(context, motionProject)
LyricsVideoProducer->>LyricsVideoProducer: extract lyrics/image from metadata JSON
LyricsVideoProducer-->>LyricsActivity: MotionVideoProducer
LyricsActivity->>MotionVideoPlayer: play(producer)
MotionVideoPlayer-->>User: display video
User->>LyricsActivity: confirm & save
LyricsActivity->>MotionStoreDao: upsert(motionProject)
LyricsActivity->>LyricsMotionWorker: startWork(context, projectId)
LyricsMotionWorker->>MotionStoreDao: query(projectId)
MotionStoreDao-->>LyricsMotionWorker: MotionProject
LyricsMotionWorker->>LyricsVideoProducer: getLyricsVideoProducer(context, motionProject)
LyricsVideoProducer-->>LyricsMotionWorker: MotionVideoProducer
LyricsMotionWorker->>LyricsMotionWorker: generate video file
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
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 |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR successfully refactors the architecture to use project-based navigation with a centralized database approach. However, there are 6 critical crash risks that must be fixed before merge:
Critical Issues Blocking Merge
- Type mismatch in ProjectDetailsCompose.kt (line 93): Reading Float as Int causes
NumberFormatException - Empty lyrics crash in LyricsVideoProducer.kt (line 21-27): Missing validation for empty lyrics list
- Empty lyrics crash in ProjectDetailsCompose.kt (line 47-49): Calling
getLyricsVideoProducerwith empty lyrics - Null pointer crash in LyricsMotionWorker.kt (line 99-100): Using
!!operators without null checks - Type display issue in ProjectDetailsCompose.kt (line 95): Text formatting needs update after type fix
Changes Overview
Refactored Files:
LyricsActivity.kt- Now uses projectId instead of raw dataAppNavHost.kt- Persists project before starting activityLyricsVideoProducer.kt- Initializes from MotionProjectLyricsMotionWorker.kt- Retrieves project from database
New Files:
ProjectDetailsCompose.kt- Immersive video playback screen
All critical issues have actionable code suggestions attached. Please review and apply the fixes.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| val motionVideoProducer = remember(project.id) { | ||
| getLyricsVideoProducer(context, project) | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: getLyricsVideoProducer will crash when project has empty lyrics list. The function calls .first() and .last() on the lyrics list without checking if it's empty, causing NoSuchElementException. Ensure projects always have at least one lyric entry or add validation before calling this function.
|
|
||
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 | ||
| Text( | ||
| text = "Starts at: ${startTime}s", |
There was a problem hiding this comment.
Update the text formatting to display the Float value correctly after fixing the type mismatch on line 93.
| text = "Starts at: ${startTime}s", | |
| text = "Starts at: ${startTime}s", |
| color = MaterialTheme.colorScheme.onSurface | ||
| ) | ||
|
|
||
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 |
There was a problem hiding this comment.
🛑 Crash Risk: Calling project.metadata.get("startTime")?.asInt will throw NumberFormatException when the stored value is a Float. The metadata stores selectedStartTimeInSeconds as Float but reads it as Int.
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 | |
| val startTime = project.metadata.get("startTime")?.asFloat ?: 0f |
| val lyrics = | ||
| motionProject.metadata.get("lyrics")?.takeIf { it.isJsonArray }?.asJsonArray?.map { | ||
| SyncedLyricFrame( | ||
| frame = it.asJsonObject.get("frame")?.takeIf { f -> f.isJsonPrimitive }?.asInt ?: 0, | ||
| text = it.asJsonObject.get("text")?.takeIf { t -> t.isJsonPrimitive }?.asString ?: "", | ||
| ) | ||
| } ?: emptyList() |
There was a problem hiding this comment.
🛑 Crash Risk: The lyrics list extracted from metadata can be empty (line 27 returns emptyList() as fallback). This will cause NoSuchElementException when LyricsContainer tries to access lyrics.first().frame and lyrics.last().frame on lines 42-43. Validate that lyrics is not empty before creating the container or handle empty case.
| override fun getMotionVideo(inputData: Data): MotionVideoProducer { | ||
| val projectId = inputData.getString(PROJECT_ID)!! | ||
| val motionProject = applicationContext.asLyricsApp().motionStoreDao.findById(projectId)!! | ||
| return getLyricsVideoProducer( | ||
| applicationContext = appContext, | ||
| song = inputData.getString(SONG) ?: "Unknown Song", | ||
| lyrics = Json.decodeFromString(inputData.getString(LYRICS)!!), | ||
| image = inputData.getString(IMAGE), | ||
| motionProject = motionProject, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Non-null assertion operator !! will throw NullPointerException when projectId is missing from inputData or when project doesn't exist in database. Add null checks with error handling to prevent worker crashes.
| override fun getMotionVideo(inputData: Data): MotionVideoProducer { | |
| val projectId = inputData.getString(PROJECT_ID)!! | |
| val motionProject = applicationContext.asLyricsApp().motionStoreDao.findById(projectId)!! | |
| return getLyricsVideoProducer( | |
| applicationContext = appContext, | |
| song = inputData.getString(SONG) ?: "Unknown Song", | |
| lyrics = Json.decodeFromString(inputData.getString(LYRICS)!!), | |
| image = inputData.getString(IMAGE), | |
| motionProject = motionProject, | |
| ) | |
| } | |
| override fun getMotionVideo(inputData: Data): MotionVideoProducer { | |
| val projectId = inputData.getString(PROJECT_ID) ?: throw IllegalArgumentException("Project ID is required") | |
| val motionProject = applicationContext.asLyricsApp().motionStoreDao.findById(projectId) | |
| ?: throw IllegalStateException("Project not found: $projectId") | |
| return getLyricsVideoProducer( | |
| applicationContext = appContext, | |
| motionProject = motionProject, | |
| ) | |
| } |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the application to a project-based workflow for creating lyrics, introducing the MotionProject model to persist metadata such as song details and timing in a local database. Significant updates include the implementation of a project details screen, enhanced navigation, and refactoring LyricsActivity and LyricsMotionWorker to utilize project identifiers for data retrieval. The reviewer feedback highlights opportunities to streamline JSON processing using Gson, optimize performance by implementing lazy loading for the lyrics property, and fix a potential precision error in start-time retrieval. Further recommendations include adopting more idiomatic Kotlin for collection operations and eliminating redundant project initialization logic.
| import android.os.Build | ||
| import android.os.Bundle | ||
| import com.google.android.material.dialog.MaterialAlertDialogBuilder | ||
| import com.tejpratapsingh.lyricsmaker.asLyricsApp |
There was a problem hiding this comment.
Add Gson import to support simplified JSON parsing.
| import com.tejpratapsingh.lyricsmaker.asLyricsApp | |
| import com.google.gson.Gson | |
| import com.tejpratapsingh.lyricsmaker.asLyricsApp |
| private val lyrics: List<SyncedLyricFrame> | ||
| get() = | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| get() { | ||
| val metadata = project?.metadata | ||
| val projectLyrics = | ||
| metadata?.get("lyrics")?.takeIf { it.isJsonArray }?.asJsonArray?.map { | ||
| SyncedLyricFrame( | ||
| frame = | ||
| it.asJsonObject | ||
| .get("frame") | ||
| ?.takeIf { f -> f.isJsonPrimitive } | ||
| ?.asInt | ||
| ?: 0, | ||
| text = | ||
| it.asJsonObject | ||
| .get("text") | ||
| ?.takeIf { t -> t.isJsonPrimitive } | ||
| ?.asString | ||
| ?: "", | ||
| ) | ||
| } | ||
| if (projectLyrics != null) return projectLyrics | ||
|
|
||
| return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| intent.getParcelableArrayListExtra(LYRICS, SyncedLyricFrame::class.java)?.toList() | ||
| ?: emptyList() | ||
| } else { | ||
| @Suppress("DEPRECATION") | ||
| intent.getParcelableArrayListExtra(LYRICS) ?: emptyList() | ||
| } | ||
|
|
||
| private val socialMeta | ||
| get() = ShareReceiverActivity.readMetadataFromIntent(intent) | ||
| } |
There was a problem hiding this comment.
The lyrics property is currently a computed getter that performs JSON parsing on every access. Since it is accessed multiple times (in onCreate and during video production), it should be a lazy property to cache the result. Additionally, using Gson simplifies the parsing logic.
private val lyrics: List<SyncedLyricFrame> by lazy {
val metadata = project?.metadata
val projectLyrics = metadata?.get("lyrics")?.let {
Gson().fromJson(it, Array<SyncedLyricFrame>::class.java)?.toList()
}
projectLyrics ?: if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
intent.getParcelableArrayListExtra(LYRICS, SyncedLyricFrame::class.java)?.toList()
?: emptyList()
} else {
@Suppress("DEPRECATION")
intent.getParcelableArrayListExtra(LYRICS) ?: emptyList()
}
}| val start = if (lyrics.isNotEmpty()) lyrics.minBy { it.frame }.frame else 0 | ||
| val end = if (lyrics.isNotEmpty()) lyrics.maxBy { it.frame }.frame else 0 |
There was a problem hiding this comment.
Use minOfOrNull and maxOfOrNull to simplify finding the frame range. This is more idiomatic and handles the empty list case gracefully without explicit checks.
| val start = if (lyrics.isNotEmpty()) lyrics.minBy { it.frame }.frame else 0 | |
| val end = if (lyrics.isNotEmpty()) lyrics.maxBy { it.frame }.frame else 0 | |
| val start = lyrics.minOfOrNull { it.frame } ?: 0 | |
| val end = lyrics.maxOfOrNull { it.frame } ?: 0 |
| Duration: ${(end - start)} frames (${(end - start) / provideCurrentConfig().fps} seconds) | ||
| """.trimIndent(), | ||
| ).setPositiveButton("OK") { dialog, _ -> | ||
| val currentProject = project ?: provideCurrentProject(id = song.md5()).copy(name = song) |
There was a problem hiding this comment.
This project initialization logic is duplicated from the video property (line 100). Consider extracting it to a shared property or helper method to ensure consistency and reduce redundancy.
| import androidx.navigation.compose.NavHost | ||
| import androidx.navigation.compose.composable | ||
| import com.google.gson.JsonArray | ||
| import com.google.gson.JsonObject |
There was a problem hiding this comment.
Add Gson import to simplify JSON serialization.
| import com.google.gson.JsonObject | |
| import com.google.gson.Gson | |
| import com.google.gson.JsonObject |
| add( | ||
| "lyrics", | ||
| JsonArray().apply { | ||
| lyrics.forEach { frame -> | ||
| add( | ||
| JsonObject().apply { | ||
| addProperty("frame", frame.frame) | ||
| addProperty("text", frame.text) | ||
| }, | ||
| ) | ||
| } | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Use Gson().toJsonTree() to serialize the lyrics list. This is much cleaner and less error-prone than manual JSON construction.
add("lyrics", Gson().toJsonTree(lyrics))| color = MaterialTheme.colorScheme.onSurface | ||
| ) | ||
|
|
||
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 |
There was a problem hiding this comment.
startTime is stored as a Float in the metadata. Using asInt here will cause precision loss (e.g., 1.5s becomes 1s). Use asFloat to maintain accuracy in the UI.
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 | |
| val startTime = project.metadata.get("startTime")?.asFloat ?: 0f |
| import com.tejpratapsingh.motionlib.core.provideCurrentConfig | ||
| import com.tejpratapsingh.motionlib.core.setCurrentConfig | ||
| import com.tejpratapsingh.motionlib.ffmpeg.FfmpegVideoProducerAdapter | ||
| import com.tejpratapsingh.motionstore.tables.MotionProject |
There was a problem hiding this comment.
Add Gson import.
| import com.tejpratapsingh.motionstore.tables.MotionProject | |
| import com.google.gson.Gson | |
| import com.tejpratapsingh.motionstore.tables.MotionProject |
| val lyrics = | ||
| motionProject.metadata.get("lyrics")?.takeIf { it.isJsonArray }?.asJsonArray?.map { | ||
| SyncedLyricFrame( | ||
| frame = it.asJsonObject.get("frame")?.takeIf { f -> f.isJsonPrimitive }?.asInt ?: 0, | ||
| text = it.asJsonObject.get("text")?.takeIf { t -> t.isJsonPrimitive }?.asString ?: "", | ||
| ) | ||
| } ?: emptyList() |
There was a problem hiding this comment.
Use Gson to deserialize the lyrics list instead of manual parsing.
val lyrics = motionProject.metadata.get("lyrics")?.let {
Gson().fromJson(it, Array<SyncedLyricFrame>::class.java)?.toList()
} ?: emptyList()![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/motion/LyricsVideoProducer.kt (1)
21-47:⚠️ Potential issue | 🔴 CriticalGuard empty project lyrics before calling
first()/last().Project-backed callers can now open legacy or malformed
MotionProjectrows. Withlyrics = emptyList(), Lines 42-43 crash immediately.💡 Suggested fix
val lyrics = motionProject.metadata.get("lyrics")?.takeIf { it.isJsonArray }?.asJsonArray?.map { SyncedLyricFrame( frame = it.asJsonObject.get("frame")?.takeIf { f -> f.isJsonPrimitive }?.asInt ?: 0, text = it.asJsonObject.get("text")?.takeIf { t -> t.isJsonPrimitive }?.asString ?: "", ) } ?: emptyList() + val sortedLyrics = lyrics.sortedBy { it.frame } + val startFrame = sortedLyrics.firstOrNull()?.frame ?: 0 + val endFrame = sortedLyrics.lastOrNull()?.frame ?: startFrame Log.d("MotionVideoProducer", "getLyricsVideoProducer: ${lyrics.size}") @@ LyricsContainer( context = applicationContext, - startFrame = lyrics.first().frame, - endFrame = lyrics.last().frame, + startFrame = startFrame, + endFrame = endFrame, songName = song, - lyrics = lyrics, + lyrics = sortedLyrics, image = image, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/motion/LyricsVideoProducer.kt` around lines 21 - 47, The code calls lyrics.first() and lyrics.last() without guarding for an empty list; update the block that constructs LyricsContainer so it checks if lyrics.isEmpty() first and handles that case (e.g., early return/create a LyricsContainer with safe default start/end frames or skip creation) before calling lyrics.first().frame or lyrics.last().frame; adjust the logic around MotionConfig/setCurrentConfig and the LyricsContainer(...) call to use the guarded values to prevent NPEs when the parsed lyrics list is empty.modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt (1)
105-122:⚠️ Potential issue | 🟠 MajorNormalize against the earliest frame, not the first selected item.
If
selectedLyricsarrives unsorted, Line 107 and Line 115 use the wrong baseline. That stores an incorrectstartTimeand can produce negative normalized frames in the metadata you persist later.💡 Suggested fix
var selectedLyrics: List<SyncedLyricFrame> = emptyList() set(value) { - field = value - selectedStartTimeInSeconds = if (value.isNotEmpty()) { - value.first().frame.toFloat() / provideCurrentConfig().fps - } else { - 0f - } + val sorted = value.sortedBy { it.frame } + field = sorted + selectedStartTimeInSeconds = + sorted.firstOrNull()?.frame?.toFloat()?.div(provideCurrentConfig().fps) ?: 0f } get() { - if (field.isEmpty()) return emptyList() - val firstFrame = field.first().frame + val firstFrame = field.firstOrNull()?.frame ?: return emptyList() return field .map { SyncedLyricFrame( frame = it.frame - firstFrame, text = it.text,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt` around lines 105 - 122, The setter/getter for selectedLyrics currently use value.first()/field.first() as the baseline which breaks when the list is unsorted; change both to compute the earliest frame (e.g., min frame via value.minByOrNull { it.frame }?.frame ?: 0 and field.minByOrNull { it.frame }?.frame ?: 0) and use that earliestFrame to compute selectedStartTimeInSeconds (divide by provideCurrentConfig().fps) and to normalize frames in the getter (subtract earliestFrame before sorting) so negative/incorrect offsets are avoided; update references in the selectedLyrics setter/getter accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivityTest.kt`:
- Around line 36-72: The test currently uses ActivityScenarioRule which launches
LyricsActivity before the `@Before` setup seeds the database; update the test to
remove the ActivityScenarioRule and instead launch the activity from within the
test method using ActivityScenario.launch(Intent(...).apply {
putExtra(LyricsActivity.PROJECT_ID, projectId) }) after setup completes,
ensuring the MotionProject inserted via
context.asLyricsApp().motionStoreDao.upsert(project) in the setup() method is
available when LyricsActivity.onCreate() calls motionStoreDao.findById(id); keep
the PROJECT_ID extra and the same Intent construction when calling
ActivityScenario.launch.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt`:
- Around line 99-106: The lazy-initialized video block creates a minimal
MotionProject when project == null, so getLyricsVideoProducer reads empty
metadata and produces an unusable project; replace that fallback with a proper
legacy project builder that populates MotionProject.metadata (image and lyrics)
from the incoming intent and song data instead of calling
provideCurrentProject(...) with only id/name — implement a new private function
(e.g., buildLegacyProjectFromIntent()) that constructs a MotionProject with id,
name, path and a metadata JsonObject containing "image" and a "lyrics" JsonArray
built from the lyrics frames, then call setCurrentProject(...) with that
complete MotionProject before calling getLyricsVideoProducer(...).
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/AppNavHost.kt`:
- Around line 29-30: The ProjectDetails.createRoute currently concatenates
projectId raw into the route which breaks for special characters; change
createRoute in the ProjectDetails Screen to URI-encode projectId before building
the string (e.g., use java.net.URLEncoder.encode(projectId, "UTF-8") or
URLEncoder.encode(projectId, StandardCharsets.UTF_8.name()) and then return
"project_details/$encodedId") so navigation works with ids containing /, ?, #,
%.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsCompose.kt`:
- Around line 93-95: The code reads project.metadata.get("startTime") as an int
which truncates fractional seconds; change the read to use .asFloat (e.g., val
startTime = project.metadata.get("startTime")?.asFloat ?: 0f) and update the
display Text in ProjectDetailsCompose to format the Float with desired precision
(e.g., using String.format or Kotlin's format specifier) so values like
selectedStartTimeInSeconds (written via addProperty in AppNavHost) show
fractional seconds instead of being truncated.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.kt`:
- Around line 241-249: Replace the blocking remember(...) thumbnail extraction
with a coroutine-backed state producer on the IO dispatcher so file I/O happens
off the composition thread: use produceState<Bitmap?>(initialValue = null, key1
= project.id, key2 = projectFile.path, key3 = projectFile.exists(), key4 =
projectFile.lastModified()) { withContext(Dispatchers.IO) { value = if
(projectFile.exists()) extractFirstFrame(projectFile.path) else null } } (or
similar), referencing the existing projectFile and extractFirstFrame symbols;
this ensures extraction runs off-main and the keys (path/exists/lastModified)
prevent caching a sticky null when the file appears or changes.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorker.kt`:
- Around line 98-105: Load and set the worker's current project after fetching
it from the DB instead of relying on a global that may be unset: in
getMotionVideo(), replace the force-unwrapped calls
(motionStoreDao.findById(projectId)!!) with a null-safe fetch
(motionStoreDao.findById(projectId)) and if null return a controlled failure;
then rehydrate the global/currentProject via
provideCurrentProject(motionProject) or assign the worker's currentProject so
getOutputFile() and onCompleted() use the freshly loaded motionProject; ensure
PROJECT_ID handling is null-safe and avoid crashes by returning a failed Result
or throwing a specific exception when the project is missing rather than using
!!.
---
Outside diff comments:
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/motion/LyricsVideoProducer.kt`:
- Around line 21-47: The code calls lyrics.first() and lyrics.last() without
guarding for an empty list; update the block that constructs LyricsContainer so
it checks if lyrics.isEmpty() first and handles that case (e.g., early
return/create a LyricsContainer with safe default start/end frames or skip
creation) before calling lyrics.first().frame or lyrics.last().frame; adjust the
logic around MotionConfig/setCurrentConfig and the LyricsContainer(...) call to
use the guarded values to prevent NPEs when the parsed lyrics list is empty.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt`:
- Around line 105-122: The setter/getter for selectedLyrics currently use
value.first()/field.first() as the baseline which breaks when the list is
unsorted; change both to compute the earliest frame (e.g., min frame via
value.minByOrNull { it.frame }?.frame ?: 0 and field.minByOrNull { it.frame
}?.frame ?: 0) and use that earliestFrame to compute selectedStartTimeInSeconds
(divide by provideCurrentConfig().fps) and to normalize frames in the getter
(subtract earliestFrame before sorting) so negative/incorrect offsets are
avoided; update references in the selectedLyrics setter/getter accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f03b063-8fe8-4cf4-91b2-01788b0ee244
📒 Files selected for processing (9)
modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivityTest.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/SearchActivity.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/AppNavHost.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsCompose.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/motion/LyricsVideoProducer.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorker.kt
| @Before | ||
| fun setup() { | ||
| val context = ApplicationProvider.getApplicationContext<Context>() | ||
| val project = | ||
| MotionProject( | ||
| id = projectId, | ||
| name = songName, | ||
| path = "/$projectId", | ||
| metadata = | ||
| JsonObject().apply { | ||
| addProperty("image", socialMeta.image) | ||
| addProperty("startTime", 0f) | ||
| add( | ||
| "lyrics", | ||
| JsonArray().apply { | ||
| lyrics.forEach { frame -> | ||
| add( | ||
| JsonObject().apply { | ||
| addProperty("frame", frame.frame) | ||
| addProperty("text", frame.text) | ||
| }, | ||
| ) | ||
| } | ||
| }, | ||
| ) | ||
| }, | ||
| ) | ||
| context.asLyricsApp().motionStoreDao.upsert(project) | ||
| } | ||
|
|
||
| @get:Rule | ||
| val activityRule = | ||
| ActivityScenarioRule<LyricsActivity>( | ||
| Intent(ApplicationProvider.getApplicationContext<Context>(), LyricsActivity::class.java).apply { | ||
| putExtra(LyricsActivity.SONG, songName) | ||
| putExtra(ShareReceiverActivity.EXTRA_METADATA, socialMeta) | ||
| putParcelableArrayListExtra(LyricsActivity.LYRICS, lyrics) | ||
| putExtra(LyricsActivity.PROJECT_ID, projectId) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the test file
find . -name "LyricsActivityTest.kt" -type fRepository: tejpratap46/AndroidVideoMotion
Length of output: 192
🏁 Script executed:
# Also find the LyricsActivity implementation to understand when data is loaded
find . -name "LyricsActivity.kt" -type f | grep -v "Test"Repository: tejpratap46/AndroidVideoMotion
Length of output: 181
🏁 Script executed:
# Read the test file to see the full context
cat -n ./modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivityTest.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 4164
🏁 Script executed:
# Read the LyricsActivity to understand when data is loaded
cat -n ./modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt | head -100Repository: tejpratap46/AndroidVideoMotion
Length of output: 4725
🏁 Script executed:
# Let me verify JUnit rule execution order by checking if there are any related docs or comments
# Also check if there are other similar tests in the codebase to see how they handle this pattern
rg "ActivityScenarioRule|@get:Rule" --type kotlin -B 3 -A 3 | head -100Repository: tejpratap46/AndroidVideoMotion
Length of output: 3625
🏁 Script executed:
# Check if there are any integration tests or similar patterns in the codebase
find . -name "*Test.kt" -type f -path "*/androidTest/*" | xargs wc -l | sort -rn | head -10Repository: tejpratap46/AndroidVideoMotion
Length of output: 752
🏁 Script executed:
# Let me research JUnit rule execution order and verify the actual behavior
# by checking Android testing documentation or any related comments in the code
rg "@Before|@Test|rule|Rule" -A 2 -B 2 ./modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivityTest.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 1036
🏁 Script executed:
# Check if LyricsActivity's onCreate or other lifecycle methods access the project property
cat -n ./modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt | sed -n '100,200p'Repository: tejpratap46/AndroidVideoMotion
Length of output: 1971
🏁 Script executed:
# Check the parent class PreviewActivity to see if it accesses project data during onCreate
find . -name "PreviewActivity.kt" -type f | head -1 | xargs cat -n | head -80Repository: tejpratap46/AndroidVideoMotion
Length of output: 1558
Move activity launch to the test method, after @Before completes.
ActivityScenarioRule launches the activity during rule evaluation, which happens before @Before runs. When LyricsActivity.onCreate() executes at launch, it immediately accesses lyrics (line 112), which queries motionStoreDao.findById(id) for the project — but the test database seed in @Before hasn't run yet, leaving project null. Use ActivityScenario.launch() in the test method instead to ensure the database is seeded first.
Suggested fix
- `@get`:Rule
- val activityRule =
- ActivityScenarioRule<LyricsActivity>(
- Intent(ApplicationProvider.getApplicationContext<Context>(), LyricsActivity::class.java).apply {
- putExtra(LyricsActivity.PROJECT_ID, projectId)
- },
- )
-
`@Test`
fun testLyricsActivityLaunch() {
+ ActivityScenario.launch<LyricsActivity>(
+ Intent(ApplicationProvider.getApplicationContext<Context>(), LyricsActivity::class.java).apply {
+ putExtra(LyricsActivity.PROJECT_ID, projectId)
+ },
+ )
+
onView(withText("Lyrics")).check(matches(isDisplayed()))
onView(withText(containsString("""Rendering video for \"$songName\""""))).check(matches(isDisplayed()))
onView(withText("OK")).check(matches(isDisplayed()))
onView(withText("Cancel")).check(matches(isDisplayed()))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivityTest.kt`
around lines 36 - 72, The test currently uses ActivityScenarioRule which
launches LyricsActivity before the `@Before` setup seeds the database; update the
test to remove the ActivityScenarioRule and instead launch the activity from
within the test method using ActivityScenario.launch(Intent(...).apply {
putExtra(LyricsActivity.PROJECT_ID, projectId) }) after setup completes,
ensuring the MotionProject inserted via
context.asLyricsApp().motionStoreDao.upsert(project) in the setup() method is
available when LyricsActivity.onCreate() calls motionStoreDao.findById(id); keep
the PROJECT_ID extra and the same Intent construction when calling
ActivityScenario.launch.
| private val video by lazy { | ||
| setCurrentProject( | ||
| provideCurrentProject(id = song.md5()).copy(name = song), | ||
| ) | ||
| val currentProject = project ?: provideCurrentProject(id = song.md5()).copy(name = song) | ||
| setCurrentProject(currentProject) | ||
|
|
||
| getLyricsVideoProducer( | ||
| applicationContext = applicationContext, | ||
| song = song, | ||
| lyrics = lyrics, | ||
| image = socialMeta?.image, | ||
| motionProject = currentProject, | ||
| ) |
There was a problem hiding this comment.
The deprecated extras-based entrypoint now builds an empty project.
When project == null, Lines 100-105 create a MotionProject with only id and name. getLyricsVideoProducer() now reads lyrics and image exclusively from motionProject.metadata, so callers still using start(context, song, lyrics, socialMeta) will preview an empty render and then persist an unusable project on Line 126.
💡 Suggested direction
- val currentProject = project ?: provideCurrentProject(id = song.md5()).copy(name = song)
+ val currentProject = project ?: buildLegacyProjectFromIntent()private fun buildLegacyProjectFromIntent(): MotionProject {
val id = song.md5()
val socialMeta =
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
intent.getParcelableExtra(ShareReceiverActivity.EXTRA_METADATA, SocialMeta::class.java)
} else {
`@Suppress`("DEPRECATION")
intent.getParcelableExtra(ShareReceiverActivity.EXTRA_METADATA)
}
return MotionProject(
id = id,
name = song,
path = "/$id",
metadata = com.google.gson.JsonObject().apply {
addProperty("image", socialMeta?.image)
add(
"lyrics",
com.google.gson.JsonArray().apply {
lyrics.forEach { frame ->
add(
com.google.gson.JsonObject().apply {
addProperty("frame", frame.frame)
addProperty("text", frame.text)
},
)
}
},
)
},
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/LyricsActivity.kt`
around lines 99 - 106, The lazy-initialized video block creates a minimal
MotionProject when project == null, so getLyricsVideoProducer reads empty
metadata and produces an unusable project; replace that fallback with a proper
legacy project builder that populates MotionProject.metadata (image and lyrics)
from the incoming intent and song data instead of calling
provideCurrentProject(...) with only id/name — implement a new private function
(e.g., buildLegacyProjectFromIntent()) that constructs a MotionProject with id,
name, path and a metadata JsonObject containing "image" and a "lyrics" JsonArray
built from the lyrics frames, then call setCurrentProject(...) with that
complete MotionProject before calling getLyricsVideoProducer(...).
| object ProjectDetails : Screen("project_details/{projectId}") { | ||
| fun createRoute(projectId: String) = "project_details/$projectId" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "AppNavHost.kt" | head -20Repository: tejpratap46/AndroidVideoMotion
Length of output: 176
🏁 Script executed:
cd modules/lyrics-maker && git ls-files | grep -E "\.kt$" | grep -i nav | head -20Repository: tejpratap46/AndroidVideoMotion
Length of output: 153
🏁 Script executed:
fd "AppNavHost" . --type fRepository: tejpratap46/AndroidVideoMotion
Length of output: 176
🏁 Script executed:
cat -n ./modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/AppNavHost.kt | head -50Repository: tejpratap46/AndroidVideoMotion
Length of output: 2286
🏁 Script executed:
rg "createRoute" ./modules/lyrics-maker --type kt -B 2 -A 5Repository: tejpratap46/AndroidVideoMotion
Length of output: 100
🏁 Script executed:
rg "createRoute" ./modules/lyrics-maker --type-list | headRepository: tejpratap46/AndroidVideoMotion
Length of output: 463
🏁 Script executed:
rg "createRoute" ./modules/lyrics-maker -A 2 -B 2Repository: tejpratap46/AndroidVideoMotion
Length of output: 1595
🏁 Script executed:
rg "ProjectDetails.createRoute" . -A 2 -B 2Repository: tejpratap46/AndroidVideoMotion
Length of output: 926
🏁 Script executed:
find . -type f -name "MotionProject*" | head -10Repository: tejpratap46/AndroidVideoMotion
Length of output: 257
🏁 Script executed:
rg "class MotionProject" . -A 10Repository: tejpratap46/AndroidVideoMotion
Length of output: 2929
🏁 Script executed:
rg "data class MotionProject" . -A 10Repository: tejpratap46/AndroidVideoMotion
Length of output: 1564
🏁 Script executed:
rg "ProjectDetails" ./modules/lyrics-maker -A 10 | grep -E "(composable|navBackStackEntry|arguments)"Repository: tejpratap46/AndroidVideoMotion
Length of output: 432
🏁 Script executed:
cat -n ./modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/AppNavHost.kt | sed -n '50,100p'Repository: tejpratap46/AndroidVideoMotion
Length of output: 2784
🏁 Script executed:
rg "navBackStackEntry\?.arguments" ./modules/lyrics-maker -A 5Repository: tejpratap46/AndroidVideoMotion
Length of output: 56
🏁 Script executed:
rg "MotionProject(" ./modules/lyrics-maker -B 2 -A 5Repository: tejpratap46/AndroidVideoMotion
Length of output: 152
🏁 Script executed:
cat -n ./modules/motion-store/src/main/java/com/tejpratapsingh/motionstore/tables/MotionProject.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 1482
🏁 Script executed:
rg "\.id" ./modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/SearchActivity.kt -B 3 -A 3Repository: tejpratap46/AndroidVideoMotion
Length of output: 508
🏁 Script executed:
rg "getString\(\"projectId" ./modules/lyrics-maker -B 5 -A 10Repository: tejpratap46/AndroidVideoMotion
Length of output: 2377
Encode projectId when building the route.
Raw interpolation breaks navigation if an id contains /, ?, #, or %. The route helper should URI-encode the parameter before concatenation.
💡 Suggested fix
object ProjectDetails : Screen("project_details/{projectId}") {
- fun createRoute(projectId: String) = "project_details/$projectId"
+ fun createRoute(projectId: String) = "project_details/${android.net.Uri.encode(projectId)}"
}📝 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.
| object ProjectDetails : Screen("project_details/{projectId}") { | |
| fun createRoute(projectId: String) = "project_details/$projectId" | |
| object ProjectDetails : Screen("project_details/{projectId}") { | |
| fun createRoute(projectId: String) = "project_details/${android.net.Uri.encode(projectId)}" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/AppNavHost.kt`
around lines 29 - 30, The ProjectDetails.createRoute currently concatenates
projectId raw into the route which breaks for special characters; change
createRoute in the ProjectDetails Screen to URI-encode projectId before building
the string (e.g., use java.net.URLEncoder.encode(projectId, "UTF-8") or
URLEncoder.encode(projectId, StandardCharsets.UTF_8.name()) and then return
"project_details/$encodedId") so navigation works with ids containing /, ?, #,
%.
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 | ||
| Text( | ||
| text = "Starts at: ${startTime}s", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the ProjectDetailsCompose.kt file and view the context around lines 93-95
find . -name "ProjectDetailsCompose.kt" -type fRepository: tejpratap46/AndroidVideoMotion
Length of output: 187
🏁 Script executed:
# Search for where startTime is written to project metadata
rg "startTime" --type kotlin -B 2 -A 2Repository: tejpratap46/AndroidVideoMotion
Length of output: 2843
🏁 Script executed:
# Search for selectedStartTimeInSeconds to understand its type
rg "selectedStartTimeInSeconds" --type kotlin -B 2 -A 2Repository: tejpratap46/AndroidVideoMotion
Length of output: 2304
startTime is stored as a float but displayed as a truncated int.
The selectedStartTimeInSeconds variable (a Float) is written to project metadata via addProperty() in AppNavHost.kt, but read back with .asInt in ProjectDetailsCompose.kt at line 93. This truncates fractional seconds: a start time of 1.5 seconds displays as "Starts at: 1s".
Use .asFloat instead and format the display to preserve precision:
Suggested fix
- val startTime = project.metadata.get("startTime")?.asInt ?: 0
+ val startTime = project.metadata.get("startTime")?.asFloat ?: 0f
Text(
- text = "Starts at: ${startTime}s",
+ text = "Starts at: ${"%.1f".format(startTime)}s",
style = MaterialTheme.typography.bodyLarge,
color = MaterialTheme.colorScheme.onSurfaceVariant
)📝 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.
| val startTime = project.metadata.get("startTime")?.asInt ?: 0 | |
| Text( | |
| text = "Starts at: ${startTime}s", | |
| val startTime = project.metadata.get("startTime")?.asFloat ?: 0f | |
| Text( | |
| text = "Starts at: ${"%.1f".format(startTime)}s", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsCompose.kt`
around lines 93 - 95, The code reads project.metadata.get("startTime") as an int
which truncates fractional seconds; change the read to use .asFloat (e.g., val
startTime = project.metadata.get("startTime")?.asFloat ?: 0f) and update the
display Text in ProjectDetailsCompose to format the Float with desired precision
(e.g., using String.format or Kotlin's format specifier) so values like
selectedStartTimeInSeconds (written via addProperty in AppNavHost) show
fractional seconds instead of being truncated.
| val projectFile = remember(project.id) { context.createProjectFile(project) } | ||
| val thumbnail: Bitmap? = | ||
| remember(project.id) { extractFirstFrame(context.createProjectFile(project).path) } | ||
| remember(project.id) { | ||
| if (projectFile.exists()) { | ||
| extractFirstFrame(projectFile.path) | ||
| } else { | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether project updates (or another stable key) change after video generation,
# so thumbnail state can be reloaded for the same project id.
# 1) Find thumbnail-loading and remember/producestate key usage.
rg -n -C3 --type=kt 'extractFirstFrame\s*\(|remember\s*\(\s*project\.id\s*\)|produceState\s*<Bitmap\?>'
# 2) Find MotionProject update write paths that touch `updated` or equivalent fields.
rg -n -C3 --type=kt 'copy\s*\(.*updated\s*=|updated\s*=|update.*MotionProject|save.*MotionProject'Repository: tejpratap46/AndroidVideoMotion
Length of output: 127
🏁 Script executed:
#!/bin/bash
# First, locate the target Kotlin file
find . -path "*/modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.kt" 2>/dev/null
# Also search broadly for the file
fd "ProjectsScreenCompose.kt"Repository: tejpratap46/AndroidVideoMotion
Length of output: 299
🏁 Script executed:
#!/bin/bash
# Find all Kotlin files to verify correct search approach
fd -e kt | head -20Repository: tejpratap46/AndroidVideoMotion
Length of output: 1922
🏁 Script executed:
#!/bin/bash
# Search for extractFirstFrame and remember(project.id) usage without --type flag
rg -n "extractFirstFrame\s*\(" --max-count=10Repository: tejpratap46/AndroidVideoMotion
Length of output: 409
🏁 Script executed:
#!/bin/bash
# Search for MotionProject definition to check for updated field
rg -n "data class.*MotionProject|class.*MotionProject" -A 15 | head -50Repository: tejpratap46/AndroidVideoMotion
Length of output: 4121
🏁 Script executed:
#!/bin/bash
# Read the actual code around lines 241-249
sed -n '235,260p' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 784
🏁 Script executed:
#!/bin/bash
# Read the extractFirstFrame implementation
sed -n '424,450p' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 738
🏁 Script executed:
#!/bin/bash
# Check for any LaunchedEffect or other async handling for thumbnail loading
rg -n "LaunchedEffect|rememberCoroutineScope|thumbnail" modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.kt | head -20Repository: tejpratap46/AndroidVideoMotion
Length of output: 665
Move thumbnail extraction off the composition thread and prevent sticky null caching.
On line 245, extractFirstFrame(...) runs inside remember(project.id), executing blocking file I/O (MediaMetadataRetriever operations) on the composition thread. This can block UI rendering. Additionally, if the video file is missing initially, null is cached forever—even if the file is created later, the cached null persists because the cache key project.id never changes.
Proposed fix
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.produceState
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
- val projectFile = remember(project.id) { context.createProjectFile(project) }
- val thumbnail: Bitmap? =
- remember(project.id) {
- if (projectFile.exists()) {
- extractFirstFrame(projectFile.path)
- } else {
- null
- }
- }
+ val projectFile = remember(project.id) { context.createProjectFile(project) }
+ val thumbnail by produceState<Bitmap?>(
+ initialValue = null,
+ key1 = project.id,
+ key2 = project.updated,
+ ) {
+ value = withContext(Dispatchers.IO) {
+ if (projectFile.exists()) {
+ runCatching { extractFirstFrame(projectFile.path) }.getOrNull()
+ } else {
+ null
+ }
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectsScreenCompose.kt`
around lines 241 - 249, Replace the blocking remember(...) thumbnail extraction
with a coroutine-backed state producer on the IO dispatcher so file I/O happens
off the composition thread: use produceState<Bitmap?>(initialValue = null, key1
= project.id, key2 = projectFile.path, key3 = projectFile.exists(), key4 =
projectFile.lastModified()) { withContext(Dispatchers.IO) { value = if
(projectFile.exists()) extractFirstFrame(projectFile.path) else null } } (or
similar), referencing the existing projectFile and extractFirstFrame symbols;
this ensures extraction runs off-main and the keys (path/exists/lastModified)
prevent caching a sticky null when the file appears or changes.
| override fun getMotionVideo(inputData: Data): MotionVideoProducer { | ||
| val projectId = inputData.getString(PROJECT_ID)!! | ||
| val motionProject = applicationContext.asLyricsApp().motionStoreDao.findById(projectId)!! | ||
| return getLyricsVideoProducer( | ||
| applicationContext = appContext, | ||
| song = inputData.getString(SONG) ?: "Unknown Song", | ||
| lyrics = Json.decodeFromString(inputData.getString(LYRICS)!!), | ||
| image = inputData.getString(IMAGE), | ||
| motionProject = motionProject, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Rehydrate currentProject inside the worker.
The worker now loads motionProject from the database here, but getOutputFile() and onCompleted() still read provideCurrentProject(). In a fresh WorkManager process that global can be unset or stale, so the render can write to the wrong project path and upsert the wrong record. Also, the !!s turn a missing project into a crash instead of a controlled failure.
💡 Suggested fix
override fun getMotionVideo(inputData: Data): MotionVideoProducer {
- val projectId = inputData.getString(PROJECT_ID)!!
- val motionProject = applicationContext.asLyricsApp().motionStoreDao.findById(projectId)!!
+ val projectId =
+ requireNotNull(inputData.getString(PROJECT_ID)) { "Missing work input: $PROJECT_ID" }
+ val motionProject =
+ requireNotNull(applicationContext.asLyricsApp().motionStoreDao.findById(projectId)) {
+ "Project not found: $projectId"
+ }
+ com.tejpratapsingh.motionstore.tables.setCurrentProject(motionProject)
return getLyricsVideoProducer(
applicationContext = appContext,
motionProject = motionProject,
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorker.kt`
around lines 98 - 105, Load and set the worker's current project after fetching
it from the DB instead of relying on a global that may be unset: in
getMotionVideo(), replace the force-unwrapped calls
(motionStoreDao.findById(projectId)!!) with a null-safe fetch
(motionStoreDao.findById(projectId)) and if null return a controlled failure;
then rehydrate the global/currentProject via
provideCurrentProject(motionProject) or assign the worker's currentProject so
getOutputFile() and onCompleted() use the freshly loaded motionProject; ensure
PROJECT_ID handling is null-safe and avoid crashes by returning a failed Result
or throwing a specific exception when the project is missing rather than using
!!.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #29 |
1 participant
LyricsActivityto useprojectIdfor data retrieval instead of passing raw song and lyrics data via intents.ProjectDetailsScreenin Compose for immersive video playback and project sharing.AppNavHostto include a newProjectDetailsroute and handle project creation/persistence before startingLyricsActivity.LyricsMotionWorkerandgetLyricsVideoProducerto initialize usingMotionProjectfrom the database.MotionProject.ProjectsScreenComposewith empty state handling for project thumbnails.LyricsViewModelto trackselectedStartTimeInSeconds.LyricsActivityTestto align with the new project-based initialization.Summary by CodeRabbit