openedx / openedx/xblocks-core
Claude suggested improvements for repo
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 0
- Forks
- 15
- Avg merge
- 5d 16m
- Merged PRs (30d)
- 7
Description
I asked claude about the code improvements in this repository with following prompt
Prompt:
Use a "Fan-out" / "fan-in" (N researchers → synthesizer) approach to research the
question "how best should I optimize this codebase"?
▎ Minimum 5 subagents, use Sonnet to do the research and individual contemplation; Opus to synthesize.
Agents run by claude:
1. Architecture & code structure — module organization, inheritance, coupling
2. Test coverage & quality — gaps, brittleness, fixtures
3. Performance & runtime — queries, caching, asset handling
4. Dependency management & build tooling — packaging, uv/pyproject.toml, CI
5. Code duplication & refactoring — DRY opportunities, dead code
6. Developer experience & documentation — docs, types, linting, onboarding
and this is what Claude analyzed and replied:
Claude reply:
Executive Summary
The codebase is functional but carries significant technical debt inherited from its
edx-platform extraction. The three most consequential issues are:
- VideoBlock's redundant database calls on every render — direct learner-facing
latency - No coverage thresholds + critically under-tested handler code
(video_handlers.pyat 16.9% line coverage), making refactoring unsafe - Legacy
setup.pybuild system with unpinned dependencies and a deadsix
dependency that blocks modernization
Pervasive copy-paste duplication across XBlocks is a natural artifact of extraction
without a consolidation pass. Addressing these in order compounds: build system → CI
safety → safe refactoring → performance.
Tier 1 — Critical / Do First
1.1 Merge the pyproject.toml branch
- Merge
origin/pyproject.tomlintomain - Remove
setup.pyentirely - Fix
[wheel] universal = 1insetup.cfg— incorrect for a
python_requires=">=3.12"package - Add lower-bound version constraints to core deps in
requirements/base.in
(currently all 28 deps are completely unpinned) - Replace
python setup.py sdist bdist_wheelwithpython -m buildin
pypi-publish.yml:34
Why: The pyproject.toml branch already exists and is ready to land. The current
setup.py is 120+ lines of bespoke requirement-loading logic that exposes zero version
constraints to downstream consumers. This unblocks uv migration, proper dep bounds,
and all other CI fixes.
1.2 Establish a coverage floor
- Add
--cov-fail-under=60topytestconfig (ratchet up over time) - Fix CI to upload coverage for all Django environments (currently
django52
coverage is silently discarded) - Write tests for
video/video_handlers.py— 16.9% line / 0% branch (AJAX
dispatch layer handling student transcript/studio interactions) - Write tests for
video/bumper_utils.py— 26.9% line / 0% branch - Write tests for
legacy_utils/xml_utils.py— 34.4% line / 10.9% branch
(shared by all 8 XBlocks) - Write tests for
poll/poll.py— 46.9% line / 9.1% branch (only 2 tests;
student_view, all handlers, and XML round-trip untested) - Write tests for
discussion/discussion.py— 63.9% line / 25% branch
(student_view,author_view,student_view_dataall untested)
Why: Every refactoring item in Tiers 2 and 3 requires this safety net first.
1.3 Fix VideoBlock's redundant VAL database calls
- Consolidate
get_html()from 3 serial DB calls to 1:get_urls_for_profiles
(video.py:323),get_video_info(video.py:349),get_course_video_image_urlin
_poster()(video.py:1164) - Fix
get_context()dual VAL calls (video.py:803+:829) — the second call
subsumes the first; first result is unused - Add
@request_cachedto_poster()(video.py:1158) - Add request-level cache to
get_available_transcript_languages()
(video_transcripts_utils.py:79) — 10 videos in a vertical currently fires 10 separate
DB calls
Why: Direct learner-facing latency. VideoBlock is the most-rendered XBlock in the
platform.
1.4 Remove the six dependency
- Replace
six.moves.map,six.moves.range,six.moves.zipin
responsetypes.py:30with stdlib equivalents - Replace
sixusage ininputtypes.py:54 - Replace
six.moves.xrangeinsafe_exec/safe_exec.py:51withrange - Replace
sixin test files:response_xml_factory.py:5,test_inputtypes.py:29,
test_safe_exec.py:19 - Remove
sixfromrequirements/base.inandrequirements/test.in:6
Why: python_requires=">=3.12" makes every six usage a dead import. All
replacements are 1:1 stdlib equivalents.
Tier 2 — High Value
2.1 Consolidate cross-XBlock duplication into legacy_utils/
The extraction left utilities copy-pasted across modules with no consolidation pass.
- Move
stringify_childrentolegacy_utils/— currently defined verbatim in
problem/stringify.py,html/html.py:111, andpoll/poll.py:52(identical docstrings
including the StackOverflow URL) - Centralize
HTML()/Text(markupsafe wrappers) — currently in
problem/markup.py,discussion/discussion.py:34-54,poll/poll.py:29-49with
copy-pasted docstrings - Create a single
ATTR_KEY_*constants module —
ATTR_KEY_DEPRECATED_ANONYMOUS_USER_IDandATTR_KEY_USER_IDredefined in
html/html.py,problem/capa_block.py:68-70,video/constants.py - Consolidate
SerializationError— identical class defined in both
annotatable/annotatable.py:27andproblem/capa_block.py:85 - Consolidate
StubUserService— duplicated acrossproblem/tests/__init__.py,
lti/tests/helpers.py, andhtml/tests/into a single
xblocks_contrib/tests/helpers.py - Extract pipeline asset-path resolution from
discussion/discussion.py:137-144and
problem/capa_block.py:436-443—video/video_utils.py:209-213already has the
correct helper; the others should import it
Why: ~200 lines of duplication eliminated; single source of truth prevents string
drift (the edx-platform.* attribute keys are platform contracts — a typo silently
breaks user attribute lookups).
2.2 Cache ProblemBlock.problem_types and VideoBlock.editable_metadata_fields
- Add
@cached_propertytoProblemBlock.problem_types(capa_block.py:631-639) —
callsetree.XML(self.data)(full DOM parse) on every access; called in
index_dictionary()twice and inhas_support() - Cache
VideoBlock.editable_metadata_fieldsacross its 3 accesses per studio
request:studio_viewline 256,get_contextlines 781 and 789
Why: Eliminates redundant XML parsing and metadata computation on every Studio
editing request.
2.3 Fix WordCloudBlock JS inlining
- Switch
word_cloud.pyfromload_unicodetoadd_javascript_urlford3.min.js
(139 KB),d3.layout.cloud.js(12.3 KB), andword_cloud.js(8.5 KB) - Same pattern applies to
annotatable.py,lti.py, andpoll.pywhich also use
load_unicodefor JS/CSS
Why: VideoBlock already does this correctly. 3 word clouds on one page = 480 KB of
non-cacheable JS inlined into the DOM on every render.
2.4 Fix CI environment naming + add Python 3.13
- Align CI
TOXENVvalues with tox envlist — CI passesdjango42/django52but
tox.inideclarespy312-django{42,52}(naming mismatch means CI environments are not
in the declared envlist) - Add Python 3.13 to CI matrix (
python-tests.yml:17) and update classifiers in
setup.py:155/pyproject.toml - Add
cache: 'pip'toactions/setup-pythonsteps
Why: Python 3.13 shipped October 2024 and is 7+ months old with no CI coverage here.
2.5 Resolve LTI block deprecation status
- Either document a concrete removal plan with target dates, or remove the
deprecation notice atlti/lti.py:297
Why: The block is marked deprecated in favor of xblock-lti-consumer yet is fully
maintained (1,019 lines). Current ambiguity discourages contribution and signals
contradictory intent to maintainers and consumers.
Tier 3 — Continuous Improvement
3.1 Break up god files in capa/
- Extract response type classes from
responsetypes.py(3,866 lines,
too-many-linessuppressed) into aresponsetypes/sub-package - Work toward reducing
capa_block.py(2,737 lines) andinputtypes.py(1,822
lines)
Prerequisite: Tier 1.2 (coverage floor) must be in place before touching these
files.
3.2 Adopt Ruff; begin adding type annotations
- Add
ruffto quality stack (replacespycodestyle+isort, gains auto-fix) - Systematically clear 35
lint-amnestymarkers (carried over from extraction;
concentrated inlti.pyx13,video.pyx8) - Add type annotations to public method signatures starting from
legacy_utils/—
currently only 14 return-type annotations exist across the entirexblocks_contrib/
source - Add
mypyorpyright(even--ignore-missing-importsmode) to thequality
tox environment
3.3 Improve developer documentation
- Create
CONTRIBUTING.rstcovering: XBlock extraction workflow, Waffle flag
integration, running the full test matrix locally, JS test setup - Add a "Quick Start" section to
README.rstcovering the Python dev loop
end-to-end (currently split between a 13-linedocs/getting_started.rstand
docs/testing.rst, neither linked from README) - Fix duplicate
0.16.0entry inCHANGELOG.rst(appears on lines 17 and 24 with
different fixes) - Fill or remove the four empty Sphinx stub files:
docs/quickstarts/index.rst,
concepts/index.rst,how-tos/index.rst,references/index.rst - Remove debug log artifact:
lti_2_util.py:130-132"#### COPY AND PASTE AUTHORIZATION HEADER ####"
3.4 Fix O(N) language scans
-
video.py:547-563validate()— O(N×M) scan over transcripts ×ALL_LANGUAGES;
replace inner scan with dict lookup -
video_transcripts_utils.py:194-235get_endonym_or_label()— linear scan of
ALL_LANGUAGESper call, called in a loop; convert to@lru_cachedict keyed by
language code
3.5 Lazy-load heavy imports in responsetypes.py
- Move module-level imports of
numpy,html5lib,shapely.geometry,symmath,
calc,pyparsingto function-level lazy imports - Evaluate the
lxml.html.soupparser.fromstring as fromstring_bsimport (comment in
code: "uses Beautiful Soup!!! FIXME?")
Why: capa_block.py → capa_problem.py → responsetypes.py — every worker process
importing ProblemBlock pays full cold-start cost for all these scientific/parsing
libraries.
3.6 Test quality fixes
- Add
pytest-xdisttorequirements/test.inand configureaddopts = -n autofor
parallelism - Move
pip install -e .fromtox.inicommandstodeps(currently defeats tox
environment caching) - Fix non-deterministic test data in
test_discussion.py—_random_string()uses
unseededrandom.choiceat collection time, making failures non-reproducible - Remove
printdebug statements fromtest_discussion.py:110and:142 - Fix
strvsbytesassertion inconsistency intest_video.py:865vs:875 - Convert five near-identical
HtmlBlockIndexingTestCasemethods to
@pytest.mark.parametrize - Convert
for-loop intest_annotatable.py test_annotation_class_attr_with_invalid_highlightto@pytest.mark.parametrize - Remove
toxfromrequirements/test.in:7(circular —toxis installed inside
tox environments)
Root Causes
Understanding why these issues accumulated helps prevent them from returning:
-
Extraction without consolidation. Each block was extracted from
edx-platform
carrying its own copy of shared utilities. No consolidation pass followed. The fix is
mechanical but requires deliberate scheduling. -
Inherited debt treated as permanent. The
lint-amnestymarkers,six,
setup.py, and pylint suppressions were carried as temporary pragmatism. Without an
explicit "pay extraction debt" phase, temporary became permanent. The unmerged
pyproject.tomlbranch is the canonical example — the work is done but not landed. -
Missing safety nets enable entropy. No coverage thresholds, no type checking, no
auto-formatter means each contribution can silently degrade quality. The codebase
doesn't deteriorate through malice — it deteriorates because there's no automated
ratchet preventing regression.
Suggested Sprint Sequencing
Sprint 1 (Foundation — parallelizable across 2-3 devs)
├── Merge pyproject.toml branch [1.1]
├── Add --cov-fail-under=60, fix CI env naming + coverage upload [1.2, 2.4]
└── Remove six [1.4]
Sprint 2 (Safety + Performance — depends on Sprint 1)
├── Write tests for video_handlers.py, xml_utils.py, poll.py [1.2 continued]
├── Consolidate VAL calls in VideoBlock [1.3]
└── Cache problem_types + editable_metadata_fields [2.2]
Sprint 3 (Consolidation — depends on Sprint 2 coverage)
├── Move shared utilities to legacy_utils/ [2.1]
├── Fix WordCloud JS inlining [2.3]
└── Resolve LTI deprecation [2.5]
Ongoing (paced by developer pain)
├── Adopt Ruff [3.2] — improves every subsequent PR immediately
├── Break up god files [3.1] — only after coverage is adequate
└── Documentation [3.3] — opportunistically
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
This issue lists many independent work areas, including setup.py, pyproject.toml, requirements/base.in, tox.ini, python-tests.yml, and several XBlock modules. Start by choosing one narrowly scoped item and reading its named files and related tests; define done as the selected change being implemented with passing focused tests and CI checks. There is no single completion criterion for the full proposal.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, build-system, developer-experience, performance, testing
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100