feat: Enhance UserStateCache and FieldDataCache with dynamic child handling - #416
feat: Enhance UserStateCache and FieldDataCache with dynamic child handling#416mraman-2U wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR lays groundwork to improve performance and authoring guardrails for large Library Content / Item Bank assessments by (a) reducing unnecessary user-state prefetch work, and (b) introducing an opt-in “shell + batch children” rendering path for incremental loading, plus Studio validation for large max_count.
Changes:
- Add dynamic-child-aware traversal in
FieldDataCacheand optimizeget_manyvia query shaping + lazy JSON parsing to reduce DB/CPU overhead. - Add Phase B1 shell rendering (
render_mode=shell) and a new authenticated batch child-render API (/api/courseware/v1/xblock_children/) with a lazy placeholder template. - Add Phase C Studio guardrails for large
max_countwith a configurable threshold and an optional hard-cap waffle flag, plus tests and implementation-plan docs.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| xmodule/tests/test_library_content.py | Adds Studio validation test coverage for large max_count warning/error behavior. |
| xmodule/tests/test_item_bank.py | Adds parallel Studio validation test coverage for Item Bank large max_count. |
| xmodule/library_content_block.py | Hooks large-max_count guardrail into legacy library content validation. |
| xmodule/item_bank_block.py | Adds shell student view, large-max_count validation helpers, and updated help text. |
| scripts/field_data_cache_integration/validate_dynamic_children_prefetch.py | Adds a devstack integration validator script for dynamic-children prefetch behavior/metrics. |
| scripts/field_data_cache_integration/B1_shell_batch_curl.rst | Adds curl/Postman examples for shell render + batch children API. |
| openedx/core/djangoapps/courseware_api/views.py | Adds DRF view for the batch child-render endpoint. |
| openedx/core/djangoapps/courseware_api/urls.py | Routes the new /api/courseware/v1/xblock_children/ endpoint. |
| openedx/core/djangoapps/courseware_api/tests/test_views.py | Adds basic request-validation tests for the new API. |
| lms/templates/vert_module_lazy.html | Introduces a lazy-placeholder vertical template + postMessage hooks for batching. |
| lms/envs/common.py | Adds LMS settings for lazy render thresholds, batch max, and Studio guardrail defaults. |
| lms/djangoapps/courseware/views/views.py | Adds render_mode handling and integrates shell-mode decision + cache-depth control. |
| lms/djangoapps/courseware/user_state_client.py | Optimizes get_many DB shape and adds lazy JSON parsing for user state. |
| lms/djangoapps/courseware/toggles.py | Adds a waffle flag for enabling lazy shell rendering in courseware. |
| lms/djangoapps/courseware/tests/test_user_state_client.py | Adds unit/integration tests for query shaping + lazy state parsing behavior. |
| lms/djangoapps/courseware/tests/test_model_data.py | Adds tests for dynamic-children traversal and lazy state behavior in caches. |
| lms/djangoapps/courseware/tests/test_lazy_xblock_render.py | Adds unit tests for shell-mode eligibility helpers. |
| lms/djangoapps/courseware/model_data.py | Implements _children_for_field_data_cache and lazy overlay behavior in UserStateCache. |
| lms/djangoapps/courseware/block_render.py | Adds shell-mode helpers and render_xblock_children batch rendering implementation. |
| docs/implementation_plans/phase-c-studio-warning-sample.rst | Documents sample Studio warning/error messaging for Phase C. |
| docs/implementation_plans/assessments-not-loading-performance.md | Adds the broader phased implementation plan and acceptance criteria. |
| cms/envs/common.py | Adds CMS-side defaults for the Studio guardrail settings. |
| cms/djangoapps/contentstore/toggles.py | Adds the Studio hard-cap waffle flag and helper. |
Suppressed comments (2)
scripts/field_data_cache_integration/validate_dynamic_children_prefetch.py:290
- Same iterator-consumption issue as above: after
keys = list(block_keys), the patched method should passkeysintooriginal_get_manyto be robust to iterator inputs.
yield from original_get_many(self, username, block_keys, scope=scope, fields=fields)
scripts/field_data_cache_integration/B1_shell_batch_curl.rst:68
- The closing triple-quote at the end of this .rst file should be removed (it looks like an accidental carryover from a Python docstring).
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
06abb77 to
cfdfa37
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
openedx/core/djangoapps/courseware_api/views.py:1013
child_usage_keysare all parsed into UsageKeys before the batch-size guardrail is enforced (it only triggers insiderender_xblock_children). For oversized requests this does unnecessary work; enforceXBLOCK_CHILDREN_BATCH_MAXon the raw comma-split list before parsing each key.
child_key_strings = [part.strip() for part in child_keys_raw.split(',') if part.strip()]
child_usage_keys = []
for key_str in child_key_strings:
lms/djangoapps/courseware/block_render.py:1240
- The batch children API returns
str(exc)for render failures, which can leak internal details to clients. Since the exception is already logged, return a generic message in the response payload instead.
'message': str(exc),
| function onParentMessage(event) { | ||
| var data = event.data || {}; | ||
| if (data.type !== 'xblock.lazy.children') { | ||
| return; | ||
| } | ||
| if (data.parent_usage_key && data.parent_usage_key !== parentUsageKey) { | ||
| return; | ||
| } | ||
| fillChildren(data.results || []); | ||
| } |
cfdfa37 to
2dac0ba
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
lms/djangoapps/courseware/block_render.py:1241
- The batch children API returns the raw exception text to the client on render failures. This can leak internal implementation details and potentially sensitive information; it’s safer to log the exception server-side and return a generic message.
errors.append({
'usage_key': child_key_str,
'error': 'render_failed',
'message': str(exc),
})
openedx/core/djangoapps/courseware_api/tests/test_views.py:901
- The new XBlockChildren endpoint has validation logic for invalid child_usage_keys and a special-case 403 response when all children are forbidden, but the tests here only cover missing parent key, invalid parent key, and oversized batch. Adding focused tests for invalid child_usage_keys and the all-forbidden 403 path will help prevent regressions.
response = self.client.get(self.children_url, {
'parent_usage_key': parent,
'child_usage_keys': children,
})
assert response.status_code == 400
| if (document.referrer) { | ||
| try { | ||
| if (event.origin !== new URL(document.referrer).origin) { | ||
| return; | ||
| } | ||
| } catch (err) { | ||
| return; | ||
| } | ||
| } |
…ndling Add lazy/shell render and selected-child prefetch, and harden review follow-ups for missing-block 404s, JS-escaped usage keys, prefetch/test correctness, and parent-only postMessage handling. Co-authored-by: Cursor <cursoragent@cursor.com>
2dac0ba to
ce7dde9
Compare
This pull request introduces several features and configuration options to improve the performance and authoring experience for large library content (item banks) in Studio and the LMS, focusing on validation, performance guardrails, and implementation planning. The most important changes are:
Studio Validation and Guardrails
LIBRARY_CONTENT_MAX_COUNT_WARNING_THRESHOLD(default 25) to control when Studio warns authors about largemax_count(item bank Count) values that may cause slow loads for learners.contentstore.hard_cap_library_content_max_count) to optionally escalate the Studio warning to an error for organizations that want a hard cap, with a helper function for checking the flag.max_countexceeds the threshold, and how an optional runbook URL can be appended.Implementation Plan Documentation
docs/implementation_plans/assessments-not-loading-performance.md) detailing phased backend and frontend improvements to address large assessment render performance, including dynamic child traversal, SQL and JSON optimizations, shell rendering, MFE lazy loading, and Studio guardrails. [1] [2]Codebase Preparation
get_block_by_usage_idto accept an optionalfield_data_cache_depthargument, supporting shell-mode rendering and batch child API for incremental loading in the LMS. [1] [2] [3]These changes collectively lay the groundwork for both immediate validation improvements in Studio and future backend/frontend optimizations to support scalable, performant delivery of large item bank assessments.…ndling
Description
Describe what this pull request changes, and why. Include implications for people using this change.
Design decisions and their rationales should be documented in the repo (docstring / ADR), per
OEP-19, and can be
linked here.
Useful information to include:
"Developer", and "Operator".
changes.
Supporting information
Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.
Testing instructions
Please provide detailed step-by-step instructions for testing this change.
Deadline
"None" if there's no rush, or provide a specific date or event (and reason) if there is one.
Other information
Include anything else that will help reviewers and consumers understand the change.