Execute REPL code like a notebook cell; the last expression is the result

Code is now executed at module level in the session's namespace (statements executed, a trailing
expression evaluated as the result), replacing the function wrapper with global declarations that
persistence had required. This yields notebook semantics throughout: top-level bindings persist by
construction, functions resolve names from the session namespace, and `return` is no longer used
(a top-level `return` yields a syntax error with a hint). Tool description and tests updated.

.serena/memories/repl.md

@@ -1,8 +1,9 @@
 # REPL (`serena.repl`)
 
 Alternative interaction paradigm: one tool (`serena_repl`) executes Python code against entrypoint `s`,
-whose attributes are facades (`s.lsp`, `s.edit`, `s.fs`, `s.mem`, `s.shell`, `s.jb`).
-Code runs as a function body (`return` defines the result); a single expression is evaluated directly.
+whose attributes are facades (`s.lsp`, `s.edit`, `s.fs`, `s.mem`, `s.shell`, `s.jb`, `s.cfg`).
+Code runs like a notebook cell (module-level exec in the session namespace); the value of a trailing
+expression is the result. No `return` (a top-level `return` yields a SyntaxError with a hint).
 
 ## Structure
 
@@ -55,11 +56,10 @@ Code runs as a function body (`return` defines the result); a single expression
   a `SerenaSession` and states its id; `serena_repl` takes a required `session_id`. `SessionRegistry` creates
   unknown ids on demand (benign: at worst docs are repeated) and evicts LRU and idle (TTL) sessions.
   Sessions survive REPL rebuilds.
-- Persistence (notebook semantics): `SerenaSession.repl_namespace` is the globals of the session's executions;
-  names bound at the top level of submitted code (assignments, def/class, imports, loop/with targets, walrus)
-  persist across calls (an AST pass wraps the code in a function and declares those names `global`, preserving
-  line numbers). `s` is re-bound in the namespace before every execution, so persisted functions always use the
-  current entrypoint (no closure over `s`). `s.vars()`/`s.clear()` list/remove persisted items. Data is tied to
+- Persistence (notebook semantics): `SerenaSession.repl_namespace` is the globals of the session's executions,
+  which run at module level (statements exec'd, a trailing expression eval'd; line numbers preserved), so all
+  top-level bindings persist across calls. `s` is re-bound in the namespace before every execution, so persisted
+  functions always use the current entrypoint. `s.vars()`/`s.clear()` list/remove persisted items. Data is tied to
   the session's lifetime (not cleared on REPL rebuild); stored facades/project objects may go stale.
   Deferred idea if memory becomes an issue: hybrid — implicit items expire after N turns, explicit store
   (e.g. `s.d`) for indefinite retention.

src/serena/repl/repl.py

@@ -200,17 +200,15 @@ class SerenaRepl:
     Executes Python code submitted by an LLM, binding the configured facades to the entrypoint object `s`
     and rendering the result of the execution as a string for the LLM.
 
-    The code is executed as the body of a function, such that the `return` statement defines the result;
-    code consisting of a single expression is evaluated and its value is the result.
-    Names assigned at the top level of the code (variables, functions, classes, imports) persist in the session's
-    namespace across executions (like the cells of a notebook), which serves as the globals of the executions.
+    The code is executed like the cell of a notebook: it is executed at module level in the session's namespace,
+    such that the names it binds (variables, functions, classes, imports) persist across executions, and if its last
+    statement is an expression, the expression's value is the result of the execution.
     The entrypoint `s` is (re)bound in the namespace before every execution, such that persisted functions always
     access the current entrypoint.
     """
 
     SOURCE_NAME = "<serena_repl>"
     ENTRYPOINT_NAME = "s"
-    _FUNCTION_NAME = "__serena_repl_fn__"
     _PERSISTED_NAME_PATTERN = re.compile(r"^(?!__)[A-Za-z_]\w*$")
 
     @classmethod
@@ -272,92 +270,26 @@ class SerenaRepl:
 
     def _run(self, code: str, namespace: dict[str, Any]) -> Any:
         """
-        Runs the given code in the given namespace (as globals) with the entrypoint bound, either as a single expression
-        or as the body of a function whose return value is the result and whose top-level assignments are made global
-        (such that they persist in the namespace).
+        Runs the given code at module level in the given namespace (as globals) with the entrypoint bound.
+
+        :return: the value of the code's last statement if it is an expression, None otherwise
         """
         namespace[self.ENTRYPOINT_NAME] = self._entrypoint
-
-        # try to evaluate the code as a single expression
-        try:
-            compiled = compile(code, self.SOURCE_NAME, "eval")
-        except SyntaxError:
-            compiled = None
-        if compiled is not None:
-            return eval(compiled, namespace)
-
-        # otherwise execute the code as the body of a function, declaring the names assigned at the top level as global.
-        # The function is constructed at the AST level, such that the line numbers of the code are preserved.
         module = ast.parse(code, self.SOURCE_NAME)
-        body: list[ast.stmt] = list(module.body)
-        assigned_names = self._collect_top_level_assigned_names(body)
-        if assigned_names:
-            body.insert(0, ast.Global(names=sorted(assigned_names)))
-        function = ast.FunctionDef(
-            name=self._FUNCTION_NAME,
-            args=ast.arguments(posonlyargs=[], args=[], kwonlyargs=[], kw_defaults=[], defaults=[]),
-            body=body,
-            decorator_list=[],
-            returns=None,
-        )
-        wrapper = ast.fix_missing_locations(ast.Module(body=[function], type_ignores=[]))
-        exec(compile(wrapper, self.SOURCE_NAME, "exec"), namespace)
-        try:
-            return namespace[self._FUNCTION_NAME]()
-        finally:
-            del namespace[self._FUNCTION_NAME]
 
-    @classmethod
-    def _collect_top_level_assigned_names(cls, statements: list[ast.stmt]) -> set[str]:
-        """
-        :param statements: the top-level statements of the code
-        :return: the names bound by the statements (assignment targets, function/class definitions, imports,
-            loop/with targets, deletions and walrus assignments outside of nested scopes)
-        """
-        names: set[str] = set()
+        # separate a trailing expression, whose value is the result
+        statements = module.body
+        trailing_expression: ast.expr | None = None
+        if statements and isinstance(statements[-1], ast.Expr):
+            trailing_expression = statements[-1].value
+            statements = statements[:-1]
 
-        def add_target(target: ast.expr) -> None:
-            if isinstance(target, ast.Name):
-                names.add(target.id)
-            elif isinstance(target, ast.Tuple | ast.List):
-                for element in target.elts:
-                    add_target(element)
-            elif isinstance(target, ast.Starred):
-                add_target(target.value)
-
-        def add_walrus_targets(node: ast.AST) -> None:
-            # walrus assignments bind in the enclosing scope, unless within a nested scope
-            for child in ast.iter_child_nodes(node):
-                if isinstance(child, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef | ast.Lambda):
-                    continue
-                if isinstance(child, ast.NamedExpr):
-                    add_target(child.target)
-                add_walrus_targets(child)
-
-        for statement in statements:
-            match statement:
-                case ast.Assign(targets=targets):
-                    for target in targets:
-                        add_target(target)
-                case ast.AnnAssign(target=target) | ast.AugAssign(target=target):
-                    add_target(target)
-                case ast.FunctionDef(name=name) | ast.AsyncFunctionDef(name=name) | ast.ClassDef(name=name):
-                    names.add(name)
-                case ast.Import(names=aliases) | ast.ImportFrom(names=aliases):
-                    for alias in aliases:
-                        if alias.name != "*":
-                            names.add(alias.asname or alias.name.split(".")[0])
-                case ast.For(target=target) | ast.AsyncFor(target=target):
-                    add_target(target)
-                case ast.With(items=items) | ast.AsyncWith(items=items):
-                    for item in items:
-                        if item.optional_vars is not None:
-                            add_target(item.optional_vars)
-                case ast.Delete(targets=targets):
-                    for target in targets:
-                        add_target(target)
-            add_walrus_targets(statement)
-        return names
+        # execute the statements, then evaluate the trailing expression (both retain the original line numbers)
+        if statements:
+            exec(compile(ast.Module(body=statements, type_ignores=[]), self.SOURCE_NAME, "exec"), namespace)
+        if trailing_expression is not None:
+            return eval(compile(ast.Expression(body=trailing_expression), self.SOURCE_NAME, "eval"), namespace)
+        return None
 
     def _format_error(self, e: Exception, code: str) -> str:
         """
@@ -373,7 +305,10 @@ class SerenaRepl:
 
         # report syntax errors in the executed code (which carry no traceback frames of their own)
         if isinstance(e, SyntaxError) and e.filename == self.SOURCE_NAME and e.lineno is not None:
-            return f"SyntaxError: {e.msg}\n" + location_line(e.lineno)
+            message = f"SyntaxError: {e.msg}\n" + location_line(e.lineno)
+            if "return" in (e.msg or "") and "outside function" in e.msg:
+                message += "\nNote: the code is executed like a notebook cell; the value of the last expression is the result (do not use `return`)."
+            return message
 
         # report runtime errors, locating them within the executed code
         location_lines = []

src/serena/tools/repl_tools.py

@@ -27,12 +27,11 @@ class SerenaReplTool(Tool, ToolMarkerOptional, ToolMarkerBeta):
         `s.info("<facade>")` documents the facade's operations only, not their result types. Result types are given
         in the method listing below (`method -> Type`); request their documentation via `s.info("<Type>")`, which
         includes the types they contain, ONLY if you intend to process results in code (filter, aggregate, chain
-        calls). If you simply want the result, return it directly: returned objects are rendered for you.
+        calls). If you simply want the result, make it the last expression: results are rendered for you.
 
-        The code is executed as the body of a function, so use `return` to define the result;
-        a single expression is evaluated and its value returned directly.
-        Returned objects are rendered in a form suitable for you; lists are rendered element-wise.
-        Returned strings are passed through unchanged.
+        The code is executed like a notebook cell: if its last statement is an expression, the expression's value is
+        the result (do not use `return`). Results are rendered in a form suitable for you; lists are rendered
+        element-wise, strings are passed through unchanged.
 
         Output size: methods with a `max_answer_chars` parameter limit the size of the rendered result (-1 uses the
         configured default). If the limit is exceeded, a shortened result (or no content) is rendered instead;

test/serena/test_repl_tool.py

@@ -25,14 +25,21 @@ class TestReplExecution:
     def repl(self) -> SerenaRepl:
         return SerenaRepl([Facade.from_api(LspApi(MagicMock()), ApiScope())], ApiScope())
 
-    def test_return_statement_defines_result(self, repl: SerenaRepl) -> None:
-        assert repl.execute("x = 20\ny = 22\nreturn x + y") == "42"
+    def test_last_expression_defines_result(self, repl: SerenaRepl) -> None:
+        assert repl.execute("x = 20\ny = 22\nx + y") == "42"
+        assert repl.execute("x = 20\ny = 22") == "None"  # no trailing expression
+        assert repl.execute("") == "None"
+
+    def test_return_yields_a_hint(self, repl: SerenaRepl) -> None:
+        result = repl.execute("x = 1\nreturn x")
+        assert result.startswith("SyntaxError") and "line 2" in result
+        assert "last expression is the result" in result
 
     def test_single_expression_is_evaluated(self, repl: SerenaRepl) -> None:
         assert repl.execute("1 + 2") == "3"
 
     def test_list_is_rendered_element_wise(self, repl: SerenaRepl) -> None:
-        assert repl.execute('return ["a", "b"]') == "a\nb"
+        assert repl.execute('["a", "b"]') == "a\nb"
 
     def test_error_reports_type_message_and_line(self, repl: SerenaRepl) -> None:
         result = repl.execute("x = 1\nraise ValueError('boom')")
@@ -47,7 +54,7 @@ class TestReplExecution:
     def test_top_level_names_persist_within_session(self, repl: SerenaRepl) -> None:
         session = SerenaSession("a")
         repl.execute("x = 20\ndef double(v):\n    return 2 * v\nfor i in range(3):\n    pass\nimport os as os_module", session)
-        assert repl.execute("return double(x) + i", session) == "42"
+        assert repl.execute("double(x) + i", session) == "42"
         assert repl.execute("os_module.sep is not None", session) == "True"
 
         # local variables of nested scopes do not persist; persisted items can be listed and cleared
@@ -56,23 +63,23 @@ class TestReplExecution:
         assert "x: int = 20" in listing and "double: function" in listing
         assert repl.execute("s.clear()", session) == "Removed 4 persisted item(s)."
         assert repl.execute("s.vars()", session) == "No persisted variables."
-        assert "NameError" in repl.execute("return x", session)
+        assert "NameError" in repl.execute("x", session)
 
     def test_sessions_have_separate_namespaces(self, repl: SerenaRepl) -> None:
         session_a = SerenaSession("a")
         repl.execute("x = 1", session_a)
-        assert "NameError" in repl.execute("return x", SerenaSession("b"))
-        assert repl.execute("return x", session_a) == "1"
+        assert "NameError" in repl.execute("x", SerenaSession("b"))
+        assert repl.execute("x", session_a) == "1"
         # ... and execution without a session persists nothing
         repl.execute("y = 1")
-        assert "NameError" in repl.execute("return y")
+        assert "NameError" in repl.execute("y")
 
     def test_persisted_functions_use_the_current_entrypoint(self) -> None:
         # a function defined against one REPL instance uses the entrypoint of the REPL that later calls it
         session = SerenaSession("a")
         SerenaRepl([Facade.from_api(LspApi(MagicMock()), ApiScope())], ApiScope()).execute("def facades():\n    return s.info()", session)
         rebuilt_repl = SerenaRepl([Facade.from_api(EditApi(MagicMock()), ApiScope())], ApiScope())
-        overview = rebuilt_repl.execute("return facades()", session)
+        overview = rebuilt_repl.execute("facades()", session)
         assert "s.edit" in overview and "s.lsp" not in overview
 
     def test_facade_discovery(self, repl: SerenaRepl) -> None:
@@ -298,13 +305,13 @@ class TestLspFacade:
             session_id = agent.create_session().session_id
 
             # a returned collection is rendered, identifying the symbol and its file
-            rendered = tool.apply(session_id, 'return s.lsp.find_symbol("create_user")')
+            rendered = tool.apply(session_id, 's.lsp.find_symbol("create_user")')
             assert "create_user" in rendered
             assert "services.py" in rendered
 
             # the underlying symbols are accessible from code, e.g. to retrieve a body without rendering the collection
             body = tool.apply(
                 session_id,
-                f'result = s.lsp.find_symbol("create_user", relative_path={self._SERVICES_FILE!r})\nreturn result.symbols[0].body',
+                f'result = s.lsp.find_symbol("create_user", relative_path={self._SERVICES_FILE!r})\nresult.symbols[0].body',
             )
             assert body.startswith("def create_user")