ADFA-3583 | Automate image import for YOLO image placeholders#1243
Merged
jatezzz merged 4 commits intoMay 5, 2026
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds per-placeholder image selection: lets users pick images for detected placeholder boxes, imports them into res/drawable, stores per-placeholder drawable references in UI state, and threads those overrides through generateXml → YoloToXmlConverter → AndroidXmlGenerator → LayoutRenderer to replace ImageView src values. ChangesRepository / XML pipeline
Drawable import & utilities
UI state, events & wiring
Activity & input mapping
Grammar
Sequence Diagram(s)sequenceDiagram
participant User
participant Zoomable as ZoomableImageView
participant Activity as ComputerVisionActivity
participant ViewModel as ComputerVisionViewModel
participant Picker as ContentPicker
participant Importer as DrawableImportHelper
participant Repo as ComputerVisionRepository
User->>Zoomable: Tap on image
Zoomable->>Zoomable: mapViewPointToImage(x,y)
Zoomable->>Activity: onImageTapListener(imageX,imageY)
Activity->>ViewModel: ImagePlaceholderTapped(imageX,imageY)
ViewModel->>ViewModel: resolve placeholder id → pendingImagePlaceholderId
ViewModel->>Activity: Emit OpenPlaceholderImagePicker
Activity->>Picker: Launch ("image/*")
User->>Picker: Select image (Uri)
Picker->>Activity: Return Uri
Activity->>ViewModel: PlaceholderImageSelected(uri)
ViewModel->>Importer: importDrawable(uri, layoutFilePath, fallbackName)
Importer-->>ViewModel: Result(ImportedDrawable)
ViewModel->>ViewModel: record selectedImagesByPlaceholderId[id]=drawableRef
ViewModel->>Repo: generateXml(..., selectedImagesByPlaceholderId)
Repo->>Repo: forward to YoloToXmlConverter / AndroidXmlGenerator
Repo-->>Activity: Return generated XML
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Daniel-ADFA
approved these changes
Apr 27, 2026
jatezzz
force-pushed
the
feat/ADFA-3583-image-import-placeholder-experimental
branch
3 times, most recently
from
b56fa24 to
00e0215
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/TextCleaner.kt (1)
6-11: Consider removing or redesigning unusedisCanvasMetadata()method.
TextCleaner.isCanvasMetadata()is not currently called anywhere in the codebase. However, if it were to be used with text processed bycleanText(), the method would fail to match metadata keywords sincecleanText()removes underscores (via the[^a-zA-Z0-9 ]regex) but the keywords use underscores:"layout_width","layout_height", etc.The method should either be removed as dead code or redesigned to normalize keywords matching the text-cleaning strategy used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/TextCleaner.kt` around lines 6 - 11, isCanvasMetadata() is unused and, if kept, will fail to match text cleaned by cleanText() because cleanText() strips underscores and non-alphanumerics while METADATA_KEYWORDS contain underscores; either remove isCanvasMetadata() entirely or update its logic to normalize both sides (e.g., apply the same cleaning/normalization as cleanText() to input text and to each METADATA_KEYWORDS entry or store a normalized keyword list without underscores) and keep the function name isCanvasMetadata() to perform the normalized comparison against METADATA_KEYWORDS.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt (1)
10-10: Strengthen placeholder sort tie-breakers for stable index mapping.The placeholder-id mapping (
ph_0,ph_1, …) depends on consistent ordering; top/left (or y/x) ties can make that less stable. Add extra keys to reduce collisions.Proposed refactor
fun List<DetectionResult>.getSortedPlaceholders(): List<DetectionResult> { return this.filter { it.label == IMAGE_PLACEHOLDER_LABEL } - .sortedWith(compareBy({ it.boundingBox.top }, { it.boundingBox.left })) + .sortedWith( + compareBy<DetectionResult>( + { it.boundingBox.top }, + { it.boundingBox.left }, + { it.boundingBox.bottom }, + { it.boundingBox.right } + ) + ) } fun List<ScaledBox>.getSortedScaledPlaceholders(): List<ScaledBox> { return this.filter { it.label == IMAGE_PLACEHOLDER_LABEL } - .sortedWith(compareBy({ it.y }, { it.x })) + .sortedWith(compareBy({ it.y }, { it.x }, { it.h }, { it.w })) }Also applies to: 15-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt` at line 10, The sort comparator using compareBy({ it.boundingBox.top }, { it.boundingBox.left }) is too weak and can produce unstable ordering when top/left tie; update the sorter in PlaceholderUtils (the sortedWith call) to add additional tie-breakers such as boundingBox.bottom, boundingBox.right (or width/height), and finally a deterministic fallback like the object's id or hashCode to guarantee stable ph_0/ph_1 mapping; ensure the comparator preserves the same top-then-left primary ordering but appends these extra keys in order of significance.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (1)
70-80: Centralize placeholder ID generation.This method hardcodes
ph_$index, andComputerVisionViewModel.resolvePlaceholderId()does the same from a separately sorted detection list. That coupling works today, but any future sort/filter tweak on either side will silently bind the wrong drawable to the wrong placeholder. Please move the ID generation into a shared helper used by both layers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt` around lines 70 - 80, The method buildSelectedImageOverrides currently hardcodes placeholder IDs using "ph_$index", which duplicates the logic in ComputerVisionViewModel.resolvePlaceholderId() and risks mismatches; extract the ID generation into a shared helper (e.g., a top-level function or companion object method named generatePlaceholderId(index: Int) or placeholderIdFor(index: Int)), replace the hardcoded "ph_$index" in buildSelectedImageOverrides with a call to that helper, and update ComputerVisionViewModel.resolvePlaceholderId() to use the same helper so both layers derive placeholder IDs from the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/AttributeValidator.kt`:
- Around line 60-63: The current normalization in AttributeValidator (the
assignment to variable 'content' derived from 'trimmed') unconditionally strips
leading/trailing bracket-like characters and can corrupt values; change this to
only unwrap when the first character is a bracket from the opening set and the
last character is the corresponding matching closing bracket (i.e., perform a
pair-aware check for matching pairs before removing them). Locate the code that
sets 'content' in AttributeValidator.kt, examine 'trimmed', and replace the
unconditional .replace(...) calls with logic that checks trimmed.first() and
trimmed.last() for matching pairs ((), {}, [], <>) and only then removes the
outer characters; otherwise leave the string intact. Ensure the behavior is
unit-testable and preserves original whitespace trimming.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 409-425: The success handler for importDrawable may apply a stale
placeholderId because placeholderId is captured before the async work; change
the state mutation in the onSuccess block to guard against stale imports by
checking the current _uiState.pendingImagePlaceholderId matches the captured
placeholderId before updating selectedImagesByPlaceholderId and clearing
pendingImagePlaceholderId. Concretely, inside the onSuccess { importedDrawable
-> ... } use _uiState.update { currentState -> if
(currentState.pendingImagePlaceholderId != placeholderId) currentState else
currentState.copy(pendingImagePlaceholderId = null,
selectedImagesByPlaceholderId = currentState.selectedImagesByPlaceholderId +
(placeholderId to SelectedImportedImage(...))) } so only the intended
placeholder is mutated.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 70-80: The method buildSelectedImageOverrides currently hardcodes
placeholder IDs using "ph_$index", which duplicates the logic in
ComputerVisionViewModel.resolvePlaceholderId() and risks mismatches; extract the
ID generation into a shared helper (e.g., a top-level function or companion
object method named generatePlaceholderId(index: Int) or placeholderIdFor(index:
Int)), replace the hardcoded "ph_$index" in buildSelectedImageOverrides with a
call to that helper, and update ComputerVisionViewModel.resolvePlaceholderId()
to use the same helper so both layers derive placeholder IDs from the single
source of truth.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt`:
- Line 10: The sort comparator using compareBy({ it.boundingBox.top }, {
it.boundingBox.left }) is too weak and can produce unstable ordering when
top/left tie; update the sorter in PlaceholderUtils (the sortedWith call) to add
additional tie-breakers such as boundingBox.bottom, boundingBox.right (or
width/height), and finally a deterministic fallback like the object's id or
hashCode to guarantee stable ph_0/ph_1 mapping; ensure the comparator preserves
the same top-then-left primary ordering but appends these extra keys in order of
significance.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/TextCleaner.kt`:
- Around line 6-11: isCanvasMetadata() is unused and, if kept, will fail to
match text cleaned by cleanText() because cleanText() strips underscores and
non-alphanumerics while METADATA_KEYWORDS contain underscores; either remove
isCanvasMetadata() entirely or update its logic to normalize both sides (e.g.,
apply the same cleaning/normalization as cleanText() to input text and to each
METADATA_KEYWORDS entry or store a normalized keyword list without underscores)
and keep the function name isCanvasMetadata() to perform the normalized
comparison against METADATA_KEYWORDS.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd77068a-fe0c-4251-b970-d7b677c26523
📒 Files selected for processing (18)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepository.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/di/ComputerVisionModule.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/AttributeValidator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/WidgetGrammar.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidXmlGenerator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/LayoutRenderer.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionEvent.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionUiState.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ZoomableImageView.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/TextCleaner.ktcv-image-to-xml/src/main/res/values/strings.xml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.kt (1)
19-43: Prefer narrow exception handling over broadrunCatchinghere.This currently collapses all failures into a generic
Resultpath. Catch only expected exceptions (IllegalArgumentException,IllegalStateException, I/O/security exceptions) and let unexpected programmer/runtime bugs fail fast.Based on learnings: In Kotlin files across this project, prefer narrow exception handling (for example,
IllegalArgumentException) over broad catch-all handling to preserve fail-fast behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.kt` around lines 19 - 43, The current runCatching block in the function that produces Result<ImportedDrawable> should be replaced with a try block that only catches expected exceptions (e.g., IllegalArgumentException, IllegalStateException, IOException, SecurityException) and returns a failed Result for those, while allowing other unexpected exceptions to propagate; specifically wrap the sequence that calls requireNotNull(layoutFilePath), resolveDrawableDir(...), resolveSupportedExtension(sourceUri), sanitizeResourceName(...), resolveAvailableFile(...), and the contentResolver.openInputStream(...) copy in the try, catch the listed exception types and return Result.failure(...) for them, and do not catch Throwable or Exception broadly so programmer/runtime bugs in methods like resolveDrawableDir, resolveSupportedExtension, sanitizeResourceName, resolveAvailableFile, or ImportedDrawable construction will fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.kt`:
- Around line 91-102: The current resolveAvailableFile uses candidate.exists()
which has a TOCTOU race; change resolveAvailableFile to attempt atomic
reservation by repeatedly constructing candidate (using drawableDir, baseName,
extension), calling candidate.createNewFile() and returning the File as soon as
createNewFile() returns true; if createNewFile() returns false (file already
exists) increment the index and retry; catch and handle IOExceptions
appropriately (either retry on transient errors or bubble up a clear exception)
so you don't return a filename that wasn't actually reserved.
- Around line 61-71: The resolveSupportedExtension function is too strict: if
resolveDisplayName(uri) returns null or has no extension it immediately rejects
valid images; update resolveSupportedExtension to try
ContentResolver.getType(uri) to derive a MIME-based extension (e.g., image/png,
image/jpeg, image/webp) and, if that fails, use the existing fallbackName (or
another fallback) before throwing; map MIME types to the same allowed strings
("png","jpg","jpeg","webp"), keep the final validation logic as in
resolveSupportedExtension, and reference resolveDisplayName,
resolveSupportedExtension, ContentResolver.getType, and fallbackName when making
the changes.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.kt`:
- Around line 19-43: The current runCatching block in the function that produces
Result<ImportedDrawable> should be replaced with a try block that only catches
expected exceptions (e.g., IllegalArgumentException, IllegalStateException,
IOException, SecurityException) and returns a failed Result for those, while
allowing other unexpected exceptions to propagate; specifically wrap the
sequence that calls requireNotNull(layoutFilePath), resolveDrawableDir(...),
resolveSupportedExtension(sourceUri), sanitizeResourceName(...),
resolveAvailableFile(...), and the contentResolver.openInputStream(...) copy in
the try, catch the listed exception types and return Result.failure(...) for
them, and do not catch Throwable or Exception broadly so programmer/runtime bugs
in methods like resolveDrawableDir, resolveSupportedExtension,
sanitizeResourceName, resolveAvailableFile, or ImportedDrawable construction
will fail fast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5237a720-a48e-4e8d-98d2-7a31040d6987
📒 Files selected for processing (15)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepository.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/di/ComputerVisionModule.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/WidgetGrammar.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidXmlGenerator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/LayoutRenderer.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionEvent.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionUiState.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ZoomableImageView.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.ktcv-image-to-xml/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (2)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionEvent.kt
- cv-image-to-xml/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (9)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/WidgetGrammar.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidXmlGenerator.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/di/ComputerVisionModule.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/LayoutRenderer.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionUiState.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
jatezzz
force-pushed
the
feat/ADFA-3583-image-import-placeholder-experimental
branch
from
644ca93 to
438b1ba
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
405-443:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStale
placeholderIdcan corrupt state after a detection reset — guard is still missing.
placeholderIdis snapshotted at line 407 before theimportDrawablecoroutine runs. If the user reloads an image or reruns detection while the file copy is in flight (both paths resetpendingImagePlaceholderId = nullandselectedImagesByPlaceholderId = emptyMap()), theonSuccessbranch at line 415 will re-insert the stale placeholder entry into the freshly-reset state using the capturedplaceholderId, making the next XML generation pick up a drawable that belongs to a different layout session.The proposed fix from the earlier review is still absent:
🛡️ Proposed guard inside the update lambda
).onSuccess { importedDrawable -> _uiState.update { currentState -> + if (currentState.pendingImagePlaceholderId != placeholderId) { + return@update currentState + } currentState.copy( pendingImagePlaceholderId = null, selectedImagesByPlaceholderId = currentState.selectedImagesByPlaceholderId +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt` around lines 405 - 443, The onSuccess handler uses a captured placeholderId which can be stale; modify the _uiState.update call inside handlePlaceholderImageSelected so the insertion of the new SelectedImportedImage only happens if the currentState.pendingImagePlaceholderId equals the captured placeholderId (otherwise skip adding), and always clear pendingImagePlaceholderId; i.e., inside the update lambda for _uiState.update in the importDrawable.onSuccess branch, check currentState.pendingImagePlaceholderId == placeholderId before copying selectedImagesByPlaceholderId to include (placeholderId -> SelectedImportedImage(...)).
🧹 Nitpick comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
176-185: ⚡ Quick winNarrow
catch (_: Exception)toIOException(and optionallySecurityException).
ExifInterface(InputStream)throwsIOException;contentResolver.openInputStreamcan throwFileNotFoundException(anIOExceptionsubclass) orSecurityException. The current broadcatch (_: Exception)silently swallows all runtime exceptions. Per the project guideline, only the specific types that are expected and safe to ignore should be caught here.♻️ Proposed narrowing
- } catch (_: Exception) { + } catch (_: IOException) { ExifInterface.ORIENTATION_NORMAL + } catch (_: SecurityException) { + ExifInterface.ORIENTATION_NORMAL }Based on learnings: In Kotlin files across the AndroidIDE project, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all
catch (e: Exception).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt` around lines 176 - 185, The current catch block around reading Exif orientation is too broad (catch (_: Exception)) and may swallow runtime issues; narrow it to catch IOException (and optionally SecurityException) only for the code in the try that uses contentResolver.openInputStream and ExifInterface(InputStream). Update the exception handling around the computation of orientation (the try that uses contentResolver.openInputStream(uri)?.use { ... } and ExifInterface(stream).getAttributeInt(...)) to catch IOException and SecurityException explicitly and return ExifInterface.ORIENTATION_NORMAL in those handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 405-443: The onSuccess handler uses a captured placeholderId which
can be stale; modify the _uiState.update call inside
handlePlaceholderImageSelected so the insertion of the new SelectedImportedImage
only happens if the currentState.pendingImagePlaceholderId equals the captured
placeholderId (otherwise skip adding), and always clear
pendingImagePlaceholderId; i.e., inside the update lambda for _uiState.update in
the importDrawable.onSuccess branch, check
currentState.pendingImagePlaceholderId == placeholderId before copying
selectedImagesByPlaceholderId to include (placeholderId ->
SelectedImportedImage(...)).
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 176-185: The current catch block around reading Exif orientation
is too broad (catch (_: Exception)) and may swallow runtime issues; narrow it to
catch IOException (and optionally SecurityException) only for the code in the
try that uses contentResolver.openInputStream and ExifInterface(InputStream).
Update the exception handling around the computation of orientation (the try
that uses contentResolver.openInputStream(uri)?.use { ... } and
ExifInterface(stream).getAttributeInt(...)) to catch IOException and
SecurityException explicitly and return ExifInterface.ORIENTATION_NORMAL in
those handlers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c23e4acb-51aa-4e49-8aef-a1eefcbbbfd4
📒 Files selected for processing (15)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepository.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/di/ComputerVisionModule.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/grammar/WidgetGrammar.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidXmlGenerator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/LayoutRenderer.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionEvent.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionUiState.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ZoomableImageView.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.ktcv-image-to-xml/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (5)
- cv-image-to-xml/src/main/res/values/strings.xml
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionEvent.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ZoomableImageView.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/DrawableImportHelper.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionUiState.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidXmlGenerator.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepository.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/di/ComputerVisionModule.kt
jatezzz
force-pushed
the
feat/ADFA-3583-image-import-placeholder-experimental
branch
from
438b1ba to
3d4f7e6
Compare
Adds tap listener and file picker to map selected device images to detected placeholders, copying them to the drawable folder.
jatezzz
force-pushed
the
feat/ADFA-3583-image-import-placeholder-experimental
branch
from
3d4f7e6 to
f34298e
Compare
… `WidgetFactory`
jatezzz
force-pushed
the
feat/ADFA-3583-image-import-placeholder-experimental
branch
from
f34298e to
d377866
Compare
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.
Description
Added functionality to allow users to tap on an image placeholder detected by the YOLO model, pick an image from their filesystem, and automatically import it. The image is safely copied to the corresponding
res/drawablefolder, and the generated XML layout is updated to reference the new drawable usingandroid:src.Details
DrawableImportHelperto handle resolving the resource directory, sanitizing filenames, avoiding duplicates, and copying the image stream.ZoomableImageViewto add anonImageTapListenerthat maps view touch coordinates to the original image coordinates.ComputerVisionViewModelto handle placeholder tap events, trigger the content picker, and maintain the state of selected images mapped by placeholder ID.YoloToXmlConverterandLayoutRendererto inject the new@drawable/...references into the generated Android XML tags.document_5143474614021655795.mp4
Ticket
ADFA-3583
Observation
The
DrawableImportHelpersafely handles special characters in image names and ensures a fallback name is provided. It dynamically traverses the directory tree from the provided layout file path to locate the rootres/drawablefolder for injection.