neurogenesis/ngen-weave

Watch

Fork 0

Title: Engine: _compile_uncached recomputes cache key without outer_scopes, causing cross-scope cache collisions #8

New issue

Closed

opened 2026-08-25 22:26:28 +00:00 by fnoble · 0 comments

fnoble commented

2026-08-25 22:26:28 +00:00

Owner

Summary

Engine._compile_uncached
(packages/ngen-weave-core/src/ngen_weave/engine/runner.py:455-462) ignores the cache
key passed in by its only caller and recomputes it locally, dropping the outer_scopes
component. Because both methods share the same cache map (self._compiled), two
compilations of the same workflow class under different outer scopes collide: the
second lookup returns the first scope's compiled graph.

Affected code

  def _compile_uncached(
      self, wf: type[Workflow], models: dict, outer_scopes: tuple, key: tuple
  ) -> CompiledGraph:
      root_path = workflow_class_path(wf)
      key = (root_path, tuple(sorted(models.items())))   # <-- drops outer_scopes
      cached = self._compiled.get(key)
      if cached is not None:
          return cached

The caller (compile(), ~line 430) computes the key correctly as:

  key = (
      root_path,
      tuple(sorted(models.items())),
      tuple(workflow_class_path(s) for s in outer_scopes),
  )

The recomputed key shadows the parameter and omits the third element.

Steps to reproduce

Declare a composite Outer wiring a single Worker child.
Compile the same child class through two different outer scopes such that the
resolved model variant for the child differs between scopes.
Inspect the second returned CompiledGraph: it carries the first scope's frozen
variant binding.

Expected behavior

Each distinct (workflow class, models, outer_scopes) tuple produces its own cache entry
and its own CompiledGraph. Variant bindings frozen into checkpoints must match the
activation path that produced them — deterministic resume depends on this.

Actual behavior

Second compilation with different outer_scopes but same (root_path, models) returns the
stale, first-scope graph. No error is raised; the wrong graph silently serves runs.

Impact

Severity: medium-high (silent correctness failure, no crash).
Wrong model-variant resolution for nested workflows; checkpoints may persist bindings
inconsistent with the run's scope chain.
Directly contradicts the engine's core value proposition (deterministic
compile/resume semantics). Contradicts the method's own contract implied by accepting
key.

Suggested fix

Use the caller-supplied key; delete the local recomputation:

  cached = self._compiled.get(key)   # key already includes outer_scopes

Note: the early-return re-check inside _compile_uncached may be redundant entirely (the
sole caller has just missed on the same map under the _compiling guard) — verify
whether nested children re-enter via recursive compile() calls before removing.

Regression test

Compile one child under two outer scopes with differing variant resolutions → assert
distinct graphs (fails today).
Compile identical (class, models, outer_scopes) twice → assert identical object
(cache still effective).

Full test plan drafted in plans/releases/v0.1.1.md.

Environment

ngen-weave v0.1.0 (packages/ngen-weave-core)
Python >= 3.12

References

Fix plan: plans/releases/v0.1.1.md
Found during architecture review vs. OpenHands comparison, 2026-08

### Summary Engine._compile_uncached (packages/ngen-weave-core/src/ngen_weave/engine/runner.py:455-462) ignores the cache key passed in by its only caller and recomputes it locally, dropping the outer_scopes component. Because both methods share the same cache map (self._compiled), two compilations of the same workflow class under different outer scopes collide: the second lookup returns the first scope's compiled graph. ### Affected code ```python def _compile_uncached( self, wf: type[Workflow], models: dict, outer_scopes: tuple, key: tuple ) -> CompiledGraph: root_path = workflow_class_path(wf) key = (root_path, tuple(sorted(models.items()))) # <-- drops outer_scopes cached = self._compiled.get(key) if cached is not None: return cached ``` The caller (compile(), ~line 430) computes the key correctly as: ```python key = ( root_path, tuple(sorted(models.items())), tuple(workflow_class_path(s) for s in outer_scopes), ) ``` The recomputed key shadows the parameter and omits the third element. ### Steps to reproduce 1. Declare a composite Outer wiring a single Worker child. 2. Compile the same child class through two different outer scopes such that the resolved model variant for the child differs between scopes. 3. Inspect the second returned CompiledGraph: it carries the first scope's frozen variant binding. ### Expected behavior Each distinct (workflow class, models, outer_scopes) tuple produces its own cache entry and its own CompiledGraph. Variant bindings frozen into checkpoints must match the activation path that produced them — deterministic resume depends on this. ### Actual behavior Second compilation with different outer_scopes but same (root_path, models) returns the stale, first-scope graph. No error is raised; the wrong graph silently serves runs. ### Impact - Severity: medium-high (silent correctness failure, no crash). - Wrong model-variant resolution for nested workflows; checkpoints may persist bindings inconsistent with the run's scope chain. - Directly contradicts the engine's core value proposition (deterministic compile/resume semantics). Contradicts the method's own contract implied by accepting key. ### Suggested fix Use the caller-supplied key; delete the local recomputation: ```python cached = self._compiled.get(key) # key already includes outer_scopes ``` Note: the early-return re-check inside _compile_uncached may be redundant entirely (the sole caller has just missed on the same map under the _compiling guard) — verify whether nested children re-enter via recursive compile() calls before removing. ### Regression test - Compile one child under two outer scopes with differing variant resolutions → assert distinct graphs (fails today). - Compile identical (class, models, outer_scopes) twice → assert identical object (cache still effective). Full test plan drafted in plans/releases/v0.1.1.md. ### Environment - ngen-weave v0.1.0 (packages/ngen-weave-core) - Python >= 3.12 ### References - Fix plan: plans/releases/v0.1.1.md - Found during architecture review vs. OpenHands comparison, 2026-08

fnoble closed this issue

2026-08-26 10:57:40 +00:00

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

neurogenesis/ngen-weave#8

No description provided.

Rows
Columns