Fix crash when stored playground language state is invalid by martin-henz · Pull Request #3646 · source-academy/frontend

martin-henz · 2026-03-10T05:47:58Z

AI-generated PR, fixing #3645

Generated with Claude Code, Sonnet 4.6.

Description

Validate the stored chapter+variant combination against ALL_LANGUAGES when loading from localStorage. Falls back to the default language config if the combination is invalid (e.g. chapter 1 + explicit-control).

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Code quality improvements

How to test

Start frontend using localhost, switch to conductor in feature flags, choose Source 1. Then reload in the browser. This would crash the frontend when the feature flag is reset to false, because the "Source 1" language does not have a CSE machine implementation.

Checklist

I have tested this code
I have updated the documentation

Validate the stored chapter+variant combination against ALL_LANGUAGES when loading from localStorage. Falls back to the default language config if the combination is invalid (e.g. chapter 1 + explicit-control). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

RichDom2185

This works but behaves differently than what the issue suggests.

coveralls · 2026-03-10T05:52:16Z

Coverage Report for CI Build 28746807720

Coverage increased (+0.02%) to 41.4%

Details

Coverage increased (+0.02%) from the base build.
Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	14127
Covered Lines:	6293
Line Coverage:	44.55%
Relevant Branches:	7506
Covered Branches:	2663
Branch Coverage:	35.48%
Branches in Coverage %:	Yes
Coverage Strength:	27.08 hits per line

💛 - Coveralls

martin-henz · 2026-03-10T11:57:52Z

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

RichDom2185 · 2026-03-16T10:42:09Z

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

The issue suggests:

invalid combination of lang + variant → fallback to default lang + variant combination

But the logic implemented is:

invalid lang → fallback to default lang && invalid variant → fallback to default variant

martin-henz · 2026-04-21T00:46:27Z

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

The issue suggests:

invalid combination of lang + variant → fallback to default lang + variant combination

But the logic implemented is:

invalid lang → fallback to default lang && invalid variant → fallback to default variant

Hmm, the implementation checks:

          const validCombination = ALL_LANGUAGES.some(
            lang => lang.chapter === chapter && lang.variant === variant
          );

so it looks for a valid combination. It falls back on the default lang/variant only if the combination is not valid.

# Conflicts: # src/pages/createStore.ts

When the stored chapter+variant combination is not in ALL_LANGUAGES, reset to `defaultLanguageConfig` (a known-valid pair) rather than to the individual default context fields, so the fallback is explicitly "the default language config" as the issue describes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-07-05T16:07:38Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e126f818-e6c6-4391-8568-82092e75bd1e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

The change updates the loadStore function in createStore.ts to validate persisted chapter/variant combinations against ALL_LANGUAGES before rehydrating playground.context, falling back to defaultLanguageConfig when the stored combination is invalid.

Changes

Language config validation

Layer / File(s)	Summary
Validate persisted chapter/variant on load `src/pages/createStore.ts`	Import is expanded to include `ALL_LANGUAGES` and `defaultLanguageConfig`; `loadStore` now computes and validates the stored `chapter`/`variant` pair against `ALL_LANGUAGES`, falling back to `defaultLanguageConfig` when the combination is invalid.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

flowchart TD
loadStore --> ReadPersistedContext
ReadPersistedContext --> CheckAllLanguages
CheckAllLanguages -->|valid combination| UseStoredChapterVariant
CheckAllLanguages -->|invalid combination| UseDefaultLanguageConfig
UseStoredChapterVariant --> SetPlaygroundContext
UseDefaultLanguageConfig --> SetPlaygroundContext

Related issues: None specified.

Related PRs: None specified.

Suggested labels: None specified.

Suggested reviewers: None specified.

🐰 A chapter stored, a variant too,
Checked against languages tried and true,
If the pairing's amiss, no need to fret,
Defaults step in, no crash just yet!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main bug fix in the PR.
Description check	✅ Passed	The description includes the required summary, type of change, testing steps, and checklist sections.
Linked Issues check	✅ Passed	The change validates stored chapter+variant state against ALL_LANGUAGES and falls back to the default config as required by `#3645`.
Out of Scope Changes check	✅ Passed	The PR stays focused on the localStorage validation fix and does not introduce unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

martin-henz · 2026-07-05T16:07:40Z

Revived this: merged master in (branch was ~227 commits behind) and resolved the conflict in createStore.ts.

@RichDom2185 — to address your note that it behaved differently from what the issue suggests: the invalid-combination fallback now resets to defaultLanguageConfig (a known-valid chapter+variant pair from ALL_LANGUAGES) rather than the individual default context fields, so it explicitly falls back to the default language config as the issue describes. When the combo is invalid we reset the whole language wholesale rather than trying to preserve a valid chapter and only fix the variant — since this is a crash-fix, dropping to the known-good default seemed the safest, most predictable behavior. Let me know if you had the chapter-preserving variant in mind instead.

Prettier/eslint/tsc clean; CI re-running.

coderabbitai

🧹 Nitpick comments (1)

src/pages/createStore.ts (1)

