#11: Re-implementation of version tracking #13
Conversation
shubertm
had a problem deploying
to
Development
GitHub Actions
Failure
kirillt
changed the title
|
|
I'm getting this crash right now: |
This crash happens because filepicker changed, we have to initialise FoldersRepo during app creation. |
…ARK-Memo into version-tracking-2
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
|
We should merge #18 first. |
|
@ShubertMunthali we need to resolve conflicts before merging. |
# Conflicts: # app/build.gradle # app/src/main/AndroidManifest.xml # app/src/main/java/dev/arkbuilders/arkmemo/App.kt # app/src/main/java/dev/arkbuilders/arkmemo/files/FilePicker.kt # app/src/main/java/dev/arkbuilders/arkmemo/files/parsers/JsonParser.kt # app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/TextNotesListAdapter.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/NoteDeleteDialogFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/SettingsFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/TextNoteVersionsFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/views/PathPreference.kt # app/src/main/java/dev/arkbuilders/arkmemo/utils/ArkFiles.kt # app/src/main/java/dev/arkbuilders/arkmemo/utils/Config.kt # app/src/main/java/space/taran/arkmemo/data/repo/notes/text/TextNotesRepo.kt # app/src/main/java/space/taran/arkmemo/data/viewmodels/EditTextNotesViewModel.kt # app/src/main/java/space/taran/arkmemo/data/viewmodels/TextNotesViewModel.kt # app/src/main/java/space/taran/arkmemo/preferences/MemoPreferences.kt # app/src/main/res/values/strings.xml # app/src/test/java/dev/arkbuilders/arkmemo/VersionStorageTest.kt # build.gradle # gradle/wrapper/gradle-wrapper.properties # settings.gradle
Yes, I will update it |
|
It works! I've checked both text notes and graphic notes. Versions are displayed correctly and forking functions correctly, too. This is really cool. We have new reusable storage primitives in arklib-android, let's use them to reduce amount of code. See FileStorage arklib-android. I think, it's possible to implement Also, we should find another icon for forking feature, I'll ask our UX team. |
|
You are right @kirillt |
|
@ShubertMunthali it's OK to keep But yes, we'll move it eventually to |
|
Alright @kirillt, so let's make important storage API from |
|
@ShubertMunthali yes, if our ready-made primitives can't be used to implement custom storages — this should be fixed. Because it would be great if 3rd party developers could write their own storages for their apps. It's possible to compile arklib-android and inject the local version into the app. It would be easier for experimentation. |
shubertm
had a problem deploying
to
Development
GitHub Actions
Failure
WalkthroughAdds note versioning capability: new storage interfaces/implementations, models, ViewModel, Fragment, utilities, tests, and resources. Extends Note to Parcelable and introduces isForked across note types. Updates adapters for media side-effects. Numerous XML/Kotlin formatting changes and minor ID/ID rename, plus CI/settings tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant VF as VersionsFragment
participant NVm as NotesViewModel
participant VVm as VersionsViewModel
participant VRepo as VersionStorageRepo
participant VStore as RootVersionStorage
User->>VF: Open Versions screen (with Note)
VF->>VVm: init()
VVm->>VRepo: provide()
VRepo->>VStore: create/init (if absent)
VRepo-->>VVm: VersionStorage
VF->>NVm: getNotes()
NVm-->>VF: List<Note>
VF->>VVm: updateLatestResourceId(note.id)
VF->>VVm: getLatestNoteFamilyTree(notes, emit)
VVm->>VStore: parentsTreeByChild(id)
VStore-->>VVm: Map<ResourceId, List<ResourceId>>
VVm-->>VF: Filtered notes (family chain)
VF-->>User: Render history list
sequenceDiagram
autonumber
actor User
participant VF as VersionsFragment
participant VVm as VersionsViewModel
participant VStore as RootVersionStorage
User->>VF: Create new version (fork/save)
VF->>VVm: createVersion(oldId, newId)
VVm->>VStore: add(Version(parent=oldId, child=newId))
VStore->>VStore: persist() to file
VStore-->>VVm: OK
VVm-->>VF: Completion
VF-->>User: UI updates (latest markers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 40
🔭 Outside diff range comments (3)
app/build.gradle (1)
90-92: Update dev.arkbuilders dependencies to latest published versionsWeb lookup shows newer releases: dev.arkbuilders:arklib → 0.3.4-blake3 and dev.arkbuilders:arkfilepicker → 0.1.2. Current lines use 0.3.3 and 0.1.1 — please bump and verify compatibility (AndroidX/Hilt/transitives) and CI access to GitHub Packages.
- File to change:
- app/build.gradle (around lines 90–92)
Diff suggestion:
- implementation 'dev.arkbuilders:arkfilepicker:0.1.1' - implementation 'dev.arkbuilders:arklib:0.3.3' + implementation 'dev.arkbuilders:arkfilepicker:0.1.2' + implementation 'dev.arkbuilders:arklib:0.3.4-blake3'Also ensure GitHub Packages is configured in repositories (and CI auth) so the artifacts can be resolved. Run a full build and smoke tests after bumping to confirm no transitive breakage.
app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt (1)
121-123: Invalidate options menu after navigating to SettingsAfter replacing the fragment with SettingsFragment, the menu should be invalidated so onCreateOptionsMenu can re-evaluate visibility. Otherwise, the Settings button may remain visible on the Settings screen.
Apply this diff:
R.id.settings -> { fragment = SettingsFragment() replaceFragment(fragment, SettingsFragment.TAG) + invalidateOptionsMenu() }app/src/main/res/layout/note.xml (1)
15-17: Remove unused ConstraintLayout attributesThis container is a LinearLayout; ConstraintLayout attributes have no effect and will trigger lint warnings. Remove them.
Apply this diff:
- app:layout_constraintEnd_toEndOf="parent" - app:layout_constraintStart_toStartOf="parent" - app:layout_constraintTop_toTopOf="parent"
🧹 Nitpick comments (30)
README.md (1)
3-3: Fix markdownlint MD036: avoid emphasis as a headingUse a proper heading instead of italic text for the status line.
Apply:
- _Implementation in progress_ +## Status +Implementation in progressapp/src/main/res/drawable/ic_content_copy.xml (1)
1-5: Avoid hardcoded tint for better theming (dark mode, dynamic color)Hardcoding @color/purple_700 limits theme adaptability. Prefer a theme attribute so the icon respects light/dark and Material dynamic color.
Minimal change:
-<vector android:height="24dp" android:tint="@color/purple_700" +<vector android:height="24dp" android:tint="?attr/colorControlNormal"Optionally, remove tint from the vector and tint at usage sites (ImageView/Button), which is often cleaner for shared assets. Also ensure usage sets contentDescription appropriately (or null if decorative).
gradle/wrapper/gradle-wrapper.properties (1)
4-4: Confirm AGP/Kotlin/JDK compatibility & add Gradle wrapper checksumGradle 9 requires AGP ≥ 8.4.0; ensure Kotlin Gradle Plugin ≥ 2.0.0 and JDK ≥ 17 before upgrading. Add the Gradle wrapper SHA-256 for supply-chain hardening.
- Files to update / verify:
- gradle/wrapper/gradle-wrapper.properties (line ~4) — add checksum
- Check project AGP/Kotlin/JDK in build.gradle(.kts), settings.gradle(.kts), and gradle.properties
Suggested diff:
distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-bin.zip +distributionSha256Sum=8fad3d78296ca518113f3d29016617c7f9367dc005f932bd9d93bf45ba46072bPlease confirm your project’s AGP, Kotlin, and JDK versions meet the minima above; if they do, apply the checksum change.
app/src/main/res/drawable/ic_note.xml (1)
1-5: Use theme-based tint to support dynamic themingSame as ic_content_copy.xml: avoid hardcoded purple tint to ensure consistency across themes and dark mode.
-<vector android:height="24dp" android:tint="@color/purple_700" +<vector android:height="24dp" android:tint="?attr/colorControlNormal"Consider tinting at usage sites for greater flexibility.
app/src/main/res/drawable/ic_history.xml (1)
1-5: Use theme-aware tint for better dark/light mode supportHardcoding tint to @color/purple_700 makes the icon ignore theme, and may reduce contrast in dark mode. Prefer using a theme attribute.
-<vector android:height="24dp" android:tint="@color/purple_700" +<vector android:height="24dp" android:tint="?attr/colorControlNormal" android:viewportHeight="24" android:viewportWidth="24" android:width="24dp" xmlns:android="http://schemas.android.com/apk/res/android">Optionally add auto-mirroring if this icon is considered directional:
- android:width="24dp" xmlns:android="http://schemas.android.com/apk/res/android"> + android:width="24dp" android:autoMirrored="true" xmlns:android="http://schemas.android.com/apk/res/android">app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Version.kt (2)
5-5: RenameVersion2to a clearer alias (e.g.,VersionId)
typealias Version2 = ResourceIdis unclear and looks like a temporary name. If the intent is to indicate “a single version identifier”, consider a clearer alias such as VersionId.-typealias Version2 = ResourceId +typealias VersionId = ResourceIdNote: This is a breaking rename across the codebase (Version2 usages must be updated).
If you’d like, I can generate a patch for the whole repo to rename
Version2→VersionId. Do you want me to proceed?
7-10: Add brief KDoc to clarify semantics of parent/childDocument whether parent → child implies “child is derived from parent” and if multiple parents or cycles are allowed. This will help future maintainers and storage/graph operations.
Example:
/**
- parent: the source/origin version
- child: the derived/forked version
- Semantics: DAG with no cycles (enforced at storage level)
*/settings.gradle (1)
28-29: Give GitHub Maven repositories unique names and keep the token fallbackGood: falling back to System.getenv("GITHUB_TOKEN") is CI-friendly.
To avoid confusion in dependency resolution logs and IDE UIs, give the two GitHub repos distinct names:
- maven { - name = "GitHubPackages" + maven { + name = "GitHubPackages-arklib-android" url = "https://maven.pkg.github.com/ARK-Builders/arklib-android" credentials { username = "token" password = localProps.getProperty("gpr.token") ?: System.getenv("GITHUB_TOKEN") } } maven { - name = "GitHubPackages" + name = "GitHubPackages-ark-android" url = "https://maven.pkg.github.com/ARK-Builders/ark-android" credentials { username = "token" password = localProps.getProperty("gpr.token") ?: System.getenv("GITHUB_TOKEN") } }Also applies to: 31-37
app/src/main/res/values/strings.xml (1)
30-33: Add contentDescription strings for accessibility (for the new action buttons)If these strings are used as labels/tooltips only, also add dedicated content descriptions for the image buttons to support TalkBack.
Consider adding alongside the current entries:
<string name="ark_memo_cd_track_versions">Track versions</string> <string name="ark_memo_cd_fork_versions">Fork versions</string> <string name="ark_memo_cd_history">History</string> <string name="ark_memo_cd_old_version">Older version</string>Then, reference them via android:contentDescription in the layout where the new icons are used.
I can patch the layout(s) to wire these up if you confirm the desired wording.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Versions.kt (2)
11-17: Prefer emptySet() and guard logging in Monoid.combine to keep it pure and low-noise
- Neutral can use emptySet() (clearer intent).
- Consider guarding or removing logging inside a pure combiner to avoid noisy logs and side-effects.
- override val neutral: Versions = setOf() + override val neutral: Versions = emptySet() - val result = a.union(b) - Log.d(LOG_PREFIX, "merging $a and $b into $result") + val result = a.union(b) + if (Log.isLoggable(LOG_TAG, Log.DEBUG)) { + Log.d(LOG_TAG, "merging $a and $b into $result") + } return result
20-20: Use a proper Log tag constant and keep it private to this fileAndroid Log tags are typically identifiers (no brackets). Also prefer const val for inlining and make it private to this file.
-internal val LOG_PREFIX: String = "[versions]" +private const val LOG_TAG: String = "Versions"app/build.gradle (1)
13-14: Remove the duplicate namespace declarationThe namespace is already set on Line 10. The second declaration is redundant and can confuse readers.
- namespace 'dev.arkbuilders.arkmemo'app/src/main/java/dev/arkbuilders/arkmemo/di/RepositoryModule.kt (1)
29-33: Consider scoping NotesRepoHelper as Singleton for consistency with other reposNotesRepoHelper likely encapsulates shared state/config. For parity with PropertiesStorageRepo and VersionStorageRepo, consider annotating this provider with @singleton as well.
Proposed change outside the selected range:
@Singleton @Provides fun provideNotesRepoHelper( memoPreferences: MemoPreferences, propertiesStorageRepo: PropertiesStorageRepo ): NotesRepoHelper = NotesRepoHelper(memoPreferences, propertiesStorageRepo)app/src/main/java/dev/arkbuilders/arkmemo/utils/ArkFiles.kt (1)
5-5: Add brief KDoc to clarify purpose and expected existence of the "versions" dirMinor, but documenting whether the caller must ensure directory existence helps avoid misuse.
Example:
/** * Returns a path to the "versions" subdirectory under this path. * Note: The directory may not exist; callers should create it when needed. */ fun Path.arkVersions(): Path = resolve("versions")app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt (1)
139-143: Provide a counterpart to restore default UI after editinginitEditUI() sets edit mode UI but there’s no public API to restore defaults (title, home-as-up, settings visibility) when leaving edit screens. Introduce a reset method for consistency.
Example implementation outside the selected range:
fun resetMainUI() { title = getString(R.string.app_name) supportActionBar?.setDisplayHomeAsUpEnabled(false) showSettingsButton(fragment is NotesFragment || fragment is VersionsFragment) invalidateOptionsMenu() }app/src/main/res/layout/note.xml (2)
25-31: Decorative ImageView missing contentDescriptionFor accessibility, mark decorative images with a null contentDescription so they’re ignored by TalkBack.
Apply this diff:
android:visibility="gone" app:srcCompat="@drawable/ic_note" + android:contentDescription="@null" />
55-76: Use ripple/selectable background instead of null for ImageButtonsNull backgrounds remove visual feedback and tap targets may feel unresponsive. Use the standard borderless ripple.
Apply this diff:
- <ImageButton + <ImageButton android:id="@+id/btn_track_versions" android:layout_width="50dp" android:layout_height="50dp" - android:background="@null" + android:background="?attr/selectableItemBackgroundBorderless" android:contentDescription="@string/ark_memo_track_versions" android:src="@drawable/ic_history" android:visibility="gone" /> - <ImageButton + <ImageButton android:id="@+id/btn_fork_version" android:layout_width="50dp" android:layout_height="50dp" - android:background="@null" + android:background="?attr/selectableItemBackgroundBorderless" android:contentDescription="@string/ark_memo_fork_versions" android:src="@drawable/ic_content_copy" android:visibility="gone" /> - <ImageButton + <ImageButton android:id="@+id/btn_delete" android:layout_width="50dp" android:layout_height="50dp" - android:background="@null" + android:background="?attr/selectableItemBackgroundBorderless" android:contentDescription="@string/delete" android:src="@drawable/delete" />app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt (1)
6-11: Document isForked semanticsAdd a brief KDoc on what constitutes a “forked” note vs “latest” and how UI should treat it (e.g., read-only for old versions). This will help third-party storage implementations later.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt (1)
13-21: Consider cache invalidation when root changes or storage needs a refreshIf the notes storage root is changed in preferences, old entries remain cached indefinitely. Expose an invalidate API.
Example addition (outside the diffed lines, for illustration):
suspend fun invalidate(root: Path? = null) = mutex.withLock { if (root == null) storageByRoot.clear() else storageByRoot.remove(root) }app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt (1)
43-45: Avoid duplicating SVG updates in the ViewThe View writes SVG commands directly while viewModel.onDrawPath likely updates the SVG too (per GraphicNotesViewModel.onDrawPath). Duplicating these responsibilities risks divergence and double writes.
Consider removing the direct SVG mutations from the View and let the ViewModel be the single source of truth:
- viewModel.svg().apply { - addCommand(SVGCommand.MoveTo(x, y)) - } + // Let ViewModel update the SVG when the stroke is finalized- viewModel.svg().apply { - addCommand(SVGCommand.AbsQuadTo(currentX, currentY, x2, y2)) - } + // Let ViewModel update the SVG when the stroke is finalizedIf you need live-preview of the stroke, render the Path on canvas (as you already do) and persist to SVG once on ACTION_UP.
Also applies to: 54-56
app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/NoteDeleteDialog.kt (1)
28-33: Optional: Strengthen parent dependency and add content descriptionUse requireParentFragment() if the dialog must be attached to a parent that implements deleteNote. Also, consider making the positive action’s intent accessible.
- .setPositiveButton(R.string.ark_memo_ok){ dialog, _ -> - parentFragment?.deleteNote(note) + .setPositiveButton(R.string.ark_memo_ok){ dialog, _ -> + requireParentFragment().deleteNote(note) toast(requireContext(), getString(R.string.note_deleted)) dialog.cancel() }If the parent isn’t guaranteed, keep the safe call but surface a clear error or disable the button.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt (1)
6-15: Add KDoc and clarify naming to avoid confusion with VersionStorageThere are both VersionStorage (singular, Version2) and VersionsStorage (plural, Versions) in the codebase. A short KDoc on the interface and methods will help contributors distinguish them quickly.
Example:
/** * Stores parent versions for a child resource (child -> set of parent ResourceIds). * Parallel to VersionStorage which stores single parent-child edges. */ interface VersionsStorage : Storage<Versions> { /** Replace the set of parents for the given child. */ fun setParents(child: ResourceId, parents: Versions) = setValue(child, parents) /** Retrieve the set of parents for the given child. */ fun getParents(child: ResourceId): Versions = getValue(child) /** Remove all parents for the given child. */ fun removeParents(child: ResourceId) = remove(child) }app/src/test/java/dev/arkbuilders/arkmemo/VersionStorageTest.kt (1)
71-114: Nit: remove unused constants and imports after refactorROOT_PATH will be unused after switching to TemporaryFolder. Clean up to keep tests tidy.
app/src/main/res/layout/fragment_notes.xml (2)
13-23: Add contentDescription for accessibility on the extended FABExtended FAB should have a contentDescription for non-text modes or assistive tech. Use the same string as its label.
<com.google.android.material.floatingactionbutton.ExtendedFloatingActionButton android:id="@+id/fab_new" android:layout_width="wrap_content" android:layout_height="wrap_content" android:text="@string/ark_memo_close" android:layout_gravity="bottom|end" android:visibility="gone" + android:contentDescription="@string/ark_memo_close" android:layout_marginEnd="16dp" android:layout_marginBottom="20dp" app:icon="@drawable/add"/>
49-55: Naming consistency: ‘fab_paste’ is a ButtonMinor naming mismatch: the id implies a FAB, but the widget is a Button. Consider renaming to btn_paste for clarity (and update references).
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
131-148: Inefficient text parsing and potential index out of bounds.The code parses the first line of text twice (Lines 144 and 154) and uses
take(20)which could be extracted to a constant. Also,split("\n")[0]could throw if the text is empty.Extract common logic and add safety:
+companion object{ + const val TAG = "text-notes-fragment" + private const val TITLE_MAX_LENGTH = 20 + + private fun extractTitleFromText(text: String): String = + text.lines().firstOrNull()?.take(TITLE_MAX_LENGTH) ?: "" private fun readArguments() { if(arguments != null) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) requireArguments().getParcelable(NOTE_KEY, TextNote::class.java)?.let { prepare(it) checkNoteForReadOnly() } else requireArguments().getParcelable<TextNote>(NOTE_KEY)?.let { prepare(it) checkNoteForReadOnly() } requireArguments().getString(NOTE_PASTE_KEY)?.let { data = it - title = it.split("\n")[0].take(20) + title = extractTitleFromText(it) } etNote.setText(data) } } private fun prepare(newNote: TextNote) { note = newNote data = newNote.text title = newNote.title.ifEmpty { // For backward compatibility, text notes before dedicated properties storage - data.split("\n")[0].take(20) + extractTitleFromText(data) } }app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt (1)
3-4: Remove duplicate import statement.There are two imports for
Build- one direct import and one forBuild.VERSION.SDK_INT. This is redundant.import android.os.Build -import android.os.Build.VERSION.SDK_INTThen update Line 95:
-if (SDK_INT >= Build.VERSION_CODES.TIRAMISU) +if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (1)
115-129: Consider extracting SDK version check logicThe repeated SDK version check pattern for
getParcelablecould be extracted into a utility function to improve maintainability and reduce duplication.Consider creating an extension function:
// In a utilities file inline fun <reified T : Parcelable> Bundle.getParcelableCompat(key: String): T? { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { getParcelable(key, T::class.java) } else { @Suppress("DEPRECATION") getParcelable(key) } }Then simplify the code:
private fun readArguments() { if (arguments != null) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) - requireArguments().getParcelable(GRAPHIC_NOTE_KEY, GraphicNote::class.java)?.let { - note = it - graphicNotesViewModel.onNoteOpened(note) - checkNoteForReadOnly() - } - else requireArguments().getParcelable<GraphicNote>(GRAPHIC_NOTE_KEY)?.let { + requireArguments().getParcelableCompat<GraphicNote>(GRAPHIC_NOTE_KEY)?.let { note = it graphicNotesViewModel.onNoteOpened(note) checkNoteForReadOnly() } } }app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt (1)
88-92: Inconsistent dialog method namingThe NoteDeleteDialog uses
setNote()method which differs from typical Android dialog patterns that usewithorcreatebuilders.Consider using a more idiomatic builder pattern or factory method for consistency with Android conventions.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (1)
176-195: TODO methods should throw NotImplementedErrorThe unimplemented methods use
TODO()which will throw at runtime, but it's better to be explicit about the error type.override fun getValue(id: ResourceId): Version2 { - TODO("Not yet implemented") + throw NotImplementedError("getValue is not yet implemented") } override fun remove(id: ResourceId) { - TODO("Not yet implemented") + throw NotImplementedError("remove is not yet implemented") } override fun setValue( id: ResourceId, value: Version2 ) { - TODO("Not yet implemented") + throw NotImplementedError("setValue is not yet implemented") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (40)
README.md(1 hunks)app/build.gradle(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/di/RepositoryModule.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/VersionsResult.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionsStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Version.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Versions.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorageRepo.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/NoteDeleteDialog.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/ArkFiles.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/Config.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/Constants.kt(1 hunks)app/src/main/res/drawable/ic_content_copy.xml(1 hunks)app/src/main/res/drawable/ic_history.xml(1 hunks)app/src/main/res/drawable/ic_note.xml(1 hunks)app/src/main/res/layout/fragment_edit_notes.xml(1 hunks)app/src/main/res/layout/fragment_notes.xml(3 hunks)app/src/main/res/layout/note.xml(3 hunks)app/src/main/res/layout/recyclerview.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)app/src/test/java/dev/arkbuilders/arkmemo/VersionStorageTest.kt(1 hunks)build.gradle(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)settings.gradle(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-18T05:40:19.372Z
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Applied to files:
README.mdapp/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt
📚 Learning: 2025-07-18T05:33:52.382Z
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Applied to files:
app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.ktapp/src/main/java/dev/arkbuilders/arkmemo/models/Note.ktapp/src/main/java/dev/arkbuilders/arkmemo/di/RepositoryModule.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.ktapp/src/main/res/layout/fragment_notes.xmlapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt
🧬 Code Graph Analysis (8)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (3)
setValue(190-195)getValue(176-178)remove(186-188)
app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt (1)
addCommand(31-33)
app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/NoteDeleteDialog.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/ui/views/Toast.kt (1)
toast(6-8)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (2)
app/src/main/java/dev/arkbuilders/arkmemo/utils/Utils.kt (1)
observeSaveResult(17-30)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (3)
initUI(66-113)readArguments(115-129)checkNoteForReadOnly(131-143)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorage.kt (2)
childrenNotParents(20-20)parentsTreeByChild(16-18)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt (5)
app/src/main/java/dev/arkbuilders/arkmemo/ui/views/Toast.kt (1)
toast(6-8)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt (1)
init(25-29)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt (2)
init(35-44)readAllNotes(46-50)app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepo.kt (2)
init(5-14)init(7-7)app/src/main/java/dev/arkbuilders/arkmemo/repo/text/TextNotesRepo.kt (1)
init(36-39)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt (3)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (2)
readArguments(131-148)initUI(88-129)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (2)
readArguments(115-129)initUI(66-113)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt (1)
initUI(103-145)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (3)
app/src/main/java/dev/arkbuilders/arkmemo/utils/Utils.kt (1)
observeSaveResult(17-30)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (3)
initUI(88-129)readArguments(131-148)checkNoteForReadOnly(166-180)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt (2)
initUI(87-91)readArguments(93-103)
🪛 detekt (1.23.8)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorageRepo.kt
[warning] 3-4: The class or object VersionsStorageRepo is empty.
(detekt.empty-blocks.EmptyClassBlock)
🪛 markdownlint-cli2 (0.17.2)
README.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (17)
app/src/main/java/dev/arkbuilders/arkmemo/utils/Config.kt (1)
4-4: Good use of SharedPreferences.edit DSLConcise, safe, and consistent with AndroidX best practices.
app/src/main/res/values/strings.xml (1)
30-33: LGTM: Strings added for versioning UX are clear and consistentThe new labels read well and align with the newly introduced UI/actions.
app/build.gradle (1)
94-97: Hilt upgrade confirmed — optional KSP migrationVerified: Hilt 2.57 and androidx.hilt:hilt-compiler:1.2.0 are present; the only kapt usage in the repo is the Hilt compilers and the kotlin-kapt plugin — no other
kapt,annotationProcessor, orkspreferences found.
- app/build.gradle
- plugins: id 'kotlin-kapt' (near top)
- dependencies: kapt "com.google.dagger:hilt-compiler:2.57" and kapt 'androidx.hilt:hilt-compiler:1.2.0' (lines ~95-96)
Snippet (unchanged):
implementation "com.google.dagger:hilt-android:2.57" kapt "com.google.dagger:hilt-compiler:2.57" kapt 'androidx.hilt:hilt-compiler:1.2.0'Optional: you can migrate these kapt entries to KSP for faster builds (add the KSP plugin and replace
kaptwithksp) — I can prepare a migration patch if you’d like.app/src/main/java/dev/arkbuilders/arkmemo/models/VersionsResult.kt (2)
6-10: LGTM: cohesive model to carry version graph slicesThe data shape is clear and aligns with storage reads.
7-9: Confirm Set/List semantics for parents vs childrenParents is a Set while children is a List. If both are identifiers, asymmetry may be accidental unless children ordering or duplicates matter. Please confirm intended semantics.
Would you like me to scan usages of VersionsResult.children to detect ordering/duplication reliance?
app/src/main/res/layout/recyclerview.xml (1)
8-10: Renamed ID verified — no stale references to R.id.recyclerView foundShort explanation: searches for R.id.recyclerView and @id/recyclerView returned no hits. The code uses the new ID (rv_notes) and view binding.
Files checked / evidence:
- app/src/main/res/layout/recyclerview.xml — android:id="@+id/rv_notes"
- app/src/main/res/layout/fragment_notes.xml — android:id="@+id/rv_notes_binding", includes layout="@layout/recyclerview"
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt — rvNotes = binding.rvNotesBinding.rvNotes
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt — rvNotes = binding.rvNotesBinding.rvNotes
Action: none required — rename appears fully migrated.
app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt (1)
12-13: Alignment with updated Note contract looks goodOverriding description and adding the isForked flag correctly aligns GraphicNote with the new Note API. Defaults and mutability are appropriate.
app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt (1)
11-16: TextNote correctly adopts new Note membersThe overrides for description and isForked match the updated Note interface. Parcelable usage remains consistent. No issues spotted here.
app/src/main/res/layout/note.xml (2)
2-7: Layout switch to LinearLayout is reasonableSimplifies the layout and matches current child structure. No functional issues spotted with the container change.
81-89: Divider placement is fineMoving the divider outside the header row makes sense visually. No issues.
app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt (1)
6-11: Note implementers updated — no action requiredVerified implementers:
- app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt — overrides description & isForked; @parcelize present; resource is annotated @IgnoredOnParcel. (OK)
- app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt — overrides description & isForked; @parcelize present; resource is annotated @IgnoredOnParcel. (OK)
Other files that reference Note (ViewModels, Fragments, Repo, Dialog) are callers, not implementers, and need no changes.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt (1)
13-21: Ignore — no early-init risk for memoPreferences.getNotesStorage()Quick verification: VersionStorageRepo.provide() is only invoked from VersionsViewModel.init(), and that init() is called from Fragments (VersionsFragment, NotesFragment) — after Application.onCreate. App is annotated with @HiltAndroidApp and MemoPreferencesImpl is injected with @ApplicationContext and constructs SharedPreferences in its constructor; getNotesStorage() simply returns Path(getPath()). Given this, memoPreferences is available by the time provide() runs and the original worry is not applicable.
Relevant locations:
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt — provide()
- app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt — versionStorageRepo.provide()
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt and NotesFragment.kt — calls to VersionsViewModel.init()
- app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.kt — getNotesStorage() uses SharedPreferences + Path(...)
- app/src/main/java/dev/arkbuilders/arkmemo/App.kt — @HiltAndroidApp (Hilt/App init)
If you still want extra defensiveness, validate that getPath() is non-empty or wrap Path(...) with a clear exception message — otherwise no change required.
Likely an incorrect or invalid review comment.
app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt (1)
21-24: Guard against uninitialized ViewModel in NotesCanvasonSizeChanged and onDraw access viewModel before it may be set via setViewModel(...), which can throw UninitializedPropertyAccessException for a lateinit property.
- File: app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt
- onSizeChanged: lines 21–24
- onDraw: lines 26–34
Suggested change:
override fun onSizeChanged(w: Int, h: Int, oldw: Int, oldh: Int) { super.onSizeChanged(w, h, oldw, oldh) - viewModel.svg().setViewBox(w.toFloat(), h.toFloat()) + if (::viewModel.isInitialized) { + viewModel.svg().setViewBox(w.toFloat(), h.toFloat()) + } } override fun onDraw(canvas: Canvas) { super.onDraw(canvas) - val paths = viewModel.paths() + if (!::viewModel.isInitialized) return + val paths = viewModel.paths() if (paths.isNotEmpty()) { paths.forEach { canvas.drawPath(it.path, it.paint) } } }Please verify call sites ensure setViewModel(viewModel) is invoked before the view is laid out/drawn.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt (1)
1-15: LGTM: Minimal, clear forwarding APIThe interface cleanly wraps Storage with intention-revealing methods. Naming reads well and the defaults are concise.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt (1)
52-56: saveVersion is provided by all callers — no change neededI searched the repo for onSaveClick usages. Every call site passes a saveVersion lambda; there’s no ArkRecorderFragment call that omits it. Keeping the parameter required is correct.
- Callers found:
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (passes saveVersion — creates version & updates latest ID)
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (passes saveVersion — creates version & updates latest ID)
- Definition:
- app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt (onSaveClick at ~line 52)
Remove the earlier suggestion to make saveVersion optional — the original review comment is incorrect.
Likely an incorrect or invalid review comment.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (1)
61-64: No leak found — MainActivity.fragment is used as a current-fragment pointer and is updated consistentlyMainActivity declares and restores
var fragment: Fragmentand the codebase updates that field when replacing/restoring fragments; fragments also setactivity.fragment = thisin onResume to keep the pointer current. Given the repository usage, this does not introduce a retained/stale reference leak.Relevant locations inspected:
- app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt —
var fragment: Fragment = NotesFragment()and updates in showFragment()/onCreate/onOptionsItemSelected/onSaveInstanceState- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt — onResume sets
activity.fragment = this- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt — sets
activity.fragment = this- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt — assigns
activity.fragmentfor editor fragments and in onResume- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/SettingsFragment.kt — sets
activity.fragment = this- app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt — assigns
activity.fragmentbefore replacingConclusion: no change required for the EditGraphicNotesFragment onResume assignment.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt (1)
63-68: Missing closing brace increateVersionfunctionThere's an extra closing brace on line 68 that doesn't match the opening braces, indicating a syntax error.
Fix the brace mismatch:
fun createVersion(oldId: ResourceId, newId: ResourceId) { viewModelScope.launch(Dispatchers.IO) { - val version = Version(oldId, newId) - versionStorage.add(version) - } + val version = Version(oldId, newId) + versionStorage.add(version) } - } +}Likely an incorrect or invalid review comment.
| @Provides | ||
| fun provideVersionStorageRepo( | ||
| memoPreferences: MemoPreferences | ||
| ) = VersionStorageRepo(memoPreferences) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope VersionStorageRepo as a Singleton and make the return type explicit
Without @singleton, Hilt may create multiple instances, defeating the cache-per-root behavior and increasing I/O. Also, adding an explicit return type improves DI clarity.
Apply this diff:
- @Provides
- fun provideVersionStorageRepo(
- memoPreferences: MemoPreferences
- ) = VersionStorageRepo(memoPreferences)
+ @Singleton
+ @Provides
+ fun provideVersionStorageRepo(
+ memoPreferences: MemoPreferences
+ ): VersionStorageRepo = VersionStorageRepo(memoPreferences)Additionally, add the missing import:
import javax.inject.Singleton🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/di/RepositoryModule.kt around lines
35 to 38, the provideVersionStorageRepo provider is missing a @Singleton scope
and an explicit return type; annotate the function with @Singleton, change its
signature to return VersionStorageRepo explicitly, and add the import
javax.inject.Singleton at the top of the file so Hilt will create a single
instance and the DI contract is clear.
| override fun valueFromString(raw: String): Versions = | ||
| raw.split(",").filter { it.isNotEmpty() }.map { | ||
| ResourceId.fromString(it) | ||
| }.toSet() |
There was a problem hiding this comment.
Handle potential parsing errors when converting strings to ResourceIds.
The valueFromString method could throw exceptions if ResourceId.fromString() encounters malformed input. This could happen if the storage file gets corrupted or manually edited.
Add error handling to prevent crashes:
override fun valueFromString(raw: String): Versions =
- raw.split(",").filter { it.isNotEmpty() }.map {
- ResourceId.fromString(it)
- }.toSet()
+ raw.split(",").filter { it.isNotEmpty() }.mapNotNull {
+ try {
+ ResourceId.fromString(it)
+ } catch (e: Exception) {
+ Log.e("RootVersionsStorage", "Failed to parse ResourceId: $it", e)
+ null
+ }
+ }.toSet()Don't forget to add the import:
import android.util.Log🤖 Prompt for AI Agents
In
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionsStorage.kt
around lines 17 to 20, the current valueFromString blindly calls
ResourceId.fromString and can throw on malformed items; wrap the mapping in safe
parsing (try/catch per item), skip or filter out invalid entries, and log parse
failures with android.util.Log so the method never throws—return the set of
successfully parsed ResourceId objects (or an empty set if none) and add the
import android.util.Log.
| val newVersion = Version( | ||
| versionIdIsChild?.parent!!, | ||
| versionIdIsParent?.child!! |
There was a problem hiding this comment.
Force unwraps could cause NPE in version graph operations
Lines 88-89 use !! which will crash if either versionIdIsChild or versionIdIsParent is null. This could happen if the version graph is corrupted or if there's a race condition.
Add null safety checks:
+ if (versionIdIsChild == null || versionIdIsParent == null) {
+ Log.e(VERSIONS_STORAGE, "Inconsistent version graph state for id: $id")
+ return
+ }
val newVersion = Version(
- versionIdIsChild?.parent!!,
- versionIdIsParent?.child!!
+ versionIdIsChild.parent,
+ versionIdIsParent.child
)📝 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 newVersion = Version( | |
| versionIdIsChild?.parent!!, | |
| versionIdIsParent?.child!! | |
| if (versionIdIsChild == null || versionIdIsParent == null) { | |
| Log.e(VERSIONS_STORAGE, "Inconsistent version graph state for id: $id") | |
| return | |
| } | |
| val newVersion = Version( | |
| versionIdIsChild.parent, | |
| versionIdIsParent.child | |
| ) |
🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt
around lines 87 to 89, the code force-unwraps versionIdIsChild and
versionIdIsParent with !! which can throw an NPE when either is null; replace
the force-unwraps with null-safe handling: check both variables before
constructing the new Version, and if either is null return a controlled failure
(e.g. throw a clear IllegalStateException/IllegalArgumentException with
contextual message or return a Result/nullable value) so you avoid crashing on
corrupted or racing version graphs and log the offending state for debugging.
| override fun parentsTreeByChild( | ||
| child: ResourceId | ||
| ): Map<ResourceId, List<ResourceId>> { | ||
| var localChild = child | ||
| var parent: ResourceId? | ||
| val parents = mutableListOf<ResourceId>() | ||
| for (version in versions) { | ||
| parent = versions.find { | ||
| it.child == localChild | ||
| }?.parent | ||
| if (parent != null && children.contains(parent)) | ||
| localChild = parent | ||
| if (parent != null) parents.add(parent) | ||
| if (!children.contains(parent)) | ||
| break | ||
| } | ||
| return mapOf(child to parents) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inefficient parent tree traversal algorithm
The parentsTreeByChild function has nested loops that could be optimized. The outer loop iterates through all versions, but the inner logic only needs to traverse the parent chain once.
Optimize the traversal:
override fun parentsTreeByChild(
child: ResourceId
): Map<ResourceId, List<ResourceId>> {
var localChild = child
- var parent: ResourceId?
val parents = mutableListOf<ResourceId>()
- for (version in versions) {
- parent = versions.find {
- it.child == localChild
- }?.parent
+ while (true) {
+ val parent = versions.find { it.child == localChild }?.parent ?: break
if (parent != null && children.contains(parent))
localChild = parent
- if (parent != null) parents.add(parent)
- if (!children.contains(parent))
- break
+ parents.add(parent)
+ if (!children.contains(parent)) break
}
return mapOf(child to parents)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt
around lines 101 to 118, replace the current nested-loop traversal with a
single-parent-chain walk using a precomputed map: build a Map<ResourceId,
ResourceId?> from version.child -> version.parent once, then set localChild =
child and loop while true to lookup parent = map[localChild]; if parent is null
break; add parent to parents; if children.contains(parent) then set localChild =
parent and continue, otherwise break; return mapOf(child to parents). This
eliminates repeated versions.find calls and reduces complexity to O(n) for the
map build plus O(depth) per lookup.
| private suspend fun readStorage(): VersionsResult = | ||
| withContext(Dispatchers.IO) { | ||
| val lines = Files.readAllLines(storageFile) | ||
| val storageVersion = lines.removeAt(0) | ||
| verifyVersion(storageVersion) | ||
| val versions = lines.map { | ||
| val parts = it.split(KEY_VALUE_SEPARATOR) | ||
| val parent = ResourceId.fromString(parts[0]) | ||
| val child = ResourceId.fromString(parts[1]) | ||
| Log.d( | ||
| VERSIONS_STORAGE, | ||
| it | ||
| ) | ||
| Version(parent, child) | ||
| } | ||
| val parents = versions.map { | ||
| Log.d( | ||
| VERSIONS_STORAGE, | ||
| "parent: ${it.parent}" | ||
| ) | ||
| it.parent | ||
| }.toSet() | ||
| val children = versions.map { | ||
| Log.d( | ||
| VERSIONS_STORAGE, | ||
| "child: ${it.child}" | ||
| ) | ||
| it.child | ||
| } | ||
| return@withContext VersionsResult( | ||
| versions, | ||
| parents, | ||
| children | ||
| ) | ||
| } |
There was a problem hiding this comment.
No error handling for malformed storage file
The readStorage() function could throw various exceptions (IndexOutOfBoundsException, IllegalArgumentException) if the storage file is corrupted or malformed.
Add proper error handling:
private suspend fun readStorage(): VersionsResult =
withContext(Dispatchers.IO) {
+ try {
val lines = Files.readAllLines(storageFile)
+ if (lines.isEmpty()) {
+ Log.w(VERSIONS_STORAGE, "Storage file is empty")
+ return@withContext VersionsResult(emptyList(), emptySet(), emptyList())
+ }
val storageVersion = lines.removeAt(0)
verifyVersion(storageVersion)
val versions = lines.map {
val parts = it.split(KEY_VALUE_SEPARATOR)
+ if (parts.size != 2) {
+ Log.e(VERSIONS_STORAGE, "Malformed version line: $it")
+ throw IllegalStateException("Malformed storage file")
+ }
val parent = ResourceId.fromString(parts[0])
val child = ResourceId.fromString(parts[1])
// ... rest of the code
}
// ... rest of the code
+ } catch (e: Exception) {
+ Log.e(VERSIONS_STORAGE, "Failed to read storage", e)
+ throw e
+ }
}📝 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.
| private suspend fun readStorage(): VersionsResult = | |
| withContext(Dispatchers.IO) { | |
| val lines = Files.readAllLines(storageFile) | |
| val storageVersion = lines.removeAt(0) | |
| verifyVersion(storageVersion) | |
| val versions = lines.map { | |
| val parts = it.split(KEY_VALUE_SEPARATOR) | |
| val parent = ResourceId.fromString(parts[0]) | |
| val child = ResourceId.fromString(parts[1]) | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| it | |
| ) | |
| Version(parent, child) | |
| } | |
| val parents = versions.map { | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| "parent: ${it.parent}" | |
| ) | |
| it.parent | |
| }.toSet() | |
| val children = versions.map { | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| "child: ${it.child}" | |
| ) | |
| it.child | |
| } | |
| return@withContext VersionsResult( | |
| versions, | |
| parents, | |
| children | |
| ) | |
| } | |
| private suspend fun readStorage(): VersionsResult = | |
| withContext(Dispatchers.IO) { | |
| try { | |
| val lines = Files.readAllLines(storageFile) | |
| if (lines.isEmpty()) { | |
| Log.w(VERSIONS_STORAGE, "Storage file is empty") | |
| return@withContext VersionsResult(emptyList(), emptySet(), emptyList()) | |
| } | |
| val storageVersion = lines.removeAt(0) | |
| verifyVersion(storageVersion) | |
| val versions = lines.map { | |
| val parts = it.split(KEY_VALUE_SEPARATOR) | |
| if (parts.size != 2) { | |
| Log.e(VERSIONS_STORAGE, "Malformed version line: $it") | |
| throw IllegalStateException("Malformed storage file") | |
| } | |
| val parent = ResourceId.fromString(parts[0]) | |
| val child = ResourceId.fromString(parts[1]) | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| it | |
| ) | |
| Version(parent, child) | |
| } | |
| val parents = versions.map { | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| "parent: ${it.parent}" | |
| ) | |
| it.parent | |
| }.toSet() | |
| val children = versions.map { | |
| Log.d( | |
| VERSIONS_STORAGE, | |
| "child: ${it.child}" | |
| ) | |
| it.child | |
| } | |
| return@withContext VersionsResult( | |
| versions, | |
| parents, | |
| children | |
| ) | |
| } catch (e: Exception) { | |
| Log.e(VERSIONS_STORAGE, "Failed to read storage", e) | |
| throw e | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt
around lines 140 to 174, readStorage() currently assumes the file format is
correct and will throw on malformed input; wrap the file parsing in a try/catch
that catches IO and parsing-related exceptions (IndexOutOfBoundsException,
IllegalArgumentException, any exceptions from ResourceId.fromString), validate
the file has at least one line before removeAt(0), ensure each data line splits
into exactly two parts before using parts[0]/parts[1], handle or log malformed
lines (skip them) and either return an empty/safe VersionsResult or rethrow a
clear, custom exception with context; ensure errors are logged with details so
callers can react appropriately.
| fun parents_tree_is_correct() { | ||
| scope.launch { | ||
| versionStorage.add(version1) | ||
| versionStorage.add(version2) | ||
| versionStorage.add(version3) | ||
| versionStorage.add(version5) | ||
| versionStorage.add(version4) | ||
| val map1 = versionStorage.parentsTreeByChild( | ||
| child1 | ||
| ) | ||
| assertEquals(parentsTree1, map1) | ||
| val map2 = versionStorage.parentsTreeByChild( | ||
| parent1 | ||
| ) | ||
| assertEquals(parentsTree2, map2) | ||
| val map3 = versionStorage.parentsTreeByChild( | ||
| parent2 | ||
| ) | ||
| assertEquals(parentsTree3, map3) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid shared mutable storage across tests and filesystem coupling
The same RootVersionStorage instance and ./ path are reused across tests, causing cross-test contamination. Use a per-test temp directory and recreate storage in @before.
🤖 Prompt for AI Agents
In app/src/test/java/dev/arkbuilders/arkmemo/VersionStorageTest.kt around lines
30 to 50, the test reuses the same RootVersionStorage and hard-coded "./" path
which causes cross-test contamination and filesystem coupling; change the test
setup to create a fresh temp directory for each test and initialize a new
VersionStorage instance in a @Before method (and delete the temp directory in
@After), then update the test to use that per-test storage (and use runBlocking
or equivalent to execute suspending calls synchronously instead of scope.launch
so the test waits for storage operations to complete).
build.gradle
Outdated
| id 'com.android.application' version '8.11.1' apply false | ||
| id 'org.jetbrains.kotlin.android' version '2.2.0' apply false | ||
| id 'com.google.dagger.hilt.android' version '2.57' apply false |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify AGP/Kotlin plugin versions and Gradle wrapper compatibility
AGP 8.11.1 and Kotlin 2.2.0 are quite new; ensure your Gradle wrapper and Android Gradle Plugin combo are supported and available.
Check current wrapper and declared plugin versions:
And confirm latest stable versions and compatibility:
🏁 Script executed:
#!/bin/bash
set -e
echo "AGP in root build.gradle:"
rg -n "id 'com.android.application' version" -n build.gradle
echo -e "\nGradle wrapper distributionUrl:"
fd gradle-wrapper.properties | xargs -I{} sh -c 'echo "== {} ==" && rg -n "distributionUrl" "{}"'Length of output: 513
🌐 Web query:
What is the latest stable Android Gradle Plugin version and which Gradle versions does it support?
💡 Result:
The latest stable Android Gradle Plugin (AGP) version is 8.11.1, released in July 2025[8][10]. (Versions 8.12.x and 8.13.x are available only as alpha or release candidates[10].)
Gradle compatibility:
AGP 8.11.x requires Gradle 8.13 as the minimum and default supported version[9].
Older AGP 8.10.x and 8.9.x require Gradle 8.11.1[1][7].
There is no official stable AGP release yet supporting Gradle 9.0 (the latest Gradle release[6][3]), so use Gradle 8.13 when using AGP 8.11.x[9].
Summary Table
| AGP Version | Stable Release? | Required Gradle Version |
|---|---|---|
| 8.11.1 | Yes | 8.13 |
| 8.12.x | Alpha/RC | Not officially stable |
| 8.10.x | Yes | 8.11.1 |
- Official compatibility and further requirements (JDK 17, SDK Build Tools 35.0.0) can be found in the AGP release notes[9].
- Using Gradle 9.0 with AGP is not currently recommended or officially supported as of August 2025[6][8].
Citations:
- 1: https://developer.android.com/build/releases/past-releases/agp-8-10-0-release-notes
- 2: https://kotlinlang.org/docs/gradle-configure-project.html
- 3: https://docs.gradle.org/current/release-notes.html
- 4: https://developer.android.com/build/releases/gradle-plugin
- 5: https://docs.gradle.org/current/userguide/compatibility.html
- 6: https://gradle.org/install/
- 7: https://developer.android.com/build/releases/past-releases/agp-8-9-0-release-notes
- 8: https://developer.android.com/reference/tools/gradle-api
- 9: https://developer.android.com/build/releases/past-releases/agp-8-11-0-release-notes
- 10: https://mvnrepository.com/artifact/com.android.tools.build/gradle
🌐 Web query:
Is Kotlin 2.2.0 officially compatible with the latest AGP, and what are the minimum supported Gradle versions?
💡 Result:
Kotlin 2.2.0 is officially compatible with the latest stable Android Gradle Plugin (AGP), and the minimum supported Gradle version for Kotlin 2.2.0 is 7.6.3.
- According to the official Kotlin documentation, Kotlin 2.2.0 is fully compatible with Gradle 7.6.3 through 8.14[4][7]. You can also use it with newer Gradle versions (up to the latest release), but if you do, you might see deprecation warnings and some new Gradle features may not be available or stable[4][7].
- The minimum supported Gradle version for Kotlin 2.2.0 is 7.6.3. Your project will not build with older Gradle versions[4][7][10].
Regarding AGP compatibility:
- Kotlin 2.2.0 can be used with the latest Android Gradle Plugin. As of August 2025, the latest AGP version is 8.12, which requires at least Gradle 8.13[9]. This means using Kotlin 2.2.0 with the latest AGP is supported, provided your Gradle version is at least 8.13 (to meet AGP’s requirements)[9].
- The official compatibility matrix and documentation note that forward compatibility between the Kotlin Gradle plugin and Kotlin compiler or libraries is not always guaranteed, so it’s best to keep all related tooling aligned as much as possible[2][4].
Summary Table
| Kotlin Version | Minimum Gradle Version | Latest Fully Supported Gradle Version | Notes on AGP |
|---|---|---|---|
| 2.2.0 | 7.6.3 | 8.14 | Compatible with latest AGP (e.g., 8.12, which requires Gradle 8.13+) |
- You should ensure your build tools are at least the minimum versions listed above for reliable support.
- For maximum stability, match Kotlin, AGP, and Gradle versions as closely as possible, and pay attention to any warnings about deprecations or incompatibilities in your build output.
References:
- Official documentation and compatibility guidance[4][7][10].
- AGP and Gradle version requirements[9].
- Recent discussions in the Kotlin community confirming all of the above[2][10].
Citations:
- 1: https://docs.gradle.org/current/userguide/compatibility.html
- 2: https://slack-chats.kotlinlang.org/t/29177439/when-updating-kotlin-to-2-2-0-i-m-getting-https-github-com-t
- 3: https://kotlinlang.org/docs/releases.html
- 4: https://kotlinlang.org/docs/whatsnew22.html
- 5: https://developer.genesys.cloud/forum/t/transport-mobile-sdk-android-minimum-gradle-and-kotlin-version/25324
- 6: https://developer.android.com/jetpack/androidx/releases/lifecycle
- 7: https://kotlinlang.org/docs/gradle-configure-project.html
- 8: Minimum supported Gradle version is 6.5 authpass/biometric_storage#33
- 9: https://developer.android.com/build/releases/gradle-plugin
- 10: https://slack-chats.kotlinlang.org/t/29719870/hey-is-it-fine-to-use-kotlin-2-2-0-without-updating-gradle-t
Action required — align Gradle wrapper with AGP 8.11.1 (or upgrade AGP)
Root build.gradle uses AGP 8.11.1 but gradle-wrapper is set to Gradle 9.0.0. AGP 8.11.1 requires Gradle 8.13; Gradle 9.0 is not officially supported for this AGP release.
Files to fix:
- build.gradle — lines ~10-12:
id 'com.android.application' version '8.11.1' apply false - gradle/wrapper/gradle-wrapper.properties — distributionUrl currently points to gradle-9.0.0
Suggested change (preferred): downgrade wrapper to Gradle 8.13:
--- gradle/wrapper/gradle-wrapper.properties
+++ gradle/wrapper/gradle-wrapper.properties
@@
-distributionUrl=https://services.gradle.org/distributions/gradle-9.0.0-bin.zip
+distributionUrl=https://services.gradle.org/distributions/gradle-8.13-bin.zip
Notes: Kotlin 2.2.0 is compatible with AGP 8.11.1; the primary issue is the Gradle wrapper mismatch. If you intentionally want Gradle 9.0, update AGP to a version that officially supports Gradle 9 (alpha/RC only as of Aug 2025) or accept instability.
🤖 Prompt for AI Agents
In build.gradle around lines 10-12 the project uses AGP 8.11.1 while
gradle/wrapper/gradle-wrapper.properties points to Gradle 9.0.0; change the
wrapper to Gradle 8.13 by updating distributionUrl in
gradle/wrapper/gradle-wrapper.properties to point to gradle-8.13-bin.zip (or
alternatively upgrade the AGP to a version that officially supports Gradle 9 if
that is intentional), then run ./gradlew wrapper to regenerate wrapper files and
verify builds succeed.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
.github/workflows/build.yml (2)
9-14: Set explicit token permissions and avoid exporting GITHUB_TOKEN globallyIf Gradle resolves dependencies from GitHub Packages, you’ll need packages: read. Also, avoid exposing GITHUB_TOKEN at job scope; inject it only where needed to minimize leak surface.
- Add permissions at the job level:
build: runs-on: ubuntu-latest environment: Development + permissions: + contents: read + packages: read env: ACRA_LOGIN: ${{ secrets.ACRARIUM_BASIC_AUTH_LOGIN }} ACRA_PASS: ${{ secrets.ACRARIUM_BASIC_AUTH_PASSWORD }} ACRA_URI: ${{ secrets.ACRARIUM_URI }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- If Gradle needs the token, pass it only to the build step (outside this range), e.g.:
- name: Build Release APK run: ./gradlew --no-daemon --stacktrace assembleRelease env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}Note: GITHUB_TOKEN is provided automatically by GitHub; you typically don’t need to re-expose it unless your build expects it in the environment explicitly. Ensure no tasks log it.
35-52: Fail fast on missing APKs and reduce artifact surprisesUpload step should error if files are missing to catch build or path issues early.
Apply these diffs:
- name: Upload Release arm64-v8a APK uses: actions/upload-artifact@v4 with: name: release-arm64-v8a-apk path: ./app/build/outputs/apk/release/app-arm64-v8a-release.apk + if-no-files-found: error- name: Upload Release armeabi-v7a APK uses: actions/upload-artifact@v4 with: name: release-armeabi-v7a-apk path: ./app/build/outputs/apk/release/app-armeabi-v7a-release.apk + if-no-files-found: error- name: Upload Release universal APK uses: actions/upload-artifact@v4 with: name: release-universal-apk path: ./app/build/outputs/apk/release/app-universal-release.apk + if-no-files-found: errorOptional:
- Add retention-days to control storage costs.
- Alternatively, upload all APKs in one step using a glob if you don’t need separate artifacts.
🧹 Nitpick comments (3)
.github/workflows/build.yml (3)
15-15: Optional: Pin actions to commit SHA for supply-chain hardeningPinning to a specific SHA reduces the risk of supply-chain attacks. Keep the major version as a comment for clarity.
Example:
- uses: actions/checkout@v4 + uses: actions/checkout@3df4ab11eba7bda6032a0b82c2376cf9d2c1a0f0 # v4
32-34: Improve Gradle invocation for CI ergonomicsAdd --no-daemon and --stacktrace for CI. Optionally adopt gradle/actions/setup-gradle for better caching and config-cache integration.
Apply this diff:
- - name: Build Release APK - run: ./gradlew assembleRelease + - name: Build Release APK + run: ./gradlew --no-daemon --stacktrace assembleReleaseOptional caching improvement (place before the build step):
- name: Setup Gradle uses: gradle/actions/setup-gradle@v3
5-9: Consider removing the environment binding for a pure build jobBinding the job to an environment (Development) can require approvals and block CI. If you only need repo secrets (not environment-scoped secrets), drop the environment to streamline builds.
Proposed change:
build: runs-on: ubuntu-latest - environment: Development
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/build.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
.github/workflows/build.yml (2)
24-26: LGTM: Using the official Gradle wrapper validationCorrect placement and version. This helps protect against tampered wrappers.
15-26: Order of steps looks correctCheckout precedes wrapper validation, and Java setup precedes Gradle. No issues here after fixing the JDK distribution.
.github/workflows/build.yml
Outdated
| name: Build the app | ||
|
|
||
| on: [push] | ||
| on: [ push ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broaden triggers: build on PRs and allow manual runs
Currently runs only on push. Recommend adding pull_request and workflow_dispatch and restricting branches to reduce noise.
Apply this diff:
-on: [ push ]
+on:
+ push:
+ branches: [ main, develop ]
+ pull_request:
+ branches: [ main, develop ]
+ workflow_dispatch:📝 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.
| on: [ push ] | |
| on: | |
| push: | |
| branches: [ main, develop ] | |
| pull_request: | |
| branches: [ main, develop ] | |
| workflow_dispatch: |
🤖 Prompt for AI Agents
In .github/workflows/build.yml at line 3, the workflow is only triggered on push
which is noisy and misses PRs and manual runs; update the workflow "on:" section
to include pull_request (restricted to the same target branches as push), add
workflow_dispatch for manual runs, and scope push/pull_request to a branches
list (e.g., main, release/*) to reduce noise; ensure YAML indentation is correct
when adding these keys.
| - name: Set up JDK 17 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '17' | ||
| distribution: 'adopt' | ||
| cache: gradle | ||
|
|
There was a problem hiding this comment.
Fix: setup-java v4 no longer supports distribution 'adopt'
actions/setup-java@v4 does not accept 'adopt'. Use 'temurin' for Java 17 (AdoptOpenJDK migrated to Eclipse Temurin).
Apply this diff:
- - name: Set up JDK 17
+ - name: Set up Temurin JDK 17
uses: actions/setup-java@v4
with:
java-version: '17'
- distribution: 'adopt'
+ distribution: 'temurin'
cache: gradle📝 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.
| - name: Set up JDK 17 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '17' | |
| distribution: 'adopt' | |
| cache: gradle | |
| - name: Set up Temurin JDK 17 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '17' | |
| distribution: 'temurin' | |
| cache: gradle |
🤖 Prompt for AI Agents
.github/workflows/build.yml around lines 17 to 23: the workflow uses
actions/setup-java@v4 with distribution: 'adopt' which is no longer supported;
update the distribution value to 'temurin' (keeping java-version: '17' and
cache: gradle) so the setup-java step uses Eclipse Temurin for Java 17.
| - name: Decrypt the keystore for signing | ||
| run: | | ||
| echo "${{ secrets.KEYSTORE_ENCRYPTED }}" > keystore.asc | ||
| gpg -d --passphrase "${{ secrets.KEYSTORE_PASSWORD }}" --batch keystore.asc > keystore.jks | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden GPG decryption and ensure non-interactive mode
Add --pinentry-mode loopback for non-interactive passphrase entry, use --output for clarity, remove stray spaces, and consider cleanup to avoid leaving secrets on disk.
Apply this diff:
- name: Decrypt the keystore for signing
run: |
echo "${{ secrets.KEYSTORE_ENCRYPTED }}" > keystore.asc
- gpg -d --passphrase "${{ secrets.KEYSTORE_PASSWORD }}" --batch keystore.asc > keystore.jks
+ gpg --batch --yes --pinentry-mode loopback \
+ --passphrase "${{ secrets.KEYSTORE_PASSWORD }}" \
+ --output keystore.jks \
+ --decrypt keystore.ascAdditionally, add a cleanup step after uploads:
- name: Cleanup signing material
if: always()
run: |
rm -f keystore.asc keystore.jks🤖 Prompt for AI Agents
.github/workflows/build.yml around lines 27 to 31: the GPG decryption step is
not fully non-interactive, includes stray spaces in the secret variable
reference, doesn’t use --output for clarity, and leaves secrets on disk; update
the gpg command to add --pinentry-mode loopback, remove the extra spaces in the
passphrase variable, and use --output keystore.jks (or equivalent) instead of
redirection, and add a post-upload cleanup step that always runs to remove
keystore.asc and keystore.jks so signing material is not left on disk.
…tracking-2 # Conflicts: # .github/workflows/build.yml # app/build.gradle # app/src/main/java/dev/arkbuilders/arkmemo/di/RepositoryModule.kt # app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt # app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt # app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/NoteDeleteDialog.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt # app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt # app/src/main/java/dev/arkbuilders/arkmemo/utils/Config.kt # app/src/main/java/dev/arkbuilders/arkmemo/utils/Constants.kt # app/src/main/res/layout/fragment_edit_notes.xml # app/src/main/res/layout/fragment_notes.xml # app/src/main/res/layout/note.xml # build.gradle # gradle/wrapper/gradle-wrapper.properties # settings.gradle
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
shubertm
temporarily deployed
to
Development
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/res/layout/dialog_common_action.xml (1)
20-56: Restore@+idwhen declaring these viewsSwitching the first declaration of each view from
@+id/...to@id/...means aapt now expects those IDs to be pre-defined (e.g., viaids.xml). Because they aren’t, the layout no longer provides IDs fortv_title,iv_close,tv_message, andtv_positive, so references to them resolve to 0 and the layout fails to build/inflate. Please revert these attributes back to@+id/....- android:id="@id/tv_title" /> + android:id="@+id/tv_title" /> ... - android:id="@id/iv_close" /> + android:id="@+id/iv_close" /> ... - android:id="@id/tv_message" /> + android:id="@+id/tv_message" /> ... - android:id="@id/tv_positive" /> + android:id="@+id/tv_positive" />app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt (1)
183-195: Keep data model in sync on Pause/Stop/ResumeOnly the UI is updated; the underlying VoiceNote flags aren’t adjusted. On rebind/recycle, the row can show an incorrect state. Update the item’s fields for consistency.
Apply this diff:
is ArkMediaPlayerSideEffect.PausePlaying -> { showPlaybackIdleState(holder, isPaused = true) + val pos = holder.bindingAdapterPosition + if (pos >= 0) { + val item = notes[pos] + if (item is VoiceNote) { + item.isPlaying = false + item.waitToBeResumed = true + } + } } is ArkMediaPlayerSideEffect.StopPlaying -> { showPlaybackIdleState(holder) holder.tvPlayingPosition.gone() + val pos = holder.bindingAdapterPosition + if (pos >= 0) { + val item = notes[pos] + if (item is VoiceNote) { + item.isPlaying = false + item.pendingForPlaybackReset = true + item.waitToBeResumed = false + } + } } is ArkMediaPlayerSideEffect.ResumePlaying -> { showPlayingState(holder) + val pos = holder.bindingAdapterPosition + if (pos >= 0) { + val item = notes[pos] + if (item is VoiceNote) { + item.isPlaying = true + item.waitToBeResumed = false + } + } }app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
193-205: Save button enablement wrongly depends on clipboard contentThis disables Save when clipboard is empty, even if note content changed.
Fix to use content state only:
- enableSaveText(!clipboardTextEmpty && isContentChanged()) + enableSaveText(isContentChanged() && !isContentEmpty())
🧹 Nitpick comments (9)
app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/BrushAdapter.kt (1)
78-97: Consider generalizing BrushColor tint resolution.With the additional color variants, the
whenbranch now repeats the sameimageTintListassignment multiple times. If theBrushColorsealed hierarchy can expose a@ColorRes(e.g., via constructor parameter), you could collapse these into a singleis BrushColorbranch and keep future color additions self-contained. Optional clean-up, no behavior change.app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt (1)
303-306: Fail‑safe default for storage checkDefaulting to true enables editing when not in NotesFragment. If the intent is to block editing when storage can’t be verified, prefer a false default.
Apply this diff:
- val storageFolderExist = - (activity.fragment as? NotesFragment)?.checkForStorageExistence() ?: true + val storageFolderExist = + (activity.fragment as? NotesFragment)?.checkForStorageExistence() ?: falsePlease confirm the desired behavior outside NotesFragment.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (2)
101-102: Remove empty TextWatcher method or switch to the lambda extensionThe empty beforeTextChanged block triggers detekt. Prefer the addTextChangedListener lambda to avoid no-op overrides.
Apply this refactor:
- val noteTitleChangeListener = - object : TextWatcher { - override fun beforeTextChanged( - s: CharSequence?, - start: Int, - count: Int, - after: Int, - ) { - } - - override fun onTextChanged( - s: CharSequence?, - start: Int, - before: Int, - count: Int, - ) { - title = s?.toString() ?: "" - if (title.isEmpty()) { - binding.edtTitle.hint = getString(R.string.hint_new_text_note) - } - enableSaveText(isContentChanged() && !isContentEmpty()) - } - - override fun afterTextChanged(s: Editable?) {} - } + noteTitle.addTextChangedListener( + onTextChanged = { s, _, _, _ -> + title = s?.toString() ?: "" + if (title.isEmpty()) { + binding.edtTitle.hint = getString(R.string.hint_new_text_note) + } + enableSaveText(isContentChanged() && !isContentEmpty()) + }, + )And remove the usage below:
- noteTitle.addTextChangedListener(noteTitleChangeListener)
175-178: Unregister listener before calling super.onDestroyView()Avoid relying on view being non-null after super.
- override fun onDestroyView() { - super.onDestroyView() - view?.viewTreeObserver?.removeOnWindowFocusChangeListener(windowFocusedListener) - } + override fun onDestroyView() { + view?.viewTreeObserver?.removeOnWindowFocusChangeListener(windowFocusedListener) + super.onDestroyView() + }app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionsStorage.kt (1)
21-21: Make serialization deterministicOrdering a Set is undefined; sort for stable files and diffs.
- override fun valueToString(value: Versions): String = value.joinToString(",") + override fun valueToString(value: Versions): String = + value.map { it.toString() }.sorted().joinToString(",")app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt (2)
81-86: Guard reads on initialization and remove nested callReturn safe defaults before init completes.
- fun getChildNotParentIds(): List<ResourceId> = versionStorage.childrenNotParents() + fun getChildNotParentIds(): List<ResourceId> = + if (isInitialized()) versionStorage.childrenNotParents() else emptyList() - fun isNotVersioned(id: ResourceId): Boolean = !isVersioned(id) - - fun isVersioned(id: ResourceId): Boolean = versionStorage.contains(id) + fun isNotVersioned(id: ResourceId): Boolean = !isVersioned(id) + fun isVersioned(id: ResourceId): Boolean = + if (isInitialized()) versionStorage.contains(id) else false
87-94: Ensure init for delete and latest-version checksAvoids access before storage is ready.
fun onDelete(id: ResourceId) { viewModelScope.launch { + ensureInitialized() versionStorage.forget(id) } } - fun isLatestResource(id: ResourceId): Boolean = versionStorage.isLatestResourceVersion(id) + fun isLatestResource(id: ResourceId): Boolean = + if (isInitialized()) versionStorage.isLatestResourceVersion(id) else falseAdd missing import once (outside shown range):
import kotlinx.coroutines.CompletableDeferredapp/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (1)
108-126: Optimize parent-chain traversal to O(depth)Current loop redundantly scans versions on each step.
- override fun parentsTreeByChild(child: ResourceId): Map<ResourceId, List<ResourceId>> { - var localChild = child - var parent: ResourceId? - val parents = mutableListOf<ResourceId>() - for (version in versions) { - parent = - versions.find { - it.child == localChild - }?.parent - if (parent != null && children.contains(parent)) { - localChild = parent - } - if (parent != null) parents.add(parent) - if (!children.contains(parent)) { - break - } - } - return mapOf(child to parents) - } + override fun parentsTreeByChild(child: ResourceId): Map<ResourceId, List<ResourceId>> { + val parentByChild = versions.associate { it.child to it.parent } + val chain = mutableListOf<ResourceId>() + var current = child + while (true) { + val parent = parentByChild[current] ?: break + chain.add(parent) + if (!children.contains(parent)) break + current = parent + } + return mapOf(child to chain) + }app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (1)
86-92: Tidy up the emptybeforeTextChangedoverrideDetekt still flags this empty block. Please collapse it to an expression body (or add a no-op comment) so the warning goes away. One option:
- override fun beforeTextChanged( - s: CharSequence?, - start: Int, - count: Int, - after: Int, - ) { - } + override fun beforeTextChanged( + s: CharSequence?, + start: Int, + count: Int, + after: Int, + ) = UnitBased on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
.github/workflows/build.yml(1 hunks)app/build.gradle(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/VersionsResult.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/models/VoiceNote.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionsStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Version.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Versions.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorageRepo.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/BrushAdapter.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/FilePickerDialog.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/AboutFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkMediaPlayerFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/GraphicNotesViewModel.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/SettingTextView.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/ArkFiles.kt(1 hunks)app/src/main/res/drawable/audio_wave_thumb.xml(1 hunks)app/src/main/res/drawable/bg_audio_view.xml(1 hunks)app/src/main/res/drawable/bg_big_radius.xml(1 hunks)app/src/main/res/drawable/bg_border_r8.xml(1 hunks)app/src/main/res/drawable/bg_brush_color_black.xml(1 hunks)app/src/main/res/drawable/bg_brush_option_selected.xml(1 hunks)app/src/main/res/drawable/bg_brush_size.xml(1 hunks)app/src/main/res/drawable/bg_graphic_control_text_selected.xml(1 hunks)app/src/main/res/drawable/bg_negative_button.xml(1 hunks)app/src/main/res/drawable/bg_positive_button.xml(1 hunks)app/src/main/res/drawable/bg_rect_button.xml(1 hunks)app/src/main/res/drawable/bg_red_button.xml(1 hunks)app/src/main/res/drawable/bg_round_button.xml(1 hunks)app/src/main/res/drawable/bg_selected_brush.xml(1 hunks)app/src/main/res/drawable/bg_tag.xml(1 hunks)app/src/main/res/drawable/ic_add.xml(1 hunks)app/src/main/res/drawable/ic_app_logo.xml(1 hunks)app/src/main/res/drawable/ic_attach.xml(1 hunks)app/src/main/res/drawable/ic_brush.xml(1 hunks)app/src/main/res/drawable/ic_btc.xml(1 hunks)app/src/main/res/drawable/ic_chevron_down.xml(1 hunks)app/src/main/res/drawable/ic_chevron_left.xml(1 hunks)app/src/main/res/drawable/ic_chevron_right.xml(1 hunks)app/src/main/res/drawable/ic_clipboard.xml(1 hunks)app/src/main/res/drawable/ic_close.xml(1 hunks)app/src/main/res/drawable/ic_coffee.xml(1 hunks)app/src/main/res/drawable/ic_copy.xml(1 hunks)app/src/main/res/drawable/ic_crash_report.xml(1 hunks)app/src/main/res/drawable/ic_debug_log.xml(1 hunks)app/src/main/res/drawable/ic_delete_note.xml(1 hunks)app/src/main/res/drawable/ic_discord.xml(1 hunks)app/src/main/res/drawable/ic_download.xml(1 hunks)app/src/main/res/drawable/ic_draw.xml(1 hunks)app/src/main/res/drawable/ic_edit.xml(1 hunks)app/src/main/res/drawable/ic_eraser.xml(1 hunks)app/src/main/res/drawable/ic_eth.xml(1 hunks)app/src/main/res/drawable/ic_feedback.xml(1 hunks)app/src/main/res/drawable/ic_globe.xml(1 hunks)app/src/main/res/drawable/ic_loading.xml(1 hunks)app/src/main/res/drawable/ic_mic.xml(1 hunks)app/src/main/res/drawable/ic_no_notes.xml(1 hunks)app/src/main/res/drawable/ic_patreon.xml(1 hunks)app/src/main/res/drawable/ic_pause.xml(1 hunks)app/src/main/res/drawable/ic_pause_circle.xml(1 hunks)app/src/main/res/drawable/ic_photo.xml(1 hunks)app/src/main/res/drawable/ic_pin.xml(1 hunks)app/src/main/res/drawable/ic_play.xml(1 hunks)app/src/main/res/drawable/ic_play_circle.xml(1 hunks)app/src/main/res/drawable/ic_privacy.xml(1 hunks)app/src/main/res/drawable/ic_rate.xml(1 hunks)app/src/main/res/drawable/ic_record.xml(1 hunks)app/src/main/res/drawable/ic_record_big.xml(1 hunks)app/src/main/res/drawable/ic_record_new.xml(1 hunks)app/src/main/res/drawable/ic_record_ongoing.xml(1 hunks)app/src/main/res/drawable/ic_record_start_over.xml(1 hunks)app/src/main/res/drawable/ic_search.xml(1 hunks)app/src/main/res/drawable/ic_search_result_empty.xml(1 hunks)app/src/main/res/drawable/ic_selected_brush_type.xml(1 hunks)app/src/main/res/drawable/ic_settings.xml(1 hunks)app/src/main/res/drawable/ic_stop.xml(1 hunks)app/src/main/res/drawable/ic_stop_record.xml(1 hunks)app/src/main/res/drawable/ic_telegram.xml(1 hunks)app/src/main/res/drawable/ic_terms_of_service.xml(1 hunks)app/src/main/res/drawable/ic_theme.xml(1 hunks)app/src/main/res/drawable/ic_toolbar_about.xml(1 hunks)app/src/main/res/drawable/ripple_border_primary_r8.xml(1 hunks)app/src/main/res/drawable/ripple_donate_copy_text.xml(1 hunks)app/src/main/res/drawable/ripple_note_item.xml(1 hunks)app/src/main/res/drawable/ripple_support_text.xml(1 hunks)app/src/main/res/drawable/ripple_web_link_text.xml(1 hunks)app/src/main/res/layout/activity_main.xml(1 hunks)app/src/main/res/layout/adapter_tag.xml(1 hunks)app/src/main/res/layout/adapter_text_note.xml(7 hunks)app/src/main/res/layout/dialog_common_action.xml(5 hunks)
⛔ Files not processed due to max files limit (24)
- app/src/main/res/layout/dialog_donate_qr.xml
- app/src/main/res/layout/fragment_about.xml
- app/src/main/res/layout/fragment_edit_notes.xml
- app/src/main/res/layout/fragment_home.xml
- app/src/main/res/layout/fragment_settings.xml
- app/src/main/res/layout/layout_audio_record.xml
- app/src/main/res/layout/layout_brush_color_chooser.xml
- app/src/main/res/layout/layout_brush_size_chooser.xml
- app/src/main/res/layout/layout_graphic_bottom_control.xml
- app/src/main/res/layout/layout_setting_text.xml
- app/src/main/res/layout/layout_support_text.xml
- app/src/main/res/layout/layout_web_link_text.xml
- app/src/main/res/layout/media_player_view.xml
- app/src/main/res/layout/note.xml
- app/src/main/res/layout/toolbar.xml
- app/src/main/res/values/attrs.xml
- app/src/main/res/values/strings.xml
- app/src/main/res/values/styles.xml
- app/src/main/res/values/switch_button_attrs.xml
- app/src/test/java/dev/arkbuilders/arkmemo/VersionStorageTest.kt
- gradle/libs.versions.toml
- logging/src/main/AndroidManifest.xml
- logging/src/main/java/dev/arkbuilders/logging/ArkLogImpl.kt
- settings.gradle
✅ Files skipped from review due to trivial changes (56)
- app/src/main/res/drawable/ic_chevron_left.xml
- app/src/main/res/drawable/ic_record_start_over.xml
- app/src/main/res/drawable/ic_eth.xml
- app/src/main/res/drawable/ic_attach.xml
- app/src/main/res/drawable/ripple_note_item.xml
- app/src/main/res/drawable/ic_stop.xml
- app/src/main/res/drawable/bg_rect_button.xml
- app/src/main/res/drawable/ic_pause.xml
- app/src/main/res/drawable/bg_round_button.xml
- app/src/main/res/drawable/ic_record_ongoing.xml
- app/src/main/res/drawable/ic_brush.xml
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt
- app/src/main/res/drawable/bg_red_button.xml
- app/src/main/res/drawable/bg_brush_size.xml
- app/src/main/res/drawable/bg_graphic_control_text_selected.xml
- app/src/main/res/drawable/ic_chevron_down.xml
- app/src/main/res/drawable/ic_edit.xml
- app/src/main/res/drawable/bg_audio_view.xml
- app/src/main/res/drawable/ic_draw.xml
- app/src/main/res/drawable/ic_no_notes.xml
- app/src/main/res/drawable/audio_wave_thumb.xml
- app/src/main/res/drawable/ic_coffee.xml
- app/src/main/res/drawable/ic_pin.xml
- app/src/main/res/drawable/ic_app_logo.xml
- app/src/main/res/drawable/bg_brush_option_selected.xml
- app/src/main/res/drawable/ic_settings.xml
- app/src/main/res/drawable/bg_border_r8.xml
- app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt
- app/src/main/res/layout/activity_main.xml
- app/src/main/res/drawable/bg_selected_brush.xml
- app/src/main/res/drawable/ic_toolbar_about.xml
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.kt
- app/src/main/res/drawable/ic_record_new.xml
- app/src/main/res/drawable/ripple_web_link_text.xml
- app/src/main/res/drawable/ic_feedback.xml
- app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/FilePickerDialog.kt
- app/src/main/java/dev/arkbuilders/arkmemo/ui/views/SettingTextView.kt
- app/src/main/res/drawable/ic_chevron_right.xml
- app/src/main/res/drawable/ripple_border_primary_r8.xml
- app/src/main/res/drawable/ic_clipboard.xml
- app/src/main/res/drawable/ic_globe.xml
- app/src/main/res/drawable/bg_brush_color_black.xml
- app/src/main/res/drawable/ic_play.xml
- app/src/main/res/layout/adapter_tag.xml
- app/src/main/res/drawable/ic_pause_circle.xml
- app/build.gradle
- app/src/main/res/drawable/bg_positive_button.xml
- app/src/main/res/drawable/ic_record_big.xml
- app/src/main/res/drawable/ic_stop_record.xml
- app/src/main/res/drawable/bg_negative_button.xml
- app/src/main/res/drawable/ic_discord.xml
- app/src/main/res/drawable/ic_close.xml
- app/src/main/res/drawable/ic_privacy.xml
- app/src/main/res/drawable/ic_search.xml
- app/src/main/res/drawable/ic_rate.xml
- app/src/main/res/drawable/ic_patreon.xml
🚧 Files skipped from review as they are similar to previous changes (9)
- app/src/main/java/dev/arkbuilders/arkmemo/models/VersionsResult.kt
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorageRepo.kt
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorageRepo.kt
- app/src/main/java/dev/arkbuilders/arkmemo/utils/ArkFiles.kt
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Versions.kt
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionsStorage.kt
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/VersionsFragment.kt
- app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/Version.kt
- app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-18T05:33:52.382Z
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Applied to files:
app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt
📚 Learning: 2025-07-18T05:40:19.372Z
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Applied to files:
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt
🧬 Code graph analysis (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorage.kt (2)
childrenNotParents(17-17)parentsTreeByChild(15-15)
🪛 detekt (1.23.8)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt
[warning] 91-92: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt
[warning] 101-102: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (43)
app/src/main/res/drawable/ripple_support_text.xml (1)
2-11: Formatting-only touch looks fineThe XML attributes are just reflowed; behavior stays unchanged. No further action needed.
app/src/main/res/drawable/ic_search_result_empty.xml (1)
6-12: Formatting-only vector tweak looks goodNo semantic or rendering changes detected; indentation now matches the surrounding XML style.
app/src/main/res/drawable/bg_tag.xml (1)
4-8: Formatting looks goodSplitting the stroke, corners, and solid attributes onto their own lines improves readability without changing behavior.
app/src/main/res/drawable/ic_crash_report.xml (1)
1-11: Vector drawable formatting LGTMOnly indentation and line breaks changed; drawable attributes remain intact.
app/src/main/res/drawable/ic_record.xml (1)
1-9: Formatting update looks goodNo functional changes introduced; vector definition remains consistent.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/AboutFragment.kt (1)
34-35: Formatting update looks goodNo behavioral changes introduced; the version string binding remains the same.
app/src/main/res/drawable/ic_selected_brush_type.xml (1)
6-14: Formatting update looks goodRe-indentation only; vector drawing data remains unchanged.
app/src/main/res/drawable/ic_theme.xml (1)
6-12: LGTM — formatting onlyVector path attributes remain unchanged; reformatting keeps rendering identical while matching project style.
app/src/main/res/drawable/ic_telegram.xml (1)
6-12: Vector formatting change looks goodOnly whitespace/layout adjustments; rendering remains identical.
app/src/main/res/drawable/bg_big_radius.xml (1)
4-8: Formatting cleanup looks goodAttribute layout is clearer now with no functional changes introduced.
app/src/main/res/drawable/ic_delete_note.xml (1)
6-12: LGTM – formatting only. Rendering semantics remain unchanged.app/src/main/res/drawable/ic_copy.xml (1)
6-12: LGTM – formatting only. Icon attributes are untouched, so behavior is preserved.app/src/main/res/drawable/ic_mic.xml (1)
1-9: Formatting change looks goodAttribute values are untouched and the drawable will render identically; thanks for cleaning up the formatting.
app/src/main/res/drawable/ic_add.xml (1)
6-12: Vector drawable formatting looks goodIndentation and self-closing tag updates are purely cosmetic; no rendering or behavioral changes introduced.
app/src/main/AndroidManifest.xml (1)
12-13: No functional change detectedWhitespace-only tweak on the self-closing permission tags—manifest semantics remain identical.
app/src/main/res/drawable/ic_terms_of_service.xml (1)
6-12: LGTM on the drawable formattingIndentation-only tweak; vector semantics remain untouched.
app/src/main/res/layout/adapter_text_note.xml (1)
1-128: Touched-up layout reads cleanlyOnly formatting/ordering tweaks; view hierarchy and attrs remain unchanged. Looks good.
app/src/main/res/drawable/ripple_donate_copy_text.xml (1)
2-13: LGTM — ripple formattingOnly whitespace/formatting adjustments; drawable behavior stays the same.
app/src/main/res/drawable/ic_btc.xml (1)
6-11: Vector formatting cleanup looks good.The path data and colors remain unchanged, so this is a safe formatting-only touchup.
app/src/main/res/drawable/ic_eraser.xml (1)
6-12: Vector formatting update looks goodAttribute set is unchanged and the reformatting keeps the drawable behavior identical while improving readability.
app/src/main/res/drawable/ic_download.xml (1)
6-12: Arrow path update looks goodStroke-only path still renders the download arrow cleanly with the correct cap/join settings.
.github/workflows/build.yml (2)
51-94: Harden keystore handling and ensure cleanup.Please make the GPG step fully non-interactive and remove signing material after artifact uploads. Suggested update:
- name: Decrypt the keystore for signing run: | echo "${{ secrets.KEYSTORE_ENCRYPTED }}" > keystore.asc - gpg -d --passphrase "${{ secrets.KEYSTORE_PASSWORD }}" --batch keystore.asc > keystore.jks + gpg --batch --yes --pinentry-mode loopback \ + --passphrase "${{ secrets.KEYSTORE_PASSWORD }}" \ + --output keystore.jks \ + --decrypt keystore.asc + + - name: Cleanup signing material + if: always() + run: | + rm -f keystore.asc keystore.jks
21-26: Fix broken JDK setup inputs.
actions/setup-java@v4no longer acceptsdistribution: 'adopt', so every job that hits this step fails before running Gradle. Switch totemurin(the Eclipse successor) in each setup block.- distribution: 'adopt' + distribution: 'temurin'Also applies to: 41-46, 103-107, 124-128
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkMediaPlayerFragment.kt (1)
109-120: Playback controls now stay in sync with pause/stop/resumeThanks for mirroring the pause/stop/resume icon handling from the list adapter—the fragment’s controls no longer get stuck in the wrong state when side effects come in.
app/src/main/java/dev/arkbuilders/arkmemo/ui/views/NotesCanvas.kt (1)
61-87: Still emitting duplicateDrawPathupdates per stroke
finishDrawingprevents the emptyPath, but we still callonDrawPathon every DOWN/MOVE, so the view model keeps receiving the same mutable instance multiple times. Please finalize the stroke insideACTION_UP(as previously discussed) and drop the global dispatch.- var finishDrawing = false when (event.action) { MotionEvent.ACTION_DOWN -> { path.moveTo(x, y) viewModel.svg().apply { addCommand( SVGCommand.MoveTo(x, y).apply { paintColor = viewModel.paint.color.getStrokeColor() brushSizeId = viewModel.paint.strokeWidth.getBrushSizeId() }, ) } currentX = x currentY = y } MotionEvent.ACTION_MOVE -> { val x2 = (currentX + x) / 2 val y2 = (currentY + y) / 2 path.quadTo(currentX, currentY, x2, y2) viewModel.svg().apply { addCommand( SVGCommand.AbsQuadTo(currentX, currentY, x2, y2).apply { paintColor = viewModel.paint.color.getStrokeColor() brushSizeId = viewModel.paint.strokeWidth.getBrushSizeId() }, ) } currentX = x currentY = y } MotionEvent.ACTION_UP, MotionEvent.ACTION_CANCEL -> { - path = Path() - finishDrawing = true + val drawPath = DrawPath(path, viewModel.paint) + viewModel.onDrawPath(drawPath) + invalidate() + path = Path() } } - if (!finishDrawing) { - val drawPath = DrawPath(path, viewModel.paint) - viewModel.onDrawPath(drawPath) - invalidate() - } - return trueapp/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt (3)
69-71: LGTM: cleaner inflationUsing view binding inflation directly is fine and clearer.
316-319: Avoid sharing mutable note instance with editorIf EditTextNotesFragment mutates the note, it can cause unexpected adapter state. Pass an immutable copy or an ID and load inside the fragment.
If TextNote is a data class:
- EditTextNotesFragment.newInstance(selectedNote) + EditTextNotesFragment.newInstance(selectedNote.copy())Otherwise, ensure the fragment treats the object as read‑only or creates its own copy.
325-328: Same for VoiceNote: pass copy or IDTo prevent shared‑mutable state issues between adapter and ArkRecorderFragment, pass a copy or an ID.
If VoiceNote is a data class:
- activity.fragment = ArkRecorderFragment.newInstance(selectedNote) + activity.fragment = ArkRecorderFragment.newInstance(selectedNote.copy())Or pass a stable identifier and load within the fragment.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/VersionStorage.kt (1)
6-19: Document VersionStorage contract semantics before exposingWe still don’t spell out ordering guarantees or graph semantics for the public methods, so third‑party storage implementers can’t tell how to implement
versions,forget,parentsTreeByChild, etc. Re-applying the earlier ask: add concise KDoc (or equivalent) describing edge/node behavior, ordering guarantees, and “latest” semantics before we freeze this API.app/src/main/res/drawable/ic_loading.xml (1)
6-12: Vector formatting looks goodOnly indentation changed; rendering stays identical.
app/src/main/java/dev/arkbuilders/arkmemo/models/Note.kt (1)
3-13: Parcelable move to the interface makes senseCentralizing
Parcelableon the interface keeps implementations consistent and avoids accidental omissions.app/src/main/java/dev/arkbuilders/arkmemo/models/TextNote.kt (1)
10-18: Fork flag integration is straightforwardAdding
isForkedalongside the other mutable flags keeps the parcelable state aligned with the new versioning needs.app/src/main/java/dev/arkbuilders/arkmemo/models/VoiceNote.kt (1)
12-27: Voice note fork state exposure looks goodThe new flag mirrors other note types and participates in parceling, so UI/state flows can track forks consistently.
app/src/main/java/dev/arkbuilders/arkmemo/models/GraphicNote.kt (1)
12-22: Graphic note fork flag matches the shared contractKeeping the default
falseand exposing it in the primary constructor aligns with the updatedNoteinterface.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
44-49: LGTM: safer paste handling with try/catch retainedFormatting-only change; behavior unchanged and exception path preserved.
app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionsStorage.kt (1)
16-19: Harden deserialization against malformed entriesA bad line will throw and break storage init. Add per-item safe parsing and logging.
- override fun valueFromString(raw: String): Versions = - raw.split(",").filter { it.isNotEmpty() }.map { - ResourceId.fromString(it) - }.toSet() + override fun valueFromString(raw: String): Versions = + raw.split(",") + .map { it.trim() } + .filter { it.isNotEmpty() } + .mapNotNull { + try { + ResourceId.fromString(it) + } catch (_: Exception) { + null + } + } + .toSet()app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt (1)
69-69: Avoid !! in getParentIds() and handle pre-init stateReturn emptyList when not initialized or missing key.
- fun getParentIds(id: ResourceId): List<ResourceId> = versionStorage.parentsTreeByChild(id)[id]!! + fun getParentIds(id: ResourceId): List<ResourceId> = + if (isInitialized()) versionStorage.parentsTreeByChild(id)[id].orEmpty() else emptyList()app/src/main/java/dev/arkbuilders/arkmemo/repo/versions/RootVersionStorage.kt (2)
93-101: Compile-time type mismatch and potential NPE in forget()Passing nullable versions to replace(...) and force-unwrapping fields will crash if not found.
- val newVersion = - Version( - versionIdIsChild?.parent!!, - versionIdIsParent?.child!!, - ) - replace(versionIdIsChild, newVersion) - versions.remove(versionIdIsParent) + if (versionIdIsChild == null || versionIdIsParent == null) { + Log.e(VERSIONS_STORAGE, "Inconsistent version graph for id=$id") + persist() + return + } + val newVersion = Version(versionIdIsChild.parent, versionIdIsParent.child) + replace(versionIdIsChild, newVersion) + versions.remove(versionIdIsParent)
148-185: Add robust parsing and logging in readStorage()Malformed files will throw; validate and skip bad lines with diagnostics.
- private suspend fun readStorage(): VersionsResult = - withContext(Dispatchers.IO) { - val lines = Files.readAllLines(storageFile) - val storageVersion = lines.removeAt(0) - verifyVersion(storageVersion) - val versions = - lines.map { - val parts = it.split(KEY_VALUE_SEPARATOR) - val parent = ResourceId.fromString(parts[0]) - val child = ResourceId.fromString(parts[1]) - Log.d( - VERSIONS_STORAGE, - it, - ) - Version(parent, child) - } - val parents = - versions.map { - Log.d( - VERSIONS_STORAGE, - "parent: ${it.parent}", - ) - it.parent - }.toSet() - val children = - versions.map { - Log.d( - VERSIONS_STORAGE, - "child: ${it.child}", - ) - it.child - } - return@withContext VersionsResult( - versions, - parents, - children, - ) - } + private suspend fun readStorage(): VersionsResult = + withContext(Dispatchers.IO) { + try { + val lines = Files.readAllLines(storageFile).toMutableList() + if (lines.isEmpty()) { + Log.w(VERSIONS_STORAGE, "Storage file is empty: $storageFile") + return@withContext VersionsResult(emptyList(), emptySet(), emptyList()) + } + val storageVersion = lines.removeAt(0) + verifyVersion(storageVersion) + val versions = lines.mapNotNull { line -> + val parts = line.split(KEY_VALUE_SEPARATOR) + if (parts.size != 2) { + Log.e(VERSIONS_STORAGE, "Malformed line: $line") + return@mapNotNull null + } + try { + val parent = ResourceId.fromString(parts[0]) + val child = ResourceId.fromString(parts[1]) + Log.d(VERSIONS_STORAGE, line) + Version(parent, child) + } catch (e: Exception) { + Log.e(VERSIONS_STORAGE, "Failed to parse line: $line", e) + null + } + } + val parents = versions.map { it.parent }.toSet() + val children = versions.map { it.child } + VersionsResult(versions, parents, children) + } catch (e: Exception) { + Log.e(VERSIONS_STORAGE, "Failed to read storage", e) + throw e + } + }app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/GraphicNotesViewModel.kt (1)
30-39: LGTM on the paint getter refactorJust a formatting tweak; behavior remains unchanged.
app/src/main/res/drawable/ic_debug_log.xml (1)
6-8: Vector drawable formatting looks goodWhitespace-only change; renders identically.
app/src/main/res/drawable/ic_photo.xml (1)
6-12: No issues spotted with the drawable reformatAttribute values are intact; purely cosmetic change.
app/src/main/res/drawable/ic_play_circle.xml (1)
6-13: Drawable reformat verifiedAll path data is unchanged; safe formatting adjustment.
| private lateinit var versionStorage: VersionStorage | ||
| private val latestResourceId = MutableStateFlow<ResourceId?>(null) | ||
|
|
||
| fun init() { | ||
| viewModelScope.launch { | ||
| versionStorage = versionStorageRepo.provide() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Guard all storage access with initialization
versionStorage is set asynchronously; direct access can crash with UninitializedPropertyAccessException.
private lateinit var versionStorage: VersionStorage
private val latestResourceId = MutableStateFlow<ResourceId?>(null)
+ private val initComplete = kotlinx.coroutines.CompletableDeferred<Unit>()
fun init() {
viewModelScope.launch {
versionStorage = versionStorageRepo.provide()
+ initComplete.complete(Unit)
}
}Add helpers outside the shown range:
private suspend fun ensureInitialized() = initComplete.await()
private fun isInitialized() = initComplete.isCompleted🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt
around lines 23 to 31, versionStorage is assigned asynchronously and can be
accessed before initialization causing UninitializedPropertyAccessException; add
a CompletableDeferred<Unit> initComplete that you complete after versionStorage
is set, implement two helpers: a suspend ensureInitialized() that awaits
initComplete and a non-suspending isInitialized() that returns
initComplete.isCompleted, then update all places that access versionStorage to
call ensureInitialized() (suspend) or check isInitialized() and handle the
uninitialized case appropriately so every storage access is guarded.
| viewModelScope.launch { | ||
| val latestNotes = | ||
| notes.filter { | ||
| getChildNotParentIds().contains(it.resource?.id) || | ||
| isNotVersioned(it.resource?.id!!) | ||
| } | ||
| emit(latestNotes) | ||
| } |
There was a problem hiding this comment.
Fix NPE and reduce per-item storage calls in getLatestNotes()
Avoid !! on nullable id, ensure storage is ready, and compute latest IDs once.
- viewModelScope.launch {
- val latestNotes =
- notes.filter {
- getChildNotParentIds().contains(it.resource?.id) ||
- isNotVersioned(it.resource?.id!!)
- }
- emit(latestNotes)
- }
+ viewModelScope.launch {
+ ensureInitialized()
+ val latestIds = versionStorage.childrenNotParents().toSet()
+ val latestNotes = notes.filter {
+ val id = it.resource?.id ?: return@filter false
+ latestIds.contains(id) || !versionStorage.contains(id)
+ }
+ emit(latestNotes)
+ }📝 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.
| viewModelScope.launch { | |
| val latestNotes = | |
| notes.filter { | |
| getChildNotParentIds().contains(it.resource?.id) || | |
| isNotVersioned(it.resource?.id!!) | |
| } | |
| emit(latestNotes) | |
| } | |
| viewModelScope.launch { | |
| ensureInitialized() | |
| val latestIds = versionStorage.childrenNotParents().toSet() | |
| val latestNotes = notes.filter { | |
| val id = it.resource?.id ?: return@filter false | |
| latestIds.contains(id) || !versionStorage.contains(id) | |
| } | |
| emit(latestNotes) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt
around lines 36–43, the filter uses !! on a nullable id and calls
getChildNotParentIds() inside the per-item predicate causing NPE risk and
repeated storage calls; first ensure the storage or repository is ready (check
or await a readiness method and return/emit empty if not ready), then call
getChildNotParentIds() once before launching the filter to compute a Set of
latest IDs, and rewrite the filter to use safe calls (e.g., it.resource?.id)
with null-safe checks (filter out null ids with let or filterNotNull) instead of
!! so no null is dereferenced.
| viewModelScope.launch { | ||
| latestResourceId.collectLatest { id -> | ||
| id?.let { | ||
| val familyTree = | ||
| notes.filter { | ||
| getParentIds(id).contains(it.resource?.id) || it.resource?.id == id | ||
| } | ||
| emit(familyTree) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Ensure init and handle null IDs in getLatestNoteFamilyTree()
Precompute parents set and avoid nullable comparisons inside filter.
- viewModelScope.launch {
- latestResourceId.collectLatest { id ->
- id?.let {
- val familyTree =
- notes.filter {
- getParentIds(id).contains(it.resource?.id) || it.resource?.id == id
- }
- emit(familyTree)
- }
- }
- }
+ viewModelScope.launch {
+ ensureInitialized()
+ latestResourceId.collectLatest { id ->
+ id?.let { target ->
+ val parentIds = versionStorage.parentsTreeByChild(target)[target].orEmpty().toSet()
+ val familyTree = notes.filter {
+ val rid = it.resource?.id ?: return@filter false
+ rid == target || parentIds.contains(rid)
+ }
+ emit(familyTree)
+ }
+ }
+ }🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt
around lines 50 to 61, getLatestNoteFamilyTree() currently calls
getParentIds(id) inside the filter and compares nullable resource ids directly;
precompute a non-null parents Set by returning early when id is null, call
getParentIds(id) once into a Set (e.g., val parentIds =
getParentIds(id).toSet()), then filter notes by checking non-null
note.resource?.id against parentIds or equality to id using safe, non-null
comparisons, and emit the resulting list; this removes repeated calls and
nullable comparisons inside the lambda.
| viewModelScope.launch(Dispatchers.IO) { | ||
| val version = Version(oldId, newId) | ||
| versionStorage.add(version) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wait for init before mutating storage in createVersion()
Prevents races on first use.
- viewModelScope.launch(Dispatchers.IO) {
- val version = Version(oldId, newId)
- versionStorage.add(version)
- }
+ viewModelScope.launch(Dispatchers.IO) {
+ ensureInitialized()
+ versionStorage.add(Version(oldId, newId))
+ }📝 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.
| viewModelScope.launch(Dispatchers.IO) { | |
| val version = Version(oldId, newId) | |
| versionStorage.add(version) | |
| } | |
| viewModelScope.launch(Dispatchers.IO) { | |
| ensureInitialized() | |
| versionStorage.add(Version(oldId, newId)) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/VersionsViewModel.kt
around lines 75 to 78, the coroutine mutates versionStorage without ensuring the
storage has finished initialization, which can cause a race on first use; update
createVersion() to wait for the storage to be initialized before calling
versionStorage.add(version) — either make createVersion suspend and call an
existing initialization/ready awaiter on versionStorage (or expose one), or
perform the initialization check/await on the main coroutine before launching
the Dispatchers.IO block, then add the version after that wait completes so no
concurrent access occurs during first-use initialization.
Re-implementation of #12
ResourceId#16Summary by CodeRabbit
New Features
UI
Documentation
Tests
Chores