ADFA-3612 | Refactor generic box resolution to domain layer#1177
Merged
jatezzz merged 1 commit intoApr 15, 2026
Conversation
Contributor
📝 Walkthrough
Risks & Best-practice issues
WalkthroughLabels were updated (removed Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ComputerVisionRepositoryImpl
participant YoloModelSource
participant GenericBoxResolver
Caller->>ComputerVisionRepositoryImpl: runYoloInference(bitmap)
ComputerVisionRepositoryImpl->>YoloModelSource: runInference(bitmap)
YoloModelSource-->>ComputerVisionRepositoryImpl: rawDetections
ComputerVisionRepositoryImpl->>GenericBoxResolver: resolve(rawDetections)
Note over GenericBoxResolver: filter out `dropdown_symbol`\nrelabel `generic_box` by proximity to symbols
GenericBoxResolver-->>ComputerVisionRepositoryImpl: processedDetections
ComputerVisionRepositoryImpl-->>Caller: Result(processedDetections)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 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.
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.kt (1)
30-31: Inject and reuseGenericBoxResolverinstead of allocating per inference.Creating
GenericBoxResolver()on every call is unnecessary for a stateless resolver. Reusing an injected instance improves testability and avoids repeated allocations.♻️ Proposed refactor
class ComputerVisionRepositoryImpl( private val assetManager: AssetManager, private val yoloModelSource: YoloModelSource, - private val regionOcrProcessor: RegionOcrProcessor + private val regionOcrProcessor: RegionOcrProcessor, + private val genericBoxResolver: GenericBoxResolver = GenericBoxResolver() ) : ComputerVisionRepository { @@ runCatching { val rawDetections = yoloModelSource.runInference(bitmap) - GenericBoxResolver().resolve(rawDetections) + genericBoxResolver.resolve(rawDetections) } }🤖 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/ComputerVisionRepositoryImpl.kt` around lines 30 - 31, The code instantiates GenericBoxResolver per inference (GenericBoxResolver().resolve(rawDetections)) which is unnecessary; modify ComputerVisionRepositoryImpl to accept a GenericBoxResolver (or an interface) via constructor or DI, store it as a private field, and call that instance's resolve method instead of newing one each time—update the class constructor/signature and all call sites that construct ComputerVisionRepositoryImpl to pass the resolver so tests can inject a mock.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
17-28: Replace magic literals with named constants and remove the unused default threshold.Using hardcoded label strings and
0.8finline makes tuning/error-proofing harder, andthresholdFactor: Float = 1.5fis currently misleading since the only call passes0.8f.🧹 Proposed cleanup
class GenericBoxResolver { + private companion object { + const val LABEL_DROPDOWN_SYMBOL = "dropdown_symbol" + const val LABEL_GENERIC_BOX = "generic_box" + const val LABEL_DROPDOWN = "dropdown" + const val LABEL_TEXT_ENTRY_BOX = "text_entry_box" + const val DROPDOWN_PROXIMITY_THRESHOLD_FACTOR = 0.8f + } fun resolve(detections: List<DetectionResult>): List<DetectionResult> { - val dropdownSymbols = detections.filter { it.label == "dropdown_symbol" } + val dropdownSymbols = detections.filter { it.label == LABEL_DROPDOWN_SYMBOL } return detections.mapNotNull { det -> when (det.label) { - "dropdown_symbol" -> null - "generic_box" -> { + LABEL_DROPDOWN_SYMBOL -> null + LABEL_GENERIC_BOX -> { val hasSymbolNearby = dropdownSymbols.any { symbol -> - isNearby(det.boundingBox, symbol.boundingBox, 0.8f) + isNearby(det.boundingBox, symbol.boundingBox, DROPDOWN_PROXIMITY_THRESHOLD_FACTOR) } - det.copy(label = if (hasSymbolNearby) "dropdown" else "text_entry_box") + det.copy(label = if (hasSymbolNearby) LABEL_DROPDOWN else LABEL_TEXT_ENTRY_BOX) } else -> det } } } - private fun isNearby(box1: RectF, box2: RectF, thresholdFactor: Float = 1.5f): Boolean { + private fun isNearby(box1: RectF, box2: RectF, thresholdFactor: Float): Boolean {🤖 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/GenericBoxResolver.kt` around lines 17 - 28, Replace magic literals in GenericBoxResolver: extract the inline label strings "dropdown" and "text_entry_box" into descriptive constants (e.g., LABEL_DROPDOWN, LABEL_TEXT_ENTRY_BOX) and replace the inline numeric literal 0.8f used when calling isNearby with a named constant (e.g., NEARBY_THRESHOLD = 0.8f). Also fix the isNearby signature by removing or changing the misleading default thresholdFactor (currently 1.5f) to use the same named NEARBY_THRESHOLD as the default or remove the default entirely so callers must pass the explicit constant; update all references to use these constants to eliminate the magic numbers/strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.kt`:
- Around line 30-31: The code instantiates GenericBoxResolver per inference
(GenericBoxResolver().resolve(rawDetections)) which is unnecessary; modify
ComputerVisionRepositoryImpl to accept a GenericBoxResolver (or an interface)
via constructor or DI, store it as a private field, and call that instance's
resolve method instead of newing one each time—update the class
constructor/signature and all call sites that construct
ComputerVisionRepositoryImpl to pass the resolver so tests can inject a mock.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt`:
- Around line 17-28: Replace magic literals in GenericBoxResolver: extract the
inline label strings "dropdown" and "text_entry_box" into descriptive constants
(e.g., LABEL_DROPDOWN, LABEL_TEXT_ENTRY_BOX) and replace the inline numeric
literal 0.8f used when calling isNearby with a named constant (e.g.,
NEARBY_THRESHOLD = 0.8f). Also fix the isNearby signature by removing or
changing the misleading default thresholdFactor (currently 1.5f) to use the same
named NEARBY_THRESHOLD as the default or remove the default entirely so callers
must pass the explicit constant; update all references to use these constants to
eliminate the magic numbers/strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7b27067-dade-4365-bd98-00b2063aed32
📒 Files selected for processing (4)
cv-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/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt
jatezzz
force-pushed
the
refactor/ADFA-3612-generic-box-resolver-experimental
branch
from
672a62a to
a76b50f
Compare
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/data/repository/ComputerVisionRepositoryImpl.kt (1)
15-19: InjectGenericBoxResolverinstead of creating it per callInstantiating the resolver inside
runYoloInferencecouples construction to execution and makes testing harder. Prefer constructor injection so the repository depends on an abstraction-ready instance.♻️ Proposed refactor
class ComputerVisionRepositoryImpl( private val assetManager: AssetManager, private val yoloModelSource: YoloModelSource, - private val regionOcrProcessor: RegionOcrProcessor + private val regionOcrProcessor: RegionOcrProcessor, + private val genericBoxResolver: GenericBoxResolver = GenericBoxResolver() ) : ComputerVisionRepository { @@ runCatching { val rawDetections = yoloModelSource.runInference(bitmap) - GenericBoxResolver().resolve(rawDetections) + genericBoxResolver.resolve(rawDetections) } }Also applies to: 30-31
🤖 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/ComputerVisionRepositoryImpl.kt` around lines 15 - 19, The repository currently constructs a GenericBoxResolver inside runYoloInference, which couples creation to execution; modify ComputerVisionRepositoryImpl to accept a GenericBoxResolver via constructor injection (add a constructor parameter named e.g. genericBoxResolver: GenericBoxResolver) and replace the internal new-instantiation in runYoloInference with using this injected genericBoxResolver instance; update all places in runYoloInference (and the occurrences around lines 30-31 where GenericBoxResolver is created/used) to use the injected symbol so tests can mock the resolver and construction no longer happens per call.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
10-21: Promote YOLO labels to constants to avoid string driftThe resolver relies on multiple hardcoded labels (
"generic_box","dropdown_symbol","dropdown","text_entry_box"). Centralizing them reduces typo/regression risk when label sets evolve.♻️ Proposed refactor
class GenericBoxResolver { + private companion object { + const val LABEL_GENERIC_BOX = "generic_box" + const val LABEL_DROPDOWN_SYMBOL = "dropdown_symbol" + const val LABEL_DROPDOWN = "dropdown" + const val LABEL_TEXT_ENTRY_BOX = "text_entry_box" + } @@ - val dropdownSymbols = detections.filter { it.label == "dropdown_symbol" } + val dropdownSymbols = detections.filter { it.label == LABEL_DROPDOWN_SYMBOL } @@ when (det.label) { - "dropdown_symbol" -> null - "generic_box" -> { + LABEL_DROPDOWN_SYMBOL -> null + LABEL_GENERIC_BOX -> { @@ - det.copy(label = if (hasSymbolNearby) "dropdown" else "text_entry_box") + det.copy(label = if (hasSymbolNearby) LABEL_DROPDOWN else LABEL_TEXT_ENTRY_BOX) }🤖 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/GenericBoxResolver.kt` around lines 10 - 21, Replace hardcoded YOLO label strings in GenericBoxResolver.kt with centralized constants: define constants (e.g., LABEL_GENERIC_BOX, LABEL_DROPDOWN_SYMBOL, LABEL_DROPDOWN, LABEL_TEXT_ENTRY_BOX) in the resolver class (companion object) or a shared LabelConstants object, then use those constants in the dropdownSymbols filter, the when branches inside detections.mapNotNull, and when setting det.copy(label = ...). Update references to dropdownSymbols, isNearby checks, and the label assignment to use the new constants so all usages (filter { it.label == ... }, when (det.label), and det.copy(label = ...)) rely on 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/GenericBoxResolver.kt`:
- Around line 26-35: The isNearby function can produce a zero distanceThreshold
for degenerate (zero-area) boxes causing valid overlaps to be missed; change the
threshold computation in isNearby to derive a non-zero baseline using both boxes
(e.g., compute avgDim1 and avgDim2 and set baseline = max(avgDim1, avgDim2,
smallEpsilon) or use max of widths/heights) and then compute distanceThreshold =
thresholdFactor * baseline; keep the rest of the distance calculation the same
so centers that coincide are correctly treated as nearby.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.kt`:
- Around line 15-19: The repository currently constructs a GenericBoxResolver
inside runYoloInference, which couples creation to execution; modify
ComputerVisionRepositoryImpl to accept a GenericBoxResolver via constructor
injection (add a constructor parameter named e.g. genericBoxResolver:
GenericBoxResolver) and replace the internal new-instantiation in
runYoloInference with using this injected genericBoxResolver instance; update
all places in runYoloInference (and the occurrences around lines 30-31 where
GenericBoxResolver is created/used) to use the injected symbol so tests can mock
the resolver and construction no longer happens per call.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt`:
- Around line 10-21: Replace hardcoded YOLO label strings in
GenericBoxResolver.kt with centralized constants: define constants (e.g.,
LABEL_GENERIC_BOX, LABEL_DROPDOWN_SYMBOL, LABEL_DROPDOWN, LABEL_TEXT_ENTRY_BOX)
in the resolver class (companion object) or a shared LabelConstants object, then
use those constants in the dropdownSymbols filter, the when branches inside
detections.mapNotNull, and when setting det.copy(label = ...). Update references
to dropdownSymbols, isNearby checks, and the label assignment to use the new
constants so all usages (filter { it.label == ... }, when (det.label), and
det.copy(label = ...)) rely on the single source of truth.
🪄 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: e1ebd3bc-f370-4d29-8169-37066388b80d
📒 Files selected for processing (4)
cv-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/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- cv-image-to-xml/src/main/assets/labels.txt
jatezzz
requested review from
a team,
Daniel-ADFA,
avestaadfa,
dara-abijo-adfa,
hal-eisen-adfa and
jomen-adfa
jatezzz
force-pushed
the
refactor/ADFA-3612-generic-box-resolver-experimental
branch
from
a76b50f to
1bc3f2e
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
26-35:⚠️ Potential issue | 🟠 MajorUse a symmetric, non-zero proximity baseline in
isNearby.At Line [27]-Line [29], the threshold is derived only from
box1; tiny/degenerategeneric_boxvalues can make the threshold effectively zero and miss valid nearby symbols, causing wrong relabeling.Proposed fix
import android.graphics.RectF import org.appdevforall.codeonthego.computervision.domain.model.DetectionResult import kotlin.math.hypot +import kotlin.math.max @@ private fun isNearby(box1: RectF, box2: RectF, thresholdFactor: Float = 1.5f): Boolean { val avgDim1 = (box1.width() + box1.height()) / 2f - val distanceThreshold = thresholdFactor * avgDim1 + val avgDim2 = (box2.width() + box2.height()) / 2f + val baseline = max(1f, max(avgDim1, avgDim2)) + val distanceThreshold = thresholdFactor * baseline @@ - return distance < distanceThreshold + return distance <= distanceThreshold }🤖 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/GenericBoxResolver.kt` around lines 26 - 35, The proximity threshold in isNearby currently uses only box1's size (avgDim1) which can be near-zero for tiny/degenerate boxes and cause false negatives; update the distanceThreshold calculation in isNearby to use a symmetric, non-zero baseline such as the average of both boxes' dimensions (avgDim1 and avgDim2) and clamp with a small minimum (e.g., MIN_BASELINE) before multiplying by thresholdFactor so the threshold is never effectively zero; adjust references to distanceThreshold computation in isNearby and keep thresholdFactor behavior unchanged.
🧹 Nitpick comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
10-21: Replace repeated label literals with shared constants.Line [10], Line [14]-Line [15], and Line [19] repeat raw string labels. Centralizing these avoids silent drift with model labels and makes remapping safer to maintain.
Refactor sketch
class GenericBoxResolver { + private object Labels { + const val GENERIC_BOX = "generic_box" + const val DROPDOWN_SYMBOL = "dropdown_symbol" + const val DROPDOWN = "dropdown" + const val TEXT_ENTRY_BOX = "text_entry_box" + } @@ - val dropdownSymbols = detections.filter { it.label == "dropdown_symbol" } + val dropdownSymbols = detections.filter { it.label == Labels.DROPDOWN_SYMBOL } @@ - "dropdown_symbol" -> null - "generic_box" -> { + Labels.DROPDOWN_SYMBOL -> null + Labels.GENERIC_BOX -> { @@ - det.copy(label = if (hasSymbolNearby) "dropdown" else "text_entry_box") + det.copy(label = if (hasSymbolNearby) Labels.DROPDOWN else Labels.TEXT_ENTRY_BOX) }🤖 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/GenericBoxResolver.kt` around lines 10 - 21, Replace the repeated raw label strings by defining shared constants (e.g., DROPDOWN_SYMBOL, GENERIC_BOX, DROPDOWN, TEXT_ENTRY_BOX) and use them wherever labels are compared or assigned; update the detections.filter { it.label == "dropdown_symbol" } (dropdownSymbols), the when branches in detections.mapNotNull (matching "generic_box" and the default mapping to "dropdown" or "text_entry_box"), and any assignments like det.copy(label = ...) to reference those constants (you can add them as companion object constants on GenericBoxResolver or as top-level constants) so label checks and remapping use 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.
Duplicate comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt`:
- Around line 26-35: The proximity threshold in isNearby currently uses only
box1's size (avgDim1) which can be near-zero for tiny/degenerate boxes and cause
false negatives; update the distanceThreshold calculation in isNearby to use a
symmetric, non-zero baseline such as the average of both boxes' dimensions
(avgDim1 and avgDim2) and clamp with a small minimum (e.g., MIN_BASELINE) before
multiplying by thresholdFactor so the threshold is never effectively zero;
adjust references to distanceThreshold computation in isNearby and keep
thresholdFactor behavior unchanged.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt`:
- Around line 10-21: Replace the repeated raw label strings by defining shared
constants (e.g., DROPDOWN_SYMBOL, GENERIC_BOX, DROPDOWN, TEXT_ENTRY_BOX) and use
them wherever labels are compared or assigned; update the detections.filter {
it.label == "dropdown_symbol" } (dropdownSymbols), the when branches in
detections.mapNotNull (matching "generic_box" and the default mapping to
"dropdown" or "text_entry_box"), and any assignments like det.copy(label = ...)
to reference those constants (you can add them as companion object constants on
GenericBoxResolver or as top-level constants) so label checks and remapping use
the single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c045087c-d6e9-4698-b877-db0f245b45f8
📒 Files selected for processing (4)
cv-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/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt
✅ Files skipped from review due to trivial changes (2)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.kt
- cv-image-to-xml/src/main/assets/labels.txt
Daniel-ADFA
approved these changes
Apr 14, 2026
Introduce GenericBoxResolver to process generic_box detections and update YOLO labels.
jatezzz
force-pushed
the
refactor/ADFA-3612-generic-box-resolver-experimental
branch
from
1bc3f2e to
0795bb3
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
26-35:⚠️ Potential issue | 🟡 MinorHandle degenerate and boundary-distance proximity cases.
On Line 28,
distanceThresholdcan become0ffor zero-area boxes, which makes valid overlaps fail. On Line 35, strict<also drops exact-threshold matches.💡 Proposed fix
import android.graphics.RectF import org.appdevforall.codeonthego.computervision.domain.model.DetectionResult import kotlin.math.hypot +import kotlin.math.max @@ private fun isNearby(box1: RectF, box2: RectF, thresholdFactor: Float = 1.5f): Boolean { val avgDim1 = (box1.width() + box1.height()) / 2f - val distanceThreshold = thresholdFactor * avgDim1 + val avgDim2 = (box2.width() + box2.height()) / 2f + val baseline = max(1f, max(avgDim1, avgDim2)) + val distanceThreshold = thresholdFactor * baseline @@ - return distance < distanceThreshold + return distance <= distanceThreshold }🤖 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/GenericBoxResolver.kt` around lines 26 - 35, The isNearby function can return false for zero-area boxes because distanceThreshold can be 0 and it also rejects exact-threshold matches; update isNearby to guard against degenerate boxes by ensuring distanceThreshold is never zero (e.g., distanceThreshold = max(thresholdFactor * avgDim1, epsilon) or compute avg from non-zero dimensions) and change the final comparison from strict < to <= so exact-threshold distances count as nearby; also handle the special case where both boxes have zero area by treating them as nearby if their centers coincide (distance == 0).
🧹 Nitpick comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt (1)
10-21: Centralize label literals to reduce drift risk.String labels are repeated in multiple branches. Extract constants (or an enum/sealed model) so future label-set changes don’t silently desync this resolver.
♻️ Proposed refactor
class GenericBoxResolver { + companion object { + private const val LABEL_GENERIC_BOX = "generic_box" + private const val LABEL_DROPDOWN_SYMBOL = "dropdown_symbol" + private const val LABEL_DROPDOWN = "dropdown" + private const val LABEL_TEXT_ENTRY_BOX = "text_entry_box" + } fun resolve(detections: List<DetectionResult>): List<DetectionResult> { - val dropdownSymbols = detections.filter { it.label == "dropdown_symbol" } + val dropdownSymbols = detections.filter { it.label == LABEL_DROPDOWN_SYMBOL } @@ - "dropdown_symbol" -> null - "generic_box" -> { + LABEL_DROPDOWN_SYMBOL -> null + LABEL_GENERIC_BOX -> { @@ - det.copy(label = if (hasSymbolNearby) "dropdown" else "text_entry_box") + det.copy(label = if (hasSymbolNearby) LABEL_DROPDOWN else LABEL_TEXT_ENTRY_BOX) }🤖 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/GenericBoxResolver.kt` around lines 10 - 21, The resolver hardcodes label strings ("dropdown_symbol", "generic_box", "dropdown", "text_entry_box") causing drift; introduce a centralized label representation (e.g., an enum or constants) and replace all literal usages in this file: use the new Label enum/constants instead of string literals in dropdownSymbols = detections.filter { it.label == ... }, the when branches checking det.label, and when assigning det.copy(label = ...); ensure any helper call like isNearby still accepts the same boundingBox types but compares against the centralized labels so future label changes only update the enum/constants (recommend adding the enum/constants near the GenericBoxResolver class or in a shared model used by detection objects).
🤖 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/GenericBoxResolver.kt`:
- Around line 26-35: The isNearby function can return false for zero-area boxes
because distanceThreshold can be 0 and it also rejects exact-threshold matches;
update isNearby to guard against degenerate boxes by ensuring distanceThreshold
is never zero (e.g., distanceThreshold = max(thresholdFactor * avgDim1, epsilon)
or compute avg from non-zero dimensions) and change the final comparison from
strict < to <= so exact-threshold distances count as nearby; also handle the
special case where both boxes have zero area by treating them as nearby if their
centers coincide (distance == 0).
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt`:
- Around line 10-21: The resolver hardcodes label strings ("dropdown_symbol",
"generic_box", "dropdown", "text_entry_box") causing drift; introduce a
centralized label representation (e.g., an enum or constants) and replace all
literal usages in this file: use the new Label enum/constants instead of string
literals in dropdownSymbols = detections.filter { it.label == ... }, the when
branches checking det.label, and when assigning det.copy(label = ...); ensure
any helper call like isNearby still accepts the same boundingBox types but
compares against the centralized labels so future label changes only update the
enum/constants (recommend adding the enum/constants near the GenericBoxResolver
class or in a shared model used by detection objects).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57ff69d8-cdc6-4257-a82e-2a2f221bf64f
📒 Files selected for processing (4)
cv-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/ComputerVisionRepositoryImpl.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/domain/GenericBoxResolver.kt
✅ Files skipped from review due to trivial changes (1)
- cv-image-to-xml/src/main/assets/labels.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/data/repository/ComputerVisionRepositoryImpl.kt
jatezzz
deleted the
refactor/ADFA-3612-generic-box-resolver-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
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
Extracted the logic that determines whether a
generic_boxis a dropdown or a text entry box into a new dedicated domain class (GenericBoxResolver), and updated the YOLO model labels.Created the
GenericBoxResolverclass to handle proximity checks betweengeneric_boxanddropdown_symbolusing declarative Kotlin functions (mapNotNull,any). Injected this step intoComputerVisionRepositoryImplimmediately after the raw YOLO inference. Updatedlabels.txtto includegeneric_boxanddropdown_symbol.Details
Ticket
ADFA-3612