From 4c4806c8dd169948b904708949185f71fff067a8 Mon Sep 17 00:00:00 2001 From: giles Date: Wed, 11 Mar 2026 09:42:04 +0000 Subject: [PATCH] Fix all 9 spec test failures: Env scope chain, IO detection, offline mutation - env.py: Add MergedEnv with dual-parent lookup (primary for set!, secondary for reads), add dict-compat methods to Env - platform_py.py: make_lambda stores env reference (no copy), env_merge uses MergedEnv for proper set! propagation, ancestor detection prevents unbounded chains in TCO recursion, sf_set_bang walks scope chain - types.py: Component/Island io_refs defaults to None (not computed) instead of empty set, so component-pure? falls through to scan - run.py: Test env uses Env class, mock execute-action calls SX lambdas via _call_sx instead of direct Python call Spec tests: 320/320 (was 311/320) Co-Authored-By: Claude Opus 4.6 --- shared/sx/env.py | 77 +++++++++++++++++++++++++++++++++++- shared/sx/ref/platform_py.py | 59 ++++++++++++++++++++++----- shared/sx/ref/sx_ref.py | 59 ++++++++++++++++++++++----- shared/sx/tests/run.py | 8 ++-- shared/sx/types.py | 4 +- 5 files changed, 180 insertions(+), 27 deletions(-) diff --git a/shared/sx/env.py b/shared/sx/env.py index 2aac736..68e0962 100644 --- a/shared/sx/env.py +++ b/shared/sx/env.py @@ -20,7 +20,7 @@ class Env: bindings: dict[str, Any] | None = None, parent: Env | None = None, ): - self._bindings: dict[str, Any] = bindings or {} + self._bindings: dict[str, Any] = {} if bindings is None else bindings self._parent = parent # -- lookup ------------------------------------------------------------- @@ -46,12 +46,30 @@ class Env: def __getitem__(self, name: str) -> Any: return self.lookup(name) + def __setitem__(self, name: str, value: Any) -> None: + """Set *name* in the **current** scope (like ``define``).""" + self._bindings[name] = value + def get(self, name: str, default: Any = None) -> Any: try: return self.lookup(name) except KeyError: return default + def update(self, other: dict[str, Any] | Env) -> None: + """Merge *other*'s bindings into the **current** scope.""" + if isinstance(other, Env): + self._bindings.update(other._bindings) + else: + self._bindings.update(other) + + def keys(self): + """All keys visible from this scope (current + parents).""" + return self.to_dict().keys() + + def __iter__(self): + return iter(self.to_dict()) + # -- mutation ----------------------------------------------------------- def define(self, name: str, value: Any) -> None: @@ -74,7 +92,7 @@ class Env: def extend(self, bindings: dict[str, Any] | None = None) -> Env: """Return a child environment.""" - return Env(bindings or {}, parent=self) + return Env({} if bindings is None else bindings, parent=self) # -- conversion --------------------------------------------------------- @@ -95,3 +113,58 @@ class Env: depth += 1 p = p._parent return f"" + + +class MergedEnv(Env): + """Env with two parent chains: primary (closure) and secondary (caller). + + Reads walk: local bindings → primary chain → secondary chain. + set! walks: local bindings → primary chain (skips secondary). + This allows set! to modify variables in the defining scope (closure) + without being confused by overlay copies from the calling scope. + """ + + __slots__ = ("_secondary",) + + def __init__( + self, + bindings: dict[str, Any] | None = None, + primary: Env | None = None, + secondary: Env | None = None, + ): + super().__init__(bindings, parent=primary) + self._secondary = secondary + + def lookup(self, name: str) -> Any: + try: + return super().lookup(name) + except KeyError: + if self._secondary is not None: + return self._secondary.lookup(name) + raise + + def __contains__(self, name: str) -> bool: + if super().__contains__(name): + return True + if self._secondary is not None: + return name in self._secondary + return False + + def get(self, name: str, default: Any = None) -> Any: + try: + return self.lookup(name) + except KeyError: + return default + + def to_dict(self) -> dict[str, Any]: + if self._secondary is not None: + d = self._secondary.to_dict() + else: + d = {} + if self._parent is not None: + d.update(self._parent.to_dict()) + d.update(self._bindings) + return d + + def extend(self, bindings: dict[str, Any] | None = None) -> Env: + return Env(bindings or {}, parent=self) diff --git a/shared/sx/ref/platform_py.py b/shared/sx/ref/platform_py.py index 0d08b78..c21be75 100644 --- a/shared/sx/ref/platform_py.py +++ b/shared/sx/ref/platform_py.py @@ -57,6 +57,7 @@ from shared.sx.types import ( HandlerDef, QueryDef, ActionDef, PageDef, _ShiftSignal, ) from shared.sx.parser import SxExpr +from shared.sx.env import Env as _Env, MergedEnv as _MergedEnv ''' # --------------------------------------------------------------------------- @@ -195,8 +196,15 @@ def make_keyword(n): return Keyword(n) +def _ensure_env(env): + """Wrap plain dict in Env if needed.""" + if isinstance(env, _Env): + return env + return _Env(env if isinstance(env, dict) else {}) + + def make_lambda(params, body, env): - return Lambda(params=list(params), body=body, closure=dict(env)) + return Lambda(params=list(params), body=body, closure=_ensure_env(env)) def make_component(name, params, has_children, body, env, affinity="auto"): @@ -490,13 +498,26 @@ def env_set(env, name, val): def env_extend(env): - return dict(env) + return _ensure_env(env).extend() def env_merge(base, overlay): - result = dict(base) - result.update(overlay) - return result + base = _ensure_env(base) + overlay = _ensure_env(overlay) + if base is overlay: + # Same env — just extend with empty local scope for params + return base.extend() + # Check if base is an ancestor of overlay — if so, no need to merge + # (common for self-recursive calls where closure == caller's ancestor) + p = overlay + depth = 0 + while p is not None and depth < 100: + if p is base: + return base.extend() + p = getattr(p, '_parent', None) + depth += 1 + # MergedEnv: reads walk base then overlay; set! walks base only + return _MergedEnv({}, primary=base, secondary=overlay) def dict_set(d, k, v): @@ -1022,8 +1043,10 @@ PLATFORM_DEPS_PY = ( ' return list(classes)\n' '\n' 'def component_io_refs(c):\n' - ' """Return cached IO refs list for a component (may be empty)."""\n' - ' return list(c.io_refs) if hasattr(c, "io_refs") and c.io_refs else []\n' + ' """Return cached IO refs list, or NIL if not yet computed."""\n' + ' if not hasattr(c, "io_refs") or c.io_refs is None:\n' + ' return NIL\n' + ' return list(c.io_refs)\n' '\n' 'def component_set_io_refs(c, refs):\n' ' """Cache IO refs on a component."""\n' @@ -1255,6 +1278,20 @@ def _wrap_aser_outputs(): return SxExpr(result) if isinstance(result, str) else result aser_call = _aser_call_wrapped aser_fragment = _aser_fragment_wrapped + + +# Override sf_set_bang to walk the Env scope chain so that (set! var val) +# updates the variable in its defining scope, not just the local copy. +def sf_set_bang(args, env): + name = symbol_name(first(args)) + value = trampoline(eval_expr(nth(args, 1), env)) + env = _ensure_env(env) + try: + env.set(name, value) + except KeyError: + # Not found in chain — define locally (matches prior behavior) + env[name] = value + return value ''' # --------------------------------------------------------------------------- @@ -1349,7 +1386,9 @@ def public_api_py(has_html: bool, has_sx: bool, has_deps: bool = False) -> str: 'def evaluate(expr, env=None):', ' """Evaluate expr in env and return the result."""', ' if env is None:', - ' env = {}', + ' env = _Env()', + ' elif isinstance(env, dict):', + ' env = _Env(env)', ' result = eval_expr(expr, env)', ' while is_thunk(result):', ' result = eval_expr(thunk_expr(result), thunk_env(result))', @@ -1369,8 +1408,8 @@ def public_api_py(has_html: bool, has_sx: bool, has_deps: bool = False) -> str: '', '', 'def make_env(**kwargs):', - ' """Create an environment dict with initial bindings."""', - ' return dict(kwargs)', + ' """Create an environment with initial bindings."""', + ' return _Env(dict(kwargs))', ]) return '\n'.join(lines) diff --git a/shared/sx/ref/sx_ref.py b/shared/sx/ref/sx_ref.py index 072a625..427dced 100644 --- a/shared/sx/ref/sx_ref.py +++ b/shared/sx/ref/sx_ref.py @@ -21,6 +21,7 @@ from shared.sx.types import ( HandlerDef, QueryDef, ActionDef, PageDef, _ShiftSignal, ) from shared.sx.parser import SxExpr +from shared.sx.env import Env as _Env, MergedEnv as _MergedEnv # ========================================================================= @@ -154,8 +155,15 @@ def make_keyword(n): return Keyword(n) +def _ensure_env(env): + """Wrap plain dict in Env if needed.""" + if isinstance(env, _Env): + return env + return _Env(env if isinstance(env, dict) else {}) + + def make_lambda(params, body, env): - return Lambda(params=list(params), body=body, closure=dict(env)) + return Lambda(params=list(params), body=body, closure=_ensure_env(env)) def make_component(name, params, has_children, body, env, affinity="auto"): @@ -449,13 +457,26 @@ def env_set(env, name, val): def env_extend(env): - return dict(env) + return _ensure_env(env).extend() def env_merge(base, overlay): - result = dict(base) - result.update(overlay) - return result + base = _ensure_env(base) + overlay = _ensure_env(overlay) + if base is overlay: + # Same env — just extend with empty local scope for params + return base.extend() + # Check if base is an ancestor of overlay — if so, no need to merge + # (common for self-recursive calls where closure == caller's ancestor) + p = overlay + depth = 0 + while p is not None and depth < 100: + if p is base: + return base.extend() + p = getattr(p, '_parent', None) + depth += 1 + # MergedEnv: reads walk base then overlay; set! walks base only + return _MergedEnv({}, primary=base, secondary=overlay) def dict_set(d, k, v): @@ -937,8 +958,10 @@ def scan_css_classes(source): return list(classes) def component_io_refs(c): - """Return cached IO refs list for a component (may be empty).""" - return list(c.io_refs) if hasattr(c, "io_refs") and c.io_refs else [] + """Return cached IO refs list, or NIL if not yet computed.""" + if not hasattr(c, "io_refs") or c.io_refs is None: + return NIL + return list(c.io_refs) def component_set_io_refs(c, refs): """Cache IO refs on a component.""" @@ -2985,6 +3008,20 @@ def _wrap_aser_outputs(): aser_fragment = _aser_fragment_wrapped +# Override sf_set_bang to walk the Env scope chain so that (set! var val) +# updates the variable in its defining scope, not just the local copy. +def sf_set_bang(args, env): + name = symbol_name(first(args)) + value = trampoline(eval_expr(nth(args, 1), env)) + env = _ensure_env(env) + try: + env.set(name, value) + except KeyError: + # Not found in chain — define locally (matches prior behavior) + env[name] = value + return value + + # ========================================================================= # Public API # ========================================================================= @@ -2998,7 +3035,9 @@ _setup_html_adapter() def evaluate(expr, env=None): """Evaluate expr in env and return the result.""" if env is None: - env = {} + env = _Env() + elif isinstance(env, dict): + env = _Env(env) result = eval_expr(expr, env) while is_thunk(result): result = eval_expr(thunk_expr(result), thunk_env(result)) @@ -3018,5 +3057,5 @@ def render(expr, env=None): def make_env(**kwargs): - """Create an environment dict with initial bindings.""" - return dict(kwargs) + """Create an environment with initial bindings.""" + return _Env(dict(kwargs)) diff --git a/shared/sx/tests/run.py b/shared/sx/tests/run.py index 9e23d49..c679d59 100644 --- a/shared/sx/tests/run.py +++ b/shared/sx/tests/run.py @@ -281,7 +281,9 @@ def eval_file(filename, env): # --- Build env --- -env = { +from shared.sx.env import Env as _Env + +env = _Env({ "try-call": try_call, "report-pass": report_pass, "report-fail": report_fail, @@ -330,7 +332,7 @@ env = { "component-body": lambda c: getattr(c, 'body', NIL), "component-closure": lambda c: dict(getattr(c, 'closure', {})), "component-has-children?": lambda c: getattr(c, 'has_children', False), -} +}) def _call_sx(fn, args, caller_env): @@ -496,7 +498,7 @@ def _load_orchestration(env): def _mock_execute_action(action, payload, on_success, on_error): """Mock: immediately calls on_success with payload as 'server truth'.""" - on_success(payload) + _call_sx(on_success, [payload], env) return NIL def _dict_delete(d, k): diff --git a/shared/sx/types.py b/shared/sx/types.py index 2b8d53d..97f0e2f 100644 --- a/shared/sx/types.py +++ b/shared/sx/types.py @@ -168,7 +168,7 @@ class Component: closure: dict[str, Any] = field(default_factory=dict) css_classes: set[str] = field(default_factory=set) # pre-scanned :class values deps: set[str] = field(default_factory=set) # transitive component deps (~names) - io_refs: set[str] = field(default_factory=set) # transitive IO primitive refs + io_refs: set[str] | None = None # transitive IO primitive refs (None = not computed) affinity: str = "auto" # "auto" | "client" | "server" @property @@ -208,7 +208,7 @@ class Island: closure: dict[str, Any] = field(default_factory=dict) css_classes: set[str] = field(default_factory=set) deps: set[str] = field(default_factory=set) - io_refs: set[str] = field(default_factory=set) + io_refs: set[str] | None = None def __repr__(self): return f""