Address review: bounded caches, MP4 cleanup, avoid double-read

- interactive.py/tui.py: cap _cache at 50 entries (FIFO eviction) to prevent
  OOM in long sessions loading many large timesteps
- renderer.py: wrap frame-rendering loop in try/finally so temp dir is always
  cleaned up even if a render_* call raises (e.g. ValueError from render_3d_slice)
- viz.py: reuse test_assembled for first step instead of reading it twice

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: sbryngelson

toolchain/mfc/viz/interactive.py

@@ -49,14 +49,20 @@
 _YELLOW = '#f9e2af'
 
 # ---------------------------------------------------------------------------
-# Server-side data cache  {step -> AssembledData}
+# Server-side data cache  {step -> AssembledData}  (bounded to avoid OOM)
 # ---------------------------------------------------------------------------
+_CACHE_MAX = 50
 _cache: dict = {}
+_cache_order: list = []
 
 
 def _load(step: int, read_func: Callable):
     if step not in _cache:
+        if len(_cache) >= _CACHE_MAX:
+            evict = _cache_order.pop(0)
+            _cache.pop(evict, None)
         _cache[step] = read_func(step)
+        _cache_order.append(step)
     return _cache[step]
 
 

toolchain/mfc/viz/renderer.py

@@ -338,42 +338,57 @@ def render_mp4(varname, steps, output, fps=10,  # pylint: disable=too-many-argum
     os.makedirs(output_dir, exist_ok=True)
     viz_dir = tempfile.mkdtemp(dir=output_dir, prefix='_frames_')
 
+    def _cleanup():
+        for fname in sorted(f for f in os.listdir(viz_dir) if f.endswith('.png')):
+            try:
+                os.remove(os.path.join(viz_dir, fname))
+            except OSError:
+                pass
+        try:
+            os.rmdir(viz_dir)
+        except OSError:
+            pass
+
     try:
         from tqdm import tqdm  # pylint: disable=import-outside-toplevel
         step_iter = tqdm(steps, desc='Rendering frames')
     except ImportError:
         step_iter = steps
 
-    for i, step in enumerate(step_iter):
-        assembled = read_func(step)
-        frame_path = os.path.join(viz_dir, f'{i:06d}.png')
-
-        if tiled and assembled.ndim == 1:
-            render_1d_tiled(assembled.x_cc, assembled.variables,
-                            step, frame_path, **opts)
-        elif assembled.ndim == 1:
-            var_data = assembled.variables.get(varname)
-            if var_data is None:
-                continue
-            render_1d(assembled.x_cc, var_data,
-                      varname, step, frame_path, **opts)
-        elif assembled.ndim == 2:
-            var_data = assembled.variables.get(varname)
-            if var_data is None:
-                continue
-            render_2d(assembled.x_cc, assembled.y_cc,
-                      var_data,
-                      varname, step, frame_path, **opts)
-        elif assembled.ndim == 3:
-            var_data = assembled.variables.get(varname)
-            if var_data is None:
-                continue
-            render_3d_slice(assembled, varname, step, frame_path, **opts)
-        else:
-            raise ValueError(
-                f"Unsupported dimensionality ndim={assembled.ndim} for step {step}. "
-                "Expected 1, 2, or 3."
-            )
+    try:
+        for i, step in enumerate(step_iter):
+            assembled = read_func(step)
+            frame_path = os.path.join(viz_dir, f'{i:06d}.png')
+
+            if tiled and assembled.ndim == 1:
+                render_1d_tiled(assembled.x_cc, assembled.variables,
+                                step, frame_path, **opts)
+            elif assembled.ndim == 1:
+                var_data = assembled.variables.get(varname)
+                if var_data is None:
+                    continue
+                render_1d(assembled.x_cc, var_data,
+                          varname, step, frame_path, **opts)
+            elif assembled.ndim == 2:
+                var_data = assembled.variables.get(varname)
+                if var_data is None:
+                    continue
+                render_2d(assembled.x_cc, assembled.y_cc,
+                          var_data,
+                          varname, step, frame_path, **opts)
+            elif assembled.ndim == 3:
+                var_data = assembled.variables.get(varname)
+                if var_data is None:
+                    continue
+                render_3d_slice(assembled, varname, step, frame_path, **opts)
+            else:
+                raise ValueError(
+                    f"Unsupported dimensionality ndim={assembled.ndim} for step {step}. "
+                    "Expected 1, 2, or 3."
+                )
+    except Exception:
+        _cleanup()
+        raise
 
     # Combine frames into MP4 using imageio + imageio-ffmpeg (bundled ffmpeg)
     frame_files = sorted(f for f in os.listdir(viz_dir) if f.endswith('.png'))
@@ -390,14 +405,5 @@ def render_mp4(varname, steps, output, fps=10,  # pylint: disable=too-many-argum
     except (OSError, ValueError, RuntimeError) as exc:
         print(f"imageio MP4 write failed: {exc}")
     finally:
-        # Always clean up temporary frame files
-        for fname in frame_files:
-            try:
-                os.remove(os.path.join(viz_dir, fname))
-            except OSError:
-                pass
-        try:
-            os.rmdir(viz_dir)
-        except OSError:
-            pass
+        _cleanup()
     return success

toolchain/mfc/viz/tui.py

@@ -38,14 +38,20 @@
 ]
 
 # ---------------------------------------------------------------------------
-# Step cache  {step -> AssembledData}
+# Step cache  {step -> AssembledData}  (bounded to avoid OOM)
 # ---------------------------------------------------------------------------
+_CACHE_MAX = 50
 _cache: Dict[int, object] = {}
+_cache_order: List[int] = []
 
 
 def _load(step: int, read_func: Callable) -> object:
     if step not in _cache:
+        if len(_cache) >= _CACHE_MAX:
+            evict = _cache_order.pop(0)
+            _cache.pop(evict, None)
         _cache[step] = read_func(step)
+        _cache_order.append(step)
     return _cache[step]
 
 

toolchain/mfc/viz/viz.py

@@ -302,7 +302,8 @@ def read_step(step):
     label = 'all' if tiled else varname
     for step in step_iter:
         try:
-            assembled = read_step(step)
+            # Reuse the already-loaded probe data for the first step
+            assembled = test_assembled if step == requested_steps[0] else read_step(step)
         except (FileNotFoundError, EOFError, ValueError) as exc:
             cons.print(f"[yellow]Warning:[/yellow] Skipping step {step}: {exc}")
             failures.append(step)