fix(mlsysim): surface load errors via last_load_error on DesignLedger

labs/tests/test_wasm_persistence.py

@@ -39,13 +39,15 @@ class body, was silently rewritten by the Python compiler to
 
 import functools
 import http.server
+import json
 import shutil
 import socketserver
 import threading
 from pathlib import Path
 
 import pytest
 
+
 STATE_PY = (
     Path(__file__).resolve().parents[2] / "mlsysim" / "mlsysim" / "labs" / "state.py"
 )
@@ -205,3 +207,146 @@ def test_design_ledger_save_async_persists_in_real_indexeddb(served_dir):
         f"from #1985 / PR #1988. A mocked test cannot catch this; only a "
         f"real Pyodide + IndexedDB check like this one can."
     )
+
+
+def test_load_async_corrupt_record_sets_last_load_error(served_dir):
+    """A stored record exists but is corrupt JSON -- json.loads() raising
+    is a Python-side failure independent of the JS resolve/reject shape,
+    so last_load_error must be populated regardless of #1988's status."""
+    from playwright.sync_api import sync_playwright
+
+    _, port = served_dir
+
+    with sync_playwright() as p:
+        browser = p.chromium.launch()
+        context = browser.new_context()
+        try:
+            page = context.new_page()
+            init_errors: list[str] = []
+            page.on(
+                "pageerror",
+                lambda exc, errors=init_errors: errors.append(str(exc)),
+            )
+
+            page.goto(f"http://127.0.0.1:{port}/index.html")
+            page.wait_for_function(
+                "window.__ready === true || window.__initError", timeout=30_000
+            )
+            init_error = page.evaluate("window.__initError || null")
+            assert not init_error, f"Pyodide init failed: {init_error}"
+            assert not init_errors, f"Uncaught page errors during init: {init_errors}"
+
+            page.evaluate(
+                """
+                () => new Promise((resolve) => {
+                    const req = indexedDB.deleteDatabase("mlsys_ledger_db");
+                    req.onsuccess = req.onerror = req.onblocked = () => resolve();
+                })
+                """
+            )
+
+            # Seed a corrupt record directly at the storage layer.
+            page.evaluate(
+                """
+                () => new Promise((resolve, reject) => {
+                    const req = indexedDB.open("mlsys_ledger_db", 1);
+                    req.onupgradeneeded = (e) => {
+                        const db = e.target.result;
+                        if (!db.objectStoreNames.contains("ledger")) {
+                            db.createObjectStore("ledger");
+                        }
+                    };
+                    req.onsuccess = (e) => {
+                        const db = e.target.result;
+                        const tx = db.transaction("ledger", "readwrite");
+                        tx.objectStore("ledger").put("{not valid json", "mlsys_design_ledger");
+                        tx.oncomplete = () => { db.close(); resolve(); };
+                        tx.onerror = () => { db.close(); reject(tx.error); };
+                    };
+                    req.onerror = () => reject(req.error);
+                })
+                """
+            )
+
+            result = page.evaluate(
+                """
+                async () => {
+                    const pyodide = window.__pyodide;
+                    return await pyodide.runPythonAsync(`
+import json
+ledger = DesignLedger()
+await ledger.load_async()
+json.dumps({"error": ledger.last_load_error})
+`);
+                }
+                """
+            )
+            page.close()
+        finally:
+            context.close()
+            browser.close()
+
+    parsed = json.loads(result)
+    assert parsed["error"] is not None, (
+        "load_async() must surface a corrupt-JSON read failure via "
+        "last_load_error instead of silently returning a blank LedgerState()."
+    )
+
+
+def test_load_async_synchronous_indexeddb_open_throw_sets_last_load_error(served_dir):
+    """indexedDB.open() throwing synchronously must reject the Promise
+    (per the Promise constructor spec) and propagate to last_load_error --
+    true today even against the pre-#1988 resolve(null)-style onerror
+    handlers, since this never reaches those handlers at all."""
+    from playwright.sync_api import sync_playwright
+
+    _, port = served_dir
+
+    with sync_playwright() as p:
+        browser = p.chromium.launch()
+        context = browser.new_context()
+        try:
+            page = context.new_page()
+            init_errors: list[str] = []
+            page.on(
+                "pageerror",
+                lambda exc, errors=init_errors: errors.append(str(exc)),
+            )
+
+            page.goto(f"http://127.0.0.1:{port}/index.html")
+            page.wait_for_function(
+                "window.__ready === true || window.__initError", timeout=30_000
+            )
+            init_error = page.evaluate("window.__initError || null")
+            assert not init_error, f"Pyodide init failed: {init_error}"
+            assert not init_errors, f"Uncaught page errors during init: {init_errors}"
+
+            page.evaluate(
+                """() => {
+                    window.indexedDB.open = () => {
+                        throw new Error('Simulated synchronous IndexedDB failure');
+                    };
+                }"""
+            )
+
+            result = page.evaluate(
+                """
+                async () => {
+                    const pyodide = window.__pyodide;
+                    return await pyodide.runPythonAsync(`
+import json
+ledger = DesignLedger()
+await ledger.load_async()
+json.dumps({"error": ledger.last_load_error})
+`);
+                }
+                """
+            )
+            page.close()
+        finally:
+            context.close()
+            browser.close()
+
+    parsed = json.loads(result)
+    assert parsed["error"] is not None
+    assert "Simulated synchronous IndexedDB failure" in parsed["error"]

