Claude PR review

Author: TomNaessens

src/backend/workers/python/papyros/papyros.py

@@ -17,98 +17,14 @@
 from pyodide_worker_runner import install_imports
 from pyodide.ffi import JsException, create_proxy
 from .util import to_py
+from .turtle_hook import TurtleImportHook
 from pyodide.http import pyfetch
 from types import ModuleType
 
 SYS_RECURSION_LIMIT = 500
 MODULE_NAME = "sandbox"
 
 
-class _TurtleImportHook:
-    """Import hook that lazily sets up SVG-based turtle graphics.
-
-    Installed in sys.meta_path. When user code does `import turtle`,
-    this hook intercepts it, imports svg-turtle, creates a shared
-    canvas/screen, and patches the turtle module to render SVG.
-
-    If user code never imports turtle, no setup occurs.
-    """
-
-    def __init__(self):
-        self.papyros = None
-        self.render = None
-        self._loading = False
-        self._turtle_module = None
-
-    def find_spec(self, name, path, target=None):
-        if name == 'turtle' and not self._loading:
-            import importlib.util
-            return importlib.util.spec_from_loader(name, self)
-        return None
-
-    def create_module(self, spec):
-        # Return the patched turtle module directly
-        return self._setup_turtle()
-
-    def exec_module(self, module):
-        # Module was already set up in create_module
-        pass
-
-    def _setup_turtle(self):
-        self._loading = True
-        try:
-            from svg_turtle import SvgTurtle
-            from svg_turtle.canvas import Canvas
-
-            if self._turtle_module is None:
-                # First import: svg_turtle stubs tkinter, then imports turtle
-                self._turtle_module = sys.modules.get('turtle')
-                if self._turtle_module is None:
-                    import turtle
-                    self._turtle_module = sys.modules['turtle']
-
-            turtle_mod = self._turtle_module
-            sys.modules['turtle'] = turtle_mod
-
-            # Fresh canvas and screen for this execution
-            canvas = Canvas(400, 400)
-            screen = SvgTurtle._Screen(canvas)
-            screen.cv.config(bg="")
-
-            class PapyrosTurtle(SvgTurtle):
-                def __init__(self):
-                    super().__init__(screen=screen)
-
-            SvgTurtle._screen = screen
-            SvgTurtle._pen = PapyrosTurtle()
-
-            rendered = [False]
-            papyros = self.papyros
-
-            def render():
-                if rendered[0]:
-                    return
-                rendered[0] = True
-                svg_string = SvgTurtle._pen.to_svg()
-                if svg_string:
-                    img = base64.b64encode(svg_string.encode("utf-8")).decode("utf-8")
-                    papyros.output("img", img, contentType="img/svg+xml;base64")
-
-            turtle_mod.Turtle = PapyrosTurtle
-            turtle_mod.done = render
-            turtle_mod.mainloop = render
-            turtle_mod.exitonclick = render
-            turtle_mod.bye = render
-
-            self.render = render
-            return turtle_mod
-        except Exception:
-            self.render = None
-            return None
-        finally:
-            self._loading = False
-
-
 class Papyros(python_runner.PyodideRunner):
     def __init__(
         self,
@@ -184,7 +100,7 @@ def _render_turtle(self):
 
     def override_turtle(self):
         if not hasattr(self, '_turtle_hook'):
-            self._turtle_hook = _TurtleImportHook()
+            self._turtle_hook = TurtleImportHook()
 
         hook = self._turtle_hook
         hook.papyros = self
@@ -345,9 +261,8 @@ async def run_async(self, source_code, mode="exec", top_level_await=True):
                         def frame_callback(frame):
                             self._flush_open_files()
                             self._emit_created_files(emit_empty=True)
-                            # Progressive turtle rendering: only emit SVG when drawing actually changed.
-                            # Non-drawing commands (pencolor, left, etc.) don't alter the SVG output,
-                            # so the string comparison naturally skips them without needing a hard cap.
+                            # The hook fires lazily when user code runs `import turtle`, so the
+                            # pen only exists partway through the trace — check per frame.
                             hook = getattr(self, '_turtle_hook', None)
                             if hook and hook.render:
                                 try:

src/backend/workers/python/papyros/turtle_hook.py

@@ -0,0 +1,85 @@
+import base64
+import sys
+
+
+class TurtleImportHook:
+    """Import hook that lazily sets up SVG-based turtle graphics.
+
+    Installed in sys.meta_path. When user code does `import turtle`,
+    this hook intercepts it, imports svg-turtle, creates a shared
+    canvas/screen, and patches the turtle module to render SVG.
+
+    If user code never imports turtle, no setup occurs.
+    """
+
+    def __init__(self):
+        self.papyros = None
+        self.render = None
+        self._loading = False
+        self._turtle_module = None
+
+    def find_spec(self, name, path, target=None):
+        if name == 'turtle' and not self._loading:
+            import importlib.util
+            return importlib.util.spec_from_loader(name, self)
+        return None
+
+    def create_module(self, spec):
+        return self._setup_turtle()
+
+    def exec_module(self, module):
+        pass
+
+    def _setup_turtle(self):
+        self._loading = True
+        try:
+            from svg_turtle import SvgTurtle
+            from svg_turtle.canvas import Canvas
+
+            if self._turtle_module is None:
+                # First import: svg_turtle stubs tkinter, then imports turtle
+                self._turtle_module = sys.modules.get('turtle')
+                if self._turtle_module is None:
+                    import turtle
+                    self._turtle_module = sys.modules['turtle']
+
+            turtle_mod = self._turtle_module
+            sys.modules['turtle'] = turtle_mod
+
+            # Fresh canvas and screen for this execution
+            canvas = Canvas(400, 400)
+            screen = SvgTurtle._Screen(canvas)
+            screen.cv.config(bg="")
+
+            class PapyrosTurtle(SvgTurtle):
+                def __init__(self):
+                    super().__init__(screen=screen)
+
+            SvgTurtle._screen = screen
+            SvgTurtle._pen = PapyrosTurtle()
+
+            rendered = [False]
+            papyros = self.papyros
+
+            def render():
+                if rendered[0]:
+                    return
+                rendered[0] = True
+                svg_string = SvgTurtle._pen.to_svg()
+                if svg_string:
+                    img = base64.b64encode(svg_string.encode("utf-8")).decode("utf-8")
+                    papyros.output("img", img, contentType="img/svg+xml;base64")
+
+            turtle_mod.Turtle = PapyrosTurtle
+            turtle_mod.done = render
+            turtle_mod.mainloop = render
+            turtle_mod.exitonclick = render
+            turtle_mod.bye = render
+
+            self.render = render
+            return turtle_mod
+        except Exception:
+            self.render = None
+            return None
+        finally:
+            self._loading = False

src/frontend/components/Output.ts

@@ -99,9 +99,7 @@ export class Output extends PapyrosElement {
     get renderedOutputs(): TemplateResult[] {
         let outputsToRender = this.outputs;
         if (this.papyros.debugger.active) {
-            // In debug mode, only show the last SVG image for progressive turtle rendering.
-            // Multiple SVG snapshots are emitted during debug (one per turtle state change),
-            // but only the latest one visible at the current frame should be displayed.
+            // Only show the latest SVG snapshot so the debugger slider shows progressive rendering.
             const lastSvgIdx = outputsToRender.findLastIndex(
                 (o) => o.type === OutputType.img && o.contentType?.includes("svg"),
             );