Skip to content

refactor: use str.removesuffix in _normalize_label_for_match - #306

Merged
dhruvbatra merged 1 commit into
mainfrom
claude/admiring-hawking-mzxbw2
Aug 29, 2026
Merged

refactor: use str.removesuffix in _normalize_label_for_match#306
dhruvbatra merged 1 commit into
mainfrom
claude/admiring-hawking-mzxbw2

Conversation

@dhruvbatra

@dhruvbatra dhruvbatra commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What

grounding.py::_normalize_label_for_match hand-rolled an endswith-check-then-slice to strip a trailing qualifier ( dropdown, menu, icon, button) from a normalized label during fuzzy matching:

for suffix in (" dropdown", " menu", " icon", " button"):
    if text.endswith(suffix):
        text = text[: -len(suffix)]

Replaced with the stdlib str.removesuffix, which is already a no-op when the suffix doesn't match:

for suffix in (" dropdown", " menu", " icon", " button"):
    text = text.removesuffix(suffix)

Why it's an improvement

  • Removes a manual len()/slice computation in favor of a purpose-built stdlib method.
  • Mirrors the sibling str.removeprefix cleanup already merged for _parse_navigation_hint in refactor: use str.removeprefix in _parse_navigation_hint #305 — same category, same file family (claim_parser.pygrounding.py).

Why it's safe

  • Pure stdlib substitution, single function, single file, no call-site changes.
  • Equivalent by construction: text[: -len(suffix)] when text.endswith(suffix) is True is exactly text.removesuffix(suffix); when the suffix doesn't match, both leave text unchanged. Verified with a standalone script comparing the old and new loop across representative inputs (single-suffix, multi-suffix chains, no match, empty string) — outputs identical in every case.
  • Full test suite: 326 passed / 21 pre-existing environment-only Chromium-sandbox failures, identical failing-test set before and after this change (verified via git stash A/B).
  • ruff check src/ tests/ clean. ruff format --check flags this file, but that drift is pre-existing (reproduced on the unmodified baseline) and unrelated to the touched lines — confirmed via ruff format --diff, which shows only pre-existing formatting elsewhere in the file, none on the lines this PR changes.

Generated by Claude Code


Note

Low Risk
Single-function stdlib substitution with equivalent semantics; no API or grounding logic changes.

Overview
_normalize_label_for_match in grounding.py now strips trailing UI qualifiers ( dropdown, menu, icon, button) with str.removesuffix instead of an endswith + slice branch inside the same loop.

Behavior for DOM label fuzzy matching is unchanged: each suffix is still applied in order, and non-matching suffixes leave the string as-is. This is a readability/stdlib cleanup only—no call sites or grounding rules change.

Reviewed by Cursor Bugbot for commit 2d75090. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Refactor
    • Simplified internal label normalization while preserving existing matching behavior.

_normalize_label_for_match hand-rolled an endswith-check-then-slice
to strip trailing " dropdown"/" menu"/" icon"/" button" qualifiers.
Use the stdlib str.removesuffix instead, which is a no-op when the
suffix doesn't match. Same behavior (each loop iteration strips at
most one matching suffix from the current text), one fewer manual
length computation. Mirrors the sibling str.removeprefix cleanup
already applied to _parse_navigation_hint in #305.

Co-authored-by: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b2e24f6a-8ab2-41db-a075-b4dcd61bdadc

📥 Commits

Reviewing files that changed from the base of the PR and between 8c18ed8 and 2d75090.

📒 Files selected for processing (1)
  • src/frontend_visualqa/grounding.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The label normalization implementation now uses str.removesuffix to remove recognized trailing suffixes. Supported suffixes and behavior remain unchanged.

Changes

Label normalization

Layer / File(s) Summary
Simplify recognized suffix removal
src/frontend_visualqa/grounding.py
_normalize_label_for_match uses str.removesuffix for each recognized trailing suffix.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 2d750

This PR changes one label-normalization loop from an equivalent manual suffix check and slice to str.removesuffix without changing call sites or matching rules. Python 3.11 supports the method, so no actionable merge-blocking risk remains beyond normal checks and review.

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: replacing manual suffix handling with str.removesuffix in _normalize_label_for_match.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/admiring-hawking-mzxbw2

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.

@dhruvbatra
dhruvbatra merged commit f6495e7 into main Aug 29, 2026
4 checks passed
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