Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive size-limiting framework for note bodies. It establishes a 1.5 MB body size limit with safe insertion, truncation, and text filtering capabilities. The legacy migration system is replaced with a new migration infrastructure that supports splitting oversized notes into linked parts. Large notes are automatically split during import/insertion while preserving span annotations. Input filters enforce the size constraint at the UI layer, and repair utilities handle span clipping during truncation. The changes span data access objects, view models, activities, utility functions, and include comprehensive test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant MainActivity
participant DataSchemaMigrations
participant BaseNoteDao
participant Database
MainActivity->>MainActivity: onCreate()
MainActivity->>MainActivity: checkForMigrations(savedInstanceState)
MainActivity->>DataSchemaMigrations: runMigrations(onProgressTitle)
DataSchemaMigrations->>Database: Query dataSchemaId
alt dataSchemaId < 2
DataSchemaMigrations->>Database: moveAttachments (if < 1)
DataSchemaMigrations->>BaseNoteDao: splitOversizedNotes (if < 2)
BaseNoteDao->>Database: Query oversized notes
BaseNoteDao->>BaseNoteDao: splitOversizedExistingNoteForMigration
BaseNoteDao->>Database: Insert split note parts
BaseNoteDao->>Database: Update original note with link
DataSchemaMigrations->>Database: setDataSchemaId = LATEST
end
DataSchemaMigrations-->>MainActivity: return migration success
MainActivity->>MainActivity: Execute navigation logic
sequenceDiagram
participant CommonDao
participant NoteSplitUtils
participant BaseNoteDao
participant Database
CommonDao->>CommonDao: importBackup(context, baseNotes, labels)
loop for each note
CommonDao->>CommonDao: Check if note exceeds MAX_BODY_CHAR_LENGTH
alt note exceeds limit
CommonDao->>NoteSplitUtils: splitAndInsertForImport(note)
NoteSplitUtils->>NoteSplitUtils: sliceIntoChunks(note, linkTextLength)
NoteSplitUtils->>BaseNoteDao: insert(final part)
BaseNoteDao->>Database: INSERT part with spans
NoteSplitUtils->>NoteSplitUtils: Create link span to final part
NoteSplitUtils->>BaseNoteDao: insert(part with link)
BaseNoteDao->>Database: INSERT part with link span
NoteSplitUtils-->>CommonDao: return firstId, parts list
else note within limit
CommonDao->>BaseNoteDao: insert(note with id=0)
BaseNoteDao->>Database: INSERT note
end
end
CommonDao->>CommonDao: Remap spans to new note IDs
CommonDao->>BaseNoteDao: updateSpans(noteId, updatedSpans)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 10
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt`:
- Around line 70-93: In insertSafe, only log and show the toast when something
was actually truncated: guard the context.log and context.showToast calls with a
check like if (truncatedNotes.isNotEmpty()) to avoid spurious messages; also fix
the size accounting by adding the truncated note's length (use note.body.length)
to truncatedCharacterSize instead of baseNote.body.length so
truncatedCharacterSize reflects the stored/truncated size, not the original
size.
- Around line 46-49: The truncation in BaseNote.truncated currently slices body
but leaves spans intact; update truncated() so when body.length >
MAX_BODY_CHAR_LENGTH you not only copy(body = body.take(MAX_BODY_CHAR_LENGTH))
but also clamp/clip the note's spans to the new length (remove spans that start
>= newLen and trim spans that partially exceed newLen by reducing their length
to fit), returning Pair(true, copy(body = truncatedBody, spans = clippedSpans));
keep MAX_BODY_CHAR_LENGTH and BaseNote.truncated as the reference points for the
change.
In `@app/src/main/java/com/philkes/notallyx/data/dao/CommonDao.kt`:
- Around line 48-61: The importBackup method in CommonDao declares an unused
context: ContextWrapper parameter; remove this unused parameter from the open
suspend fun importBackup signature and update all implementations/overrides and
call sites to match the new signature. Ensure the method body still references
database.getBaseNoteDao(), MAX_BODY_CHAR_LENGTH,
NoteSplitUtils.splitAndInsertForImport(note, dao) and dao.insert(note.copy(id =
0)) unchanged; if any external interface or tests declared the old signature,
update them accordingly.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.kt`:
- Around line 144-145: Remove the unconditional preferences.setDataSchemaId(1)
call so runMigrations() can read the actual stored schema and perform all needed
migrations; specifically, delete the line that calls setDataSchemaId(1) before
the if-check and let preferences.dataSchemaId.value and the runMigrations() flow
(which updates the schema to LATEST_DATA_SCHEMA) handle applying the attachment
relocation (for dataSchemaId < 1) and subsequent migrations; references:
setDataSchemaId, dataSchemaId, runMigrations, LATEST_DATA_SCHEMA.
In `@app/src/main/java/com/philkes/notallyx/utils/backup/ImportExtensions.kt`:
- Around line 269-288: When catching SQLiteBlobTooBigException in toBaseNote
(the catch block that falls back to the substr(body...) query), the code
currently returns the truncated body but leaves spansTmp unchanged so
out-of-bounds spans are persisted; update the truncation path to clip/validate
spans before returning the BaseNote by invoking the existing routine used
elsewhere (either call NoteRepairUtils.truncateBodyAndFixSpans() with the new
body length or apply NoteRepairUtils.sliceSpans() to spansTmp based on the
truncated body length), ensuring spansTmp is replaced with the clipped span
string prior to constructing the BaseNote (this mirrors the protection used by
NoteRepairUtils and prevents dao.insert() in CommonDao.importBackup from
persisting invalid spans).
In `@app/src/main/java/com/philkes/notallyx/utils/DataSchemaMigrations.kt`:
- Around line 76-84: The SQLiteBlobTooBigException thrown when calling
dao.get(id) is currently swallowed; before calling truncateBodyAndFixSpans(dao,
id) and retrying dao.get(id), catch the exception and log it (including the
exception message/stack) so the failure is recorded — update the try/catch
around dao.get(id) that assigns original to log the caught
SQLiteBlobTooBigException (and context like id) then increment repaired and call
truncateBodyAndFixSpans(dao, id) and retry dao.get(id).
In `@app/src/main/java/com/philkes/notallyx/utils/LengthFilterWithToast.kt`:
- Around line 9-22: The constructor parameter `max` in class
LengthFilterWithToast is not stored as a property so it isn't accessible inside
filter(); change the primary constructor to store it (e.g., add `private val
max`) so the filter() method can reference `max`, then rebuild and ensure the
existing call to
context.showToast(context.getString(R.string.note_text_too_long_truncated, max))
compiles and works as intended.
In `@app/src/main/java/com/philkes/notallyx/utils/MiscExtensions.kt`:
- Around line 61-71: The KDoc for charLimit() claims it returns a Pair of
min/max characters but the function actually returns a single Int (minChars);
update the documentation to match the implementation by removing the Pair
description and describing that charLimit(): Int returns the minimum guaranteed
characters (worst-case 4 bytes/char) computed as minChars, and adjust the
`@return` tag to indicate an Int instead of a Pair.
- Around line 47-58: The String.truncateToMb implementation can produce a String
whose UTF-8 byte length exceeds maxBytes because slicing arbitrary bytes may
produce replacement characters (U+FFFD) that are 3 bytes each; fix
String.truncateToMb so it returns a String whose UTF-8 encoded byte size is
guaranteed ≤ maxBytes by trimming to the last valid UTF-8 character boundary or
by re-encoding and, if necessary, shortening the byte slice/character count
iteratively until String(bytes).toByteArray(Charsets.UTF_8).size <= maxBytes;
update the function (String.truncateToMb) to locate a safe cut point (or loop
reducing end index) rather than returning a direct decode of an arbitrary byte
slice.
In `@app/src/main/res/values/strings.xml`:
- Around line 252-257: The string resource notes_too_big_truncating has
incorrect grammar ("Note are")—update its value to "Notes are too big to save,
they were truncated to %2$s characters" (preserve the %1$s/%2$s placeholders as
intended) so the plural subject and verb agree; edit the string with
name="notes_too_big_truncating" accordingly.
🧹 Nitpick comments (5)
app/src/test/kotlin/com/philkes/notallyx/utils/NoteSplitUtilsDbTest.kt (1)
85-89: Add a comment clarifying the expected insertion order.The assignment of
finalPart = notes[0]andfirstPart = notes[1]relies on the implementation detail thatsplitAndInsertForImportinserts the final part first to obtain its ID for linking. Adding a brief comment would make this test more maintainable.💡 Suggested improvement
// Load all notes and sort by id val notes = dao.getAll().sortedBy { it.id } assertEquals(2, notes.size) + // splitAndInsertForImport inserts final part first (to get ID for link), then first part val finalPart = notes[0] val firstPart = notes[1]app/src/test/kotlin/com/philkes/notallyx/utils/NoteSplitUtilsTest.kt (1)
49-49: Minor: redundant-0in shift parameter.
shift = -0is mathematically equivalent toshift = 0. Consider simplifying for clarity.- val sliced = NoteSplitUtils.sliceSpans(spans, rangeStart = 0, rangeEnd = 8, shift = -0) + val sliced = NoteSplitUtils.sliceSpans(spans, rangeStart = 0, rangeEnd = 8, shift = 0)app/src/test/kotlin/com/philkes/notallyx/utils/NoteRepairUtilsDbTest.kt (1)
80-80: Minor:varshould beval.
beforeis never reassigned, so it should be declared asval.- var before = dao.get(id)!! + val before = dao.get(id)!!app/src/main/java/com/philkes/notallyx/utils/NoteSplitUtils.kt (2)
1-16: Derive link-span offsets fromLINK_TEXTto prevent drift.Hard-coded
"\n\n"and"Open next part"appear in two places; ifLINK_TEXTchanges, offsets will break.♻️ Suggested refactor
- const val LINK_TEXT: String = "\n\nOpen next part" + private const val LINK_PREFIX = "\n\n" + private const val LINK_LABEL = "Open next part" + const val LINK_TEXT: String = LINK_PREFIX + LINK_LABEL @@ - val linkStart = chunkBody.length + ("\n\n").length - val linkEnd = linkStart + ("Open next part").length + val linkStart = chunkBody.length + LINK_PREFIX.length + val linkEnd = linkStart + LINK_LABEL.length @@ - val linkStart = chunkBody.length + ("\n\n").length - val linkEnd = linkStart + ("Open next part").length + val linkStart = chunkBody.length + LINK_PREFIX.length + val linkEnd = linkStart + LINK_LABEL.lengthAlso applies to: 104-115, 190-199
23-49: UseSpanRepresentation.copy(...)to avoid manual field reconstruction.Since
SpanRepresentationis a data class, manual reconstruction risks missing new properties if the class expands. The copy method automatically includes all fields, making it the safer and more maintainable approach.♻️ Suggested refactor
- result.add( - SpanRepresentation( - start = newStart, - end = newEnd, - bold = s.bold, - link = s.link, - linkData = s.linkData, - italic = s.italic, - monospace = s.monospace, - strikethrough = s.strikethrough, - ) - ) + result.add(s.copy(start = newStart, end = newEnd))
Comment on lines
+46
to
+49
| private fun BaseNote.truncated(): Pair<Boolean, BaseNote> { | ||
| return if (body.length > MAX_BODY_CHAR_LENGTH) { | ||
| return Pair(true, copy(body = body.take(MAX_BODY_CHAR_LENGTH))) | ||
| } else Pair(false, this) |
Contributor
There was a problem hiding this comment.
Clamp spans when truncating bodies.
Body truncation without span clipping can leave out-of-bounds spans and corrupt rendering.
🛠️ Suggested fix
-import com.philkes.notallyx.utils.charLimit
+import com.philkes.notallyx.utils.charLimit
+import com.philkes.notallyx.utils.NoteSplitUtils.sliceSpans
@@
private fun BaseNote.truncated(): Pair<Boolean, BaseNote> {
- return if (body.length > MAX_BODY_CHAR_LENGTH) {
- return Pair(true, copy(body = body.take(MAX_BODY_CHAR_LENGTH)))
- } else Pair(false, this)
+ return if (body.length > MAX_BODY_CHAR_LENGTH) {
+ val newBody = body.take(MAX_BODY_CHAR_LENGTH)
+ val newSpans =
+ if (spans.isNotEmpty()) sliceSpans(spans, 0, newBody.length, 0) else emptyList()
+ Pair(true, copy(body = newBody, spans = newSpans))
+ } else Pair(false, this)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private fun BaseNote.truncated(): Pair<Boolean, BaseNote> { | |
| return if (body.length > MAX_BODY_CHAR_LENGTH) { | |
| return Pair(true, copy(body = body.take(MAX_BODY_CHAR_LENGTH))) | |
| } else Pair(false, this) | |
| private fun BaseNote.truncated(): Pair<Boolean, BaseNote> { | |
| return if (body.length > MAX_BODY_CHAR_LENGTH) { | |
| val newBody = body.take(MAX_BODY_CHAR_LENGTH) | |
| val newSpans = | |
| if (spans.isNotEmpty()) sliceSpans(spans, 0, newBody.length, 0) else emptyList() | |
| Pair(true, copy(body = newBody, spans = newSpans)) | |
| } else Pair(false, this) | |
| } |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt` around lines
46 - 49, The truncation in BaseNote.truncated currently slices body but leaves
spans intact; update truncated() so when body.length > MAX_BODY_CHAR_LENGTH you
not only copy(body = body.take(MAX_BODY_CHAR_LENGTH)) but also clamp/clip the
note's spans to the new length (remove spans that start >= newLen and trim spans
that partially exceed newLen by reducing their length to fit), returning
Pair(true, copy(body = truncatedBody, spans = clippedSpans)); keep
MAX_BODY_CHAR_LENGTH and BaseNote.truncated as the reference points for the
change.
Comment on lines
+70
to
+93
| suspend fun insertSafe(context: ContextWrapper, baseNotes: List<BaseNote>): List<Long> { | ||
| val truncatedNotes = mutableListOf<BaseNote>() | ||
| var truncatedCharacterSize = 0 | ||
| val notes = | ||
| baseNotes.map { baseNote -> | ||
| val (truncated, note) = baseNote.truncated() | ||
| if (truncated) { | ||
| truncatedCharacterSize += baseNote.body.length | ||
| truncatedNotes.add(note) | ||
| } | ||
| note | ||
| } | ||
| context.log( | ||
| TAG, | ||
| "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters", | ||
| ) | ||
| context.showToast( | ||
| context.getString( | ||
| R.string.notes_too_big_truncating, | ||
| truncatedNotes.size, | ||
| truncatedCharacterSize, | ||
| ) | ||
| ) | ||
| return insert(notes) |
Contributor
There was a problem hiding this comment.
Avoid logging/toasting when nothing was truncated and fix size count.
The current implementation logs and shows a toast even when no note was truncated, and the size metric uses the original length rather than the truncated length.
🛠️ Suggested fix
val notes =
baseNotes.map { baseNote ->
val (truncated, note) = baseNote.truncated()
if (truncated) {
- truncatedCharacterSize += baseNote.body.length
+ truncatedCharacterSize += note.body.length
truncatedNotes.add(note)
}
note
}
- context.log(
- TAG,
- "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters",
- )
- context.showToast(
- context.getString(
- R.string.notes_too_big_truncating,
- truncatedNotes.size,
- truncatedCharacterSize,
- )
- )
+ if (truncatedNotes.isNotEmpty()) {
+ context.log(
+ TAG,
+ "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters",
+ )
+ context.showToast(
+ context.getString(
+ R.string.notes_too_big_truncating,
+ truncatedNotes.size,
+ truncatedCharacterSize,
+ )
+ )
+ }
return insert(notes)🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt` around lines
70 - 93, In insertSafe, only log and show the toast when something was actually
truncated: guard the context.log and context.showToast calls with a check like
if (truncatedNotes.isNotEmpty()) to avoid spurious messages; also fix the size
accounting by adding the truncated note's length (use note.body.length) to
truncatedCharacterSize instead of baseNote.body.length so truncatedCharacterSize
reflects the stored/truncated size, not the original size.
Comment on lines
+48
to
+61
| open suspend fun importBackup( | ||
| context: ContextWrapper, | ||
| baseNotes: List<BaseNote>, | ||
| labels: List<Label>, | ||
| ) { | ||
| val dao = database.getBaseNoteDao() | ||
| // Insert notes, splitting oversized text notes instead of truncating | ||
| baseNotes.forEach { note -> | ||
| if (note.type == Type.NOTE && note.body.length > MAX_BODY_CHAR_LENGTH) { | ||
| NoteSplitUtils.splitAndInsertForImport(note, dao) | ||
| } else { | ||
| dao.insert(note.copy(id = 0)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify splitAndInsertForImport signature and importBackup usage
echo "=== Checking splitAndInsertForImport signature ==="
rg -n "fun splitAndInsertForImport" --type=kt -A5
echo ""
echo "=== Checking all calls to importBackup ==="
rg -n "importBackup" --type=kt -B2 -A2
echo ""
echo "=== Checking if context is used anywhere else in importBackup method ==="
# Get the full importBackup method to check for context usage
rg -n "open suspend fun importBackup" --type=kt -A20Repository: Crustack/NotallyX
Length of output: 327
🏁 Script executed:
#!/bin/bash
# Find Kotlin files and search for the relevant signatures
echo "=== Finding Kotlin files ==="
fd -e kt | head -20
echo ""
echo "=== Checking splitAndInsertForImport signature ==="
rg "fun splitAndInsertForImport" -A5
echo ""
echo "=== Checking all calls to importBackup ==="
rg "importBackup" -B2 -A2
echo ""
echo "=== Checking the full importBackup method signature ==="
rg "open suspend fun importBackup" -A30Repository: Crustack/NotallyX
Length of output: 50373
Remove unused context parameter from importBackup.
The context: ContextWrapper parameter is declared but never used. NoteSplitUtils.splitAndInsertForImport(note, dao) doesn't accept a context parameter, and no other code in the method references it. Either remove the parameter or confirm if it's intended for future use.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/data/dao/CommonDao.kt` around lines 48
- 61, The importBackup method in CommonDao declares an unused context:
ContextWrapper parameter; remove this unused parameter from the open suspend fun
importBackup signature and update all implementations/overrides and call sites
to match the new signature. Ensure the method body still references
database.getBaseNoteDao(), MAX_BODY_CHAR_LENGTH,
NoteSplitUtils.splitAndInsertForImport(note, dao) and dao.insert(note.copy(id =
0)) unchanged; if any external interface or tests declared the old signature,
update them accordingly.
Comment on lines
+144
to
+145
| preferences.setDataSchemaId(1) | ||
| if (preferences.dataSchemaId.value < LATEST_DATA_SCHEMA) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the expected behavior of dataSchemaId and LATEST_DATA_SCHEMA
echo "=== Checking LATEST_DATA_SCHEMA value ==="
rg -n "LATEST_DATA_SCHEMA\s*=" --type=kt
echo ""
echo "=== Checking setDataSchemaId usage ==="
rg -n "setDataSchemaId" --type=kt -C3
echo ""
echo "=== Checking dataSchemaId preference definition ==="
rg -n "dataSchemaId" --type=kt -C3
echo ""
echo "=== Checking runMigrations implementation ==="
ast-grep --pattern 'suspend fun $_.runMigrations($$$) {
$$$
}'
echo ""
echo "=== Checking the exact context in MainActivity.kt around lines 144-145 ==="
sed -n '140,160p' app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.ktRepository: Crustack/NotallyX
Length of output: 1353
🏁 Script executed:
#!/bin/bash
# Search without file type restrictions and find kotlin files
echo "=== Finding kotlin files in project ==="
find . -name "*.kt" -type f | head -20
echo ""
echo "=== Searching for LATEST_DATA_SCHEMA definition ==="
rg -n "LATEST_DATA_SCHEMA" | head -20
echo ""
echo "=== Searching for setDataSchemaId implementation ==="
rg -n "fun setDataSchemaId" -A5
echo ""
echo "=== Searching for dataSchemaId definition ==="
rg -n "val dataSchemaId\|var dataSchemaId" -B2 -A2
echo ""
echo "=== Searching for runMigrations implementation ==="
rg -n "fun runMigrations\|suspend fun runMigrations" -A10
echo ""
echo "=== Full context: lines 130-160 in MainActivity.kt ==="
sed -n '130,160p' app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.ktRepository: Crustack/NotallyX
Length of output: 4409
🏁 Script executed:
#!/bin/bash
echo "=== Searching for dataSchemaId property definition ==="
rg -n "dataSchemaId" app/src/main/java/com/philkes/notallyx/presentation/viewmodel/preference/ -B2 -A2
echo ""
echo "=== Full NotallyXPreferences file to understand dataSchemaId ==="
grep -n "dataSchemaId" app/src/main/java/com/philkes/notallyx/presentation/viewmodel/preference/NotallyXPreferences.kt
echo ""
echo "=== Checking runMigrations implementation ==="
rg -n "fun runMigrations\|suspend fun runMigrations" -A15
echo ""
echo "=== Full DataSchemaMigrations.kt file ==="
cat app/src/main/java/com/philkes/notallyx/utils/DataSchemaMigrations.kt
echo ""
echo "=== Checking after migrations complete - full checkForMigrations method ==="
sed -n '134,170p' app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.ktRepository: Crustack/NotallyX
Length of output: 7196
Remove unconditional dataSchemaId reset that skips attachment migration on fresh installs.
Line 144 unconditionally sets dataSchemaId to 1 before the check, causing a critical bug:
- On fresh install (default
dataSchemaId= 0),setDataSchemaId(1)modifies the preference beforerunMigrations()reads it runMigrations()then reads the modified value (1), so the migration fordataSchemaId < 1(attachment relocation) is skipped- Only the migration for
dataSchemaId < 2(note splitting) runs
Remove line 144. Let runMigrations() read the actual current dataSchemaId and handle all necessary migrations; it already updates the preference to the final schema after completion.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.kt`
around lines 144 - 145, Remove the unconditional preferences.setDataSchemaId(1)
call so runMigrations() can read the actual stored schema and perform all needed
migrations; specifically, delete the line that calls setDataSchemaId(1) before
the if-check and let preferences.dataSchemaId.value and the runMigrations() flow
(which updates the schema to LATEST_DATA_SCHEMA) handle applying the attachment
relocation (for dataSchemaId < 1) and subsequent migrations; references:
setDataSchemaId, dataSchemaId, runMigrations, LATEST_DATA_SCHEMA.
Comment on lines
+269
to
288
| val id = getLong(getColumnIndexOrThrow("id")) | ||
| val body = | ||
| try { | ||
| getString(getColumnIndexOrThrow("body")) | ||
| } catch (_: SQLiteBlobTooBigException) { | ||
| // Fall back to truncated read from source DB to avoid cursor window overflow | ||
| val cursor = | ||
| sourceDb.rawQuery( | ||
| "SELECT substr(body, 1, ?) AS body FROM BaseNote WHERE id = ?", | ||
| arrayOf( | ||
| com.philkes.notallyx.data.dao.BaseNoteDao.Companion.MAX_BODY_CHAR_LENGTH | ||
| .toString(), | ||
| id.toString(), | ||
| ), | ||
| ) | ||
| val value = if (cursor.moveToFirst()) cursor.getString(0) else "" | ||
| cursor.close() | ||
| value | ||
| } | ||
| val spansTmp = getString(getColumnIndexOrThrow("spans")) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that importBackup or related methods handle span clipping for truncated bodies
echo "=== Checking CommonDao.importBackup for span handling ==="
rg -n -A 10 "fun importBackup" --type kotlin
echo ""
echo "=== Checking if sliceSpans or span clipping is called during import ==="
rg -n "sliceSpans|clipSpans" --type kotlin
echo ""
echo "=== Checking NoteSplitUtils for span adjustment ==="
rg -n "sliceSpans|clipSpans" --type kotlin
echo ""
echo "=== Looking for span adjustment logic more broadly ==="
rg -n "Span.*length|length.*Span" --type kotlin -A 2 -B 2Repository: Crustack/NotallyX
Length of output: 11140
🏁 Script executed:
#!/bin/bash
# Check the full context of toBaseNote function
echo "=== Full toBaseNote function ==="
sed -n '240,300p' app/src/main/java/com/philkes/notallyx/utils/backup/ImportExtensions.kt
echo ""
echo "=== Check how toBaseNote is called ==="
rg -n "toBaseNote" app/src/main/java/com/philkes/notallyx/utils/backup/ImportExtensions.kt -B 2 -A 2
echo ""
echo "=== Check if spans are validated after reading ==="
rg -n "from.*Source\|toBaseNote" app/src/main/java/com/philkes/notallyx/utils/backup/ImportExtensions.kt -A 5Repository: Crustack/NotallyX
Length of output: 3033
🏁 Script executed:
#!/bin/bash
# Check the full importBackup function and the else branch handling
echo "=== Full importBackup function from line 48 ==="
sed -n '48,85p' app/src/main/java/com/philkes/notallyx/data/dao/CommonDao.kt
echo ""
echo "=== Check if spans are validated or clipped during normal insertion ==="
rg -n "insertBaseNote\|insert.*spans" app/src/main/java/com/philkes/notallyx/data/dao -A 5 --type kotlin
echo ""
echo "=== Check BaseNoteDao insert method ==="
rg -n "fun insert" app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt -A 10Repository: Crustack/NotallyX
Length of output: 3726
🏁 Script executed:
#!/bin/bash
# Final verification: Check if spans are validated anywhere else in the import path
echo "=== Check NoteRepairUtils to see when repair/clipping happens ==="
cat -n app/src/main/java/com/philkes/notallyx/utils/NoteRepairUtils.kt
echo ""
echo "=== Verify that insert() doesn't clip spans ==="
grep -n "fun insert" app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.kt -A 1Repository: Crustack/NotallyX
Length of output: 2058
Spans with out-of-bounds positions are persisted when body is truncated during import.
When the body read triggers SQLiteBlobTooBigException and falls back to a truncated read, the spans are read in full without adjustment. The truncated note (with body.length <= MAX_BODY_CHAR_LENGTH) then bypasses NoteSplitUtils.splitAndInsertForImport (which would clip spans) and is inserted directly via dao.insert() with no span validation.
The codebase already has the pattern for fixing this in NoteRepairUtils.truncateBodyAndFixSpans(), which clips spans using sliceSpans() after truncation. Apply the same logic during import: when toBaseNote truncates the body, clip the spans to the new body length before returning the note. Alternatively, validate and clip spans in the normal insertion path of CommonDao.importBackup.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/utils/backup/ImportExtensions.kt`
around lines 269 - 288, When catching SQLiteBlobTooBigException in toBaseNote
(the catch block that falls back to the substr(body...) query), the code
currently returns the truncated body but leaves spansTmp unchanged so
out-of-bounds spans are persisted; update the truncation path to clip/validate
spans before returning the BaseNote by invoking the existing routine used
elsewhere (either call NoteRepairUtils.truncateBodyAndFixSpans() with the new
body length or apply NoteRepairUtils.sliceSpans() to spansTmp based on the
truncated body length), ensuring spansTmp is replaced with the clipped span
string prior to constructing the BaseNote (this mirrors the protection used by
NoteRepairUtils and prevents dao.insert() in CommonDao.importBackup from
persisting invalid spans).
Comment on lines
+76
to
+84
| val original = | ||
| try { | ||
| dao.get(id) | ||
| } catch (e: SQLiteBlobTooBigException) { | ||
| // Repair the single offending row, then retry | ||
| repaired += 1 | ||
| truncateBodyAndFixSpans(dao, id) | ||
| dao.get(id) | ||
| } |
Contributor
There was a problem hiding this comment.
Log the SQLiteBlobTooBigException before truncation.
The exception is currently swallowed, which makes diagnosing data loss or corrupted rows harder.
🛠️ Suggested fix
} catch (e: SQLiteBlobTooBigException) {
// Repair the single offending row, then retry
repaired += 1
+ log(TAG, "SQLite blob too big for note id=$id; truncating before retry", e)
truncateBodyAndFixSpans(dao, id)
dao.get(id)
}📝 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 original = | |
| try { | |
| dao.get(id) | |
| } catch (e: SQLiteBlobTooBigException) { | |
| // Repair the single offending row, then retry | |
| repaired += 1 | |
| truncateBodyAndFixSpans(dao, id) | |
| dao.get(id) | |
| } | |
| val original = | |
| try { | |
| dao.get(id) | |
| } catch (e: SQLiteBlobTooBigException) { | |
| // Repair the single offending row, then retry | |
| repaired += 1 | |
| log(TAG, "SQLite blob too big for note id=$id; truncating before retry", e) | |
| truncateBodyAndFixSpans(dao, id) | |
| dao.get(id) | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 79-79: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/utils/DataSchemaMigrations.kt` around
lines 76 - 84, The SQLiteBlobTooBigException thrown when calling dao.get(id) is
currently swallowed; before calling truncateBodyAndFixSpans(dao, id) and
retrying dao.get(id), catch the exception and log it (including the exception
message/stack) so the failure is recorded — update the try/catch around
dao.get(id) that assigns original to log the caught SQLiteBlobTooBigException
(and context like id) then increment repaired and call
truncateBodyAndFixSpans(dao, id) and retry dao.get(id).
Comment on lines
+47
to
+58
| fun String.truncateToMb(mb: Double): String { | ||
| val maxBytes = (mb * 1024 * 1024).toInt() | ||
| val bytes = this.toByteArray(Charsets.UTF_8) | ||
|
|
||
| if (bytes.size <= maxBytes) return this | ||
|
|
||
| // Take only the allowed bytes | ||
| val truncatedBytes = bytes.sliceArray(0 until maxBytes) | ||
|
|
||
| // Converting back to String handles partial UTF-8 characters | ||
| // by using the default replacement behavior. | ||
| return String(truncatedBytes, Charsets.UTF_8) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/main/java/com/philkes/notallyx/utils/MiscExtensions.kt | sed -n '40,65p'Repository: Crustack/NotallyX
Length of output: 1052
🏁 Script executed:
# Search for usages of truncateToMb in the codebase
rg "truncateToMb" --type kotlin -B 2 -A 2Repository: Crustack/NotallyX
Length of output: 519
🏁 Script executed:
# Also check if there's a charLimit function mentioned in the review scratchpad
rg "charLimit" --type kotlin -B 2 -A 2Repository: Crustack/NotallyX
Length of output: 1551
🏁 Script executed:
rg "\.truncateToMb|truncateToMb\(" --type kotlin -B 3 -A 3Repository: Crustack/NotallyX
Length of output: 664
🏁 Script executed:
# Also check the full charLimit function to understand the complete picture
cat -n app/src/main/java/com/philkes/notallyx/utils/MiscExtensions.kt | sed -n '61,75p'Repository: Crustack/NotallyX
Length of output: 670
🏁 Script executed:
# Broader search for truncateToMb usage
rg "truncateToMb" --type kotlinRepository: Crustack/NotallyX
Length of output: 169
🏁 Script executed:
# Test the UTF-8 replacement behavior with a Python script
python3 << 'EOF'
# UTF-8 replacement character
import sys
# Test case: simulate truncation mid-multibyte character
# 4-byte UTF-8 sequence: F0 90 80 80 (𐀀 in UTF-8)
test_bytes = bytes([0xF0, 0x90, 0x80]) # Incomplete 4-byte sequence (missing last byte)
# Decode with replacement
decoded = test_bytes.decode('utf-8', errors='replace')
print(f"Incomplete sequence: {test_bytes.hex()}")
print(f"Decoded string: {repr(decoded)}")
# Re-encode
re_encoded = decoded.encode('utf-8')
print(f"Re-encoded bytes: {re_encoded.hex()}")
print(f"Original byte count: {len(test_bytes)}")
print(f"Re-encoded byte count: {len(re_encoded)}")
print(f"Overflow: {len(re_encoded) - len(test_bytes)} bytes")
print()
# Replacement character info
replacement_char = '\ufffd'
replacement_bytes = replacement_char.encode('utf-8')
print(f"Replacement character U+FFFD: {replacement_bytes.hex()}")
print(f"Size of U+FFFD in UTF-8: {len(replacement_bytes)} bytes")
EOFRepository: Crustack/NotallyX
Length of output: 147
🏁 Script executed:
# Check if truncateToMb is referenced in comments or documentation
rg "truncateToMb" --all-types -B 5 -A 5Repository: Crustack/NotallyX
Length of output: 474
🏁 Script executed:
# Check git history for this function (recent commits)
git log --oneline --all --grep="truncate" | head -20Repository: Crustack/NotallyX
Length of output: 43
🏁 Script executed:
# Check if there are any DB size limits referenced
rg "1024.*1024|MB|maxBytes" --type kotlin | head -20Repository: Crustack/NotallyX
Length of output: 1269
Fix UTF-8 truncation to prevent byte limit overflow from replacement characters.
The current implementation can exceed maxBytes when decoding invalid UTF-8 sequences. The replacement character U+FFFD encodes to 3 bytes; truncating mid-multibyte sequence creates invalid bytes that are replaced, causing re-encoding to potentially exceed the limit. Although truncateToMb is currently unused in the codebase, if deployed for enforcing strict byte limits (e.g., the 1.5 MB database constraint), it will silently violate the constraint. Trim to a valid UTF-8 boundary or verify re-encoded size stays within maxBytes.
🛠️ Proposed fix (keep re‑encoded bytes ≤ maxBytes)
- // Take only the allowed bytes
- val truncatedBytes = bytes.sliceArray(0 until maxBytes)
-
- // Converting back to String handles partial UTF-8 characters
- // by using the default replacement behavior.
- return String(truncatedBytes, Charsets.UTF_8)
+ // Take only the allowed bytes
+ val truncatedBytes = bytes.sliceArray(0 until maxBytes)
+ var result = String(truncatedBytes, Charsets.UTF_8)
+ // Ensure re-encoded size stays within maxBytes (replacement chars can exceed)
+ while (result.toByteArray(Charsets.UTF_8).size > maxBytes) {
+ result = result.dropLast(1)
+ }
+ return result🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/utils/MiscExtensions.kt` around lines
47 - 58, The String.truncateToMb implementation can produce a String whose UTF-8
byte length exceeds maxBytes because slicing arbitrary bytes may produce
replacement characters (U+FFFD) that are 3 bytes each; fix String.truncateToMb
so it returns a String whose UTF-8 encoded byte size is guaranteed ≤ maxBytes by
trimming to the last valid UTF-8 character boundary or by re-encoding and, if
necessary, shortening the byte slice/character count iteratively until
String(bytes).toByteArray(Charsets.UTF_8).size <= maxBytes; update the function
(String.truncateToMb) to locate a safe cut point (or loop reducing end index)
rather than returning a direct decode of an arbitrary byte slice.
Comment on lines
+61
to
+71
| /** | ||
| * Calculates the character limit for a given MB size. | ||
| * * @param mb The size limit in Megabytes (e.g., 1.5) | ||
| * | ||
| * @return A Pair where: first = Minimum characters guaranteed (worst-case: 4 bytes/char) second = | ||
| * Maximum characters possible (best-case: 1 byte/char) | ||
| */ | ||
| fun Double.charLimit(): Int { | ||
| val totalBytes = (this * 1024 * 1024).toInt() | ||
| val minChars = totalBytes / 4 // Every character is an Emoji/Complex | ||
| return minChars |
Contributor
There was a problem hiding this comment.
KDoc return type mismatch.
The KDoc describes returning a Pair of min/max characters, but the function returns a single Int. Please align the documentation with the actual return type.
✏️ Suggested KDoc update
- * `@return` A Pair where: first = Minimum characters guaranteed (worst-case: 4 bytes/char) second =
- * Maximum characters possible (best-case: 1 byte/char)
+ * `@return` Minimum characters guaranteed (worst-case: 4 bytes/char).📝 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
| /** | |
| * Calculates the character limit for a given MB size. | |
| * * @param mb The size limit in Megabytes (e.g., 1.5) | |
| * | |
| * @return A Pair where: first = Minimum characters guaranteed (worst-case: 4 bytes/char) second = | |
| * Maximum characters possible (best-case: 1 byte/char) | |
| */ | |
| fun Double.charLimit(): Int { | |
| val totalBytes = (this * 1024 * 1024).toInt() | |
| val minChars = totalBytes / 4 // Every character is an Emoji/Complex | |
| return minChars | |
| /** | |
| * Calculates the character limit for a given MB size. | |
| * * `@param` mb The size limit in Megabytes (e.g., 1.5) | |
| * | |
| * `@return` Minimum characters guaranteed (worst-case: 4 bytes/char). | |
| */ | |
| fun Double.charLimit(): Int { | |
| val totalBytes = (this * 1024 * 1024).toInt() | |
| val minChars = totalBytes / 4 // Every character is an Emoji/Complex | |
| return minChars |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/philkes/notallyx/utils/MiscExtensions.kt` around lines
61 - 71, The KDoc for charLimit() claims it returns a Pair of min/max characters
but the function actually returns a single Int (minChars); update the
documentation to match the implementation by removing the Pair description and
describing that charLimit(): Int returns the minimum guaranteed characters
(worst-case 4 bytes/char) computed as minChars, and adjust the `@return` tag to
indicate an Int instead of a Pair.
Comment on lines
+252
to
257
| <string name="note_text_too_long_truncated">Text too long, truncated to %1$d characters</string> | ||
| <string name="note_too_big_truncating">Note is too big to save, it was truncated to %1$s characters (was: %2$s)</string> | ||
| <string name="notes">Notes</string> | ||
| <string name="notes_sorted_by">Notes sorted by</string> | ||
| <string name="notes_too_big_truncating">%1$s Note are too big to save, they were truncated to %2$s characters</string> | ||
| <string name="open_link">Open link</string> |
Contributor
There was a problem hiding this comment.
Fix plural grammar in notes_too_big_truncating.
“Note are” should be “Notes are”.
✏️ Suggested fix
- <string name="notes_too_big_truncating">%1$s Note are too big to save, they were truncated to %2$s characters</string>
+ <string name="notes_too_big_truncating">%1$s Notes are too big to save, they were truncated to %2$s characters</string>📝 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
| <string name="note_text_too_long_truncated">Text too long, truncated to %1$d characters</string> | |
| <string name="note_too_big_truncating">Note is too big to save, it was truncated to %1$s characters (was: %2$s)</string> | |
| <string name="notes">Notes</string> | |
| <string name="notes_sorted_by">Notes sorted by</string> | |
| <string name="notes_too_big_truncating">%1$s Note are too big to save, they were truncated to %2$s characters</string> | |
| <string name="open_link">Open link</string> | |
| <string name="note_text_too_long_truncated">Text too long, truncated to %1$d characters</string> | |
| <string name="note_too_big_truncating">Note is too big to save, it was truncated to %1$s characters (was: %2$s)</string> | |
| <string name="notes">Notes</string> | |
| <string name="notes_sorted_by">Notes sorted by</string> | |
| <string name="notes_too_big_truncating">%1$s Notes are too big to save, they were truncated to %2$s characters</string> | |
| <string name="open_link">Open link</string> |
🤖 Prompt for AI Agents
In `@app/src/main/res/values/strings.xml` around lines 252 - 257, The string
resource notes_too_big_truncating has incorrect grammar ("Note are")—update its
value to "Notes are too big to save, they were truncated to %2$s characters"
(preserve the %1$s/%2$s placeholders as intended) so the plural subject and verb
agree; edit the string with name="notes_too_big_truncating" accordingly.
This was referenced Feb 18, 2026
Merged
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.
Fixes #734
Fixes #739
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.