[#33] reduce download_all + _download_one complexity below grade C#36
Merged
Conversation
After round-1's inline-the-helpers fix, download_all hit radon C(18) and _download_one C(11) — they carried inherent orchestration complexity that flat inlining could not shed. Targeted refactor without re-introducing single-caller-rename helpers: - Extract _iter_eligible_attempts as a generator yielding the (item_id, item, index, entry) work tuples. The generator carries the filter + empty-media skip + eligibility cascade + global limit countdown — real state-machine logic, not a renamed parameter list. download_all becomes a flat loop with one decision per iteration. - Extract _filter_by_ids for the items-filter restriction (was inlined inside the generator; pulling it out shaves the and-conjunction from the inner loop). - Extract _classify_status to fold the 4xx / 5xx / other branches in _download_one into a single lookup-then-bucket line. Result: download_all C(18) -> B(7) _download_one C(11) -> B(9) _iter_eligible_attempts (new) B(10) No grade C functions remain in media.py. All 431 tests pass; ruff lint + format + mypy clean; behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
After round-1's "inline single-caller helpers" fix on PR #35,
download_allhit radon C(18) and_download_oneC(11). Local CI gate showed⚠️ Radon: grade C function(s) detected (warning only).This is a focused refactor that reduces those two functions to B without re-introducing the gold-plating that round-1 explicitly told me to inline.
What changed
_iter_eligible_attempts(new generator) yields(item_id, item, index, entry)work tuples. Carries the filter + empty-media skip + eligibility cascade + global limit countdown — real state-machine logic, not a renamed parameter list.download_allbecomes a flat for-loop with one decision per iteration._filter_by_ids(new) handles the items-filter restriction up front._classify_status(new) folds the 4xx / 5xx / other branches in_download_oneinto a single lookup-then-bucket.Result
download_all_download_one_iter_eligible_attempts(new)No grade C functions remain in
media.py.The remaining 3 C-grade functions in the repo (
generate.py:_render_index,validate.py:validate_judgment,archive.py:_archive_tweet_to_item) are pre-existing and outside the scope of #33. The Radon WARN may still trigger on them; this PR clears my contribution to the warning.Test plan
uv run pytest→ 431 passed (no test changes — behaviour preserved)uv run ruff check src/→ cleanuv run ruff format --check src/→ cleanuv run mypy --ignore-missing-imports src/→ cleanuv run radon cc src/xbrain/media.py -nc -s→ emptybash scripts/check.sh→ ALL CRITICAL CHECKS PASSED🤖 Generated with Claude Code