[BB-10108] Refactor Branching XBlock to include Ren'Py like features by pkulkark · Pull Request #10 · open-craft/branching-xblock

pkulkark · 2026-02-19T16:12:16Z

Description

This PR refactors Branching XBlock to introduce Ren’Py-like branching/storytelling behavior, with updated authoring and learner UX aligned to the design reference in Figma: Figma design file.

Key updates included:

Refactored Studio authoring flow into a step-based editor with improved node management and validation UX.
Added richer image node support (left_image_url, right_image_url, overlay text, background image settings).
Strengthened save-time validation (cycle detection, referenced-node deletion checks, choice score validation).
Updated learner runtime for per-choice scoring, undo/reset behavior, and completion report output.

Supporting Information

OpenCraft Internal Jira task: BB-10108

Testing Instructions

Deploy this branch
In Studio, verify:
- node create/edit/delete flows work
- deleting referenced nodes is blocked
- cycles are rejected on save
- background image accessibility validation works (alt text required unless decorative)
Create a branching scenario with all the options in place
In learner view, verify:
- scoring accumulates by selected choice scores
- undo reverses the most recent scored choice
- reset clears progress and score
- completion report renders with expected values

Additional notes from author

The grade-scale currently implements only Pass/Fail levels and does not support creating multiple grade bands like Studio grading. This was an intentional scoping decision to reduce implementation complexity for this iteration, and can be extended in a future iteration if requirements call for it.
This branch and main both changed the same enable_hints → enable_reset_activity areas. Although 3 related commits from main were cherry-picked here (ab2770c, 8e5d707, c05093e), Git still reported conflicts because cherry-picks copy changes but not commit ancestry. To reconcile both ancestry and overlapping edits, origin/main was merged into this branch and conflicts were resolved in merge commit febfb7b.

Needed as we are scoring per choice and cyclic nodes can result in score loops making highest score unbound

Using topo+dp to find max path sum

samuelallan72

Thanks @pkulkark ! This is a pretty large PR, so I've made a first pass, and left several code review comments inline. I haven't done a deep review of code logic yet, or played around too much with the UI.

Also a comment on UI/UX:

Saving when there's an issue with the main background image is confusing - the error message may be hidden because it's in the body rather than a popup or next to the save button, and the studio still displays the saving animation in the lower right corner:

This is confusing, as you are thinking that it's just taking a long time to save, when in fact validation has failed and you need to scroll down to view the error:

Maybe these errors too need to go in the bottom bar or as a popup message?

branching_xblock/branching_xblock.py

samuelallan72 · 2026-02-23T00:10:35Z

branching_xblock/branching_xblock.py

+        return node
+
+    @staticmethod
+    def _coerce_choice_score(raw_score):


Rather than coercing the score everywhere, can we simply validate it in studio_submit (or a function only called from that), and trust the data structures after that?

In general I see a lot of noise working with data in the backend, due to all the validating and normalising happening. It would be nice to validate and normalise in one place, where it's submitted from the user (ie. studio_submit), and then treat it as known data types everywhere else.

If we need validation for the case where the xblock is imported from olx, then perhaps we can use a standard validation method to validate everything - like validate or validate_field_data - ref https://docs.openedx.org/projects/xblock/en/latest/xblock-utils/index.html or https://docs.openedx.org/projects/xblock/en/latest/xblock.html#xblock.core.XBlock.validate . I'm not sure exactly how these interact, but either way, if we can move all validation related things to the boundaries, that will leave things cleaner in the rest of the code.

tests/test_branching_xblock.py

samuelallan72 · 2026-02-23T00:12:33Z

branching_xblock/branching_xblock.py

