ADFA-3910 | Fix single placeholder dropdown array generation in strings file#1274
Open
ADFA-3910 | Fix single placeholder dropdown array generation in strings file#1274
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughRelease NotesFeatures & Improvements
Technical Changes
Risks & Best Practice Concerns
Migration Notes
WalkthroughThis PR centralizes widget tag parsing into a new ChangesTag Parsing Centralization
OCR-Aware Entry Extraction & Widget Enhancement
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 docstrings
🧪 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: 1
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/OcrExtensions.kt (1)
12-29: 💤 Low valueSolid OCR-aware splitting; one optional simplification.
The branching (punctuation-first, then digit-only whitespace fallback) sensibly avoids over-splitting non-numeric phrases while still handling space-separated numeric OCR like
"1 2 3". The empty-input case (size != 1) gracefully short-circuits to an empty list.Minor nit: the
this.trim()on line 22 is redundant given thatWHITESPACE_DELIMITERS = "\\s+"plusfilter { it.isNotBlank() }already discards leading/trailing whitespace tokens. Removing it would reduce one allocation per call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/OcrExtensions.kt` around lines 12 - 29, In extractOcrEntries(), remove the redundant this.trim() call when creating whitespaceTokens since WHITESPACE_DELIMITERS ("\\s+") plus filter { it.isNotBlank() } already handles leading/trailing spaces; update the whitespaceTokens assignment in the function (referencing WHITESPACE_DELIMITERS and whitespaceTokens) to split directly on WHITESPACE_DELIMITERS and filter blanks, leaving the rest of the function logic (punctuatedTokens, isSpaceSeparatedOcrNumbers, and the return) unchanged.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidWidget.kt (1)
123-178: 💤 Low valueSingle-placeholder bypass implemented correctly; consider unifying placeholder knowledge.
The core fix —
isSinglePlaceholderEntry()gating string-array creation inprocessAttributes(line 158) — is precise and matches the PR objective. Nicely scoped: ID derivation/resolution still proceeds normally, only thestring-arraywrite is suppressed.Optional consistency nit:
isMeaningfulDropdownText(line 202) only excludes"dropdown", whileplaceholderEntriescoversyear/month/day/select/choose/dropdown. As a result, abox.textof"select"will still passisMeaningfulDropdownTextand feed intorawEntries, only to be filtered out later byisSinglePlaceholderEntry. Routing both checks through the same set would make the intent uniform and avoid future drift if the placeholder list grows.♻️ Suggested alignment
private fun String.isMeaningfulDropdownText(): Boolean { - val cleaned = normalizedDropdownLabel() - return cleaned.isNotBlank() && !cleaned.equals("dropdown", ignoreCase = true) + val cleaned = normalizedDropdownLabel() + return cleaned.isNotBlank() && cleaned.lowercase() !in placeholderEntries }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidWidget.kt` around lines 123 - 178, The placeholder list is duplicated: companion object placeholderEntries is used by isSinglePlaceholderEntry but isMeaningfulDropdownText uses a different hardcoded check; unify them by making isMeaningfulDropdownText consult the same placeholderEntries set (or move placeholderEntries to a shared util/constant) and ensure both use the same normalization/lowercasing (call normalizedDropdownLabel().lowercase() or equivalent) so processAttributes, isSinglePlaceholderEntry, and isMeaningfulDropdownText all reference the same canonical placeholder set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetTagParser.kt`:
- Around line 60-83: normalizeTagToken calls normalizeOcrDigits with an
UPPERCASED token but normalizeOcrDigits currently targets lowercase letters and
only maps O/I/! → digits, causing inconsistency with isNumericLikeOcrChar and
EntriesValidator.cleanNumberArtifacts; fix by making normalizeOcrDigits operate
on the uppercase input and include the same OCR-to-digit mappings (e.g., map 'I'
and '!' and 'L' → '1', 'O' → '0', and add 'Z' → '2', 'S' → '5', 'B' → '6') so
normalizeOcrDigits, isNumericLikeOcrChar, and extractOrdinal behavior are
consistent (update the normalizeOcrDigits implementation referenced by
normalizeTagToken and keep isNumericLikeOcrChar as-is).
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/xml/AndroidWidget.kt`:
- Around line 123-178: The placeholder list is duplicated: companion object
placeholderEntries is used by isSinglePlaceholderEntry but
isMeaningfulDropdownText uses a different hardcoded check; unify them by making
isMeaningfulDropdownText consult the same placeholderEntries set (or move
placeholderEntries to a shared util/constant) and ensure both use the same
normalization/lowercasing (call normalizedDropdownLabel().lowercase() or
equivalent) so processAttributes, isSinglePlaceholderEntry, and
isMeaningfulDropdownText all reference the same canonical placeholder set.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/OcrExtensions.kt`:
- Around line 12-29: In extractOcrEntries(), remove the redundant this.trim()
call when creating whitespaceTokens since WHITESPACE_DELIMITERS ("\\s+") plus
filter { it.isNotBlank() } already handles leading/trailing spaces; update the
whitespaceTokens assignment in the function (referencing WHITESPACE_DELIMITERS
and whitespaceTokens) to split directly on WHITESPACE_DELIMITERS and filter
blanks, leaving the rest of the function logic (punctuatedTokens,
isSpaceSeparatedOcrNumbers, and the return) unchanged.
🪄 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: 3c853016-4505-4a8b-827a-8ca2c2356296
📒 Files selected for processing (6)
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/WidgetAnnotationMatcher.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/WidgetTagParser.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/xml/AndroidWidget.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/OcrExtensions.kt
Extract WidgetTagParser and ignore isolated placeholder entries like `select` or `dropdown` from strings.xml
jatezzz
force-pushed
the
fix/ADFA-3910-dropdown-placeholder-string-xml-experimental
branch
from
9f74584 to
fda1b16
Compare
Daniel-ADFA
reviewed
May 6, 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
Fixed an issue where single placeholder labels (e.g., "dropdown", "select", "choose") sketched inside a spinner/dropdown widget incorrectly generated a
string-arrayinstrings.xml. Additionally, OCR tag parsing logic has been refactored and centralized into a newWidgetTagParserobject.Details
isSinglePlaceholderEntry()check inSpinnerWidgetto bypass string array creation when the only item is a known placeholder.MarginAnnotationParserandWidgetAnnotationMatcherintoWidgetTagParser.AttributeValidator.document_5174968672900351910.mp4
Ticket
ADFA-3910
Observation
Centralizing the tag parsing logic into
WidgetTagParsersignificantly cleans up the margin and annotation matching domains, ensuring that OCR text normalization (e.g., replacing 'l' with '1', 'O' with '0') is applied consistently across the entire pipeline.