Conversation
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds an auto-empty bin feature: new preference and UI, WorkManager worker with scheduling/cancellation, DAO/database changes to record deletion timestamps and test DB support, background cleanup logic to remove old deleted notes and attachments, translations/strings update, and Robolectric tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsUI as Settings UI
participant Prefs as Preferences
participant App as NotallyXApplication
participant WorkMgr as WorkManager
participant Worker as AutoRemoveWorker
participant DB as Database
participant Files as FileSystem
User->>SettingsUI: Adjust "Auto remove days" slider
SettingsUI->>Prefs: Save autoRemoveDeletedNotesAfterDays
Prefs->>App: Preference observer emits change
App->>WorkMgr: scheduleAutoRemoveOldDeletedNotes() if days > 0
App->>WorkMgr: cancelAutoRemoveOldDeletedNotes() if days == 0
Note over WorkMgr: Periodic trigger (daily)
WorkMgr->>Worker: doWork()
Worker->>Prefs: Read autoRemoveDeletedNotesAfterDays
Worker->>DB: getDeletedNoteIdsOlderThan(cutoff)
DB-->>Worker: oldDeletedNoteIds
Worker->>DB: getImages/getFiles/getAudios(ids)
DB-->>Worker: attachment lists
Worker->>DB: delete notes by ids
Worker->>Files: deleteAttachments(attachment paths)
Files-->>Worker: deletion results
Worker-->>WorkMgr: Result.success()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
app/src/main/res/values/strings.xml (1)
35-36: Consider replacing literal "X" with a format argument or clearer phrasing in the hint.The hint uses a literal "X" (
"permanently removed after X days") rather than a format argument like%d. If the hint is displayed alongside the seekbar value, this is a cosmetic issue, but it could be confusing if the hint is shown in isolation (e.g., as a tooltip). Consider rephrasing to"...removed after the configured number of days"or using a string format argument.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/values/strings.xml` around lines 35 - 36, The hint string auto_remove_deleted_notes_hint uses a literal "X"; replace it with either a format placeholder (e.g., "%d" or "%s") and update the code that sets the hint to call getString(R.string.auto_remove_deleted_notes_hint, days) so the actual seekbar value is inserted, or reword the string to a clearer standalone phrase such as "By enabling this, Notes that were deleted will be permanently removed after the configured number of days" to avoid needing a runtime substitution; update the string resource and any code that reads it (references to auto_remove_deleted_notes_hint) accordingly.app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.kt (1)
117-128: Missing cleanup of test database singleton between test classes.
testInstanceis never reset to null afterdb.close()inAutoRemoveDeletedNotesDbTest.tearDown(). This means if test classes execute sequentially in the same JVM process, a subsequent call togetFreshDatabase()could receive a closed database instance from a previous test class, causing test failures.While
AutoRemoveDeletedNotesDbTestclears data between individual test methods viadeleteAll()in its@Afterhook, the singleton is not reset after the class-level teardown. This is particularly problematic if the test framework or build environment reuses static state across test class boundaries.Additionally,
testInstancelacks the@Volatileannotation present on theinstancefield (line 58). For consistency and correctness in the double-checked locking pattern, it should be annotated.Recommended fix:
Add a reset of
testInstanceinAutoRemoveDeletedNotesDbTest.tearDown()or implement a dedicated cleanup method called from test teardown:`@JvmStatic` `@AfterClass` fun tearDown() { db.close() NotallyDatabase.resetTestInstance() // or set testInstance = null directly if accessible }And add
@Volatileto thetestInstancefield.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.kt` around lines 117 - 128, The test singleton testInstance in NotallyDatabase is never reset and also lacks `@Volatile`, causing closed DB reuse across test classes; update the NotallyDatabase companion by annotating testInstance with `@Volatile` and add a public static resetTestInstance() (or resetTestDatabase()) method that closes and/or sets testInstance = null, and then call that reset from AutoRemoveDeletedNotesDbTest.tearDown() (or its `@AfterClass` teardown) after db.close(); ensure getTestDatabase still lazy-initializes testInstance via the synchronized block (the function name getTestDatabase remains unchanged).app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt (2)
19-21: Verify that theContextWrappercast is safe for all Android contexts.
context.applicationContextis cast toContextWrapperon line 20. WhileApplicationextendsContextWrapper, if the application context were ever wrapped differently (e.g., by instrumentation), this would throw aClassCastException. Since this is a WorkManager worker, in practice the application context should always be anApplicationinstance — but consider casting toApplicationdirectly for clarity (consistent with line 24 which already casts toApplication).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt` around lines 19 - 21, The cast of context.applicationContext to ContextWrapper inside AutoRemoveDeletedNotesWorker.doWork is unsafe; change it to cast to Application (or check instance and safely unwrap) before calling removeOldDeletedNotes so it works with any application context variant—locate the doWork() method in AutoRemoveDeletedNotesWorker and replace the (context.applicationContext as ContextWrapper) cast with a safe Application cast or conditional instance check that then calls removeOldDeletedNotes().
23-59:removeOldDeletedNotesis a member extension function — not reusable outside the worker.This function is defined as
suspend fun ContextWrapper.removeOldDeletedNotes()inside the class, making it accessible only withinAutoRemoveDeletedNotesWorker. The test file imports this function fromcom.philkes.notallyx.utils.backup, but the function does not actually exist in the backup module — the import is incorrect and should reference the worker instead. Consider extracting the core logic into a shared top-level extension to enable reusability across modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt` around lines 23 - 59, The removeOldDeletedNotes extension is currently defined as a member extension inside AutoRemoveDeletedNotesWorker (suspend fun ContextWrapper.removeOldDeletedNotes), which prevents reuse from other modules/tests; extract the core logic into a top-level suspend extension (e.g., suspend fun ContextWrapper.removeOldDeletedNotes or suspend fun Context.removeOldDeletedNotes) in a shared utils file (the same package the tests expect, e.g., com.philkes.notallyx.utils.backup) and have AutoRemoveDeletedNotesWorker call that top-level function; update any incorrect test imports to reference the new top-level function and keep usages of NotallyXPreferences, NotallyDatabase, Converters, and deleteAttachments unchanged within the extracted function.app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.kt (1)
35-56: Database lifecycle inconsistency:@Beforerecreatesdbstored incompanion object, but only@AfterClasscloses it.
setUp()creates a freshNotallyDatabaseon every test and assigns it to thecompanionfield, buttearDown()(annotated@AfterClass) only runs once and closes the last-assigned instance. Earlier database instances are never closed. Either:
- Move
db/daoout ofcompanionand use@Afterto close, or- Initialize
dbonce in a@BeforeClass/@JvmStaticmethod and only calldao.deleteAll()in@Before/@Afterfor isolation.Example: one-time init with `@BeforeClass`
companion object { private lateinit var db: NotallyDatabase private lateinit var dao: BaseNoteDao + `@JvmStatic` + `@BeforeClass` + fun initDb() { + val context = ApplicationProvider.getApplicationContext<ContextWrapper>() + db = NotallyDatabase.getFreshDatabase(context, false) + dao = db.getBaseNoteDao() + } + `@JvmStatic` `@AfterClass` fun tearDown() { db.close() } } `@Before` fun setUp() { - val context = ApplicationProvider.getApplicationContext<ContextWrapper>() - db = NotallyDatabase.getFreshDatabase(context, false) - dao = db.getBaseNoteDao() + runBlocking { dao.deleteAll() } } `@After` fun cleanup() { runBlocking { dao.deleteAll() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.kt` around lines 35 - 56, The test currently recreates NotallyDatabase in setUp() and stores it in the companion object field db but only closes it once in the `@JvmStatic` `@AfterClass` tearDown(), leaking earlier instances; fix by making the lifecycle consistent: either move db and dao out of the companion so setUp()/cleanup() manage a per-test instance (create db via NotallyDatabase.getFreshDatabase(...) in setUp, call db.close() in `@After` cleanup after running runBlocking { dao.deleteAll() }) or change to one-time init by adding a `@JvmStatic` `@BeforeClass` method to create db = NotallyDatabase.getFreshDatabase(...) and dao = db.getBaseNoteDao(), keep per-test isolation via runBlocking { dao.deleteAll() } in `@Before/`@After, and leave `@JvmStatic` `@AfterClass` tearDown() to call db.close(); update references to setUp, cleanup, tearDown, db, dao, NotallyDatabase.getFreshDatabase, and BaseNoteDao.deleteAll accordingly.app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt (1)
467-472: Nit: preferitinside theletblock.Line 470 re-references
tooltipResIdinsidelet { ... }where the shadoweditparameter is already the non-null value. Usingitis more idiomatic Kotlin.- tooltipResId?.let { tooltipText = context.getString(tooltipResId) } + tooltipResId?.let { tooltipText = context.getString(it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt` around lines 467 - 472, The code in Title.apply re-checks tooltipResId inside the let block instead of using the non-null shadowed parameter; update the lambda in Title.apply so that tooltipResId?.let { tooltipText = context.getString(it) } uses the provided it value (referencing tooltipResId, tooltipText, context.getString, and Title.apply) to be more idiomatic Kotlin.app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt (1)
261-276: Thecontextparameter is unused for scheduling — only used in the error-logging catch block.The
ContextWrapperparameter exists solely to callcontext.log(...)on line 274 inside thecatch. Consider whether passing a logging lambda or usingLog.e(like line 262) would be simpler and avoid the extra parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt` around lines 261 - 276, The scheduleAutoRemoveOldDeletedNotes function currently takes a ContextWrapper only to call context.log in the catch; remove the ContextWrapper parameter and replace the call to context.log(TAG, "...", throwable = e) with a direct logging call (e.g., Log.e(TAG, "Scheduling auto removal of old deleted notes failed", e)). Specifically, update the WorkManager.scheduleAutoRemoveOldDeletedNotes signature to remove context, leave enqueueUniquePeriodicWork usage unchanged, and change the catch block to call Log.e(TAG, ..., e) (or accept an optional logging lambda parameter with a default that delegates to Log.e if you prefer injectable logging).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/SettingsFragment.kt`:
- Around line 343-358: The code in SettingsFragment inside the AutoEmptyBin
callback unsafely casts requireContext() to ContextWrapper when calling
WorkManager.scheduleAutoRemoveOldDeletedNotes, which can throw
ClassCastException; change the call to pass requireActivity() (or use a safe
cast like (requireContext() as? ContextWrapper) ?: return) so
scheduleAutoRemoveOldDeletedNotes receives a guaranteed ContextWrapper, and keep
cancelAutoRemoveOldDeletedNotes as-is; update the invocation in the
binding.AutoEmptyBin.setupAutoEmptyBin lambda where model.savePreference and
WorkManager calls occur.
In
`@app/src/main/java/com/philkes/notallyx/presentation/viewmodel/preference/NotallyXPreferences.kt`:
- Around line 118-126: The reload() method is missing a call to update the
autoRemoveDeletedNotesAfterDays IntPreference, so observers (e.g., in
NotallyXApplication and SettingsFragment) don't see imported/reset values; open
the reload() function in NotallyXPreferences and add a line calling
autoRemoveDeletedNotesAfterDays.reload() following the same pattern used for
periodicBackups.reload(), so the LiveData is refreshed immediately after
import/reset.
In
`@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`:
- Around line 32-35: The Log.d call in AutoRemoveDeletedNotesWorker uses an
interpolated string with an unclosed parenthesis around "(since:
${Date(before).toText()"; fix the Log.d message by adding the missing closing
parenthesis inside the string (e.g. "... (since: ${Date(before).toText()})") so
the parentheses are balanced; ensure the call to Log.d(TAG, ...) remains
syntactically correct around Date(before).toText().
- Around line 37-58: The database opened by NotallyDatabase.getFreshDatabase in
AutoRemoveDeletedNotesWorker (inside doWork) is never closed; wrap the try/catch
in a try/finally (or use Kotlin's use/auto-close pattern) and call
database.close() in the finally block to ensure the Room instance is always
closed after using baseNoteDao.getDeletedNoteIdsOlderThan,
getImages/getFiles/getAudios, delete, and deleteAttachments; reference the
NotallyDatabase.getFreshDatabase call and the local variable database to locate
where to insert the close.
---
Nitpick comments:
In `@app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.kt`:
- Around line 117-128: The test singleton testInstance in NotallyDatabase is
never reset and also lacks `@Volatile`, causing closed DB reuse across test
classes; update the NotallyDatabase companion by annotating testInstance with
`@Volatile` and add a public static resetTestInstance() (or resetTestDatabase())
method that closes and/or sets testInstance = null, and then call that reset
from AutoRemoveDeletedNotesDbTest.tearDown() (or its `@AfterClass` teardown) after
db.close(); ensure getTestDatabase still lazy-initializes testInstance via the
synchronized block (the function name getTestDatabase remains unchanged).
In `@app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt`:
- Around line 261-276: The scheduleAutoRemoveOldDeletedNotes function currently
takes a ContextWrapper only to call context.log in the catch; remove the
ContextWrapper parameter and replace the call to context.log(TAG, "...",
throwable = e) with a direct logging call (e.g., Log.e(TAG, "Scheduling auto
removal of old deleted notes failed", e)). Specifically, update the
WorkManager.scheduleAutoRemoveOldDeletedNotes signature to remove context, leave
enqueueUniquePeriodicWork usage unchanged, and change the catch block to call
Log.e(TAG, ..., e) (or accept an optional logging lambda parameter with a
default that delegates to Log.e if you prefer injectable logging).
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt`:
- Around line 467-472: The code in Title.apply re-checks tooltipResId inside the
let block instead of using the non-null shadowed parameter; update the lambda in
Title.apply so that tooltipResId?.let { tooltipText = context.getString(it) }
uses the provided it value (referencing tooltipResId, tooltipText,
context.getString, and Title.apply) to be more idiomatic Kotlin.
In
`@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`:
- Around line 19-21: The cast of context.applicationContext to ContextWrapper
inside AutoRemoveDeletedNotesWorker.doWork is unsafe; change it to cast to
Application (or check instance and safely unwrap) before calling
removeOldDeletedNotes so it works with any application context variant—locate
the doWork() method in AutoRemoveDeletedNotesWorker and replace the
(context.applicationContext as ContextWrapper) cast with a safe Application cast
or conditional instance check that then calls removeOldDeletedNotes().
- Around line 23-59: The removeOldDeletedNotes extension is currently defined as
a member extension inside AutoRemoveDeletedNotesWorker (suspend fun
ContextWrapper.removeOldDeletedNotes), which prevents reuse from other
modules/tests; extract the core logic into a top-level suspend extension (e.g.,
suspend fun ContextWrapper.removeOldDeletedNotes or suspend fun
Context.removeOldDeletedNotes) in a shared utils file (the same package the
tests expect, e.g., com.philkes.notallyx.utils.backup) and have
AutoRemoveDeletedNotesWorker call that top-level function; update any incorrect
test imports to reference the new top-level function and keep usages of
NotallyXPreferences, NotallyDatabase, Converters, and deleteAttachments
unchanged within the extracted function.
In `@app/src/main/res/values/strings.xml`:
- Around line 35-36: The hint string auto_remove_deleted_notes_hint uses a
literal "X"; replace it with either a format placeholder (e.g., "%d" or "%s")
and update the code that sets the hint to call
getString(R.string.auto_remove_deleted_notes_hint, days) so the actual seekbar
value is inserted, or reword the string to a clearer standalone phrase such as
"By enabling this, Notes that were deleted will be permanently removed after the
configured number of days" to avoid needing a runtime substitution; update the
string resource and any code that reads it (references to
auto_remove_deleted_notes_hint) accordingly.
In
`@app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.kt`:
- Around line 35-56: The test currently recreates NotallyDatabase in setUp() and
stores it in the companion object field db but only closes it once in the
`@JvmStatic` `@AfterClass` tearDown(), leaking earlier instances; fix by making the
lifecycle consistent: either move db and dao out of the companion so
setUp()/cleanup() manage a per-test instance (create db via
NotallyDatabase.getFreshDatabase(...) in setUp, call db.close() in `@After`
cleanup after running runBlocking { dao.deleteAll() }) or change to one-time
init by adding a `@JvmStatic` `@BeforeClass` method to create db =
NotallyDatabase.getFreshDatabase(...) and dao = db.getBaseNoteDao(), keep
per-test isolation via runBlocking { dao.deleteAll() } in `@Before/`@After, and
leave `@JvmStatic` `@AfterClass` tearDown() to call db.close(); update references to
setUp, cleanup, tearDown, db, dao, NotallyDatabase.getFreshDatabase, and
BaseNoteDao.deleteAll accordingly.
Comment on lines
+167
to
+168
| @Query("SELECT id FROM BaseNote WHERE folder = 'DELETED' AND modifiedTimestamp < :before") | ||
| suspend fun getDeletedNoteIdsOlderThan(before: Long): LongArray |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "modifiedTimestamp|getDeletedNoteIdsOlderThan|removeOldDeletedNotes|AutoRemoveWorker" --type=kotlin -C 5Repository: Crustack/NotallyX
Length of output: 50373
🏁 Script executed:
rg -n "\.move\(" --type=kotlin -B 2 -A 2 | grep -A 4 -B 4 "DELETED\|folder = Folder\|Folder.DELETED" | head -100Repository: Crustack/NotallyX
Length of output: 3023
🏁 Script executed:
rg -n "Migration7|migration|DELETED.*modifiedTimestamp|backfill" --type=kotlin -A 5 | head -80Repository: Crustack/NotallyX
Length of output: 8400
🏁 Script executed:
rg -n "autoRemoveDeletedNotes|AutoRemoveDeletedNotes" --type=kotlin -B 3 -A 3 | grep -v test | head -100Repository: Crustack/NotallyX
Length of output: 8814
🏁 Script executed:
rg -n "checkUpdateAutoRemoveOldDeletedNotes" --type=kotlin -A 20Repository: Crustack/NotallyX
Length of output: 4405
Pre-existing deleted notes may be removed prematurely when the auto-remove feature is first enabled.
The implementation correctly updates modifiedTimestamp to the current time when notes are moved to DELETED going forward (BaseNoteModel.kt line 635). However, notes already in the DELETED folder before this feature was deployed have modifiedTimestamp set to their last edit time (via Migration6's default). When a user enables "auto remove after 30 days", the worker immediately runs getDeletedNoteIdsOlderThan(), which filters on modifiedTimestamp < before. This means notes deleted recently but last edited months ago will be permanently removed on the next worker execution, with no safeguard or one-time migration.
Consider either:
- A one-time data migration that sets
modifiedTimestamptoSystem.currentTimeMillis()for all notes currently in the DELETED folder when the feature is first enabled. - Adding a dedicated
deletedTimestampcolumn to clearly separate edit time from deletion time. - At minimum, documenting this as a known limitation so users understand the behavior.
Comment on lines
+343
to
+358
| autoRemoveDeletedNotesAfterDays.observe(viewLifecycleOwner) { value -> | ||
| binding.AutoEmptyBin.setupAutoEmptyBin( | ||
| autoRemoveDeletedNotesAfterDays, | ||
| requireContext(), | ||
| ) { newValue -> | ||
| model.savePreference(autoRemoveDeletedNotesAfterDays, newValue) | ||
| val workManager = WorkManager.getInstance(requireContext()) | ||
| if (newValue > 0) { | ||
| workManager.scheduleAutoRemoveOldDeletedNotes( | ||
| requireContext() as ContextWrapper | ||
| ) | ||
| } else { | ||
| workManager.cancelAutoRemoveOldDeletedNotes() | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Unsafe cast requireContext() as ContextWrapper on line 352.
requireContext() returns a Context, which is typically the host Activity (a ContextWrapper subclass), but the unchecked cast will throw ClassCastException if the context is ever not a ContextWrapper. Consider using the existing requireActivity() (which is guaranteed to be an Activity extends ContextWrapper) or a safe cast.
Proposed fix
if (newValue > 0) {
workManager.scheduleAutoRemoveOldDeletedNotes(
- requireContext() as ContextWrapper
+ requireActivity()
)
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| autoRemoveDeletedNotesAfterDays.observe(viewLifecycleOwner) { value -> | |
| binding.AutoEmptyBin.setupAutoEmptyBin( | |
| autoRemoveDeletedNotesAfterDays, | |
| requireContext(), | |
| ) { newValue -> | |
| model.savePreference(autoRemoveDeletedNotesAfterDays, newValue) | |
| val workManager = WorkManager.getInstance(requireContext()) | |
| if (newValue > 0) { | |
| workManager.scheduleAutoRemoveOldDeletedNotes( | |
| requireContext() as ContextWrapper | |
| ) | |
| } else { | |
| workManager.cancelAutoRemoveOldDeletedNotes() | |
| } | |
| } | |
| } | |
| autoRemoveDeletedNotesAfterDays.observe(viewLifecycleOwner) { value -> | |
| binding.AutoEmptyBin.setupAutoEmptyBin( | |
| autoRemoveDeletedNotesAfterDays, | |
| requireContext(), | |
| ) { newValue -> | |
| model.savePreference(autoRemoveDeletedNotesAfterDays, newValue) | |
| val workManager = WorkManager.getInstance(requireContext()) | |
| if (newValue > 0) { | |
| workManager.scheduleAutoRemoveOldDeletedNotes( | |
| requireActivity() | |
| ) | |
| } else { | |
| workManager.cancelAutoRemoveOldDeletedNotes() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/SettingsFragment.kt`
around lines 343 - 358, The code in SettingsFragment inside the AutoEmptyBin
callback unsafely casts requireContext() to ContextWrapper when calling
WorkManager.scheduleAutoRemoveOldDeletedNotes, which can throw
ClassCastException; change the call to pass requireActivity() (or use a safe
cast like (requireContext() as? ContextWrapper) ?: return) so
scheduleAutoRemoveOldDeletedNotes receives a guaranteed ContextWrapper, and keep
cancelAutoRemoveOldDeletedNotes as-is; update the invocation in the
binding.AutoEmptyBin.setupAutoEmptyBin lambda where model.savePreference and
WorkManager calls occur.
app/src/main/java/com/philkes/notallyx/presentation/viewmodel/preference/NotallyXPreferences.kt
Show resolved
Hide resolved
Comment on lines
+32
to
+35
| Log.d( | ||
| TAG, | ||
| "Removing notes that have been deleted for $days days or more (since: ${Date(before).toText()}", | ||
| ) |
Contributor
There was a problem hiding this comment.
Missing closing parenthesis in log message.
Line 34: the interpolated string opens (since: but never closes it.
Fix
- "Removing notes that have been deleted for $days days or more (since: ${Date(before).toText()}",
+ "Removing notes that have been deleted for $days days or more (since: ${Date(before).toText()})",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`
around lines 32 - 35, The Log.d call in AutoRemoveDeletedNotesWorker uses an
interpolated string with an unclosed parenthesis around "(since:
${Date(before).toText()"; fix the Log.d message by adding the missing closing
parenthesis inside the string (e.g. "... (since: ${Date(before).toText()})") so
the parentheses are balanced; ensure the call to Log.d(TAG, ...) remains
syntactically correct around Date(before).toText().
Comment on lines
+37
to
+58
| val database = NotallyDatabase.getFreshDatabase(this, preferences.dataInPublicFolder.value) | ||
| val baseNoteDao = database.getBaseNoteDao() | ||
|
|
||
| return try { | ||
| val ids = baseNoteDao.getDeletedNoteIdsOlderThan(before) | ||
| if (ids.isNotEmpty()) { | ||
| val imageStrings = baseNoteDao.getImages(ids) | ||
| val fileStrings = baseNoteDao.getFiles(ids) | ||
| val audioStrings = baseNoteDao.getAudios(ids) | ||
|
|
||
| val images = imageStrings.flatMap { json -> Converters.jsonToFiles(json) } | ||
| val files = fileStrings.flatMap { json -> Converters.jsonToFiles(json) } | ||
| val audios = audioStrings.flatMap { json -> Converters.jsonToAudios(json) } | ||
|
|
||
| baseNoteDao.delete(ids) | ||
| deleteAttachments(images + files + audios, ids) | ||
| } | ||
| Result.success() | ||
| } catch (e: Exception) { | ||
| log(TAG, msg = "Auto remove deleted notes after $days days failed", throwable = e) | ||
| Result.failure() | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the getFreshDatabase implementation
rg "fun getFreshDatabase" -A 10Repository: Crustack/NotallyX
Length of output: 1183
🏁 Script executed:
#!/bin/bash
# Find NotallyDatabase class definition
rg "class NotallyDatabase" -A 20Repository: Crustack/NotallyX
Length of output: 1975
🏁 Script executed:
#!/bin/bash
# Check for other usages of getFreshDatabase
rg "getFreshDatabase" -B 2 -A 5Repository: Crustack/NotallyX
Length of output: 13807
🏁 Script executed:
#!/bin/bash
# Check if database is being closed anywhere else
rg "database\.close\(\)" -B 2 -A 2Repository: Crustack/NotallyX
Length of output: 1468
🏁 Script executed:
#!/bin/bash
# Find createInstance implementation
rg "fun createInstance" -A 20Repository: Crustack/NotallyX
Length of output: 2221
🏁 Script executed:
#!/bin/bash
# Check the full context of createInstance calls - see if there's a singleton
rg "private var.*database|companion object" -A 5 app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.ktRepository: Crustack/NotallyX
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Check the BaseNoteModel usages to see the full pattern
rg "getFreshDatabase" -B 2 -A 15 app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.ktRepository: Crustack/NotallyX
Length of output: 1847
🏁 Script executed:
#!/bin/bash
# Look for all notallyDatabase usages in BaseNoteModel to see the full pattern including closing
rg "notallyDatabase" -B 5 -A 20 app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.ktRepository: Crustack/NotallyX
Length of output: 2764
🏁 Script executed:
#!/bin/bash
# Check if the test also closes the database
cat -n app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.ktRepository: Crustack/NotallyX
Length of output: 4792
🏁 Script executed:
#!/bin/bash
# Check if AutoRemoveDeletedNotesWorker's database is ever closed anywhere
rg "AutoRemoveDeletedNotesWorker" -A 100 app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.ktRepository: Crustack/NotallyX
Length of output: 2086
Add database.close() in finally block to prevent resource leak.
getFreshDatabase() creates a new Room database instance via Room.databaseBuilder() that must be explicitly closed. Currently, the database opened in doWork() is never closed, causing a connection leak on each daily execution.
Suggested fix
val database = NotallyDatabase.getFreshDatabase(this, preferences.dataInPublicFolder.value)
val baseNoteDao = database.getBaseNoteDao()
- return try {
+ return try {
val ids = baseNoteDao.getDeletedNoteIdsOlderThan(before)
if (ids.isNotEmpty()) {
val imageStrings = baseNoteDao.getImages(ids)
val fileStrings = baseNoteDao.getFiles(ids)
val audioStrings = baseNoteDao.getAudios(ids)
val images = imageStrings.flatMap { json -> Converters.jsonToFiles(json) }
val files = fileStrings.flatMap { json -> Converters.jsonToFiles(json) }
val audios = audioStrings.flatMap { json -> Converters.jsonToAudios(json) }
baseNoteDao.delete(ids)
deleteAttachments(images + files + audios, ids)
}
Result.success()
} catch (e: Exception) {
log(TAG, msg = "Auto remove deleted notes after $days days failed", throwable = e)
Result.failure()
+ } finally {
+ database.close()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val database = NotallyDatabase.getFreshDatabase(this, preferences.dataInPublicFolder.value) | |
| val baseNoteDao = database.getBaseNoteDao() | |
| return try { | |
| val ids = baseNoteDao.getDeletedNoteIdsOlderThan(before) | |
| if (ids.isNotEmpty()) { | |
| val imageStrings = baseNoteDao.getImages(ids) | |
| val fileStrings = baseNoteDao.getFiles(ids) | |
| val audioStrings = baseNoteDao.getAudios(ids) | |
| val images = imageStrings.flatMap { json -> Converters.jsonToFiles(json) } | |
| val files = fileStrings.flatMap { json -> Converters.jsonToFiles(json) } | |
| val audios = audioStrings.flatMap { json -> Converters.jsonToAudios(json) } | |
| baseNoteDao.delete(ids) | |
| deleteAttachments(images + files + audios, ids) | |
| } | |
| Result.success() | |
| } catch (e: Exception) { | |
| log(TAG, msg = "Auto remove deleted notes after $days days failed", throwable = e) | |
| Result.failure() | |
| } | |
| val database = NotallyDatabase.getFreshDatabase(this, preferences.dataInPublicFolder.value) | |
| val baseNoteDao = database.getBaseNoteDao() | |
| return try { | |
| val ids = baseNoteDao.getDeletedNoteIdsOlderThan(before) | |
| if (ids.isNotEmpty()) { | |
| val imageStrings = baseNoteDao.getImages(ids) | |
| val fileStrings = baseNoteDao.getFiles(ids) | |
| val audioStrings = baseNoteDao.getAudios(ids) | |
| val images = imageStrings.flatMap { json -> Converters.jsonToFiles(json) } | |
| val files = fileStrings.flatMap { json -> Converters.jsonToFiles(json) } | |
| val audios = audioStrings.flatMap { json -> Converters.jsonToAudios(json) } | |
| baseNoteDao.delete(ids) | |
| deleteAttachments(images + files + audios, ids) | |
| } | |
| Result.success() | |
| } catch (e: Exception) { | |
| log(TAG, msg = "Auto remove deleted notes after $days days failed", throwable = e) | |
| Result.failure() | |
| } finally { | |
| database.close() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`
around lines 37 - 58, The database opened by NotallyDatabase.getFreshDatabase in
AutoRemoveDeletedNotesWorker (inside doWork) is never closed; wrap the try/catch
in a try/finally (or use Kotlin's use/auto-close pattern) and call
database.close() in the finally block to ensure the Room instance is always
closed after using baseNoteDao.getDeletedNoteIdsOlderThan,
getImages/getFiles/getAudios, delete, and deleteAttachments; reference the
NotallyDatabase.getFreshDatabase call and the local variable database to locate
where to insert the close.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt (1)
263-265: Prefer the idiomatic KotlinPeriodicWorkRequestBuilder<T>overPeriodicWorkRequest.Builder(Class, ...).The
PeriodicWorkRequestBuilderinline helper was moved fromwork-runtime-ktxintowork-runtime, making it the standard Kotlin API. The class-based Builder is the Java-style approach.♻️ Proposed refactor
- val request = - PeriodicWorkRequest.Builder(AutoRemoveDeletedNotesWorker::class.java, 1, TimeUnit.DAYS) - .build() + val request = + PeriodicWorkRequestBuilder<AutoRemoveDeletedNotesWorker>(1, TimeUnit.DAYS) + .build()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt` around lines 263 - 265, Replace the Java-style PeriodicWorkRequest.Builder usage with the Kotlin idiomatic PeriodicWorkRequestBuilder for AutoRemoveDeletedNotesWorker: locate the PeriodicWorkRequest.Builder(...) construction that creates "request" for AutoRemoveDeletedNotesWorker and swap it to use PeriodicWorkRequestBuilder<AutoRemoveDeletedNotesWorker>(1, TimeUnit.DAYS) (ensure the import for PeriodicWorkRequestBuilder is present) so the code uses the Kotlin inline helper instead of the class-based Builder.app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt (1)
469-471: Useitinsideletbody instead of re-referencing the nullable parameter.Using
itis the idiomatic Kotlin pattern here and makes the smart-cast explicit.♻️ Proposed refactor
- tooltipResId?.let { tooltipText = context.getString(tooltipResId) } + tooltipResId?.let { tooltipText = context.getString(it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt` around lines 469 - 471, The nullable `tooltipResId` is being re-referenced inside its let block instead of using the lambda parameter; change the body of the let to use the implicit `it` (e.g., replace usage of `tooltipResId` inside the block with `it`) so the line becomes `tooltipResId?.let { tooltipText = context.getString(it) }`, updating the code around `tooltipResId`, `tooltipText`, and `context.getString` in PreferenceBindingExtensions.kt.app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt (1)
110-110:deleteAll()deletes the entireBaseNotetable and is only needed by tests — annotate or restrict it.This query has no
WHEREclause and is used only in test teardown. Exposing it on the productionBaseNoteDaointerface without any guard increases the risk of accidental use.♻️ Proposed: add `@VisibleForTesting`
+ `@VisibleForTesting` `@Query`("DELETE FROM BaseNote") suspend fun deleteAll()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt` at line 110, The deleteAll() method on BaseNoteDao performs an unguarded DELETE of the entire table and is test-only; annotate the method to prevent accidental production use by adding testing-only annotations (e.g., `@VisibleForTesting` and/or `@RestrictTo`(RestrictTo.Scope.TESTS)) to the deleteAll() function in BaseNoteDao, or alternatively move the method into a test-only DAO interface used only in tests; ensure the annotation(s) are applied to the suspend fun deleteAll() declaration so it’s clearly marked/limited to tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.kt`:
- Around line 117-128: testInstance is not thread-safe and remains pointing at a
closed DB; mark testInstance as `@Volatile` and ensure it is cleared when the test
DB is closed so subsequent tests get a fresh instance. Specifically, add
`@Volatile` to the testInstance declaration and update the teardown/close path
used by getTestDatabase/getFreshDatabase (or override the DB close listener) to
set testInstance = null immediately after calling close() so double-checked
locking remains valid and closed DB references are not reused.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt`:
- Around line 542-546: The label formatter in setupAutoEmptyBin uses a hardcoded
plural string and yields "1 days"; add a plurals resource named "days" (with
singular and other forms) and change the setLabelFormatter lambda to use
context.resources.getQuantityString(R.plurals.days, sliderValue.toInt(),
sliderValue.toInt()) for non-zero values so the label shows "1 day" vs "N days";
keep the zero branch returning context.getString(R.string.disabled).
In
`@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`:
- Around line 23-59: removeOldDeletedNotes is declared as a member extension
inside AutoRemoveDeletedNotesWorker which makes it inaccessible to external
callers/tests; move it out of the class body and declare it as a top-level
extension function on ContextWrapper (same signature: suspend fun
ContextWrapper.removeOldDeletedNotes(): Result) in the utilities file the tests
reference (or the same file but outside the class) so it can be imported as
com.philkes.notallyx.utils.backup.removeOldDeletedNotes; ensure all references
(including tests) import the top-level symbol and remove any dependency on
AutoRemoveDeletedNotesWorker instance state.
In
`@app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.kt`:
- Line 18: Remove the invalid import
com.philkes.notallyx.utils.backup.removeOldDeletedNotes from the test; that
function is a member extension defined inside AutoRemoveDeletedNotesWorker in
package com.philkes.notallyx.utils and cannot be imported from the backup
package. Delete the import line and either call the extension via an
AutoRemoveDeletedNotesWorker instance or adjust the test to use the publicly
accessible API in AutoRemoveDeletedNotesWorker / com.philkes.notallyx.utils
instead.
---
Duplicate comments:
In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt`:
- Around line 167-168: Pre-existing notes in DELETED can be erroneously purged
because their modifiedTimestamp reflects last edit time; add a one-time
migration or feature-activation step that updates modifiedTimestamp = current
system time for all notes where folder = 'DELETED' when the auto-remove feature
is first enabled; implement this by calling a DAO update (or new DAO method) to
set modifiedTimestamp = now for BaseNote rows with folder = 'DELETED' before any
calls to getDeletedNoteIdsOlderThan(...), and ensure the move(ids, folder,
timestamp) logic remains unchanged for future deletions so newly moved notes
keep correct deletion timestamps.
In
`@app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt`:
- Around line 32-35: The log message in AutoRemoveDeletedNotesWorker (Log.d
called with TAG) has an unclosed parenthesis in the interpolated string for
"(since: ${Date(before).toText()"; fix the string in the Log.d call by adding
the missing closing parenthesis so it becomes "(since:
${Date(before).toText()})" (ensure the rest of the string punctuation remains
correct).
- Around line 37-58: The Room database opened via
NotallyDatabase.getFreshDatabase(...) inside the worker
(AutoRemoveDeletedNotesWorker.doWork / the shown block) is never closed; wrap
the existing try/catch in a try/catch/finally (or add a finally) and call
database.close() in the finally to ensure the DB is closed on success or
exception, keeping the rest of the logic (ids retrieval, delete,
deleteAttachments) unchanged and referencing the same database variable.
---
Nitpick comments:
In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt`:
- Line 110: The deleteAll() method on BaseNoteDao performs an unguarded DELETE
of the entire table and is test-only; annotate the method to prevent accidental
production use by adding testing-only annotations (e.g., `@VisibleForTesting`
and/or `@RestrictTo`(RestrictTo.Scope.TESTS)) to the deleteAll() function in
BaseNoteDao, or alternatively move the method into a test-only DAO interface
used only in tests; ensure the annotation(s) are applied to the suspend fun
deleteAll() declaration so it’s clearly marked/limited to tests.
In `@app/src/main/java/com/philkes/notallyx/NotallyXApplication.kt`:
- Around line 263-265: Replace the Java-style PeriodicWorkRequest.Builder usage
with the Kotlin idiomatic PeriodicWorkRequestBuilder for
AutoRemoveDeletedNotesWorker: locate the PeriodicWorkRequest.Builder(...)
construction that creates "request" for AutoRemoveDeletedNotesWorker and swap it
to use PeriodicWorkRequestBuilder<AutoRemoveDeletedNotesWorker>(1,
TimeUnit.DAYS) (ensure the import for PeriodicWorkRequestBuilder is present) so
the code uses the Kotlin inline helper instead of the class-based Builder.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt`:
- Around line 469-471: The nullable `tooltipResId` is being re-referenced inside
its let block instead of using the lambda parameter; change the body of the let
to use the implicit `it` (e.g., replace usage of `tooltipResId` inside the block
with `it`) so the line becomes `tooltipResId?.let { tooltipText =
context.getString(it) }`, updating the code around `tooltipResId`,
`tooltipText`, and `context.getString` in PreferenceBindingExtensions.kt.
Comment on lines
+117
to
+128
| private var testInstance: NotallyDatabase? = null | ||
|
|
||
| private fun getTestDatabase(context: ContextWrapper): NotallyDatabase { | ||
| return testInstance | ||
| ?: synchronized(this) { | ||
| testInstance = | ||
| Room.inMemoryDatabaseBuilder(context, NotallyDatabase::class.java) | ||
| .allowMainThreadQueries() | ||
| .build() | ||
| return testInstance!! | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
testInstance lacks @Volatile and is never reset after close(), risking stale/closed DB for subsequent test classes.
Two issues:
- Unlike the production
instancefield,testInstanceis not@Volatile, breaking the double-checked locking invariant. - After a test class's
@AfterClasscloses the database,testInstancestill holds the closed reference. Any subsequent test class callinggetFreshDatabase(isTestRunner=true)will receive the already-closedNotallyDatabase.
🛡️ Proposed fix
- private var testInstance: NotallyDatabase? = null
+ `@Volatile` private var testInstance: NotallyDatabase? = null
private fun getTestDatabase(context: ContextWrapper): NotallyDatabase {
return testInstance
?: synchronized(this) {
+ // Re-check in case a previously closed instance was cleared
+ if (testInstance?.isOpen == false) testInstance = null
testInstance =
Room.inMemoryDatabaseBuilder(context, NotallyDatabase::class.java)
.allowMainThreadQueries()
.build()
return testInstance!!
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/philkes/notallyx/data/NotallyDatabase.kt` around lines
117 - 128, testInstance is not thread-safe and remains pointing at a closed DB;
mark testInstance as `@Volatile` and ensure it is cleared when the test DB is
closed so subsequent tests get a fresh instance. Specifically, add `@Volatile` to
the testInstance declaration and update the teardown/close path used by
getTestDatabase/getFreshDatabase (or override the DB close listener) to set
testInstance = null immediately after calling close() so double-checked locking
remains valid and closed DB references are not reused.
Comment on lines
+542
to
+546
| setLabelFormatter { sliderValue -> | ||
| if (sliderValue == 0f) { | ||
| context.getString(R.string.disabled) | ||
| } else "${sliderValue.toInt()} ${context.getString(R.string.days)}" | ||
| } |
Contributor
There was a problem hiding this comment.
setupAutoEmptyBin label produces "1 days" — use plural string resource.
"${sliderValue.toInt()} ${context.getString(R.string.days)}" always appends the plural form, producing "1 days" when the slider is at 1. Use getQuantityString with a plurals resource to correctly render "1 day" vs "N days".
🌐 Proposed fix
In res/values/strings.xml, replace/add a plural:
<plurals name="days">
<item quantity="one">%d day</item>
<item quantity="other">%d days</item>
</plurals>Then in setupAutoEmptyBin:
- } else "${sliderValue.toInt()} ${context.getString(R.string.days)}"
+ } else context.resources.getQuantityString(R.plurals.days, sliderValue.toInt(), sliderValue.toInt())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setLabelFormatter { sliderValue -> | |
| if (sliderValue == 0f) { | |
| context.getString(R.string.disabled) | |
| } else "${sliderValue.toInt()} ${context.getString(R.string.days)}" | |
| } | |
| setLabelFormatter { sliderValue -> | |
| if (sliderValue == 0f) { | |
| context.getString(R.string.disabled) | |
| } else context.resources.getQuantityString(R.plurals.days, sliderValue.toInt(), sliderValue.toInt()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt`
around lines 542 - 546, The label formatter in setupAutoEmptyBin uses a
hardcoded plural string and yields "1 days"; add a plurals resource named "days"
(with singular and other forms) and change the setLabelFormatter lambda to use
context.resources.getQuantityString(R.plurals.days, sliderValue.toInt(),
sliderValue.toInt()) for non-zero values so the label shows "1 day" vs "N days";
keep the zero branch returning context.getString(R.string.disabled).
app/src/main/java/com/philkes/notallyx/utils/AutoRemoveDeletedNotesWorker.kt
Outdated
Show resolved
Hide resolved
app/src/test/kotlin/com/philkes/notallyx/utils/AutoRemoveDeletedNotesDbTest.kt
Show resolved
Hide resolved
Crustack
force-pushed
the
feat/393
branch
2 times, most recently
from
99f7af8 to
98b8fc0
Compare
This was referenced Mar 1, 2026
This was referenced Apr 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #393
Summary by CodeRabbit
New Features
Documentation
Tests