mlsysim/mlsysim/labs/state.py

@@ -31,6 +31,7 @@ def __init__(self):
         self.file_path = self.config_dir / "ledger.json"
 
         self._state = LedgerState()
+        self._last_load_error: Optional[str] = None
 
         # WASM save tasks remain tracked until flush() observes them. Keeping
         # completed tasks lets a later flush() re-raise persistence failures.
@@ -49,6 +50,11 @@ def last_save_error(self) -> Optional[str]:
         """
         return self._last_save_error
 
+    @property
+    def last_load_error(self) -> Optional[str]:
+        """Error message from the most recent failed load."""
+        return self._last_load_error
+
     @property
     def save_pending(self) -> bool:
         """True while at least one WASM background save is still running."""
@@ -75,6 +81,7 @@ def _parse_history(self, data: dict) -> dict:
 
     def load(self) -> LedgerState:
         """Loads the ledger from the best available persistent storage."""
+        self._last_load_error = None
 
         # WASM loading is asynchronous, so synchronous load()
         # simply returns the current in-memory state.
@@ -88,10 +95,10 @@ def load(self) -> LedgerState:
                     data = json.load(f)
 
                 data["history"] = self._parse_history(data)
-
                 self._state = LedgerState(**data)
 
-            except Exception:
+            except Exception as e:
+                self._last_load_error = f"{type(e).__name__}: {e}"
                 self._state = LedgerState()
 
         return self._state
@@ -100,6 +107,7 @@ async def load_async(self) -> LedgerState:
         """
         Async load for WASM environments using IndexedDB.
         """
+        self._last_load_error = None
 
         if not self.is_wasm:
             return self.load()
@@ -187,14 +195,12 @@ async def load_async(self) -> LedgerState:
 
             if raw:
                 data = json.loads(raw)
-
                 data["history"] = self._parse_history(data)
-
                 self._state = LedgerState(**data)
 
         except Exception as e:
+            self._last_load_error = f"{type(e).__name__}: {e}"
             print(f"Failed to load from IndexedDB: {e}")
-
             self._state = LedgerState()
 
         return self._state

mlsysim/tests/test_state.py

@@ -1,4 +1,5 @@
-"""Tests for DesignLedger persistence (mlsysim/labs/state.py).
+"""
+Tests for DesignLedger persistence (mlsysim/labs/state.py).
 
 Covers the WASM background-save failure path fixed in #1985: previously
 `save()` used `asyncio.create_task(...)` fire-and-forget, so IndexedDB
@@ -11,7 +12,10 @@
 
 import pytest
 
-from mlsysim.labs.state import DesignLedger
+import json
+from pathlib import Path
+
+from mlsysim.labs.state import DesignLedger, LedgerState
 import mlsysim.labs.state as state_mod
 
 
@@ -122,3 +126,94 @@ async def run():
             await ledger.asave(step=2, design={"x": 1})
 
     asyncio.run(run())
+
+
+"""--- Read-path error handling (#1994) ---
+Native/local-filesystem path only. WASM/IndexedDB is covered separately in labs/tests/test_wasm_persistence.py."""
+
+def test_init_does_not_raise(tmp_path, monkeypatch):
+    """Regression guard: last_load_error is a read-only @property backed
+    by _last_load_error. Assigning self.last_load_error = ... anywhere
+    (including __init__) raises AttributeError immediately, since the
+    property has no setter. This is the exact bug that would otherwise
+    only surface at runtime, not at review time."""
+    _ledger_with_home(monkeypatch, tmp_path)  # must not raise
+
+
+def test_load_missing_file_is_not_an_error(tmp_path, monkeypatch):
+    """First run for a student -- no ledger.json exists yet. This is
+        expected, not a failure, and must not populate last_load_error."""
+    ledger = _ledger_with_home(monkeypatch, tmp_path)
+    state = ledger.load()
+    assert isinstance(state, LedgerState)
+    assert ledger.last_load_error is None
+
+
+def test_load_corrupt_file_sets_last_load_error(tmp_path, monkeypatch):
+    """A save file exists but isn't valid JSON -- e.g. truncated by a
+    crash mid-write. Must fail safe (blank state, no crash) but the
+    failure must be visible via last_load_error, not silently discarded."""
+    ledger = _ledger_with_home(monkeypatch, tmp_path)
+    ledger.config_dir.mkdir(exist_ok=True)
+    ledger.file_path.write_text("{not valid json")
+
+    state = ledger.load()
+
+    assert isinstance(state, LedgerState)
+    assert ledger.last_load_error is not None
+    assert "json" in ledger.last_load_error.lower()
+
+
+def test_load_corrupt_file_resets_to_blank_state(tmp_path, monkeypatch):
+    """A corrupt file must not leave stale/partial in-memory state around
+    -- the fallback is a fresh LedgerState(), not a half-populated one."""
+    ledger = _ledger_with_home(monkeypatch, tmp_path)
+    ledger.config_dir.mkdir(exist_ok=True)
+    ledger.file_path.write_text("{not valid json")
+
+    state = ledger.load()
+
+    assert state.track is None
+    assert state.current_step == 0
+    assert state.history == {}
+
+
+def test_load_valid_file_clears_previous_error(tmp_path, monkeypatch):
+    """last_load_error must reset on a subsequent successful load --
+    it's a snapshot of the *most recent* attempt, not sticky forever."""
+    ledger = _ledger_with_home(monkeypatch, tmp_path)
+    ledger.config_dir.mkdir(exist_ok=True)
+    ledger.file_path.write_text("{not valid json")
+    ledger.load()
+    assert ledger.last_load_error is not None
+
+    ledger.file_path.write_text(json.dumps({
+        "track": "edge",
+        "current_step": 3,
+        "history": {},
+        "last_updated": "2026-08-10T00:00:00",
+    }))
+    state = ledger.load()
+
+    assert ledger.last_load_error is None
+    assert state.track == "edge"
+    assert state.current_step == 3
+
+
+def test_load_valid_file_round_trips_history(tmp_path, monkeypatch):
+    """Sanity check that the happy path (already-existing behavior)
+    wasn't broken by the error-handling changes."""
+    ledger = _ledger_with_home(monkeypatch, tmp_path)
+    ledger.config_dir.mkdir(exist_ok=True)
+    ledger.file_path.write_text(json.dumps({
+        "track": "cloud",
+        "current_step": 5,
+        "history": {"1": {"choice": "gpu"}, "5": {"choice": "spot"}},
+        "last_updated": "2026-08-10T00:00:00",
+    }))
+
+    state = ledger.load()
+
+    assert ledger.last_load_error is None
+    assert state.history[1] == {"choice": "gpu"}
+    assert state.history[5] == {"choice": "spot"}