ADFA-3617 | Support horizontal widgets in XML generation#1204
Merged
Conversation
jatezzz
requested review from
a team,
Daniel-ADFA,
avestaadfa,
hal-eisen-adfa and
itsaky-adfa
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 4 seconds. ⌛ 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 (5)
📝 WalkthroughWalkthroughRefactors the YOLO→XML pipeline into smaller components: adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client as YoloToXmlConverter
participant Geo as LayoutGeometryProcessor
participant Matcher as WidgetAnnotationMatcher
participant XML as AndroidXmlGenerator
participant Detections as DetectionResults
Client->>Geo: scaleDetection(...) for each detection
Geo-->>Client: List<ScaledBox>
Client->>Geo: assignTextToParents(parents, texts, allBoxes)
Geo-->>Client: populated boxes with text
Client->>Matcher: matchAnnotationsToElements(canvasTags, uiElements, annotations)
Matcher-->>Client: Map<ScaledBox, String>
Client->>XML: buildXml(boxes, annotationsMap, targetDpHeight, wrapInScroll)
XML-->>Client: Android XML string
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/AndroidXmlGenerator.kt`:
- Around line 215-216: The current code in AndroidXmlGenerator.kt
unconditionally writes android:contentDescription using the image label and then
adds "android:contentDescription" to writtenAttrs, which discards any
contentDescription parsed earlier by FuzzyAttributeParser; change the logic in
the method that contains the xml.append("$indent
android:contentDescription=\"${escapeXmlAttr(label)}\"\n") and
writtenAttrs.add("android:contentDescription") so it only writes the label
fallback when no parsed contentDescription exists and the attribute hasn't
already been written — i.e., check the parsed attributes from
FuzzyAttributeParser (the parsed contentDescription key) and writtenAttrs first,
write parsed value if present, otherwise write the label, and only then add
"android:contentDescription" to writtenAttrs.
- Around line 65-71: The current loop always uses DEFAULT_SPACING_DP for end
margin; change the index < row.lastIndex branch in row.forEachIndexed to compute
the actual horizontal gap between the current box and the next box (e.g., gapPx
= nextBox.right - currentBox.left or next.boundingBox.left -
(box.boundingBox.left + box.boundingBox.width) depending on your box model),
convert gapPx to dp (round and clamp to >= 0), format it as "${gapDp}dp" and
place it in extraAttrs ("android:layout_marginEnd"), falling back to
DEFAULT_SPACING_DP if conversion fails or gap is negative; keep calling
appendSimpleView(xml, box, counters, " ", annotations, extraAttrs)
unchanged.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/model/ScaledBox.kt`:
- Around line 5-8: ScaledBox currently has mutable fields (var text and mutable
Rect) which break Map/Set membership when mutated; make text immutable (change
var text to val) and stop mutating Rect instances — replace the rect field with
an immutable copy (or store immutable coordinates/Rect-like data) so
equals/hashCode remain stable; update callers such as LayoutGeometryProcessor
(where parent.text = it.text is used) to create new ScaledBox instances with the
updated text instead of mutating, and ensure usages in WidgetAnnotationMatcher
and AndroidXmlGenerator continue to construct/lookup ScaledBox by the stable,
immutable fields.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetAnnotationMatcher.kt`:
- Around line 87-110: The normalizeWidgetType function must map "icon" into the
image-placeholder group so P-* parsed annotations apply to icon detections:
update normalizeWidgetType (and keep getTagType as-is) to include a branch that
returns "image_placeholder" when label.startsWith("icon") (place it near the
other image_placeholder branch so "icon" is normalized to "image_placeholder").
- Around line 126-135: The code currently prefers zero-based lookup before
one-based which causes e.g. "B-1" to match candidates[1] instead of
candidates[0]; in the matching logic around the variables ordinal, candidates
and claimedWidgets (in WidgetAnnotationMatcher), reverse the checks so you first
compute and test oneBasedMatch = candidates.getOrNull(ordinal - 1) against
claimedWidgets and return it if available, and only if that fails fall back to
zeroBasedMatch = candidates.getOrNull(ordinal); keep the ordinal != null guard
and claimedWidgets exclusion intact.
🪄 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: 84293198-706f-4d26-8972-ee71757e1ab7
📒 Files selected for processing (5)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/AndroidXmlGenerator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/LayoutGeometryProcessor.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetAnnotationMatcher.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/model/ScaledBox.kt
jatezzz
force-pushed
the
feat/ADFA-3617-horizontal-widgets-xml-experimental
branch
3 times, most recently
from
16923fc to
97116aa
Compare
Daniel-ADFA
requested changes
Apr 20, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/AndroidXmlGenerator.kt`:
- Around line 82-89: The escapeXmlAttr function currently strips pipe characters
which corrupts valid Android attribute values (e.g., android:inputType flags);
remove only the .replace("|", "") call from escapeXmlAttr so pipes are
preserved, leaving the trim() and the escaping of &, <, >, ", ' intact (ensure
the method named escapeXmlAttr still returns a String with the same chaining of
trim() and replace calls for XML characters).
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/LayoutGeometryProcessor.kt`:
- Around line 99-106: normW and normH use integer division causing truncation;
change their calculations to use floating-point division by converting the
numerator or denominator to Float (e.g., (rect.right - rect.left).toFloat() /
sourceWidth or / sourceWidth.toFloat()) so normW and normH are Float values
consistent with normCx/normCy; update the two lines that define normW and normH
in LayoutGeometryProcessor.kt accordingly and keep subsequent math (multiplying
by targetW/targetH and roundToInt()) unchanged.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetAnnotationMatcher.kt`:
- Line 8: The prefix group in TAG_EXTRACT_REGEX inside WidgetAnnotationMatcher
doesn't capture the two-character prefix "S8", so tags like "S8-1" aren't
normalized to "SW-1"; update TAG_EXTRACT_REGEX to explicitly allow "S8" as an
alternative in the prefix group (for example use
(?i)((?:S8|[BPDTCRS]\s*W?))[^a-zA-Z0-9]*([\d lIoO!]+)$ equivalent replacement)
so the normalization logic used later (around the S8 -> SW recovery in the
normalization code) will see "S8" as the prefix and normalize it to "SW" as
intended.
🪄 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: c8398a54-5d2e-4922-84bd-d5b6c22ca741
📒 Files selected for processing (5)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/AndroidXmlGenerator.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/LayoutGeometryProcessor.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetAnnotationMatcher.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/model/ScaledBox.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/model/ScaledBox.kt
Refactor XML generation into specific domain components to group and render widgets in horizontal rows.
- Model: Make `ScaledBox` properties immutable (`val text`) to prevent silently breaking `hashCode` lookups in Maps and Sets. - Geometry: Refactor `assignTextToParents` to safely update widget text using `.copy()` instead of mutating instances directly. - Generator: Calculate `android:layout_marginEnd` dynamically from bounding box geometry to preserve actual horizontal gaps, replacing the hardcoded 16dp margin. - Generator: Honor parsed `android:contentDescription` for ImageViews so user-provided annotations are not overwritten by default labels. - Matcher: Prioritize 1-based ordinal matching over 0-based fallback so tags like "B-1" correctly map to the first element in the list. - Matcher: Include "icon" in "image_placeholder" widget normalization.
- Rename `appendTextViewAttributes` to `appendTextWidgetAttributes` to accurately reflect that it handles multiple text-based widgets (TextView, Button, CheckBox, RadioButton, Switch). - Simplify the default text fallback logic by introducing a `WIDGET_TAGS` constant set, replacing the verbose `when` block.
…lc `normCx` and `normCy` values and improve `TAG_EXTRACT_REGEX` regex to detect tags like `S8-1`
jatezzz
force-pushed
the
feat/ADFA-3617-horizontal-widgets-xml-experimental
branch
from
609430b to
bd4eecd
Compare
Daniel-ADFA
approved these changes
Apr 20, 2026
This was referenced Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR introduces the ability to generate horizontal alignments in the Android XML output.
YoloToXmlConverterinto smaller, specialized domain components (AndroidXmlGenerator,LayoutGeometryProcessor, andWidgetAnnotationMatcher). AddedgroupIntoRowslogic to geometry processing which groups overlapping or adjacent boxes, and updated the XML generation to output a horizontalLinearLayoutwhen multiple items fall into the same row.Details
LayoutGeometryProcessorto handle row grouping thresholds and overlaps.AndroidXmlGeneratorwith anappendHorizontalRowfunction to map horizontal widgets with correct margins.ScaledBoxinto its own domain model file.document_5123232377321032509.mp4
Ticket
ADFA-3617
Observation
The refactor heavily decouples the geometry parsing, annotation matching, and XML string building into isolated, testable classes, making future layout enhancements much easier to implement.