Skip to content

fix: raise ValueError instead of UnboundLocalError in _get_indexed_match on zero matches - #4401

Open
chuenchen309 wants to merge 1 commit into
Unstructured-IO:mainfrom
chuenchen309:fix/get-indexed-match-unbound-local-error
Open

fix: raise ValueError instead of UnboundLocalError in _get_indexed_match on zero matches#4401
chuenchen309 wants to merge 1 commit into
Unstructured-IO:mainfrom
chuenchen309:fix/get-indexed-match-unbound-local-error

Conversation

@chuenchen309

@chuenchen309 chuenchen309 commented Jul 15, 2026

Copy link
Copy Markdown

Problem

_get_indexed_match (unstructured/cleaners/extract.py) does for i, result in enumerate(re.finditer(pattern, text)), then references i in the "not found" branch's error message. When the pattern matches zero times, the loop body never runs, so i is never bound — the intended ValueError("Result with index ... was not found...") instead raises UnboundLocalError: cannot access local variable 'i', breaking extract_text_before()'s and extract_text_after()'s documented "pattern not found" contract.

(The existing test_get_indexed_match_raises_value_error_with_index_too_high test doesn't catch this — it uses a pattern that does match, just fewer times than the requested index, so i was always bound there.)

Fix

Initialize i = -1 before the loop, so the existing ValueError branch can safely report a sensible "largest index" (-1, meaning none) even with zero matches.

Testing

  • Added test_get_indexed_match_raises_value_error_with_zero_matches (a pattern with zero matches in the text) alongside the existing index-too-high test.
  • Confirmed red→green: reverting only extract.py reproduces UnboundLocalError: cannot access local variable 'i'; reapplying passes.
  • Full test_unstructured/cleaners/ (140 tests) — all pass.
  • ruff check — clean.
  • xref: no open PR touches unstructured/cleaners/extract.py.

Note: this bumps CHANGELOG.md/__version__.py to 0.25.1, matching the same version already proposed by two of my other open PRs (#4397, #4398) and one already-merged... actually still-open (#4399) against this same base — all four independently propose the same next-patch version bump since none have merged yet. Whichever merges first should "win" the bump; the rest will need a trivial rebase, which I'm happy to do once the ordering is known.

AI-Generated disclosure

Found via an AI-assisted code review pass (Claude Code) over unstructured/cleaners/. I personally reproduced the UnboundLocalError, verified the fix, and ran the relevant test suite plus lint before submitting.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 4 files

Shadow auto-approve: would auto-approve. Fixes a clear bug where zero matches raised UnboundLocalError instead of the intended ValueError; the change is a one-line initialization plus a regression test, bounded and clearly beneficial.

Re-trigger cubic

…tch on zero matches

`for i, result in enumerate(re.finditer(pattern, text))` never binds
`i` when the pattern matches zero times, so the "not found" branch's
error message (which references `i`) raises UnboundLocalError instead
of the intended ValueError -- breaking extract_text_before()'s and
extract_text_after()'s documented "pattern not found" contract.

Initialize `i = -1` before the loop so the existing ValueError branch
can safely report a sensible "largest index" even with zero matches.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chuenchen309
chuenchen309 force-pushed the fix/get-indexed-match-unbound-local-error branch from b5d6953 to 3e6064b Compare July 15, 2026 22:05
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.

1 participant