87-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fallback logic correctly validates the combined pair.

This correctly matches the PR intent: it checks the stored chapter+variant as a single combination against ALL_LANGUAGES and falls back to defaultLanguageConfig for both fields together (rather than resolving chapter and variant independently), avoiding the "Language config not found" crash described in the linked issue.

One nit: this validity check duplicates the same ALL_LANGUAGES.find logic already encapsulated in getLanguageConfig (src/commons/application/ApplicationTypes.ts). Consider reusing that helper (wrapped in try/catch) instead of re-implementing the lookup here, to keep a single source of truth for "what counts as a valid chapter/variant pair."

♻️ Optional refactor using existing helper

         context: (() => {
           const chapter = loadedStore.playgroundSourceChapter
             ? loadedStore.playgroundSourceChapter
             : defaultState.workspaces.playground.context.chapter;
           const variant = loadedStore.playgroundSourceVariant
             ? loadedStore.playgroundSourceVariant
             : defaultState.workspaces.playground.context.variant;
-          const validCombination = ALL_LANGUAGES.some(
-            lang => lang.chapter === chapter && lang.variant === variant,
-          );
+          let validCombination = true;
+          try {
+            getLanguageConfig(chapter, variant);
+          } catch {
+            validCombination = false;
+          }
           // Invalid stored combinations (e.g. chapter 1 + explicit-control, which can be
           // persisted when toggling feature flags) would crash on load, so fall back to
           // the default language config as a coherent, always-valid pair.
           return {
             ...defaultState.workspaces.playground.context,
             chapter: validCombination ? chapter : defaultLanguageConfig.chapter,
             variant: validCombination ? variant : defaultLanguageConfig.variant,
           };
         })(),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/createStore.ts` around lines 87 - 105, The fallback in
createStore() is validating chapter and variant correctly, but it reimplements
the language-pair lookup logic already centralized in getLanguageConfig. Update
the playground context initialization to reuse getLanguageConfig from
ApplicationTypes instead of duplicating ALL_LANGUAGES.some, and wrap that helper
call in try/catch so invalid stored combinations still fall back to
defaultLanguageConfig as a pair.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pages/createStore.ts`:
- Around line 87-105: The fallback in createStore() is validating chapter and
variant correctly, but it reimplements the language-pair lookup logic already
centralized in getLanguageConfig. Update the playground context initialization
to reuse getLanguageConfig from ApplicationTypes instead of duplicating
ALL_LANGUAGES.some, and wrap that helper call in try/catch so invalid stored
combinations still fall back to defaultLanguageConfig as a pair.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 50216437-776a-4e6f-9d01-7d8a5f6a9aec

📥 Commits

Reviewing files that changed from the base of the PR and between 339cb22 and 08645da.

📒 Files selected for processing (1)

src/pages/createStore.ts

RichDom2185 requested changes Mar 10, 2026

View reviewed changes

martin-henz mentioned this pull request Mar 10, 2026

CSE Machine : Function Descriptions modification for Frame overlap #3627

Merged

7 tasks

martin-henz added 3 commits March 10, 2026 19:58

Merge branch 'master' into invalidLang

33375a8

Merge branch 'master' into invalidLang

3c0f115

Merge branch 'master' into invalidLang

466df3b

RichDom2185 and others added 2 commits March 16, 2026 23:24

Merge branch 'master' into invalidLang

7a5290f

Merge branch 'master' into invalidLang

66a3eb9

martin-henz and others added 2 commits July 6, 2026 00:02

Merge remote-tracking branch 'origin/master' into invalidLang

677c418

# Conflicts: # src/pages/createStore.ts

Merge branch 'master' into invalidLang

b78753d

coderabbitai Bot reviewed Jul 5, 2026

View reviewed changes

martin-henz merged commit 5b9b157 into master Jul 5, 2026
10 checks passed

martin-henz deleted the invalidLang branch July 5, 2026 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix crash when stored playground language state is invalid#3646

Fix crash when stored playground language state is invalid#3646
martin-henz merged 9 commits into
masterfrom
invalidLang

martin-henz commented Mar 10, 2026 •

edited

Loading

Uh oh!

RichDom2185 left a comment

Uh oh!

coveralls commented Mar 10, 2026 •

edited

Loading

Uh oh!

martin-henz commented Mar 10, 2026

Uh oh!

RichDom2185 commented Mar 16, 2026

Uh oh!

martin-henz commented Apr 21, 2026

Uh oh!

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading

Review skipped

Uh oh!

martin-henz commented Jul 5, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

martin-henz commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How to test

Checklist

Uh oh!

RichDom2185 left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Coverage Report for CI Build 28746807720

Coverage increased (+0.02%) to 41.4%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

martin-henz commented Mar 10, 2026

Uh oh!

RichDom2185 commented Mar 16, 2026

Uh oh!

martin-henz commented Apr 21, 2026

Uh oh!

coderabbitai Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

martin-henz commented Jul 5, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martin-henz commented Mar 10, 2026 •

edited

Loading

coveralls commented Mar 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading