Skip to content

ADFA-3912 | Fix fuzzy attribute parsing bounds and UI element tag filtering#1275

Merged
jatezzz merged 2 commits into
stagefrom
fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental
May 7, 2026
Merged

ADFA-3912 | Fix fuzzy attribute parsing bounds and UI element tag filtering#1275
jatezzz merged 2 commits into
stagefrom
fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental

Conversation

@jatezzz
Copy link
Copy Markdown
Collaborator

@jatezzz jatezzz commented May 6, 2026

Description

Fixed an out-of-bounds string extraction error in FuzzyAttributeParser by adding a safeguard check (current.valueStart > valueEnd) before processing annotations. Additionally, updated the UI element filtering logic in YoloToXmlConverter so that the isTag exclusion only applies to elements explicitly labeled as "text". This ensures valid UI elements are not improperly excluded during conversion.

Details

document_5179472272527722523.mp4

Ticket

ADFA-3912

Observation

⚠️ Must be merged after #1274

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cac4df6e-4691-4ce3-bb6f-9171e6854e98

📥 Commits

Reviewing files that changed from the base of the PR and between efe479f and a8c2576.

📒 Files selected for processing (1)
  • cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt

📝 Walkthrough

Release Notes - ADFA-3912 | Fix fuzzy attribute parsing bounds and UI element tag filtering

  • FuzzyAttributeParser bounds check

    • Added defensive guard in parseByColonScanning to skip processing when computed valueEnd precedes valueStart (prevents StringIndexOutOfBounds during substring extraction).
  • YoloToXmlConverter refactor & behavior fix

    • Refactored XML generation into private helpers: extractUiCandidates, scaleDetections, associateTextToWidgets, finalizeUiElements, extractCanvasTags.
    • Changed tag-filtering so isTag exclusion is applied only to elements labeled "text" (prevents valid non-text UI elements from being excluded).
  • PlaceholderOverrides utility

    • Added List.buildPlaceholderOverrides to produce a mapping from sorted placeholders to drawable references (uses indexed keys like "ph_0", "ph_1", ...).
  • Files & size impact

    • FuzzyAttributeParser.kt: small guard (+2 lines)
    • YoloToXmlConverter.kt: refactor (+~73/-37 lines)
    • PlaceholderUtils.kt: new extension (+14 lines)
  • Blocking dependency

    • Must be merged after PR #1274 into branch stage.

Risks & considerations

  • Refactor surface area: The YoloToXmlConverter internal flow was restructured — exercise comprehensive tests for UI candidate extraction, text-to-widget association, canvas tag extraction, placeholder overrides, and rendering order.
  • Behavioral change: Tag filtering now only affects text-labeled elements; validate that non-text widgets previously excluded are now retained and that true tag texts are still filtered.
  • Silent skipping: FuzzyAttributeParser now skips malformed annotations rather than throwing; monitor logs/metrics for missed attribute parsing.
  • New public extension: Confirm buildPlaceholderOverrides does not conflict with existing APIs and document its usage.

Walkthrough

The PR adds a substring-range guard in FuzzyAttributeParser, introduces List.buildPlaceholderOverrides, and refactors YoloToXmlConverter.generateXmlLayout into ordered private helpers for candidate extraction, scaling, text association, finalization, and canvas/tag extraction.

Changes

Attribute Parsing Validation Guard

Layer / File(s) Summary
Input Validation
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/FuzzyAttributeParser.kt
Guard added in parseByColonScanning loop to skip processing when valueStart > valueEnd, preventing invalid substring range extraction attempts.

Placeholder override utility

Layer / File(s) Summary
New Utility Function
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/PlaceholderUtils.kt
Extension function buildPlaceholderOverrides added to sort placeholders and map each indexed placeholder key ph_<index> to a drawable reference from the provided map, returning Map<ScaledBox, String>.

XML Generation Pipeline Refactoring

Layer / File(s) Summary
Core Pipeline Refactoring
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/YoloToXmlConverter.kt
generateXmlLayout refactored to delegate to private helpers: extractUiCandidates, scaleDetections, associateTextToWidgets, finalizeUiElements, and extractCanvasTags. Removed buildSelectedImageOverrides; now uses uiElements.buildPlaceholderOverrides(selectedImagesByPlaceholderId).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • avestaadfa
  • Daniel-ADFA

Poem

🐰 I hop through code with careful paws,

A guard to stop the substring claws,
Placeholders mapped in tidy rows,
XML flows where the pipeline goes,
Hooray — small fixes, tidy prose!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the two main changes: fixing fuzzy attribute parsing bounds and updating UI element tag filtering logic.
Description check ✅ Passed The description directly relates to the changeset, explaining the safeguard check added to FuzzyAttributeParser and the UI element filtering logic updates in YoloToXmlConverter with clear context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jatezzz jatezzz requested review from a team, Daniel-ADFA and avestaadfa May 6, 2026 17:50
@jatezzz jatezzz force-pushed the fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental branch from 48bb962 to 35ac06e Compare May 6, 2026 19:56
Added bounds validation in FuzzyAttributeParser and updated UI element filter logic.
@jatezzz jatezzz force-pushed the fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental branch from 35ac06e to efe479f Compare May 6, 2026 21:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/YoloToXmlConverter.kt`:
- Around line 87-95: In associateTextToWidgets change the parent selection so
you do not drop non-text boxes based on annotationMatcher.isTag: keep parents as
all boxes where it.label != "text" (remove the isTag check), while still
filtering initialTexts with annotationMatcher.isTag so only text boxes get
tag-filtered; then call geometryProcessor.assignTextToParents(parents,
initialTexts, scaledBoxes) and proceed with the existing remainingTexts and
geometryProcessor.assignNearbyTextToWidgets call unchanged (references:
associateTextToWidgets, annotationMatcher.isTag,
geometryProcessor.assignTextToParents,
geometryProcessor.assignNearbyTextToWidgets).
🪄 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: b21178be-4d63-4592-b3a7-27fcbba14f63

📥 Commits

Reviewing files that changed from the base of the PR and between 69e03bf and efe479f.

📒 Files selected for processing (3)
  • cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/FuzzyAttributeParser.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/utils/PlaceholderUtils.kt

@jatezzz jatezzz merged commit 27af4e9 into stage May 7, 2026
2 checks passed
@jatezzz jatezzz deleted the fix/ADFA-3912-parsing-bounds-and-tag-filter-experimental branch May 7, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants