Refactor: Profiles Grid, Profile Management (Reorder/Rename/Delete), and Search Improvements#386
Open
ALE-ARME wants to merge 5 commits intoyogeshpaliyal:masterfrom
Open
Refactor: Profiles Grid, Profile Management (Reorder/Rename/Delete), and Search Improvements#386ALE-ARME wants to merge 5 commits intoyogeshpaliyal:masterfrom
ALE-ARME wants to merge 5 commits intoyogeshpaliyal:masterfrom
Conversation
ALE-ARME
force-pushed
the
issue-360-partial
branch
from
d50811c to
cceefc9
Compare
ALE-ARME
force-pushed
the
issue-360-partial
branch
from
396876d to
4dc3302
Compare
Contributor
Author
|
If AI didn't mess up this PR should contain only partial fixes of the Home page improvements issue and the one about profile reordering. |
📝 WalkthroughWalkthroughThis PR removes a legacy APK build workflow, introduces profile priority-based reordering with database schema changes, adds a profiles grid UI for managing profiles, enhances clipboard link detection with DisposableEffect, customizes QR code colors, adds web server localization, removes clipboard UI settings, and extends string resources for new profile and web interface features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeUI as Home UI<br/>(ProfilesGrid)
participant ViewModel as AccountViewModel
participant Repo as LinkRepository
participant DB as SQLite Database
User->>HomeUI: Clicks move up on profile
HomeUI->>ViewModel: moveProfileUp(profileId)
ViewModel->>ViewModel: normalizePriorities()
Note over ViewModel: Reorder priorities<br/>to match index
ViewModel->>Repo: updateProfilePriority(neighborId, newPriority)
Repo->>DB: UPDATE Profile SET priority = ?
ViewModel->>Repo: updateProfilePriority(profileId, neighborPriority)
Repo->>DB: UPDATE Profile SET priority = ?
ViewModel->>Repo: getAllProfiles()
Repo->>DB: SELECT * FROM Profile<br/>ORDER BY priority ASC
DB-->>Repo: Ordered profiles
Repo-->>ViewModel: Updated profiles
ViewModel-->>HomeUI: Emit updated profiles
HomeUI-->>User: Display reordered grid
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt (2)
176-195:⚠️ Potential issue | 🟡 Minor
toLong()on comma-split tag IDs can throw if data is malformed.Line 181:
tagId.trim().toLong()will throwNumberFormatExceptionif a tag ID is empty or non-numeric (e.g., trailing comma intagsIds). Consider usingtoLongOrNull()with a filter to silently skip invalid entries.Suggested defensive parsing
val existingTags = - selectedLink.tagsIds?.split(",")?.mapIndexed { index, tagId -> - Tags( - tagId.trim().toLong(), + selectedLink.tagsIds?.split(",")?.mapIndexedNotNull { index, tagId -> + val id = tagId.trim().toLongOrNull() ?: return@mapIndexedNotNull null + Tags( + id, selectedLink.tagsNames ?.split(",") ?.getOrNull(index) ?.trim() ?: context.getString(R.string.unknown), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt` around lines 176 - 195, The parsing of comma-separated tag IDs in the LaunchedEffect (inside AddLinkScreen.kt) is unsafe because tagId.trim().toLong() can throw; update the block that builds existingTags to use safe parsing (e.g., use toLongOrNull() and mapIndexedNotNull) so invalid or empty IDs are skipped, while still pairing each successful Long with its corresponding name from selectedLink.tagsNames?.split(",")?.getOrNull(index)?.trim() (falling back to context.getString(R.string.unknown) when name is missing); then populate selectedTags and initialSelectedTags with the filtered Tags objects to avoid exceptions on malformed tagsIds.
833-909:⚠️ Potential issue | 🟠 MajorDuplicate-link checks use the raw (non-normalized) link, diverging from
save()which normalizes first.The "Save & Execute" (line 837) and "Save" (line 871) buttons call
deeprQueries.getDeeprByLink(deeprInfo.link)with the raw user input, while thesavelambda (line 199) normalizes vianormalizeLink()before the same check. This means a user enteringexample.comwill pass the button-level guard (DB hashttps://example.com, no match forexample.com), thensave()normalizes, finds the duplicate, shows a toast, and returns — without executing even if "Save & Execute" was clicked.Additionally, the validation+duplicate logic is repeated three times with subtle differences (
executeAsOneOrNullinsavevsexecuteAsList().isNotEmpty()in the buttons). Consider extracting a single validation helper that normalizes then checks, and call it from all paths.Proposed refactor: extract shared validation and normalize consistently
val save: (executeAfterSave: Boolean) -> Unit = save@{ executeAfterSave -> - // Normalize the link before saving val normalizedLink = normalizeLink(deeprInfo.link) - - if (isCreate && deeprQueries.getDeeprByLink(normalizedLink).executeAsOneOrNull() != null) { - Toast.makeText(context, deeplinkExistsText, Toast.LENGTH_SHORT).show() - return@save - } // ... rest of save logic } + + val validateAndSave: (executeAfterSave: Boolean) -> Unit = { executeAfterSave -> + val normalizedLink = normalizeLink(deeprInfo.link) + if (!isValidDeeplink(normalizedLink)) { + isError = true + } else if (isCreate && deeprQueries.getDeeprByLink(normalizedLink).executeAsList().isNotEmpty()) { + Toast.makeText(context, deeplinkExistsText, Toast.LENGTH_SHORT).show() + } else { + save(executeAfterSave) + } + }Then replace all three button
onClickblocks withvalidateAndSave(true)/validateAndSave(false).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt` around lines 833 - 909, The button handlers use the raw deeprInfo.link and call deeprQueries.getDeeprByLink(...) directly, but save() first normalizes the link via normalizeLink() and uses executeAsOneOrNull(), causing inconsistent duplicate detection and duplicated logic; create a single helper e.g. validateAndSave(execute: Boolean) that calls normalizeLink(deeprInfo.link), then queries deeprQueries.getDeeprByLink(normalizedLink) using the same existence check as save (executeAsOneOrNull or a consistent API), sets isError on invalid links, shows the duplicate Toast when found, and calls save(execute) (or openDeeplink for execute-only flows) as appropriate; replace the three onClick blocks (the Button and two FilledTonalButton handlers referencing deeprInfo.link, save(true)/save(false), and openDeeplink) to call validateAndSave(true/false) so normalization and duplicate-checking are centralized and consistent.app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt (1)
200-233:⚠️ Potential issue | 🟠 Major
getPrimaryClip()insideOnPrimaryClipChangedListenertriggers an Android 12+ system toast on every clipboard changeOn Android 12 and higher, when an app calls
getPrimaryClip()to access clip data from a different app for the first time, a toast message notifies the user of this clipboard access. On Android 12 (API level 31) and higher, the system usually shows a toast message when your app callsgetPrimaryClip().Because
clipboard.primaryClipis called unconditionally inside theOnPrimaryClipChangedListenercallback, the system will show a "Deepr pasted from your clipboard" toast every time the user copies anything (passwords, addresses, random text) while this app is running — regardless of whether the content is a deeplink. UsinggetPrimaryClipDescription()avoids this: data is not copied and the toast message is not displayed.The fix is to gate the full
getPrimaryClip()call on the MIME type: usegetPrimaryClipDescription()first (no toast) to confirm the clip is plain text, then callgetPrimaryClip()only if warranted.🛡️ Proposed fix to avoid spurious clipboard-access toasts on Android 12+
val listener = ClipboardManager.OnPrimaryClipChangedListener { - val clipData = clipboard.primaryClip - if (clipData != null && clipData.itemCount > 0) { - val text = clipData.getItemAt(0).text?.toString() - if (!text.isNullOrBlank()) { - val normalizedLink = normalizeLink(text) - if (isValidDeeplink(normalizedLink)) { - clipboardLink = ClipboardLink(normalizedLink) - } - } - } + val description = clipboard.primaryClipDescription + if (description != null && description.hasMimeType(android.content.ClipDescription.MIMETYPE_TEXT_PLAIN)) { + val clipData = clipboard.primaryClip + if (clipData != null && clipData.itemCount > 0) { + val text = clipData.getItemAt(0).text?.toString() + if (!text.isNullOrBlank()) { + val normalizedLink = normalizeLink(text) + if (isValidDeeplink(normalizedLink)) { + clipboardLink = ClipboardLink(normalizedLink) + } + } + } + } } // Initial check -val clipData = clipboard.primaryClip -if (clipData != null && clipData.itemCount > 0) { - val text = clipData.getItemAt(0).text?.toString() - if (!text.isNullOrBlank()) { - val normalizedLink = normalizeLink(text) - if (isValidDeeplink(normalizedLink)) { - clipboardLink = ClipboardLink(normalizedLink) - } - } -} +val description = clipboard.primaryClipDescription +if (description != null && description.hasMimeType(android.content.ClipDescription.MIMETYPE_TEXT_PLAIN)) { + val clipData = clipboard.primaryClip + if (clipData != null && clipData.itemCount > 0) { + val text = clipData.getItemAt(0).text?.toString() + if (!text.isNullOrBlank()) { + val normalizedLink = normalizeLink(text) + if (isValidDeeplink(normalizedLink)) { + clipboardLink = ClipboardLink(normalizedLink) + } + } + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt` around lines 200 - 233, The clipboard listener calls clipboard.primaryClip unconditionally which triggers Android 12+ clipboard-access toasts; change both the listener body and the initial check in the DisposableEffect to first call clipboard.primaryClipDescription (or getPrimaryClipDescription()) and verify the MIME type is text/plain (or hasMimeType(ClipDescription.MIMETYPE_TEXT_PLAIN)) before calling clipboard.primaryClip/getItemAt(0); keep using normalizeLink, isValidDeeplink and ClipboardLink as before, and ensure you only call clipboard.primaryClip when the description indicates text to avoid spurious toasts.app/src/main/res/values/strings.xml (1)
267-267:⚠️ Potential issue | 🟡 MinorRemove unused
profile_name_cannot_be_emptystring resource
profile_name_cannot_be_empty(line 344) is not referenced anywhere in the codebase—onlyprofile_name_cannot_be_blankis actively used. The unused key creates unnecessary translation maintenance burden (currently defined in only 2 locales: values and values-it) and should be removed.🤖 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` at line 267, Remove the unused string resource named profile_name_cannot_be_empty: delete its entry from the default strings.xml and the corresponding translation in values-it/strings.xml, and verify there are no references (grep/search for "profile_name_cannot_be_empty") before committing; keep the active string profile_name_cannot_be_blank intact.app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt (1)
309-347:⚠️ Potential issue | 🟠 MajorN+1 query pattern in
GET /api/tagswith hardcodedprofileId = 1LFor every tag in the database, a full
getLinksAndTags(...)query is executed and.sizeis called on the result set. This loads entire link rows just to count them — O(n_tags × m_links) in terms of DB work and memory allocation.The repository already exposes
getAllTagsWithCount(profileId)which is a single, optimized SQL query designed for exactly this use case. TheprofileIdis also hardcoded to1Lhere, which will return wrong counts for users who have links in other profiles.♻️ Proposed fix
get("/api/tags") { try { + val profileId = call.request.queryParameters["profileId"]?.toLongOrNull() ?: 1L - val allTags = deeprQueries.getAllTags().executeAsList() - val response = allTags.map { tag -> - val linkCount = deeprQueries.getLinksAndTags( - 1L, "", "", "", "", -1L, -1L, - tag.id.toString(), tag.id.toString(), - "DESC", "createdAt", "DESC", "createdAt", - ).executeAsList().size - TagResponse(id = tag.id, name = tag.name, count = linkCount) - } + val tagsWithCount = deeprQueries.getAllTagsWithCount(profileId).executeAsList() + val response = tagsWithCount.map { row -> + TagResponse(id = row.id, name = row.name, count = row.linkCount.toInt()) + } call.respond(HttpStatusCode.OK, response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt` around lines 309 - 347, The GET /api/tags handler is performing an N+1 query by calling deeprQueries.getLinksAndTags(...) for each tag and hardcodes profileId = 1L; replace that loop with the optimized single-query method deeprQueries.getAllTagsWithCount(profileId) to return tags with counts in one call, and obtain the correct profileId from the request/context (instead of 1L). Map the results of getAllTagsWithCount(...) to TagResponse(id, name, count) and respond with that list; remove the per-tag getLinksAndTags(...) calls and their .size usage to eliminate the N+1 issue and ensure counts respect the actual profileId.
🧹 Nitpick comments (14)
app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt (1)
139-145: Auto-fetch has no cancellation of in-flight requests and no debounce.
fetchMetadatais a plain lambda (not a suspend function), so whendeeprInfo.linkchanges andLaunchedEffectrestarts, the previously firedviewModel.fetchMetaData()call is not cancelled. If the link changes rapidly (e.g., QR scan immediately followed by a manual edit), stale callbacks can overwrite the current metadata.Consider making
fetchMetaDataa suspend function invoked directly inside theLaunchedEffectcoroutine (so it's automatically cancelled on re-key), or track/ignore stale responses by comparing the link at callback time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt` around lines 139 - 145, The LaunchedEffect block starts new work on deeprInfo.link but calls a non-cancellable callback (fetchMetadata / viewModel.fetchMetaData), causing stale responses to overwrite newer state; change the implementation so the metadata fetch is cancellable or ignores stale results: either convert fetchMetadata (and underlying viewModel.fetchMetaData) into a suspend function and call it directly inside the LaunchedEffect coroutine (so coroutine cancellation will abort in-flight work), or add a request token (capture the current normalized link or a UUID in LaunchedEffect) and have the fetch callback verify that the token/link still matches before applying results (compare against deeprInfo.link or the captured token in the callback). Ensure you update references to normalizeLink, isValidDeeplink, fetchMetadata, and viewModel.fetchMetaData accordingly so only the latest request can update UI.app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt (2)
200-200: MissingDisposableEffectimport; staleLaunchedEffectimport
DisposableEffectis invoked via its fully qualified name (androidx.compose.runtime.DisposableEffect) rather than being imported like every other Compose API in this file. At the same time, the oldimport androidx.compose.runtime.LaunchedEffectat line 24 appears unused after this refactor.♻️ Proposed fix
+import androidx.compose.runtime.DisposableEffect -import androidx.compose.runtime.LaunchedEffectThen on line 200:
- androidx.compose.runtime.DisposableEffect(Unit) { + DisposableEffect(Unit) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt` at line 200, File uses androidx.compose.runtime.DisposableEffect with a fully-qualified name and still has an unused import for LaunchedEffect; replace the fully-qualified call with a proper import and remove the stale import. Add "import androidx.compose.runtime.DisposableEffect" to the imports, remove "import androidx.compose.runtime.LaunchedEffect" if unused, and update any usages (e.g., the call currently written as androidx.compose.runtime.DisposableEffect(Unit) { ... }) to use the imported DisposableEffect symbol so imports are consistent and no unused LaunchedEffect remains.
203-226: Extract duplicated clipboard-reading logic into a shared helper lambdaThe listener body (lines 203–213) and the initial check (lines 217–226) are byte-for-byte identical. Even after applying the
getPrimaryClipDescription()guard above, both blocks will remain duplicated.♻️ Proposed refactor
+ val processClip = { + val description = clipboard.primaryClipDescription + if (description != null && description.hasMimeType(android.content.ClipDescription.MIMETYPE_TEXT_PLAIN)) { + val clipData = clipboard.primaryClip + if (clipData != null && clipData.itemCount > 0) { + val text = clipData.getItemAt(0).text?.toString() + if (!text.isNullOrBlank()) { + val normalizedLink = normalizeLink(text) + if (isValidDeeplink(normalizedLink)) { + clipboardLink = ClipboardLink(normalizedLink) + } + } + } + } + } + - val listener = ClipboardManager.OnPrimaryClipChangedListener { - val clipData = clipboard.primaryClip - ... - } + val listener = ClipboardManager.OnPrimaryClipChangedListener { processClip() } - // Initial check - val clipData = clipboard.primaryClip - ... + // Initial check + processClip()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt` around lines 203 - 226, The clipboard-reading logic is duplicated between the ClipboardManager.OnPrimaryClipChangedListener block and the initial check; extract that shared code into a single helper (either a private function or a lambda) — e.g., create a processClipboard() function that reads clipboard.primaryClip, gets text, calls normalizeLink(text), checks isValidDeeplink(normalizedLink) and assigns clipboardLink = ClipboardLink(normalizedLink) — then call that helper from the listener body and from the initial check so ClipboardManager.OnPrimaryClipChangedListener, normalizeLink, isValidDeeplink, ClipboardLink and clipboard are reused without duplication.app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/TransferLinkLocalServer.kt (1)
45-45: Optional: importQrCodeColorsinstead of using its fully qualified name.
QrCodeViewis imported at Line 59, butQrCodeColorsis referenced with its fully qualified class name inline. Adding an import keeps the call-site consistent and easier to read.♻️ Proposed change
import com.lightspark.composeqr.QrCodeView +import com.lightspark.composeqr.QrCodeColors- colors = - com.lightspark.composeqr.QrCodeColors( - background = Color.White, - foreground = Color.Black, - ), + colors = + QrCodeColors( + background = Color.White, + foreground = Color.Black, + ),Also applies to: 323-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/TransferLinkLocalServer.kt` at line 45, Replace the fully-qualified QrCodeColors usages with a simple import: add an import for QrCodeColors at the top of the file and update all call-sites (the places where QrCodeView is used and the occurrences around the previous lines 323-327) to reference QrCodeColors directly instead of its fully-qualified name; this keeps the import style consistent with QrCodeView and improves readability.app/src/main/java/com/yogeshpaliyal/deepr/ui/components/NoteViewDialog.kt (1)
91-104: Migrate from deprecatedClickableTexttoBasicTextwithLinkAnnotation.
ClickableTextis deprecated as of Compose Foundation 1.7.0-alpha07, and your project's Compose BOM (2025.09.00) is well past this point. Replace theStringAnnotation("URL", ...)offset-lookup pattern withBasicText+LinkAnnotation.ClickableusingwithLink()inbuildAnnotatedString. PassLinkInteractionListenertoLinkAnnotation.Clickableto retain the customopenDeeplink()dispatch.Migration sketch
-import androidx.compose.foundation.text.ClickableText +import androidx.compose.foundation.text.BasicText +import androidx.compose.ui.text.LinkAnnotation +import androidx.compose.ui.text.TextLinkStyles +import androidx.compose.ui.text.withLinkval annotatedString = buildAnnotatedString { val text = deepr.notes - append(text) val matcher = Patterns.WEB_URL.matcher(text) + var cursor = 0 while (matcher.find()) { + append(text.substring(cursor, matcher.start())) val url = text.substring(matcher.start(), matcher.end()) - addStringAnnotation(tag = "URL", annotation = url, start = matcher.start(), end = matcher.end()) - addStyle(SpanStyle(color = primaryColor, textDecoration = TextDecoration.Underline), start = matcher.start(), end = matcher.end()) + withLink( + LinkAnnotation.Clickable( + tag = url, + styles = TextLinkStyles(SpanStyle(color = primaryColor, textDecoration = TextDecoration.Underline)), + linkInteractionListener = { openDeeplink(context, url) } + ) + ) { append(url) } + cursor = matcher.end() } + append(text.substring(cursor)) } SelectionContainer { - ClickableText( + BasicText( text = annotatedString, style = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.onSurface), - onClick = { offset -> - annotatedString.getStringAnnotations(tag = "URL", start = offset, end = offset) - .firstOrNull()?.let { openDeeplink(context, it.item) } - }, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/components/NoteViewDialog.kt` around lines 91 - 104, Replace the deprecated ClickableText usage by building annotatedString with LinkAnnotation.Clickable via withLink() in buildAnnotatedString and render it with BasicText; update the annotation creation code where you currently use StringAnnotation("URL", ...) so it uses withLink(url) / LinkAnnotation.Clickable and attach a LinkInteractionListener that calls openDeeplink(context, url) (preserve the existing annotatedString variable name and the openDeeplink dispatch logic), then replace the ClickableText block (which performs getStringAnnotations and offset lookup) with a BasicText that uses the new annotatedString and the LinkAnnotation.Clickable listener to handle taps.app/src/main/sqldelight/com/yogeshpaliyal/deepr/Deepr.sq (1)
218-220: Stray blank line betweenGROUP BYandORDER BYis inconsistent with all other queries in this file.SQLDelight's parser treats this as harmless whitespace, but every other multi-line query in
Deepr.sqhas no blank lines mid-body. Remove line 219 for consistency.♻️ Proposed cleanup
GROUP BY Tags.id, Tags.name - ORDER BY linkCount DESC, Tags.name;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/sqldelight/com/yogeshpaliyal/deepr/Deepr.sq` around lines 218 - 220, Remove the stray blank line between the GROUP BY clause and the ORDER BY clause so the query remains consistent with others in the file; specifically, join "GROUP BY Tags.id, Tags.name" and "ORDER BY linkCount DESC, Tags.name" with no blank line between them (the relevant symbols to locate are GROUP BY, Tags.id, Tags.name, ORDER BY, and linkCount).app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/TagSelectionScreen.kt (1)
692-709:split(tag.name)loses trailing content when the name occurs more than once in the formatted string.If the user somehow names a tag with a value that appears multiple times in the resolved format string,
parts.size > 2and onlyparts[0] + bold + parts[1]is rendered — content after the second occurrence is silently dropped. While the currentdelete_tag_confirmation_with_namehas a single%s, this is fragile for future copy changes. UsingindexOfis more robust.♻️ Robust AnnotatedString construction via indexOf
-val fullMessage = stringResource(R.string.delete_tag_confirmation_with_name, tag.name) -val parts = fullMessage.split(tag.name) -val annotatedString = buildAnnotatedString { - if (parts.size >= 2) { - append(parts[0]) - withStyle(style = SpanStyle(fontWeight = FontWeight.Bold)) { - append(tag.name) - } - append(parts[1]) - } else { - append(fullMessage) - } -} +val fullMessage = stringResource(R.string.delete_tag_confirmation_with_name, tag.name) +val annotatedString = buildAnnotatedString { + val startIndex = fullMessage.indexOf(tag.name) + if (startIndex != -1) { + append(fullMessage.substring(0, startIndex)) + withStyle(style = SpanStyle(fontWeight = FontWeight.Bold)) { + append(tag.name) + } + append(fullMessage.substring(startIndex + tag.name.length)) + } else { + append(fullMessage) + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/TagSelectionScreen.kt` around lines 692 - 709, The current construction in TagSelectionScreen.kt builds annotatedString by splitting fullMessage with tag.name (variables fullMessage, tag.name, parts, annotatedString), which drops content after the second occurrence; instead find the first occurrence using indexOf(tag.name), then append substring before, append the tag name with bold SpanStyle, and append the remainder (substring after first match) so any further occurrences are preserved; update buildAnnotatedString usage accordingly and keep the Text(...) rendering as-is.app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt (2)
107-107: Redundantval port = port— no-op self-assignmentThis creates a local
valthat shadows the function parameter with its own value. It serves no purpose and may generate a compiler warning.✏️ Proposed fix
- val port = port - server = embeddedServer(CIO, host = "0.0.0.0", port = port) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt` at line 107, Remove the redundant local shadowing of the parameter by deleting the line "val port = port" in LocalServerRepositoryImpl (the local variable shadows the function parameter named port); just use the existing parameter "port" directly everywhere in the function (or rename if you intended a different variable), ensuring no other code relies on that local declaration.
386-402:POST /api/links/increment-countreads the resource ID from a query parameterUsing a query parameter for a mutable state change on a POST endpoint is non-idiomatic (query params belong on GET/safe methods). The conventional REST design is either
POST /api/links/{id}/increment-count(path parameter) orPATCH /api/links/{id}with a body. This also makes the endpoint easier to document and harder to accidentally call via a GET proxy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt` around lines 386 - 402, Change the POST handler that currently reads the link id from call.request.queryParameters in the route "/api/links/increment-count" to accept the id as a path parameter (e.g. POST "/api/links/{id}/increment-count") or accept a JSON body with an id, then parse that value instead of using queryParameters; update the route declaration and replace call.request.queryParameters["id"]?.toLongOrNull() with code that retrieves the id from call.parameters["id"] or from the request body, and continue calling accountViewModel.incrementOpenedCount(id) and responding with SuccessResponse/ErrorResponse as before.app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt (3)
1596-1607: Fragile annotated-string construction:split(profile.name)can break for certain names or locales.If
profile.nameis a short/common substring (e.g.,"a","de"),splitmay fragment the translated message at unintended positions. Also, if the name appears more than once in the message,parts.size >= 2is true butparts[1]won't contain the full trailing text.A more robust approach is to locate the name's position in the formatted string and split only at that known offset:
Proposed fix
- val parts = fullMessage.split(profile.name) - val annotatedString = buildAnnotatedString { - if (parts.size >= 2) { - append(parts[0]) - withStyle(style = SpanStyle(fontWeight = FontWeight.Bold)) { - append(profile.name) - } - append(parts[1]) - } else { - append(fullMessage) - } - } + val annotatedString = buildAnnotatedString { + val nameIndex = fullMessage.indexOf(profile.name) + if (nameIndex >= 0) { + append(fullMessage.substring(0, nameIndex)) + withStyle(style = SpanStyle(fontWeight = FontWeight.Bold)) { + append(profile.name) + } + append(fullMessage.substring(nameIndex + profile.name.length)) + } else { + append(fullMessage) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` around lines 1596 - 1607, The annotated-string construction is fragile because split(profile.name) can incorrectly fragment translations or handle multiple occurrences; instead, find the first occurrence index of profile.name in the formatted fullMessage (use fullMessage.indexOf(profile.name)) and if index >= 0 build the AnnotatedString by taking fullMessage.substring(0, index), append the bold profile.name, then append fullMessage.substring(index + profile.name.length), otherwise fall back to appending fullMessage; update the logic around fullMessage, parts, and annotatedString in Home.kt to use this index-based splitting so only the intended occurrence is made bold.
1688-1690: Save button enabled check should compare trimmed names for consistent UX.
enabled = newName.isNotBlank() && newName != profile.nameallows enabling Save when only leading/trailing whitespace differs (e.g.," Default "vs"Default"), but theonClickhandler trims before comparing, resulting in a "name already exists" error after the user clicks.Proposed fix
- enabled = newName.isNotBlank() && newName != profile.name + enabled = newName.isNotBlank() && newName.trim() != profile.name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` around lines 1688 - 1690, The Save button's enabled check uses raw newName and profile.name, allowing only-whitespace differences to enable the button but failing later because onClick trims; update the enabled logic to compare trimmed values instead (use newName.trim().isNotBlank() and compare newName.trim() with profile.name.trim() or the canonical stored name) so the enabled state matches the onClick trimming behavior; locate the Save button enabled assignment (enabled = newName.isNotBlank() && newName != profile.name) and change it to use trimmed comparisons consistent with the onClick handler.
569-613: FAB tap/long-press via innerpointerInputbypasses the FAB's ripple and accessibility semantics.The
FloatingActionButton(onClick = {})has an empty click handler while actual tap handling is done viadetectTapGestureson the innerBox. This means the FAB's Material ripple effect and accessibility click action may not behave as expected — the innerpointerInputconsumes the gesture before the FAB's clickable modifier processes it.Consider using
combinedClickable(fromfoundation) on the FAB's modifier instead, which natively supports bothonClickandonLongClickwhile preserving ripple and semantics:Sketch of alternative approach
- if (!showProfilesGrid) { - FloatingActionButton( - onClick = {}, - ) { - Box( - modifier = - Modifier - .size(56.dp) - .pointerInput(Unit) { - detectTapGestures( - onTap = { - localNavigator.add(AddLinkScreen(createDeeprObject())) - }, - onLongPress = { - hapticFeedback.performHapticFeedback(HapticFeedbackType.LongPress) - // ... clipboard logic ... - localNavigator.add(AddLinkScreen(createDeeprObject(link = linkToPass))) - }, - ) - }, - contentAlignment = Alignment.Center, - ) { - Icon( - TablerIcons.Plus, - contentDescription = stringResource(R.string.add_link), - ) - } - } + if (!showProfilesGrid) { + FloatingActionButton( + onClick = { + localNavigator.add(AddLinkScreen(createDeeprObject())) + }, + modifier = Modifier.combinedClickable( + onClick = { + localNavigator.add(AddLinkScreen(createDeeprObject())) + }, + onLongClick = { + hapticFeedback.performHapticFeedback(HapticFeedbackType.LongPress) + // ... clipboard logic ... + localNavigator.add(AddLinkScreen(createDeeprObject(link = linkToPass))) + }, + ), + ) { + Icon( + TablerIcons.Plus, + contentDescription = stringResource(R.string.add_link), + ) + }app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt (2)
849-857:getMaxPriority()+insertProfile(name, maxPriority + 1)is not atomic.If two profile insertions run concurrently, both can read the same max priority and insert with the same value. Low risk since this is UI-driven (single FAB), but worth noting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt` around lines 849 - 857, The current insertProfile in AccountViewModel uses linkRepository.getMaxPriority() and then linkRepository.insertProfile(name, maxPriority + 1) separately, which is not atomic and can cause duplicate priorities under concurrency; change this by adding a single atomic repository/DAO operation (e.g., linkRepository.insertProfileWithNextPriority(name) or a DAO method annotated with `@Transaction`) that reads the current max priority and inserts the new profile inside the same transaction, then call that new method from AccountViewModel.insertProfile instead of calling getMaxPriority and insertProfile separately; alternatively, if you prefer in-memory serialization, guard the two calls with a dedicated Mutex/synchronized block in the repository so the read+insert cannot interleave.
141-148:normalizePriorities()rewrites every profile's priority on every app launch and every move.On init (line 136), this runs unconditionally for existing profiles, issuing up to N update queries. On each move, it runs again before the swap. For a small number of profiles this is tolerable, but consider tracking whether normalization is actually needed (e.g., a dirty flag or checking if priorities are already sequential) to avoid unnecessary DB writes on every cold start.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt` around lines 141 - 148, normalizePriorities() currently issues updates for every profile on init and before moves; change it to detect whether normalization is required and only perform writes when priorities are out of order or a "dirty" flag is set. Specifically, in normalizePriorities() (and where it is called) first load profiles with linkRepository.getAllProfiles().executeAsList() and check if any profile.priority != index.toLong()—if none mismatch, return early; alternatively, add a persistent or in-memory dirty flag that is set when profiles are added/removed/moved (e.g., in your move/swap methods) and only run updateProfilePriority(profile.id, index.toLong()) when the flag is true, then clear the flag after successful normalization. Ensure callers (init and move) use the new check/flag to avoid unconditional DB writes.
🤖 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/yogeshpaliyal/deepr/backup/importer/CsvBookmarkImporter.kt`:
- Line 71: Imported profiles are all saved with priority = 0L at the
deeprQueries.insertProfile(it, 0L) call in CsvBookmarkImporter, causing
indeterminate ordering; modify the import transaction so it queries the current
max priority (using the same pattern as AccountViewModel.insertProfile(), i.e.,
call deeprQueries.maxPriority() or equivalent) before inserting each new
profile, set the profile's priority to maxPriority + 1, and increment that in
the loop for subsequent inserts so each imported profile gets a sequentially
higher priority rather than 0.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt`:
- Around line 124-178: The GET handler currently opens assets and runs the
43-entry placeholders replacement on every request (see htmlContent and
placeholders in LocalServerRepositoryImpl); move that work into server
initialization (e.g., in startServer or the class constructor) so the file is
read once, replacements performed once, and the resulting string stored in a
cached property (e.g., cachedIndexHtml) that the GET / handler returns directly;
update the GET handler to use cachedIndexHtml instead of re-reading assets and
looping through placeholders.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt`:
- Around line 494-496: The hardcoded contentDescription strings in Home.kt
(e.g., the conditional contentDescription using "Finish Reordering" with
isReordering, and the "Move Up"/"Move Down" captions) break i18n/accessibility;
replace those literal strings with stringResource lookups (e.g., use
stringResource(R.string.finish_reordering) for the isReordering branch and
stringResource(R.string.move_up) / stringResource(R.string.move_down) for the
move buttons), and add corresponding entries finish_reordering, move_up, and
move_down to your strings.xml so screen readers and translations receive the
localized text; keep the existing ternary/conditional logic and icon/tint usage
(TablerIcons.Check, TablerIcons.ArrowsSort) intact.
In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt`:
- Around line 86-91: Initialize the profiles grid state so the app lands on it
by default: change the initial value of the MutableStateFlow _showProfilesGrid
in AccountViewModel (the backing property for showProfilesGrid) from false to
true, or ensure setShowProfilesGrid(true) is invoked during the ViewModel's
initialization (e.g., in the init block) so the profiles grid is shown before
the first frame; update only the _showProfilesGrid declaration or add the init
call in AccountViewModel.kt.
- Around line 859-893: The moveProfileUp and moveProfileDown flows call
normalizePriorities(), then read profiles via
linkRepository.getAllProfiles().executeAsList() and perform two
updateProfilePriority() calls outside any atomic boundary, allowing concurrent
coroutines to interleave and corrupt priorities; fix by wrapping the entire
normalize-and-swap sequence for a given operation in a single serialized
boundary — either execute a DB transaction that includes normalizePriorities(),
the read, and both updateProfilePriority() calls (using explicit BEGIN/COMMIT on
the SQL driver) or protect the sequence with a shared Mutex / single-threaded
CoroutineDispatcher so moveProfileUp, moveProfileDown and normalizePriorities
cannot run concurrently.
In `@app/src/main/res/values-it/strings.xml`:
- Line 256: The string resource named "reorder" uses a non-standard calque
"Reordina"; update the value for string name="reorder" to the correct Italian
imperative "Riordina" in the strings.xml so the app uses the proper
localization.
- Line 272: Update the two Italian string resources: add a terminal period to
the value of profile_delete_confirmation_with_name so it ends with "verranno
eliminati." to match surrounding confirmation strings, and fix the missing
elision apostrophe in web_error_add_link by changing "Errore nell aggiunta" to
"Errore nell'aggiunta". Ensure you edit the string values for the resource names
profile_delete_confirmation_with_name and web_error_add_link accordingly.
In `@app/src/main/res/values/strings.xml`:
- Around line 271-273: Remove the unused duplicate string resource
delete_profile_confirmation across all localization files (it is identical to
profile_delete_confirmation and unused), update
profile_delete_confirmation_with_name to end with a period, and make the
substitution consistent by quoting the %s placeholder (use the same quoting
style as delete_link_confirmation_with_name, e.g. wrap %s in single quotes) so
its text matches neighboring strings; change only the string resource entries
(profile_delete_confirmation_with_name and removal of
delete_profile_confirmation) and apply the same edits to every
values-*/localized XML file.
In `@app/src/main/sqldelight/com/yogeshpaliyal/deepr/Deepr.sq`:
- Around line 80-82: The WHERE clause filters out non-matching Tag rows before
aggregation, causing GROUP_CONCAT(Tags.name) and GROUP_CONCAT(Tags.id) to miss a
link's other tags; change the tag-search predicate (Tags.name LIKE '%' || ? ||
'%') so it is used in an EXISTS correlated subquery (e.g., EXISTS(SELECT 1 FROM
Tags t WHERE t.linkId = Deepr.linkId AND t.name LIKE '%' || ? || '%')) while
leaving the LEFT JOIN to Tags intact for aggregation, preserving all joined tag
rows for GROUP_CONCAT; ensure you keep the same number/order of bind parameters
used by the current query so the Kotlin API surface remains unchanged.
In `@README.md`:
- Around line 35-39: Update the markdown table rows to fix two typos: change the
string "Github" in the column header/cell to the correct brand casing "GitHub",
and correct the feature label "Profile speicific themes" to "Profile specific
themes" so the table reads exactly "GitHub" and "Profile specific themes" where
those exact strings appear.
---
Outside diff comments:
In `@app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt`:
- Around line 200-233: The clipboard listener calls clipboard.primaryClip
unconditionally which triggers Android 12+ clipboard-access toasts; change both
the listener body and the initial check in the DisposableEffect to first call
clipboard.primaryClipDescription (or getPrimaryClipDescription()) and verify the
MIME type is text/plain (or hasMimeType(ClipDescription.MIMETYPE_TEXT_PLAIN))
before calling clipboard.primaryClip/getItemAt(0); keep using normalizeLink,
isValidDeeplink and ClipboardLink as before, and ensure you only call
clipboard.primaryClip when the description indicates text to avoid spurious
toasts.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt`:
- Around line 309-347: The GET /api/tags handler is performing an N+1 query by
calling deeprQueries.getLinksAndTags(...) for each tag and hardcodes profileId =
1L; replace that loop with the optimized single-query method
deeprQueries.getAllTagsWithCount(profileId) to return tags with counts in one
call, and obtain the correct profileId from the request/context (instead of 1L).
Map the results of getAllTagsWithCount(...) to TagResponse(id, name, count) and
respond with that list; remove the per-tag getLinksAndTags(...) calls and their
.size usage to eliminate the N+1 issue and ensure counts respect the actual
profileId.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt`:
- Around line 176-195: The parsing of comma-separated tag IDs in the
LaunchedEffect (inside AddLinkScreen.kt) is unsafe because tagId.trim().toLong()
can throw; update the block that builds existingTags to use safe parsing (e.g.,
use toLongOrNull() and mapIndexedNotNull) so invalid or empty IDs are skipped,
while still pairing each successful Long with its corresponding name from
selectedLink.tagsNames?.split(",")?.getOrNull(index)?.trim() (falling back to
context.getString(R.string.unknown) when name is missing); then populate
selectedTags and initialSelectedTags with the filtered Tags objects to avoid
exceptions on malformed tagsIds.
- Around line 833-909: The button handlers use the raw deeprInfo.link and call
deeprQueries.getDeeprByLink(...) directly, but save() first normalizes the link
via normalizeLink() and uses executeAsOneOrNull(), causing inconsistent
duplicate detection and duplicated logic; create a single helper e.g.
validateAndSave(execute: Boolean) that calls normalizeLink(deeprInfo.link), then
queries deeprQueries.getDeeprByLink(normalizedLink) using the same existence
check as save (executeAsOneOrNull or a consistent API), sets isError on invalid
links, shows the duplicate Toast when found, and calls save(execute) (or
openDeeplink for execute-only flows) as appropriate; replace the three onClick
blocks (the Button and two FilledTonalButton handlers referencing
deeprInfo.link, save(true)/save(false), and openDeeplink) to call
validateAndSave(true/false) so normalization and duplicate-checking are
centralized and consistent.
In `@app/src/main/res/values/strings.xml`:
- Line 267: Remove the unused string resource named
profile_name_cannot_be_empty: delete its entry from the default strings.xml and
the corresponding translation in values-it/strings.xml, and verify there are no
references (grep/search for "profile_name_cannot_be_empty") before committing;
keep the active string profile_name_cannot_be_blank intact.
---
Nitpick comments:
In `@app/src/main/java/com/yogeshpaliyal/deepr/MainActivity.kt`:
- Line 200: File uses androidx.compose.runtime.DisposableEffect with a
fully-qualified name and still has an unused import for LaunchedEffect; replace
the fully-qualified call with a proper import and remove the stale import. Add
"import androidx.compose.runtime.DisposableEffect" to the imports, remove
"import androidx.compose.runtime.LaunchedEffect" if unused, and update any
usages (e.g., the call currently written as
androidx.compose.runtime.DisposableEffect(Unit) { ... }) to use the imported
DisposableEffect symbol so imports are consistent and no unused LaunchedEffect
remains.
- Around line 203-226: The clipboard-reading logic is duplicated between the
ClipboardManager.OnPrimaryClipChangedListener block and the initial check;
extract that shared code into a single helper (either a private function or a
lambda) — e.g., create a processClipboard() function that reads
clipboard.primaryClip, gets text, calls normalizeLink(text), checks
isValidDeeplink(normalizedLink) and assigns clipboardLink =
ClipboardLink(normalizedLink) — then call that helper from the listener body and
from the initial check so ClipboardManager.OnPrimaryClipChangedListener,
normalizeLink, isValidDeeplink, ClipboardLink and clipboard are reused without
duplication.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt`:
- Line 107: Remove the redundant local shadowing of the parameter by deleting
the line "val port = port" in LocalServerRepositoryImpl (the local variable
shadows the function parameter named port); just use the existing parameter
"port" directly everywhere in the function (or rename if you intended a
different variable), ensuring no other code relies on that local declaration.
- Around line 386-402: Change the POST handler that currently reads the link id
from call.request.queryParameters in the route "/api/links/increment-count" to
accept the id as a path parameter (e.g. POST "/api/links/{id}/increment-count")
or accept a JSON body with an id, then parse that value instead of using
queryParameters; update the route declaration and replace
call.request.queryParameters["id"]?.toLongOrNull() with code that retrieves the
id from call.parameters["id"] or from the request body, and continue calling
accountViewModel.incrementOpenedCount(id) and responding with
SuccessResponse/ErrorResponse as before.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/components/NoteViewDialog.kt`:
- Around line 91-104: Replace the deprecated ClickableText usage by building
annotatedString with LinkAnnotation.Clickable via withLink() in
buildAnnotatedString and render it with BasicText; update the annotation
creation code where you currently use StringAnnotation("URL", ...) so it uses
withLink(url) / LinkAnnotation.Clickable and attach a LinkInteractionListener
that calls openDeeplink(context, url) (preserve the existing annotatedString
variable name and the openDeeplink dispatch logic), then replace the
ClickableText block (which performs getStringAnnotations and offset lookup) with
a BasicText that uses the new annotatedString and the LinkAnnotation.Clickable
listener to handle taps.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt`:
- Around line 139-145: The LaunchedEffect block starts new work on
deeprInfo.link but calls a non-cancellable callback (fetchMetadata /
viewModel.fetchMetaData), causing stale responses to overwrite newer state;
change the implementation so the metadata fetch is cancellable or ignores stale
results: either convert fetchMetadata (and underlying viewModel.fetchMetaData)
into a suspend function and call it directly inside the LaunchedEffect coroutine
(so coroutine cancellation will abort in-flight work), or add a request token
(capture the current normalized link or a UUID in LaunchedEffect) and have the
fetch callback verify that the token/link still matches before applying results
(compare against deeprInfo.link or the captured token in the callback). Ensure
you update references to normalizeLink, isValidDeeplink, fetchMetadata, and
viewModel.fetchMetaData accordingly so only the latest request can update UI.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt`:
- Around line 1596-1607: The annotated-string construction is fragile because
split(profile.name) can incorrectly fragment translations or handle multiple
occurrences; instead, find the first occurrence index of profile.name in the
formatted fullMessage (use fullMessage.indexOf(profile.name)) and if index >= 0
build the AnnotatedString by taking fullMessage.substring(0, index), append the
bold profile.name, then append fullMessage.substring(index +
profile.name.length), otherwise fall back to appending fullMessage; update the
logic around fullMessage, parts, and annotatedString in Home.kt to use this
index-based splitting so only the intended occurrence is made bold.
- Around line 1688-1690: The Save button's enabled check uses raw newName and
profile.name, allowing only-whitespace differences to enable the button but
failing later because onClick trims; update the enabled logic to compare trimmed
values instead (use newName.trim().isNotBlank() and compare newName.trim() with
profile.name.trim() or the canonical stored name) so the enabled state matches
the onClick trimming behavior; locate the Save button enabled assignment
(enabled = newName.isNotBlank() && newName != profile.name) and change it to use
trimmed comparisons consistent with the onClick handler.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/TagSelectionScreen.kt`:
- Around line 692-709: The current construction in TagSelectionScreen.kt builds
annotatedString by splitting fullMessage with tag.name (variables fullMessage,
tag.name, parts, annotatedString), which drops content after the second
occurrence; instead find the first occurrence using indexOf(tag.name), then
append substring before, append the tag name with bold SpanStyle, and append the
remainder (substring after first match) so any further occurrences are
preserved; update buildAnnotatedString usage accordingly and keep the Text(...)
rendering as-is.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/TransferLinkLocalServer.kt`:
- Line 45: Replace the fully-qualified QrCodeColors usages with a simple import:
add an import for QrCodeColors at the top of the file and update all call-sites
(the places where QrCodeView is used and the occurrences around the previous
lines 323-327) to reference QrCodeColors directly instead of its fully-qualified
name; this keeps the import style consistent with QrCodeView and improves
readability.
In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt`:
- Around line 849-857: The current insertProfile in AccountViewModel uses
linkRepository.getMaxPriority() and then linkRepository.insertProfile(name,
maxPriority + 1) separately, which is not atomic and can cause duplicate
priorities under concurrency; change this by adding a single atomic
repository/DAO operation (e.g.,
linkRepository.insertProfileWithNextPriority(name) or a DAO method annotated
with `@Transaction`) that reads the current max priority and inserts the new
profile inside the same transaction, then call that new method from
AccountViewModel.insertProfile instead of calling getMaxPriority and
insertProfile separately; alternatively, if you prefer in-memory serialization,
guard the two calls with a dedicated Mutex/synchronized block in the repository
so the read+insert cannot interleave.
- Around line 141-148: normalizePriorities() currently issues updates for every
profile on init and before moves; change it to detect whether normalization is
required and only perform writes when priorities are out of order or a "dirty"
flag is set. Specifically, in normalizePriorities() (and where it is called)
first load profiles with linkRepository.getAllProfiles().executeAsList() and
check if any profile.priority != index.toLong()—if none mismatch, return early;
alternatively, add a persistent or in-memory dirty flag that is set when
profiles are added/removed/moved (e.g., in your move/swap methods) and only run
updateProfilePriority(profile.id, index.toLong()) when the flag is true, then
clear the flag after successful normalization. Ensure callers (init and move)
use the new check/flag to avoid unconditional DB writes.
In `@app/src/main/sqldelight/com/yogeshpaliyal/deepr/Deepr.sq`:
- Around line 218-220: Remove the stray blank line between the GROUP BY clause
and the ORDER BY clause so the query remains consistent with others in the file;
specifically, join "GROUP BY Tags.id, Tags.name" and "ORDER BY linkCount DESC,
Tags.name" with no blank line between them (the relevant symbols to locate are
GROUP BY, Tags.id, Tags.name, ORDER BY, and linkCount).
| val profile = deeprQueries.getProfileByName(it).executeAsOneOrNull() | ||
| if (profile == null) { | ||
| deeprQueries.insertProfile(it) | ||
| deeprQueries.insertProfile(it, 0L) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "maxPriority|insertProfile" --type=kotlin -A 2 -B 1Repository: yogeshpaliyal/Deepr
Length of output: 6312
🏁 Script executed:
rg -n "maxPriority\|insertProfile" --include="*.sq" -A 2 -B 1Repository: yogeshpaliyal/Deepr
Length of output: 504
🏁 Script executed:
# Also check the CsvBookmarkImporter.kt file directly to see the context around line 71
fd "CsvBookmarkImporter.kt" --type=f -x cat -n {}Repository: yogeshpaliyal/Deepr
Length of output: 7603
Imported profiles all receive priority = 0, producing indeterminate grid ordering.
When a CSV contains multiple new profiles, they are all inserted with priority = 0L at line 71. Since the profile grid orders by priority, imported profiles with identical priority values will appear in undefined order relative to each other.
The codebase already follows the correct pattern elsewhere (see AccountViewModel.insertProfile() which queries maxPriority and assigns maxPriority + 1). Apply the same approach here—query maxPriority for each new profile within the transaction loop and assign sequential priorities.
💡 Suggested approach
if (profile == null) {
- deeprQueries.insertProfile(it, 0L)
+ val nextPriority = (deeprQueries.maxPriority().executeAsOneOrNull()?.MAX ?: 0L) + 1L
+ deeprQueries.insertProfile(it, nextPriority)
deeprQueries.lastInsertRowId().executeAsOneOrNull()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/CsvBookmarkImporter.kt`
at line 71, Imported profiles are all saved with priority = 0L at the
deeprQueries.insertProfile(it, 0L) call in CsvBookmarkImporter, causing
indeterminate ordering; modify the import transaction so it queries the current
max priority (using the same pattern as AccountViewModel.insertProfile(), i.e.,
call deeprQueries.maxPriority() or equivalent) before inserting each new
profile, set the profile's priority to maxPriority + 1, and increment that in
the loop for subsequent inserts so each imported profile gets a sequentially
higher priority rather than 0.
Comment on lines
+124
to
+178
| var htmlContent = | ||
| context.assets | ||
| .open("index.html") | ||
| .bufferedReader() | ||
| .use { it.readText() } | ||
|
|
||
| // Replace placeholders with translations | ||
| val placeholders = | ||
| mapOf( | ||
| "{{WEB_TITLE}}" to R.string.web_title, | ||
| "{{WEB_SUBTITLE}}" to R.string.web_subtitle, | ||
| "{{WEB_PROFILE_LABEL}}" to R.string.web_profile_label, | ||
| "{{WEB_NEW_PROFILE}}" to R.string.web_new_profile, | ||
| "{{WEB_ADD_NEW_LINK}}" to R.string.web_add_new_link, | ||
| "{{WEB_LINK_URL}}" to R.string.web_link_url, | ||
| "{{WEB_LINK_NAME}}" to R.string.web_link_name, | ||
| "{{WEB_OPTIONAL}}" to R.string.web_optional, | ||
| "{{WEB_NOTES_OPTIONAL}}" to R.string.web_notes_optional, | ||
| "{{WEB_TAGS_OPTIONAL}}" to R.string.web_tags_optional, | ||
| "{{WEB_ADD_LINK_BUTTON}}" to R.string.web_add_link_button, | ||
| "{{WEB_YOUR_LINKS}}" to R.string.web_your_links, | ||
| "{{WEB_REFRESH}}" to R.string.web_refresh, | ||
| "{{WEB_SEARCH_PLACEHOLDER}}" to R.string.web_search_placeholder, | ||
| "{{WEB_ALL_TAGS}}" to R.string.web_all_tags, | ||
| "{{WEB_SORT_DATE}}" to R.string.web_sort_date, | ||
| "{{WEB_SORT_NAME}}" to R.string.web_sort_name, | ||
| "{{WEB_SORT_OPENS}}" to R.string.web_sort_opens, | ||
| "{{WEB_CLEAR}}" to R.string.web_clear, | ||
| "{{WEB_LOADING_LINKS}}" to R.string.web_loading_links, | ||
| "{{WEB_LOADING_DESCRIPTION}}" to R.string.web_loading_description, | ||
| "{{WEB_CREATE_PROFILE_TITLE}}" to R.string.web_create_profile_title, | ||
| "{{WEB_PROFILE_NAME_LABEL}}" to R.string.web_profile_name_label, | ||
| "{{WEB_CANCEL}}" to R.string.web_cancel, | ||
| "{{WEB_CREATE_PROFILE_BUTTON}}" to R.string.web_create_profile_button, | ||
| "{{WEB_STATUS_CREATING}}" to R.string.web_status_creating, | ||
| "{{WEB_STATUS_ADDED}}" to R.string.web_status_added, | ||
| "{{WEB_SUCCESS_LINK_ADDED}}" to R.string.web_success_link_added, | ||
| "{{WEB_ERROR_ADD_LINK}}" to R.string.web_error_add_link, | ||
| "{{WEB_ERROR_LOADING}}" to R.string.web_error_loading, | ||
| "{{WEB_ERROR_REFRESH}}" to R.string.web_error_refresh, | ||
| "{{WEB_ERROR_FETCH}}" to R.string.web_error_fetch, | ||
| "{{WEB_MSG_SWITCHED}}" to R.string.web_msg_switched, | ||
| "{{WEB_MSG_FILTERED}}" to R.string.web_msg_filtered, | ||
| "{{WEB_MSG_FILTERS_CLEARED}}" to R.string.web_msg_filters_cleared, | ||
| "{{WEB_NO_LINKS}}" to R.string.web_no_links, | ||
| "{{WEB_NO_LINKS_DESCRIPTION}}" to R.string.web_no_links_description, | ||
| "{{WEB_NO_MATCHING}}" to R.string.web_no_matching, | ||
| "{{WEB_NO_MATCHING_DESCRIPTION}}" to R.string.web_no_matching_description, | ||
| "{{WEB_OPEN_LINK}}" to R.string.web_open_link, | ||
| "{{WEB_OPENS}}" to R.string.web_opens, | ||
| ) | ||
|
|
||
| placeholders.forEach { (placeholder, resId) -> | ||
| htmlContent = htmlContent.replace(placeholder, context.getString(resId)) | ||
| } |
There was a problem hiding this comment.
HTML asset is re-read and all 43 placeholders replaced on every GET / request
The full file I/O (assets.open("index.html")) and the 43-entry forEach replacement loop run on every web page load. Since the content doesn't change during the server's lifetime, this work should be done once at server startup and the result cached.
♻️ Proposed refactor
+ // Cache the localized HTML content at server start
+ private var cachedHtmlContent: String? = null
+
+ private fun buildLocalizedHtml(): String {
+ val html = context.assets.open("index.html").bufferedReader().use { it.readText() }
+ val placeholders = mapOf(
+ "{{WEB_TITLE}}" to R.string.web_title,
+ // ... (same map as today)
+ )
+ return placeholders.entries.fold(html) { acc, (key, resId) ->
+ acc.replace(key, context.getString(resId))
+ }
+ }Then in startServer:
+ cachedHtmlContent = buildLocalizedHtml()
server = embeddedServer(CIO, host = "0.0.0.0", port = port) {
...
get("/") {
try {
- var htmlContent = context.assets.open("index.html")...
- placeholders.forEach { ... }
+ val htmlContent = cachedHtmlContent ?: buildLocalizedHtml()
call.respondText(htmlContent, ContentType.Text.Html)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt`
around lines 124 - 178, The GET handler currently opens assets and runs the
43-entry placeholders replacement on every request (see htmlContent and
placeholders in LocalServerRepositoryImpl); move that work into server
initialization (e.g., in startServer or the class constructor) so the file is
read once, replacements performed once, and the resulting string stored in a
cached property (e.g., cachedIndexHtml) that the GET / handler returns directly;
update the GET handler to use cachedIndexHtml instead of re-reading assets and
looping through placeholders.
Comment on lines
+494
to
+496
| imageVector = if (isReordering) TablerIcons.Check else TablerIcons.ArrowsSort, | ||
| contentDescription = if (isReordering) "Finish Reordering" else stringResource(R.string.reorder), | ||
| tint = if (isReordering) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.onSurface, |
There was a problem hiding this comment.
Hardcoded English content descriptions break i18n/accessibility.
"Finish Reordering" (line 495), "Move Up" (line 899), and "Move Down" (line 913) are hardcoded strings while other UI text correctly uses stringResource(). These should use string resources for proper localization and screen-reader support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` around
lines 494 - 496, The hardcoded contentDescription strings in Home.kt (e.g., the
conditional contentDescription using "Finish Reordering" with isReordering, and
the "Move Up"/"Move Down" captions) break i18n/accessibility; replace those
literal strings with stringResource lookups (e.g., use
stringResource(R.string.finish_reordering) for the isReordering branch and
stringResource(R.string.move_up) / stringResource(R.string.move_down) for the
move buttons), and add corresponding entries finish_reordering, move_up, and
move_down to your strings.xml so screen readers and translations receive the
localized text; keep the existing ternary/conditional logic and icon/tint usage
(TablerIcons.Check, TablerIcons.ArrowsSort) intact.
Comment on lines
+86
to
+91
| private val _showProfilesGrid = MutableStateFlow(false) | ||
| val showProfilesGrid: StateFlow<Boolean> = _showProfilesGrid | ||
|
|
||
| fun setShowProfilesGrid(show: Boolean) { | ||
| _showProfilesGrid.value = show | ||
| } |
There was a problem hiding this comment.
_showProfilesGrid defaults to false, but the PR objective states the profiles grid should be the main landing view.
The PR description says "Replaces the default Home screen with a profiles grid as the main landing view," yet _showProfilesGrid is initialized to false, meaning users will land on the links/content view instead of the profiles grid on app start. Either initialize to true or ensure it's set before the first frame.
Proposed fix
- private val _showProfilesGrid = MutableStateFlow(false)
+ private val _showProfilesGrid = MutableStateFlow(true)📝 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 val _showProfilesGrid = MutableStateFlow(false) | |
| val showProfilesGrid: StateFlow<Boolean> = _showProfilesGrid | |
| fun setShowProfilesGrid(show: Boolean) { | |
| _showProfilesGrid.value = show | |
| } | |
| private val _showProfilesGrid = MutableStateFlow(true) | |
| val showProfilesGrid: StateFlow<Boolean> = _showProfilesGrid | |
| fun setShowProfilesGrid(show: Boolean) { | |
| _showProfilesGrid.value = show | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt`
around lines 86 - 91, Initialize the profiles grid state so the app lands on it
by default: change the initial value of the MutableStateFlow _showProfilesGrid
in AccountViewModel (the backing property for showProfilesGrid) from false to
true, or ensure setShowProfilesGrid(true) is invoked during the ViewModel's
initialization (e.g., in the init block) so the profiles grid is shown before
the first frame; update only the _showProfilesGrid declaration or add the init
call in AccountViewModel.kt.
Comment on lines
+859
to
+893
| fun moveProfileUp(profileId: Long) { | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| // First, ensure all profiles have unique, sequential priorities | ||
| normalizePriorities() | ||
|
|
||
| val profiles = linkRepository.getAllProfiles().executeAsList() | ||
| val index = profiles.indexOfFirst { it.id == profileId } | ||
| if (index > 0) { | ||
| val currentProfile = profiles[index] | ||
| val previousProfile = profiles[index - 1] | ||
|
|
||
| val tempPriority = currentProfile.priority | ||
| linkRepository.updateProfilePriority(currentProfile.id, previousProfile.priority) | ||
| linkRepository.updateProfilePriority(previousProfile.id, tempPriority) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun moveProfileDown(profileId: Long) { | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| // First, ensure all profiles have unique, sequential priorities | ||
| normalizePriorities() | ||
|
|
||
| val profiles = linkRepository.getAllProfiles().executeAsList() | ||
| val index = profiles.indexOfFirst { it.id == profileId } | ||
| if (index != -1 && index < profiles.size - 1) { | ||
| val currentProfile = profiles[index] | ||
| val nextProfile = profiles[index + 1] | ||
|
|
||
| val tempPriority = currentProfile.priority | ||
| linkRepository.updateProfilePriority(currentProfile.id, nextProfile.priority) | ||
| linkRepository.updateProfilePriority(nextProfile.id, tempPriority) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check database configuration and transaction support
find . -type f -name "*.kt" | xargs grep -l "Room\|Database\|Transaction" | head -20Repository: yogeshpaliyal/Deepr
Length of output: 206
🏁 Script executed:
# Look at the repository implementation for these methods
rg -n "updateProfilePriority|getAllProfiles|normalizePriorities" -A 5 -B 2 --type=ktRepository: yogeshpaliyal/Deepr
Length of output: 89
🏁 Script executed:
# Check for existing transaction patterns in the codebase
rg -n "withTransaction|transaction\(" --type=kt -C 2Repository: yogeshpaliyal/Deepr
Length of output: 89
🏁 Script executed:
# Examine the full AccountViewModel file to understand context
wc -l app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.ktRepository: yogeshpaliyal/Deepr
Length of output: 138
🏁 Script executed:
# Fix the file type for Kotlin and search for repository methods
rg -n "updateProfilePriority|getAllProfiles|normalizePriorities" -A 5 -B 2 --type=kotlinRepository: yogeshpaliyal/Deepr
Length of output: 13414
🏁 Script executed:
# Search for transaction patterns with correct syntax
rg -n "withTransaction|transaction\(" --type=kotlin -C 2Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look at the repository files to understand the database setup
find . -type f -name "*Repository*.kt" | head -10Repository: yogeshpaliyal/Deepr
Length of output: 785
🏁 Script executed:
# Examine the Room database setup
rg -n "@Database|@Entity|@Dao" --type=kotlin -C 1Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check the database implementation file
cat -n app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt | head -100Repository: yogeshpaliyal/Deepr
Length of output: 4508
🏁 Script executed:
# Find database configuration files
find . -type f \( -name "*.sqldelight" -o -name "*Database*.kt" -o -name "*Deepr*.kt" \) | grep -E "(database|queries|sqldelight)" | head -20Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look for the DeeprQueries interface definition
rg -n "interface DeeprQueries|class DeeprQueries" -A 20Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check if this is SQLiteDelight by looking for .sqldelight files
find . -type f -name "*.sqldelight"Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look for database initialization
rg -n "DatabaseDriverFactory|DatabaseDriver|createDriver" -B 2 -A 5Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check for any transaction usage in the codebase
rg -n "\.transactionWithResult|\.transaction\(|withTransaction"Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look at the DeeprApplication.kt which was found earlier
cat -n app/src/main/java/com/yogeshpaliyal/deepr/DeeprApplication.kt | head -150Repository: yogeshpaliyal/Deepr
Length of output: 6954
🏁 Script executed:
# Search more broadly for database setup
rg -n "Database\|SqlDelight\|Room\|sqlite" -i | head -50Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check the imports in LinkRepositoryImpl to understand the DB setup
cat -n app/src/main/java/com/yogeshpaliyal/deepr/data/LinkRepositoryImpl.kt | head -80Repository: yogeshpaliyal/Deepr
Length of output: 3105
🏁 Script executed:
# Look for gradle files to see dependencies
find . -name "build.gradle*" -o -name "*.gradle.kts" | head -5Repository: yogeshpaliyal/Deepr
Length of output: 126
🏁 Script executed:
# Check for transaction support in SQLiteDelight
rg -n "transactionWithResult|transactionWithoutResult"Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look at the build.gradle to see SQLiteDelight version
cat -n app/build.gradle.kts | head -100Repository: yogeshpaliyal/Deepr
Length of output: 3415
🏁 Script executed:
# Search for database.transaction or db.transaction patterns
rg -n "database\.|db\." | grep -i transactionRepository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check the DeeprDB class to see available transaction methods
rg -n "class DeeprDB" -A 30Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look at how other database operations handle concurrency
rg -n "launch\(Dispatchers.IO\)" -B 2 -A 10 | head -100Repository: yogeshpaliyal/Deepr
Length of output: 10530
🏁 Script executed:
# Find the DeeprDB file or generated code
find . -type f -name "DeeprDB.kt" -o -name "*DB.kt" | grep -i deeprRepository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Check if there's a .sqldelight file
find . -type f -name "*.sqldelight"Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Look at the full LinkRepositoryImpl to understand the complete picture
cat -n app/src/main/java/com/yogeshpaliyal/deepr/data/LinkRepositoryImpl.ktRepository: yogeshpaliyal/Deepr
Length of output: 9256
Priority swap operations lack atomicity — concurrent reorders can corrupt or duplicate priorities.
moveProfileUp and moveProfileDown perform normalizePriorities() (multiple writes), a fresh getAllProfiles().executeAsList(), then two updateProfilePriority calls in separate viewModelScope.launch(Dispatchers.IO) coroutines without synchronization. If the user taps move buttons quickly, these can interleave mid-execution. For example, normalizePriorities() on coroutine A could write while coroutine B's getAllProfiles() reads stale data, leading to duplicate or inconsistent priorities.
SQLiteDelight does not provide a high-level withTransaction API. Wrap the entire normalize-and-swap logic in a database transaction (via direct SQL BEGIN/COMMIT), or serialize these operations with a mutex or single-threaded dispatcher to prevent concurrent reorders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt`
around lines 859 - 893, The moveProfileUp and moveProfileDown flows call
normalizePriorities(), then read profiles via
linkRepository.getAllProfiles().executeAsList() and perform two
updateProfilePriority() calls outside any atomic boundary, allowing concurrent
coroutines to interleave and corrupt priorities; fix by wrapping the entire
normalize-and-swap sequence for a given operation in a single serialized
boundary — either execute a DB transaction that includes normalizePriorities(),
the read, and both updateProfilePriority() calls (using explicit BEGIN/COMMIT on
the SQL driver) or protect the sequence with a shared Mutex / single-threaded
CoroutineDispatcher so moveProfileUp, moveProfileDown and normalizePriorities
cannot run concurrently.
|
|
||
| <!-- Profiles --> | ||
| <string name="profiles">Profili</string> | ||
| <string name="reorder">Reordina</string> |
There was a problem hiding this comment.
Non-standard Italian word for "reorder"
"Reordina" is an English calque. The standard Italian imperative/command form of riordinare is "Riordina".
✏️ Proposed fix
- <string name="reorder">Reordina</string>
+ <string name="reorder">Riordina</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="reorder">Reordina</string> | |
| <string name="reorder">Riordina</string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/values-it/strings.xml` at line 256, The string resource
named "reorder" uses a non-standard calque "Reordina"; update the value for
string name="reorder" to the correct Italian imperative "Riordina" in the
strings.xml so the app uses the proper localization.
| <string name="failed_to_update_profile">Impossibile aggiornare il profilo</string> | ||
| <string name="cannot_delete_only_profile">Impossibile eliminare l\'unico profilo esistente</string> | ||
| <string name="profile_delete_confirmation">Sei sicuro di voler eliminare questo profilo? Tutti i link e le etichette in questo profilo verranno eliminati.</string> | ||
| <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati</string> |
There was a problem hiding this comment.
Missing terminal period and missing apostrophe in web_error_add_link
Two punctuation issues in this file:
- Line 272 —
profile_delete_confirmation_with_nameends with"verranno eliminati"without a period, inconsistent with the surrounding confirmation strings (lines 271, 273) that end with. - Line 326 —
web_error_add_link:"Errore nell aggiunta"is missing the elision apostrophe; must be"Errore nell'aggiunta".
✏️ Proposed fixes
- <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati</string>
+ <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati.</string>- <string name="web_error_add_link">Errore nell aggiunta del link. Riprova.</string>
+ <string name="web_error_add_link">Errore nell'aggiunta del link. Riprova.</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="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati</string> | |
| <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati.</string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/values-it/strings.xml` at line 272, Update the two Italian
string resources: add a terminal period to the value of
profile_delete_confirmation_with_name so it ends with "verranno eliminati." to
match surrounding confirmation strings, and fix the missing elision apostrophe
in web_error_add_link by changing "Errore nell aggiunta" to "Errore
nell'aggiunta". Ensure you edit the string values for the resource names
profile_delete_confirmation_with_name and web_error_add_link accordingly.
Comment on lines
271
to
+273
| <string name="profile_delete_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string> | ||
| <string name="profile_delete_confirmation_with_name">Are you sure you want to delete %s profile? All links and tags in this profile will be deleted</string> | ||
| <string name="delete_profile_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the actual content of the strings.xml file at the specified lines
cat -n app/src/main/res/values/strings.xml | sed -n '270,275p'Repository: yogeshpaliyal/Deepr
Length of output: 777
🏁 Script executed:
# Search for all usages of the potentially duplicate keys across the codebase
rg -n "profile_delete_confirmation|delete_profile_confirmation" --type=kt --type=xml --type=javaRepository: yogeshpaliyal/Deepr
Length of output: 89
🏁 Script executed:
# Also check for the quoted vs unquoted %s pattern in other confirmation strings
rg -n "delete_.*_confirmation|profile_.*_confirmation" app/src/main/res/values/strings.xml | head -20Repository: yogeshpaliyal/Deepr
Length of output: 851
🏁 Script executed:
# Search for usages of the two potentially duplicate keys in all files
rg "profile_delete_confirmation|delete_profile_confirmation" --type=java --type=xmlRepository: yogeshpaliyal/Deepr
Length of output: 1658
🏁 Script executed:
# Also check broader context - search in all code files
find . -type f \( -name "*.java" -o -name "*.kt" -o -name "*.xml" \) -not -path "*/\.*" | xargs rg "profile_delete_confirmation|delete_profile_confirmation" 2>/dev/null | head -30Repository: yogeshpaliyal/Deepr
Length of output: 2041
🏁 Script executed:
# Check if delete_profile_confirmation is used anywhere in the codebase
rg "delete_profile_confirmation" --type=java --type=xml app/src/main/java/Repository: yogeshpaliyal/Deepr
Length of output: 45
🏁 Script executed:
# Also verify the exact usages of profile_delete_confirmation and profile_delete_confirmation_with_name
rg "R\.string\.profile_delete_confirmation|R\.string\.delete_profile_confirmation" app/src/main/java/Repository: yogeshpaliyal/Deepr
Length of output: 425
Remove unused duplicate string key and fix missing period
delete_profile_confirmation (line 273) is identical to profile_delete_confirmation (line 271) and is never referenced in the codebase—it's dead code present across multiple localization files. Remove it.
Additionally, profile_delete_confirmation_with_name (line 272) is missing a terminal period, inconsistent with both neighboring strings that end with ..
The quoting of %s also differs: delete_link_confirmation_with_name quotes the substitution (\'%s\'), while profile_delete_confirmation_with_name does not (%s). Align the quoting style for consistency.
✏️ Proposed fix
- <string name="profile_delete_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string>
- <string name="profile_delete_confirmation_with_name">Are you sure you want to delete %s profile? All links and tags in this profile will be deleted</string>
- <string name="delete_profile_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string>
+ <string name="profile_delete_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string>
+ <string name="profile_delete_confirmation_with_name">Are you sure you want to delete '%s' profile? All links and tags in this profile will be deleted.</string>Remove delete_profile_confirmation from all localization files (values-*, values-et, values-it, values-hi, etc.).
🤖 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 271 - 273, Remove the
unused duplicate string resource delete_profile_confirmation across all
localization files (it is identical to profile_delete_confirmation and unused),
update profile_delete_confirmation_with_name to end with a period, and make the
substitution consistent by quoting the %s placeholder (use the same quoting
style as delete_link_confirmation_with_name, e.g. wrap %s in single quotes) so
its text matches neighboring strings; change only the string resource entries
(profile_delete_confirmation_with_name and removal of
delete_profile_confirmation) and apply the same edits to every
values-*/localized XML file.
Comment on lines
80
to
+82
| WHERE | ||
| Deepr.profileId = ? | ||
| AND (Deepr.link LIKE '%' || ? || '%' OR Deepr.name LIKE '%' || ? || '%' OR Deepr.notes LIKE '%' || ? || '%') | ||
| AND (Deepr.link LIKE '%' || ? || '%' OR Deepr.name LIKE '%' || ? || '%' OR Deepr.notes LIKE '%' || ? || '%' OR Tags.name LIKE '%' || ? || '%') |
There was a problem hiding this comment.
Tag search in WHERE with LEFT JOIN causes incomplete GROUP_CONCAT — matched links will display only the searched tag, not all their tags.
With LEFT JOIN Tags producing one row per tag per link, the WHERE filter Tags.name LIKE '%' || ? || '%' eliminates all non-matching tag rows for a link before GROUP BY. As a result, GROUP_CONCAT(Tags.name) in the SELECT only aggregates the surviving (matching) tag rows — every other tag of the same link is silently dropped from tagsNames and tagsIds in the result set.
Concrete example: Link "My App" has tags ["React", "JavaScript"]. User searches "React".
- Left-join row for
"React"passes theWHERE→ survives intoGROUP BY. - Left-join row for
"JavaScript"is filtered out. GROUP_CONCATproduces"React"only;"JavaScript"is invisible in the UI for this result.
The fix is to decouple the tag-search predicate from the join used for aggregation by moving it into an EXISTS correlated subquery, so the LEFT JOIN rows remain intact for GROUP_CONCAT.
🐛 Proposed fix — preserve all tag rows via EXISTS
WHERE
Deepr.profileId = ?
- AND (Deepr.link LIKE '%' || ? || '%' OR Deepr.name LIKE '%' || ? || '%' OR Deepr.notes LIKE '%' || ? || '%' OR Tags.name LIKE '%' || ? || '%')
+ AND (
+ Deepr.link LIKE '%' || ? || '%'
+ OR Deepr.name LIKE '%' || ? || '%'
+ OR Deepr.notes LIKE '%' || ? || '%'
+ OR EXISTS (
+ SELECT 1 FROM Tags t2
+ INNER JOIN LinkTags lt2 ON t2.id = lt2.tagId
+ WHERE lt2.linkId = Deepr.id
+ AND t2.name LIKE '%' || ? || '%'
+ )
+ )The number of ? parameters for the search query stays at 4, so the generated Kotlin API surface is unchanged.
📝 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
| WHERE | |
| Deepr.profileId = ? | |
| AND (Deepr.link LIKE '%' || ? || '%' OR Deepr.name LIKE '%' || ? || '%' OR Deepr.notes LIKE '%' || ? || '%') | |
| AND (Deepr.link LIKE '%' || ? || '%' OR Deepr.name LIKE '%' || ? || '%' OR Deepr.notes LIKE '%' || ? || '%' OR Tags.name LIKE '%' || ? || '%') | |
| WHERE | |
| Deepr.profileId = ? | |
| AND ( | |
| Deepr.link LIKE '%' || ? || '%' | |
| OR Deepr.name LIKE '%' || ? || '%' | |
| OR Deepr.notes LIKE '%' || ? || '%' | |
| OR EXISTS ( | |
| SELECT 1 FROM Tags t2 | |
| INNER JOIN LinkTags lt2 ON t2.id = lt2.tagId | |
| WHERE lt2.linkId = Deepr.id | |
| AND t2.name LIKE '%' || ? || '%' | |
| ) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/sqldelight/com/yogeshpaliyal/deepr/Deepr.sq` around lines 80 -
82, The WHERE clause filters out non-matching Tag rows before aggregation,
causing GROUP_CONCAT(Tags.name) and GROUP_CONCAT(Tags.id) to miss a link's other
tags; change the tag-search predicate (Tags.name LIKE '%' || ? || '%') so it is
used in an EXISTS correlated subquery (e.g., EXISTS(SELECT 1 FROM Tags t WHERE
t.linkId = Deepr.linkId AND t.name LIKE '%' || ? || '%')) while leaving the LEFT
JOIN to Tags intact for aggregation, preserving all joined tag rows for
GROUP_CONCAT; ensure you keep the same number/order of bind parameters used by
the current query so the Kotlin API surface remains unchanged.
Comment on lines
+35
to
+39
| | Feature | Github Release | F-droid | Play Store | Pro Version on Play Store | | ||
| |-----------------------------------|----------------|---------|------------|-----------| | ||
| | Firebase Analytics | ❌ | ❌ | ✅ | ✅ | | ||
| | Google Drive Backup | ❌ | ❌ | ❌ | ✅ | | ||
| | Profile speicific themes | ❌ | ❌ | ❌ | ✅ | |
There was a problem hiding this comment.
Two text errors in the changed table rows.
- Line 35/39: "Github" → "GitHub" (official branding; flagged by static analysis).
- Line 39: "Profile speicific themes" → "Profile specific themes" (transposed letters).
✏️ Proposed fix
-| Feature | Github Release | F-droid | Play Store | Pro Version on Play Store |
-|-----------------------------------|----------------|---------|------------|-----------|
-| Firebase Analytics | ❌ | ❌ | ✅ | ✅ |
-| Google Drive Backup | ❌ | ❌ | ❌ | ✅ |
-| Profile speicific themes | ❌ | ❌ | ❌ | ✅ |
+| Feature | GitHub Release | F-droid | Play Store | Pro Version on Play Store |
+|-----------------------------------|----------------|---------|------------|-----------|
+| Firebase Analytics | ❌ | ❌ | ✅ | ✅ |
+| Google Drive Backup | ❌ | ❌ | ❌ | ✅ |
+| Profile specific themes | ❌ | ❌ | ❌ | ✅ |🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ... | Feature | Github Release | F-droid | Play Store | Pro Ve...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 35 - 39, Update the markdown table rows to fix two
typos: change the string "Github" in the column header/cell to the correct brand
casing "GitHub", and correct the feature label "Profile speicific themes" to
"Profile specific themes" so the table reads exactly "GitHub" and "Profile
specific themes" where those exact strings appear.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt (1)
499-499:⚠️ Potential issue | 🟡 MinorLocalize the remaining reorder content descriptions.
Line [499], Line [927], and Line [941] still use hardcoded English strings (
"Finish Reordering","Move Up","Move Down"), which breaks localization/accessibility consistency.Proposed fix
- contentDescription = if (isReordering) "Finish Reordering" else stringResource(R.string.reorder), + contentDescription = + if (isReordering) stringResource(R.string.finish_reordering) + else stringResource(R.string.reorder), ... - contentDescription = "Move Up", + contentDescription = stringResource(R.string.move_up), ... - contentDescription = "Move Down", + contentDescription = stringResource(R.string.move_down),Also applies to: 927-927, 941-941
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` at line 499, The contentDescription values are using hardcoded English strings; update them to use string resources so they're localizable — replace the literal "Finish Reordering" in the contentDescription expression that checks isReordering with stringResource(R.string.finish_reordering) (or equivalent), and replace "Move Up" and "Move Down" usages with stringResource(R.string.move_up) and stringResource(R.string.move_down) respectively; locate these in Home.kt where contentDescription is passed (the composable using isReordering and the up/down buttons) and ensure the corresponding string resource keys are added to strings.xml.app/src/main/res/values-it/strings.xml (2)
257-257:⚠️ Potential issue | 🟡 MinorUse the standard Italian imperative for reorder.
Line 257 still uses “Reordina”; the standard UI imperative is “Riordina”.
✏️ Suggested fix
- <string name="reorder">Reordina</string> + <string name="reorder">Riordina</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/values-it/strings.xml` at line 257, The string resource named "reorder" currently uses the non-standard Italian form "Reordina"; update the value of the resource with name "reorder" to the standard imperative "Riordina" so the UI uses the correct Italian wording.
273-273:⚠️ Potential issue | 🟡 MinorFix unresolved Italian punctuation/elision issues.
Line 273 is missing the terminal period, and Line 327 should use “nell'aggiunta” (apostrophe required).
✏️ Suggested fix
- <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati</string> + <string name="profile_delete_confirmation_with_name">Sei sicuro di voler eliminare il profilo %s? Tutti i link e le etichette in questo profilo verranno eliminati.</string>- <string name="web_error_add_link">Errore nell aggiunta del link. Riprova.</string> + <string name="web_error_add_link">Errore nell'aggiunta del link. Riprova.</string>Also applies to: 327-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/values-it/strings.xml` at line 273, The Italian strings have punctuation and elision issues: add a terminal period to the string with name profile_delete_confirmation_with_name and fix the other affected string to use the elided form "nell'aggiunta" (include the apostrophe) — locate the string resource profile_delete_confirmation_with_name and append a final period, and locate the other Italian string (line ~327) and replace its word with "nell'aggiunta" to correct the apostrophe/elision.app/src/main/res/values/strings.xml (1)
272-273:⚠️ Potential issue | 🟡 MinorClean up profile-delete confirmation strings.
Line 272 is missing terminal punctuation and reads awkwardly; Line 273 duplicates Line 271 and adds localization maintenance overhead.
✏️ Suggested fix
- <string name="profile_delete_confirmation_with_name">Are you sure you want to delete %s profile? All links and tags in this profile will be deleted</string> - <string name="delete_profile_confirmation">Are you sure you want to delete this profile? All links and tags in this profile will be deleted.</string> + <string name="profile_delete_confirmation_with_name">Are you sure you want to delete '%s' profile? All links and tags in this profile will be deleted.</string>🤖 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 272 - 273, Update the two string resources to remove duplication and fix punctuation/grammar: eliminate the redundant delete_profile_confirmation entry and standardize on a single, well-formed resource (profile_delete_confirmation_with_name) whose text includes proper terminal punctuation and reads smoothly when a profile name is inserted (adjust "%s profile" wording to "%s profile?" or better "%s profile." as appropriate); ensure the final string resource name remains unique (profile_delete_confirmation_with_name) and update any callers to use that resource.
🧹 Nitpick comments (1)
app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt (1)
838-842: Avoid O(n²) index lookup inProfilesGrid.Line [841] uses
profiles.indexOf(profile)inside item rendering; this adds linear lookup per item. Use indexed items directly.Proposed refactor
+import androidx.compose.foundation.lazy.grid.itemsIndexed ... - items(profiles, key = { it.id }) { profile -> + itemsIndexed(profiles, key = { _, profile -> profile.id }) { index, profile -> val isSelected = profile.id == currentProfileId val hapticFeedback = LocalHapticFeedback.current - val index = profiles.indexOf(profile)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` around lines 838 - 842, The code in the items(...) block does an O(n) lookup with profiles.indexOf(profile) for every item; change the Lazy/Compose items call to an indexed variant so the index is provided directly (e.g., use itemsIndexed with key = { index, profile -> profile.id } and the lambda signature (index, profile) -> ...), then remove the profiles.indexOf(profile) call and use the provided index; keep existing uses of currentProfileId, LocalHapticFeedback.current, and isSelected as before.
🤖 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/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt`:
- Around line 139-145: The LaunchedEffect tied to deeprInfo.link currently calls
fetchMetadata on every link change (while the user types); modify the effect to
only auto-fetch once for a provided/initial link by adding a one-time guard
(e.g., a remembered Boolean like hasPrefilled or initialPrefillDone) and only
call normalizeLink/isValidDeeplink/fetchMetadata when that flag is false and
deeprInfo.name.isEmpty(), then set the flag true after calling fetchMetadata;
update the LaunchedEffect(deeprInfo.link) block and use normalizeLink,
isValidDeeplink, and fetchMetadata as-is.
- Around line 123-131: The callback from viewModel.fetchMetaData can write stale
metadata if the user changed the link before the async response; capture the
requested URL (e.g., val expectedLink = urlToFetch) before calling fetchMetaData
and inside the callback verify the current deeprInfo.link (or current input)
still equals expectedLink before updating deeprInfo, isNameError, and thumbnail;
only apply the metadata when they match and still relevant, and still set
isFetchingMetadata.value = false regardless.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt`:
- Around line 1594-1663: The delete confirmation AlertDialog is shown while the
Edit Profile dialog is always rendered, causing stacked dialogs; update the
edit-dialog rendering (the block controlled by the edit dialog state, e.g.,
showEditDialog or showEditProfileDialog) to only render when the delete
confirmation is not visible by adding a guard like if (!showDeleteConfirmation
&& showEditDialog) { ... } around the edit dialog UI, and ensure the edit
dialog's onDismiss clears its own flag so both dialogs cannot remain open
simultaneously (references: showDeleteConfirmation, the edit dialog boolean
state, and viewModel.deleteProfile/onDismiss handlers).
In `@app/src/main/res/values/strings.xml`:
- Line 31: The string resource save_your_link_below has a singular/plural
mismatch ("link" vs "them"); update its text to use a consistent singular form
by replacing "them" with "it" so the value reads "Save your link below to
quickly access it later" (modify the string value for save_your_link_below).
---
Duplicate comments:
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt`:
- Line 499: The contentDescription values are using hardcoded English strings;
update them to use string resources so they're localizable — replace the literal
"Finish Reordering" in the contentDescription expression that checks
isReordering with stringResource(R.string.finish_reordering) (or equivalent),
and replace "Move Up" and "Move Down" usages with
stringResource(R.string.move_up) and stringResource(R.string.move_down)
respectively; locate these in Home.kt where contentDescription is passed (the
composable using isReordering and the up/down buttons) and ensure the
corresponding string resource keys are added to strings.xml.
In `@app/src/main/res/values-it/strings.xml`:
- Line 257: The string resource named "reorder" currently uses the non-standard
Italian form "Reordina"; update the value of the resource with name "reorder" to
the standard imperative "Riordina" so the UI uses the correct Italian wording.
- Line 273: The Italian strings have punctuation and elision issues: add a
terminal period to the string with name profile_delete_confirmation_with_name
and fix the other affected string to use the elided form "nell'aggiunta"
(include the apostrophe) — locate the string resource
profile_delete_confirmation_with_name and append a final period, and locate the
other Italian string (line ~327) and replace its word with "nell'aggiunta" to
correct the apostrophe/elision.
In `@app/src/main/res/values/strings.xml`:
- Around line 272-273: Update the two string resources to remove duplication and
fix punctuation/grammar: eliminate the redundant delete_profile_confirmation
entry and standardize on a single, well-formed resource
(profile_delete_confirmation_with_name) whose text includes proper terminal
punctuation and reads smoothly when a profile name is inserted (adjust "%s
profile" wording to "%s profile?" or better "%s profile." as appropriate);
ensure the final string resource name remains unique
(profile_delete_confirmation_with_name) and update any callers to use that
resource.
---
Nitpick comments:
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt`:
- Around line 838-842: The code in the items(...) block does an O(n) lookup with
profiles.indexOf(profile) for every item; change the Lazy/Compose items call to
an indexed variant so the index is provided directly (e.g., use itemsIndexed
with key = { index, profile -> profile.id } and the lambda signature (index,
profile) -> ...), then remove the profiles.indexOf(profile) call and use the
provided index; keep existing uses of currentProfileId,
LocalHapticFeedback.current, and isSelected as before.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.ktapp/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.ktapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-et/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values-es/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/values-et/strings.xml
Comment on lines
+123
to
+131
| val fetchMetadata: (String?) -> Unit = { link -> | ||
| val urlToFetch = link ?: deeprInfo.link | ||
| if (urlToFetch.isNotBlank()) { | ||
| isFetchingMetadata.value = true | ||
| viewModel.fetchMetaData(urlToFetch) { | ||
| isFetchingMetadata.value = false | ||
| if (it != null) { | ||
| deeprInfo = deeprInfo.copy(name = it.title ?: "", thumbnail = it.image ?: "") | ||
| isNameError = false |
There was a problem hiding this comment.
Guard against stale metadata responses overwriting current input.
The async callback applies name/thumbnail unconditionally; if the link changes before completion, stale metadata can be written into the new link form state.
✏️ Suggested fix (apply only if response matches current link)
val fetchMetadata: (String?) -> Unit = { link ->
- val urlToFetch = link ?: deeprInfo.link
+ val urlToFetch = normalizeLink(link ?: deeprInfo.link)
if (urlToFetch.isNotBlank()) {
+ val requestLink = urlToFetch
isFetchingMetadata.value = true
- viewModel.fetchMetaData(urlToFetch) {
+ viewModel.fetchMetaData(requestLink) {
isFetchingMetadata.value = false
- if (it != null) {
+ val currentLink = normalizeLink(deeprInfo.link)
+ if (it != null && currentLink == requestLink) {
deeprInfo = deeprInfo.copy(name = it.title ?: "", thumbnail = it.image ?: "")
isNameError = false
} else {
Toast.makeText(context, fetchMetadataErrorText, Toast.LENGTH_SHORT).show()
}
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt`
around lines 123 - 131, The callback from viewModel.fetchMetaData can write
stale metadata if the user changed the link before the async response; capture
the requested URL (e.g., val expectedLink = urlToFetch) before calling
fetchMetaData and inside the callback verify the current deeprInfo.link (or
current input) still equals expectedLink before updating deeprInfo, isNameError,
and thumbnail; only apply the metadata when they match and still relevant, and
still set isFetchingMetadata.value = false regardless.
Comment on lines
+139
to
+145
| // Pre-populate and fetch if link is provided (e.g. from long press) | ||
| LaunchedEffect(deeprInfo.link) { | ||
| val normalized = normalizeLink(deeprInfo.link) | ||
| if (isValidDeeplink(normalized) && deeprInfo.name.isEmpty()) { | ||
| fetchMetadata(normalized) | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid auto-fetching metadata on every link edit.
This effect runs for each deeprInfo.link change and can repeatedly call metadata fetch while the user types.
✏️ Suggested fix (gate to one-time prefill behavior)
+ var hasAutoFetchedMetadata by remember(selectedLink.id) { mutableStateOf(false) }
// Pre-populate and fetch if link is provided (e.g. from long press)
- LaunchedEffect(deeprInfo.link) {
+ LaunchedEffect(deeprInfo.link, hasAutoFetchedMetadata, selectedLink.id) {
val normalized = normalizeLink(deeprInfo.link)
- if (isValidDeeplink(normalized) && deeprInfo.name.isEmpty()) {
+ if (
+ !hasAutoFetchedMetadata &&
+ selectedLink.link.isNotBlank() &&
+ isValidDeeplink(normalized) &&
+ deeprInfo.name.isEmpty()
+ ) {
+ hasAutoFetchedMetadata = true
fetchMetadata(normalized)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/addlink/AddLinkScreen.kt`
around lines 139 - 145, The LaunchedEffect tied to deeprInfo.link currently
calls fetchMetadata on every link change (while the user types); modify the
effect to only auto-fetch once for a provided/initial link by adding a one-time
guard (e.g., a remembered Boolean like hasPrefilled or initialPrefillDone) and
only call normalizeLink/isValidDeeplink/fetchMetadata when that flag is false
and deeprInfo.name.isEmpty(), then set the flag true after calling
fetchMetadata; update the LaunchedEffect(deeprInfo.link) block and use
normalizeLink, isValidDeeplink, and fetchMetadata as-is.
Comment on lines
+1594
to
+1663
| if (showDeleteConfirmation) { | ||
| AlertDialog( | ||
| onDismissRequest = { showDeleteConfirmation = false }, | ||
| icon = { | ||
| Surface( | ||
| color = MaterialTheme.colorScheme.errorContainer, | ||
| shape = RoundedCornerShape(16.dp), | ||
| ) { | ||
| Icon( | ||
| imageVector = TablerIcons.Trash, | ||
| contentDescription = null, | ||
| tint = MaterialTheme.colorScheme.onErrorContainer, | ||
| modifier = Modifier.padding(12.dp), | ||
| ) | ||
| } | ||
| }, | ||
| title = { | ||
| Text( | ||
| text = stringResource(R.string.delete_profile), | ||
| style = MaterialTheme.typography.headlineSmall, | ||
| textAlign = TextAlign.Center, | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) | ||
| }, | ||
| text = { | ||
| Column( | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) { | ||
| val fullMessage = stringResource(R.string.profile_delete_confirmation_with_name, profile.name) | ||
| val parts = fullMessage.split(profile.name) | ||
| val annotatedString = buildAnnotatedString { | ||
| if (parts.size >= 2) { | ||
| append(parts[0]) | ||
| withStyle(style = SpanStyle(fontWeight = FontWeight.Bold)) { | ||
| append(profile.name) | ||
| } | ||
| append(parts[1]) | ||
| } else { | ||
| append(fullMessage) | ||
| } | ||
| } | ||
| Text( | ||
| text = annotatedString, | ||
| textAlign = TextAlign.Center, | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) | ||
| } | ||
| }, | ||
| confirmButton = { | ||
| Button( | ||
| onClick = { | ||
| viewModel.deleteProfile(profile.id) | ||
| showDeleteConfirmation = false | ||
| onDismiss() | ||
| Toast.makeText(context, context.getString(R.string.profile_deleted_successfully), Toast.LENGTH_SHORT).show() | ||
| }, | ||
| colors = ButtonDefaults.buttonColors(containerColor = MaterialTheme.colorScheme.error) | ||
| ) { | ||
| Text(stringResource(R.string.delete)) | ||
| } | ||
| }, | ||
| dismissButton = { | ||
| TextButton(onClick = { showDeleteConfirmation = false }) { | ||
| Text(stringResource(android.R.string.cancel)) | ||
| } | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Prevent stacked dialogs in profile management flow.
Line [1665] always renders the edit dialog, even when showDeleteConfirmation is true (Line [1594]), so both dialogs can appear together.
Proposed fix
- if (showDeleteConfirmation) {
+ if (showDeleteConfirmation) {
AlertDialog(
onDismissRequest = { showDeleteConfirmation = false },
...
)
- }
-
- AlertDialog(
- onDismissRequest = onDismiss,
- title = { Text(stringResource(R.string.edit_profile)) },
- text = {
- Column(verticalArrangement = Arrangement.spacedBy(8.dp)) {
- OutlinedTextField(
- value = newName,
- onValueChange = {
- newName = it
- error = null
- },
- label = { Text(stringResource(R.string.profile_name)) },
- singleLine = true,
- isError = error != null,
- modifier = Modifier.fillMaxWidth(),
- shape = RoundedCornerShape(12.dp),
- )
- error?.let {
- Text(text = it, color = MaterialTheme.colorScheme.error, style = MaterialTheme.typography.bodySmall)
- }
- }
- },
- confirmButton = {
- Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
- if (allProfiles.size > 1) {
- TextButton(
- onClick = {
- showDeleteConfirmation = true
- },
- colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error)
- ) {
- Text(stringResource(R.string.delete))
- }
- }
-
- Button(
- onClick = {
- val trimmed = newName.trim()
- if (trimmed.isBlank()) {
- error = context.getString(R.string.profile_name_cannot_be_blank)
- return@Button
- }
- if (allProfiles.any { it.name.equals(trimmed, ignoreCase = true) && it.id != profile.id }) {
- error = context.getString(R.string.profile_name_exists)
- return@Button
- }
-
- coroutineScope.launch {
- viewModel.updateProfile(profile.id, trimmed, profile.themeMode, profile.colorTheme)
- onDismiss()
- Toast.makeText(context, context.getString(R.string.profile_updated_successfully), Toast.LENGTH_SHORT).show()
- }
- },
- enabled = newName.isNotBlank() && newName != profile.name
- ) {
- Text(stringResource(R.string.save))
- }
- }
- },
- dismissButton = {
- TextButton(onClick = onDismiss) {
- Text(stringResource(android.R.string.cancel))
- }
- }
- )
+ } else {
+ AlertDialog(
+ onDismissRequest = onDismiss,
+ title = { Text(stringResource(R.string.edit_profile)) },
+ text = {
+ Column(verticalArrangement = Arrangement.spacedBy(8.dp)) {
+ OutlinedTextField(
+ value = newName,
+ onValueChange = {
+ newName = it
+ error = null
+ },
+ label = { Text(stringResource(R.string.profile_name)) },
+ singleLine = true,
+ isError = error != null,
+ modifier = Modifier.fillMaxWidth(),
+ shape = RoundedCornerShape(12.dp),
+ )
+ error?.let {
+ Text(text = it, color = MaterialTheme.colorScheme.error, style = MaterialTheme.typography.bodySmall)
+ }
+ }
+ },
+ confirmButton = {
+ Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
+ if (allProfiles.size > 1) {
+ TextButton(
+ onClick = { showDeleteConfirmation = true },
+ colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error)
+ ) {
+ Text(stringResource(R.string.delete))
+ }
+ }
+ Button(
+ onClick = {
+ val trimmed = newName.trim()
+ if (trimmed.isBlank()) {
+ error = context.getString(R.string.profile_name_cannot_be_blank)
+ return@Button
+ }
+ if (allProfiles.any { it.name.equals(trimmed, ignoreCase = true) && it.id != profile.id }) {
+ error = context.getString(R.string.profile_name_exists)
+ return@Button
+ }
+ coroutineScope.launch {
+ viewModel.updateProfile(profile.id, trimmed, profile.themeMode, profile.colorTheme)
+ onDismiss()
+ Toast.makeText(context, context.getString(R.string.profile_updated_successfully), Toast.LENGTH_SHORT).show()
+ }
+ },
+ enabled = newName.isNotBlank() && newName != profile.name
+ ) {
+ Text(stringResource(R.string.save))
+ }
+ }
+ },
+ dismissButton = {
+ TextButton(onClick = onDismiss) {
+ Text(stringResource(android.R.string.cancel))
+ }
+ }
+ )
+ }Also applies to: 1665-1729
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/yogeshpaliyal/deepr/ui/screens/home/Home.kt` around
lines 1594 - 1663, The delete confirmation AlertDialog is shown while the Edit
Profile dialog is always rendered, causing stacked dialogs; update the
edit-dialog rendering (the block controlled by the edit dialog state, e.g.,
showEditDialog or showEditProfileDialog) to only render when the delete
confirmation is not visible by adding a guard like if (!showDeleteConfirmation
&& showEditDialog) { ... } around the edit dialog UI, and ensure the edit
dialog's onDismiss clears its own flag so both dialogs cannot remain open
simultaneously (references: showDeleteConfirmation, the edit dialog boolean
state, and viewModel.deleteProfile/onDismiss handlers).
| <string name="add_tag">Add tag</string> | ||
| <string name="no_links_saved_yet">No links saved yet</string> | ||
| <string name="save_your_link_below">Save your link below to quickly access them later.</string> | ||
| <string name="save_your_link_below">Save your link below to quickly access them later</string> |
There was a problem hiding this comment.
Fix singular/plural mismatch in helper text.
Line 31 says “Save your link … access them later”; use singular pronoun for consistency.
✏️ Suggested fix
- <string name="save_your_link_below">Save your link below to quickly access them later</string>
+ <string name="save_your_link_below">Save your link below to quickly access it later</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="save_your_link_below">Save your link below to quickly access them later</string> | |
| <string name="save_your_link_below">Save your link below to quickly access it later</string> |
🤖 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` at line 31, The string resource
save_your_link_below has a singular/plural mismatch ("link" vs "them"); update
its text to use a consistent singular form by replacing "them" with "it" so the
value reads "Save your link below to quickly access it later" (modify the string
value for save_your_link_below).
684d9be to
a8e76b5
Compare
Owner
|
@ALE-ARME |
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
2 participants
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.
This Pull Request implements significant UI/UX enhancements focused on profile management and search functionality, resolving various data integrity and navigation issues.
🎩 Key Changes:
ON DELETE CASCADEcorrectly removes all associated links when a profile is deleted.HometoProfiles.README.mdwith instructions for new navigation and profile management features.🔗 Link Pre-population:
+(Add Link) button to open the creation screen with the clipboard link auto-populated.Fixes #360 (Partial)
Fixes #385
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation