diff --git a/.serena/memories/repl.md b/.serena/memories/repl.md index 2a27e1e1..0b67db46 100644 --- a/.serena/memories/repl.md +++ b/.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. diff --git a/src/serena/repl/repl.py b/src/serena/repl/repl.py index 0b8a4f2e..b631777b 100644 --- a/src/serena/repl/repl.py +++ b/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 = "" 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 = [] diff --git a/src/serena/tools/repl_tools.py b/src/serena/tools/repl_tools.py index 72633ae5..04b6895d 100644 --- a/src/serena/tools/repl_tools.py +++ b/src/serena/tools/repl_tools.py @@ -27,12 +27,11 @@ class SerenaReplTool(Tool, ToolMarkerOptional, ToolMarkerBeta): `s.info("")` 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("")`, 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; diff --git a/test/serena/test_repl_tool.py b/test/serena/test_repl_tool.py index fdd46b62..4b7b348e 100644 --- a/test/serena/test_repl_tool.py +++ b/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")