[PR #1350] [MERGED] fix(labs): relax widget-gated-cell check to catch only multi-widget leaks #6491

New Issue

GiteaMirror · 2026-04-21T22:22:20-05:00

GiteaMirror commented

2026-04-21 22:22:20 -05:00

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1350
Author: @profvjreddi
Created: 4/16/2026
Status: ✅ Merged
Merged: 4/16/2026
Merged by: @profvjreddi

Base: dev ← Head: fix/widget-gated-test-strictness

📝 Commits (1)

888ef19 fix(labs): relax widget-gated-cell check to only catch multi-widget leaks

📊 Changes

1 file changed (+76 additions, -31 deletions)

View changed files

📝 labs/tests/test_static.py (+76 -31)

📄 Description

corrects a mistake in #1346's test_no_widget_defined_in_gated_cell. the check was over-strict and flagged the sequential-unlock idiom that actually works, so 32 of 33 labs were xfailed.

what i got wrong

the check treated ANY widget in a gated cell's return tuple as a violation. but the sequential-unlock idiom in labs 02 through 16 looks like this:

@app.cell
def _(mo, partA_prediction):
    mo.stop(partA_prediction.value is None, mo.md("..."))
    partB_prediction = mo.ui.radio(...)
    return (partB_prediction,)

this works: gate fires while partA is unanswered, user sees the unlock msg. once partA is answered, partB_prediction is defined and the next gated cell unblocks. test_engine.py confirms all 33 labs actually execute cleanly with this pattern (70 passed).

what the real bug was

lab_01 pre-#1339 had a gated cell defining MULTIPLE widgets:

@app.cell
def _(mo, partA_prediction):
    mo.stop(partA_prediction.value is None, ...)
    partA_instrument = mo.ui.slider(...)
    partB_prediction = mo.ui.radio(...)
    partB_instrument = mo.ui.slider(...)
    return (partA_instrument, partB_prediction, partB_instrument)

when this cell's gate fires, THREE widgets go undefined. that cascade is what broke lab_01 visually.

changes

renamed to test_no_multi_widget_leak_in_gated_cell
raised threshold from >=1 leaked widget to >=2
dropped the blanket xfail; the 14 labs with actual multi-widget debt are grandfathered via a _KNOWN_MULTI_LEAK_LABS set pointing at [GH-ISSUE #296] Create reference labels for all sections (#1347)
updated docstring to reflect the real rule, cite lab_01 post-#1339 as the canonical fix

verification

pytest labs/tests/test_static.py — 675 passed, 18 skipped (14 grandfathered + 4 pre-existing), 1 xfailed (unrelated)
19 currently-clean labs are now strictly enforced
as labs get refactored, remove them from the grandfather set; when empty, bug class is closed

this supersedes my abandoned attempt at a mass refactor in #1349 (closed). that attempt removed cell-level mo.stop entirely, which would have broken the sequential-unlock UX that works today.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1350 **Author:** [@profvjreddi](https://github.com/profvjreddi) **Created:** 4/16/2026 **Status:** ✅ Merged **Merged:** 4/16/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/widget-gated-test-strictness` --- ### 📝 Commits (1) - [`888ef19`](https://github.com/harvard-edge/cs249r_book/commit/888ef19379fa999acc91e56eeda7f599f8e6cc55) fix(labs): relax widget-gated-cell check to only catch multi-widget leaks ### 📊 Changes **1 file changed** (+76 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `labs/tests/test_static.py` (+76 -31) </details> ### 📄 Description corrects a mistake in #1346's `test_no_widget_defined_in_gated_cell`. the check was over-strict and flagged the sequential-unlock idiom that actually works, so 32 of 33 labs were xfailed. ## what i got wrong the check treated ANY widget in a gated cell's return tuple as a violation. but the sequential-unlock idiom in labs 02 through 16 looks like this: ```python @app.cell def _(mo, partA_prediction): mo.stop(partA_prediction.value is None, mo.md("...")) partB_prediction = mo.ui.radio(...) return (partB_prediction,) ``` this works: gate fires while partA is unanswered, user sees the unlock msg. once partA is answered, partB_prediction is defined and the next gated cell unblocks. test_engine.py confirms all 33 labs actually execute cleanly with this pattern (70 passed). ## what the real bug was lab_01 pre-#1339 had a gated cell defining MULTIPLE widgets: ```python @app.cell def _(mo, partA_prediction): mo.stop(partA_prediction.value is None, ...) partA_instrument = mo.ui.slider(...) partB_prediction = mo.ui.radio(...) partB_instrument = mo.ui.slider(...) return (partA_instrument, partB_prediction, partB_instrument) ``` when this cell's gate fires, THREE widgets go undefined. that cascade is what broke lab_01 visually. ## changes - renamed to `test_no_multi_widget_leak_in_gated_cell` - raised threshold from `>=1` leaked widget to `>=2` - dropped the blanket xfail; the 14 labs with actual multi-widget debt are grandfathered via a `_KNOWN_MULTI_LEAK_LABS` set pointing at #1347 - updated docstring to reflect the real rule, cite lab_01 post-#1339 as the canonical fix ## verification - `pytest labs/tests/test_static.py` — 675 passed, 18 skipped (14 grandfathered + 4 pre-existing), 1 xfailed (unrelated) - 19 currently-clean labs are now strictly enforced - as labs get refactored, remove them from the grandfather set; when empty, bug class is closed ## related this supersedes my abandoned attempt at a mass refactor in #1349 (closed). that attempt removed cell-level mo.stop entirely, which would have broken the sequential-unlock UX that works today. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-04-21 22:22:20 -05:00

GiteaMirror closed this issue

2026-04-21 22:22:21 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#6491