+        help="Whether the background image is decorative"
+    )
+
    max_score = Float(


Score fields appear to be treated as integers everywhere else, so can these be integer fields instead?

branching_xblock/branching_xblock.py

samuelallan72 · 2026-02-23T00:26:47Z

branching_xblock/branching_xblock.py

+            return "Final grade range must end at 100."
+        return None
+
+    def _normalize_grade_ranges(self, raw_grade_ranges, strict=False):


This overlaps with _validate_grade_ranges above. Can we simply drop this and use something like:

if error := _validate_grade_ranges(raw_ranges): raise ValidationError(error) else: self.grade_ranges = raw_ranges

I'm not sure why we need so much normalising of values throughout? Can we just validate, and ensure the frontend editor always submits the expected types of values?

branching_xblock/branching_xblock.py

samuelallan72 · 2026-02-23T00:44:50Z

branching_xblock/branching_xblock.py

-                new_id = f"node-{uuid.uuid4().hex[:6]}"
-            else:
-                new_id = old_id
+            new_id = f"node-{uuid.uuid4().hex[:6]}" if old_id.startswith('temp-') or not old_id else old_id


It would be interesting to explore letting the frontend provide unique ids for the nodes, to avoid this node id re-assigning and mapping.

branching_xblock/static/js/src/studio_editor.js

- move studio save validation to a single backend validation path - return structured field/global/node errors for UI rendering - remove frontend normalization/coercion and rely on backend responses

pkulkark · 2026-03-02T18:50:52Z

Thanks @pkulkark ! This is a pretty large PR, so I've made a first pass, and left several code review comments inline. I haven't done a deep review of code logic yet, or played around too much with the UI.

Also a comment on UI/UX:

Saving when there's an issue with the main background image is confusing - the error message may be hidden because it's in the body rather than a popup or next to the save button, and the studio still displays the saving animation in the lower right corner:
This is confusing, as you are thinking that it's just taking a long time to save, when in fact validation has failed and you need to scroll down to view the error: Maybe these errors too need to go in the bottom bar or as a popup message?

Good catch. I've fixed this now.

samuelallan72 · 2026-03-03T03:10:56Z

Thanks @pkulkark, I see your responses ; let me know when it's ready for another review pass.

branching_xblock/static/js/src/studio_editor.js

samuelallan72 · 2026-03-04T05:47:43Z

branching_xblock/branching_xblock.py

+
+    def _normalize_scenario_nodes(self) -> None:
+        """
+        Normalize stored scenario node payloads once per nodes object.


Could you expand these docs with information about why this is required, what it actually does, and a note about potentially removing it in the future when we no longer need to normalise old data?

Also, maybe we can run the normalising in the init method, so we don't need to remember to use it in the various view methods?

I've added more information in the doc strings - let me know if that's good enough.

I don't think we should move the normalising in an init method, because we'd risk invalid stored values being auto-corrected during learner requests, which hides data issues instead of surfacing them through authoring validation. Keeping normalization in Studio/save flow will avoid that. Alternative option is to drop the whole normalizing and assume the old saved values wouldn't be needed to be preserved but i'm a bit hesitant to do that since this xblock rolls in the old branching xblock too.

Hmm if it's in init, then any invalid stored values will be automatically corrected whenever they're saved next. Currently, existing components may be in a slightly broken state after upgrade, until they are edited and re-saved in studio.

Alternative option is to drop the whole normalizing and assume the old saved values wouldn't be needed to be preserved but i'm a bit hesitant to do that since this xblock rolls in the old branching xblock too.

Is this definitely to be a in-place update to the branching xblock? If not, and if we don't need to have backward compatibility, then perhaps a better option might be to change the xblock label (to register it as a different/new xblock) and maybe even the package name, and drop normalising altogether.

Yes it was part of the requirement to have this as an in-place update.

branching_xblock/branching_xblock.py

branching_xblock/static/js/src/branching_xblock.js

samuelallan72 · 2026-03-11T04:06:59Z

@pkulkark I'm still seeing the red general error feedback text within the main content:

I'm not sure where that should go though, as it's not in the designs.

samuelallan72

@pkulkark in general, it works as expected functionally, thanks! I flagged a few things though - please see my inline comments. :)

branching_xblock/branching_xblock.py

samuelallan72

@pkulkark everything looks pretty good to me. 👍 I have left a couple of comments/suggestions, but otherwise feel free to response as you see fit and merge.

This was a very large change, so I may have missed things.

I tested this: read the code, tested manually on the sandbox
I read through the code
I checked for accessibility issues (my screenreader wasn't working, so I didn't test that. I did test keyboard controls though.)
Includes documentation

branching_xblock/branching_xblock.py

branching_xblock/static/js/src/branching_xblock.js

Co-authored-by: Samuel Allan <samuel.allan@fastmail.com>

pkulkark added 26 commits January 28, 2026 11:36

feat: Add overlay option for image node

75017ed

refactor: change choices to radio single choice

06e40d3

fix: button appearance

e35a6cf

refactor: replace hint settings with reset option

3f6f546

refactor: updated studio editor

62f601b

feat: composite images + styling

8e9ab8f

fix: styling changes

d074d83

feat: Add scores to each choice

5e0a83f

fix: update styling

cbc7212

fix: hide choices only if no branches flag is selected

aa9b8a4

feat: add soft delete for nodes

e36d8ab

feat: Add save validation for fields

fdb2ad9

fix: disable leaf flag if choices are added

d33cb3d

feat: display unlinked nodes

982c931

feat: replace hint settings with reset option

ab2770c

fix: update styling

8e5d707

fix: update tests

c05093e

fix: refactoring errors

a44a971

fix: Prevent cyclic nodes

15e2995

Needed as we are scoring per choice and cyclic nodes can result in score loops making highest score unbound

feat: compute maximum attainable score in scenario

cd5b7c6

Using topo+dp to find max path sum

feat: Add grade-range slider

c53152d

feat: Final grade report

549df1b

fix: clean up + fix regressions

a46658b

fix: Add test cases

f797f27

Merge origin/main into pooja/bb10108-implement-renpy

febfb7b

fix: CI errors

acc3151

samuelallan72 assigned pkulkark Feb 22, 2026

samuelallan72 self-requested a review February 22, 2026 23:34

samuelallan72 requested changes Feb 23, 2026

View reviewed changes

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from a31f6f6 to 69cfac0 Compare March 2, 2026 14:59

pkulkark force-pushed the pooja/bb10108-implement-renpy branch 3 times, most recently from 717966f to aaf609b Compare March 2, 2026 16:54

refactor: cleanup redundant functions and improve validation flow

aa3e3e7

- move studio save validation to a single backend validation path - return structured field/global/node errors for UI rendering - remove frontend normalization/coercion and rely on backend responses

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from aaf609b to aa3e3e7 Compare March 2, 2026 18:36

samuelallan72 reviewed Mar 4, 2026

View reviewed changes

fix: minor fixes

8fbc77f

samuelallan72 reviewed Mar 11, 2026

View reviewed changes

branching_xblock/static/js/src/branching_xblock.js Outdated Show resolved Hide resolved

samuelallan72 reviewed Mar 11, 2026

View reviewed changes

branching_xblock/branching_xblock.py Show resolved Hide resolved

branching_xblock/branching_xblock.py Outdated Show resolved Hide resolved

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from b191692 to 16a805a Compare March 11, 2026 16:00

fix: Refine score handling and validation feedback

855077c

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from 16a805a to 855077c Compare March 11, 2026 16:03

fix: inconsistent validation errors

ab9c996

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from c58cc0d to ab9c996 Compare March 11, 2026 19:42

fix: adjust styling

c1540a9

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from faf8448 to c1540a9 Compare March 11, 2026 20:13

samuelallan72 approved these changes Mar 11, 2026

View reviewed changes

branching_xblock/branching_xblock.py Outdated Show resolved Hide resolved

branching_xblock/static/js/src/branching_xblock.js Show resolved Hide resolved

fix: minor fixes

b1bfc1a

Co-authored-by: Samuel Allan <samuel.allan@fastmail.com>

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from 4e989aa to f5c81e1 Compare March 12, 2026 15:06

fix: adjust button styling

2f3b7a0

pkulkark force-pushed the pooja/bb10108-implement-renpy branch from f5c81e1 to 2f3b7a0 Compare March 12, 2026 15:33

pkulkark merged commit 5d20f81 into main Mar 12, 2026
4 of 6 checks passed

Conversation

pkulkark commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting Information

Testing Instructions

Additional notes from author

Uh oh!

samuelallan72 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

samuelallan72 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samuelallan72 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samuelallan72 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samuelallan72 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkulkark commented Mar 2, 2026

Uh oh!

samuelallan72 commented Mar 3, 2026

Uh oh!

Uh oh!

samuelallan72 Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pkulkark Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

samuelallan72 Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkulkark Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

samuelallan72 commented Mar 11, 2026

Uh oh!

samuelallan72 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

samuelallan72 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pkulkark commented Feb 19, 2026 •

edited

Loading

samuelallan72 Mar 11, 2026 •

edited

Loading