Build/tests/kernel.test.ts (1)

216-239: Consider consolidating duplicate path tests.

These path tests overlap with the more comprehensive coverage in Build/tests/path.test.ts. While having some basic path tests here for quick validation is fine, you may want to remove these to avoid maintaining duplicate test code.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/kernel.test.ts` around lines 216 - 239, Remove the duplicate path
tests in the describe block that exercises the path helper (the describe('path',
...) block containing tests that call path("user","name"), path("components", 0,
"value"), path("user","*"), and path("user","**")), or replace them with a
single minimal smoke test that only asserts path(...) constructs without
duplicating coverage already present in path.test.ts; update or delete the
describe('path', ...) block in Build/tests/kernel.test.ts so only the
non-duplicative, quick-validation test (or none) remains.

Build/src/store/map.ts (4)

71-77: values() creates intermediate array unnecessarily.

The method collects all values into an array then returns its iterator. This loses the lazy iteration benefit. Consider using a generator function instead.

♻️ Proposed fix using generator

-  values(): IterableIterator<V> {
-    const values: V[] = [];
-    for (const sig of this.entries.values()) {
-      values.push(sig.get());
-    }
-    return values[Symbol.iterator]();
+  *values(): IterableIterator<V> {
+    for (const sig of this.entries.values()) {
+      yield sig.get();
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 71 - 77, The values() method currently
builds an intermediate array then returns its iterator, losing laziness; change
values() into a generator function that iterates over this.entries.values() and
yields sig.get() for each entry (referencing the values() method and the
sig.get() call) so consumers receive a lazy IterableIterator<V> without
allocating the temporary V[].

---

4-8: Module-level counter shares same concern as reactive.ts.

Similar to storeId in reactive.ts, the mapId counter persists across tests. Consider a consistent approach to resetting these counters for test isolation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 4 - 8, The module-level counter mapId
used by nextMapId persists across test runs leading to flaky tests; mirror the
fix used for storeId in reactive.ts by making the counter resettable: refactor
mapId so it’s not permanently module-global (or export a resetMapId function)
and update nextMapId to use the refactored counter; ensure tests call the new
resetMapId (or the same centralized reset helper) to restore mapId between
tests.

---

29-31: get() returns value directly without dependency tracking.

The get() method calls sig.get() which tracks the signal dependency, but the ReactiveMap itself doesn't track the map structure. If entries are added/removed, consumers using get() won't be notified unless they also subscribe to the map.

This is likely intentional design but worth documenting.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 29 - 31, The get(key: K) method
currently returns this.entries.get(key)?.get() which only tracks the individual
entry's signal (sig.get()) and does not track the ReactiveMap's structure, so
consumers won't be notified when entries are added/removed; either update
get(key: K) in the ReactiveMap to also read a map-structure signal (so callers
subscribe to structural changes) whenever returning an entry, or add a clear
comment/docstring on the ReactiveMap/get method explaining that get only tracks
the entry signal and does not subscribe to map additions/removals (reference
symbols: get(key: K): V | undefined, this.entries, sig.get(), ReactiveMap).

---

79-85: Same intermediate array pattern in entriesIterable().

This method also creates an intermediate array. Consider using a generator for consistency and efficiency.

♻️ Proposed fix using generator

-  entriesIterable(): IterableIterator<[K, V]> {
-    const result: [K, V][] = [];
-    for (const [key, sig] of this.entries.entries()) {
-      result.push([key, sig.get()]);
-    }
-    return result[Symbol.iterator]();
+  *entriesIterable(): IterableIterator<[K, V]> {
+    for (const [key, sig] of this.entries.entries()) {
+      yield [key, sig.get()];
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 79 - 85, entriesIterable() currently
builds an intermediate array then returns its iterator which wastes memory;
change it to a generator by making entriesIterable a generator function (use
function* or add the * prefix) that iterates this.entries and yields [key,
sig.get()] directly (referencing the entries Map via this.entries and the
signature accessor sig.get()), removing the temporary result array so callers
receive an IterableIterator without the intermediate allocation.

Build/src/store/reactive.ts (2)

4-8: Module-level counter may cause issues in test isolation.

The storeId counter is module-level and never reset. While __resetRegistryForTesting resets the graph registry, stores created in different tests will have incrementing IDs that persist across test runs. This could lead to non-deterministic test behavior if tests depend on specific path names.

Consider exposing a reset function or making the counter resettable for testing purposes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/reactive.ts` around lines 4 - 8, The module-level counter
storeId used by nextStoreId causes persistent, non-deterministic IDs across
tests; add a reset mechanism (e.g., export a function like
resetStoreIdForTesting or extend __resetRegistryForTesting to reset storeId to
0) and update tests to call it where the registry is reset so store IDs start
from a deterministic value; ensure the reset function only used in test builds
or clearly named to indicate test-only usage.

---

43-60: Proxy set trap allows arbitrary property assignment.

When target[prop] is not a Signal, the set trap assigns newValue directly to the target without any validation. This could allow consumers to accidentally pollute the reactive object with non-reactive properties, breaking the invariant that all properties are Signals.

Consider throwing an error or creating a new signal for unknown properties instead of direct assignment.

♻️ Proposed fix to prevent arbitrary assignments

     set(target, prop, newValue) {
       if (prop === "$" || prop === "$raw") {
         throw new Error("Cannot set $ or $raw directly");
       }
       if (target[prop] && target[prop] instanceof Signal) {
         target[prop].set(newValue);
-      } else {
-        target[prop] = newValue as any;
+      } else if (prop in target) {
+        // Only allow setting existing non-signal properties
+        target[prop] = newValue as any;
+      } else {
+        throw new Error(`Cannot set unknown property "${String(prop)}" on reactive object`);
       }
       return true;
     },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/reactive.ts` around lines 43 - 60, The Proxy set trap
currently assigns newValue directly to target[prop] when the existing property
is not a Signal, which allows arbitrary non-reactive properties; update the set
handler in the proxy (the set trap on the `proxy` wrapping `result`) so that
instead of direct assignment it either throws for unknown keys or converts the
value into a Signal (e.g., create a new Signal for newValue and assign that to
target[prop]) to preserve the invariant that all properties are Signals; keep
the existing guard that prevents setting "$" or "$raw" and ensure the branch
that checks `target[prop] instanceof Signal` still calls
`target[prop].set(newValue)` for existing signals.

Build/src/kernel/index.ts (1)

28-28: Path alias may cause confusion.

Re-exporting path (a function) as Path (capitalized, typically denoting a type/class) could mislead consumers into thinking it's a type or constructor rather than a factory function. Consider renaming to createPath or documenting this clearly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/index.ts` at line 28, The export alias export { path as Path
} is misleading because capitalized identifiers imply a type/class; rename the
exported symbol to a clear verb form (e.g., export { path as createPath } or
export { path as makePath }) and update any imports using Path to the new name,
or alternatively keep Path but add explicit documentation comment clarifying
it's a factory function; update references to the function (the path function
and any imports) to match the new export name so consumers aren't misled.

Build/src/kernel/effect.ts (2)

92-96: effectIdle arrow formatting could be clearer.

The multi-line arrow function is harder to read. Consider reformatting for clarity.

♻️ Proposed formatting improvement

-export const effectIdle = (fn: () => CleanupFn) => createEffect(fn,
-  r => typeof requestIdleCallback !== "undefined"
-    ? requestIdleCallback(() => r())
-    : setTimeout(() => r(), 0)
-);
+export const effectIdle = (fn: () => CleanupFn) =>
+  createEffect(fn, (r) => {
+    if (typeof requestIdleCallback !== "undefined") {
+      requestIdleCallback(() => r());
+    } else {
+      setTimeout(() => r(), 0);
+    }
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 92 - 96, The arrow formatting of
effectIdle is hard to read; update the createEffect call for clarity by
expanding the arrow callback into a clearer multi-line function: put the
parameter (r) and the conditional logic on separate lines, use braces and an
explicit return (or explicit invocation) for the handler that chooses between
requestIdleCallback and setTimeout, and align indentation so the ternary branch
is easier to scan; focus on editing the export const effectIdle = (fn: () =>
CleanupFn) => createEffect(fn, r => ...) expression so the callback using
requestIdleCallback and setTimeout is formatted across several lines for
readability.

---

17-32: Effect pool stores stale function references.

When an effect is returned to the pool (line 86), it stores both the original fn and the current cleanupFn. However, getEffectFromPool() returns these stale references. The pool is then reused in createEffect() but pooled.fn is never used - only pooled.cleanup is assigned to cleanupFn. This seems intentional but the pool structure storing fn is misleading.

♻️ Proposed simplification of pool structure

-const effectPool: { fn: () => CleanupFn; cleanup: CleanupFn }[] = [];
+const effectPool: { cleanup: CleanupFn }[] = [];

-function getEffectFromPool(): { fn: () => CleanupFn; cleanup: CleanupFn } {
+function getEffectFromPool(): { cleanup: CleanupFn } {
   if (effectPool.length > 0) {
     return effectPool.pop()!;
   }
-  return { fn: () => undefined, cleanup: undefined };
+  return { cleanup: undefined };
 }

-function returnEffectToPool(effect: { fn: () => CleanupFn; cleanup: CleanupFn }): void {
+function returnEffectToPool(effect: { cleanup: CleanupFn }): void {
   if (effectPool.length < EFFECT_POOL_SIZE) {
     effect.cleanup = undefined;
     effectPool.push(effect);
   }
 }

And update line 86:

-    returnEffectToPool({ fn, cleanup: cleanupFn });
+    returnEffectToPool({ cleanup: cleanupFn });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 17 - 32, The pool currently stores
objects with both fn and cleanup but only cleanup is reused, causing misleading
stale fn references; fix by simplifying the pool to store only CleanupFn items
(use a type like CleanupFn[] or alias) and update getEffectFromPool to return a
CleanupFn (or undefined), update returnEffectToPool to push only the cleanup
function after setting it to undefined when returning, and adjust createEffect
(and any code using pooled.fn) to expect a pooled cleanup value instead of an
object with fn—remove references to pooled.fn and ensure the pool capacity uses
EFFECT_POOL_SIZE as before.

Build/tests/graph.test.ts (2)

175-181: Config restoration may not reset all fields.

The afterEach restores lockViolation to "throw" and satori to null, but originalConfig is captured once at module load time. If other tests modify additional config fields, they won't be restored. Consider using configureSairin(originalConfig) instead.

♻️ Proposed fix to restore full config

   describe('configureSairin', () => {
-    const originalConfig = getSairinConfig();
-
     afterEach(() => {
-      configureSairin({ lockViolation: "throw", satori: null });
+      // Reset to default config state
+      configureSairin({ lockViolation: "throw", satori: null });
+      // Also reset the registry to ensure clean state
+      __resetRegistryForTesting();
     });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/graph.test.ts` around lines 175 - 181, The afterEach currently
only resets lockViolation and satori, which misses other fields; change the
teardown to restore the full original config by calling
configureSairin(originalConfig) and ensure originalConfig is captured fresh for
each test (move const originalConfig = getSairinConfig() into a beforeEach so
each test gets a fresh snapshot) referencing getSairinConfig and configureSairin
in the describe('configureSairin') block.

---

115-153: Test relies on internal implementation details.

Accessing (sig as any)._node and manipulating subscribers directly couples the test to internal implementation. While this may be necessary for testing cleanup behavior, consider whether there's a public API that could be used instead.

Build/tests/benchmark.test.ts (1)

8-20: Benchmark threshold is very permissive.

The threshold of 2000 nanoseconds (2μs) per read operation is quite high for a simple signal read. This may not catch performance regressions effectively. Consider tightening this threshold based on baseline measurements.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/benchmark.test.ts` around lines 8 - 20, The benchmark test
"signal read should be fast" uses a very permissive threshold
(expect(timePerOp).toBeLessThan(2000)) which won't catch regressions; tighten
this by measuring a realistic baseline and lowering the threshold (e.g., to
~500ns–1000ns) or compute a dynamic threshold from a short calibration run,
update the assertion accordingly, and document the chosen baseline; locate the
test around the Signal instantiation (Signal(path("bench","read"), 0)), the loop
that calls sig.get(), the iterations variable, and the timePerOp/expect
assertion to make the change.

Build/tests/flow-async.test.ts (2)

142-149: Timing-based test assertions could be flaky.

This test relies on setTimeout delays (20ms waits) to assert resource refetch behavior. In CI environments with variable load, these tight timings might cause intermittent failures.

Consider increasing the wait times or using a more reliable synchronization mechanism if available.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/flow-async.test.ts` around lines 142 - 149, The test uses fragile
20ms setTimeout waits after creating and after calling res.refetch(), which can
flake under CI; change the test to avoid fixed short sleeps by either awaiting a
real signal from the resource (e.g., await the Promise returned by res.refetch()
if it exists) or replacing the fixed waits with a small polling loop that
repeatedly checks res.value.get() until it equals the expected value (with a
generous timeout like 2000ms) so the assertions on res.value.get() become
robust; update references in this test to use res.refetch() and res.value.get()
accordingly.

---

52-57: Test timing may be fragile.

The test waits 100ms after starting a flow that takes 50ms, then asserts counter is 1. This timing-based assertion could be flaky on slower CI systems. Consider using await on the first start() or a more deterministic synchronization approach.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/flow-async.test.ts` around lines 52 - 57, The test is using
fragile timing: it calls myFlow.start() twice (one without awaiting) and then
waits 100ms to assert counter, which can flake; change to a deterministic wait
by awaiting the start promise (replace the first myFlow.start(); await
myFlow.start(); with a single await myFlow.start()) or, if start does not return
a completion promise, modify the test to await a provided completion signal
(e.g., a returned promise or event from myFlow like myFlow.waitForCompletion())
and then assert counter to avoid using setTimeout.

Build/src/kernel/derived.ts (1)

42-45: Tracker function notifies without checking for actual changes.

The tracker callback marks the node as dirty and notifies subscribers whenever any dependency changes. This is correct for invalidation, but consider whether notification should be deferred until the next get() call to avoid redundant subscriber notifications when multiple dependencies change.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/derived.ts` around lines 42 - 45, The tracker callback
currently sets this._node.dirty = true and immediately calls
notifySubscribers(this._node) on every dependency change; change it to avoid
redundant notifications by either (a) only calling notifySubscribers when the
node transitions from clean to dirty (i.e., if !this._node.dirty then set dirty
= true and notify), or (b) set this._node.dirty = true unconditionally but defer
notifySubscribers until the next get() or via a single scheduled
microtask/next-tick so multiple dependency changes are coalesced; update the
tracker function accordingly (references: tracker, this._node.dirty,
notifySubscribers, get()).

Build/src/kernel/graph.ts (3)

44-44: Unused pathCache variable.

pathCache is declared but never used in this file. Consider removing it or implementing the intended caching logic.

🧹 Remove unused variable

-const pathCache = new Map<string, PathKey>();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` at line 44, The declared but unused variable
pathCache (const pathCache = new Map<string, PathKey>();) should be removed or
wired into the intended caching logic: either delete the declaration from
graph.ts if caching is not needed, or implement usage where PathKey-based path
lookups are repeated (e.g., in functions that compute paths use
pathCache.get/set with the PathKey key inside the path computation functions) so
the Map is actually read from and written to; reference the symbol pathCache and
the type PathKey when applying the change.

---

269-293: Duplicated lock violation handling logic.

handleLockViolation() in graph.ts and the lock violation handling in Signal.set() (lines 48-66 in signal.ts) implement nearly identical logic for logging and throwing on lock violations. Consider having Signal.set() call assertLock() or a shared utility to reduce duplication.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 269 - 293, The lock-violation
branching in Signal.set duplicates the behavior already implemented in
handleLockViolation; refactor by removing the duplicated logic from Signal.set
and instead call the shared handler (either call handleLockViolation(path,
owner, attemptedOwner) directly or move the logic into a new shared
assertLock(path, owner, attemptedOwner) utility and use it from both Signal.set
and handleLockViolation). Ensure the call preserves the same behavior (uses
getSairinConfig/getSairinLogger, logs with tags ["lock","write"], falls back to
console for missing logger, and throws when config.lockViolation === "throw")
and keep unique symbols Signal.set, handleLockViolation (or assertLock) to
locate the change.

---

150-176: watch() only observes nodes existing at call time.

Nodes created after watch() is called won't trigger the callback. If dynamic observation of future nodes matching the pattern is intended, this would need a registry hook for node creation events.

Additionally, the cleanup function iterates all nodes to remove handlers. Consider storing node-to-handler mappings directly for more efficient cleanup.

Build/src/kernel/signal.ts (2)

76-78: Consider allowing options in update() for lock owner support.

The update() method calls set() but doesn't forward an options parameter. If a caller needs to update a locked signal with owner permissions, they cannot use update().

♻️ Optional enhancement

- update(fn: (value: T) => T): void {
-   this.set(fn(this._node.value));
+ update(fn: (value: T) => T, options?: { owner?: string }): void {
+   this.set(fn(this._node.value), options);
  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/signal.ts` around lines 76 - 78, The update method on the
signal doesn't accept or forward an options argument for lock/owner semantics;
change update in the Signal (or equivalent) class to accept an optional options
parameter (e.g., update(fn: (value: T) => T, options?: SetOptions)) and forward
it to set by calling this.set(fn(this._node.value), options); ensure the options
type matches the existing set signature so callers can pass lock/owner options
through update to set.

---

101-117: Use isPathKey() instead of duck-typing for path detection.

The current check ("segments" in pathOrInitial) could false-positive on objects that happen to have a segments property but aren't valid PathKey instances. Use the isPathKey() type guard which is already re-exported from ./graph for a more robust check.

♻️ Proposed fix

 export function signal<T>(pathOrInitial: PathKey | T, initial?: T): Signal<T> {
-  if (
-    typeof pathOrInitial === "object" &&
-    pathOrInitial !== null &&
-    "segments" in pathOrInitial
-  ) {
+  if (isPathKey(pathOrInitial)) {
     let path = pathOrInitial as PathKey;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/signal.ts` around lines 101 - 117, The signal function
currently detects PathKey by duck-typing ("segments" in pathOrInitial) which can
false-positive; replace that check with the exported type guard isPathKey from
./graph (use isPathKey(pathOrInitial)) and then narrow/cast pathOrInitial to
PathKey; keep the existing logic that calls resolveAlias(path), getNode(path),
and constructs new Signal(path, ...), so only change the path detection to use
isPathKey and adjust imports if necessary.

Build/src/store/array.ts (1)

25-27: Avoid returning values from forEach callback.

The arrow function (fn) => fn() implicitly returns the result of fn(). While forEach ignores the return value, this pattern can be confusing. Use a block body or for...of loop.

♻️ Proposed fix

 private notify(): void {
-  this.subscribers.forEach((fn) => fn());
+  for (const fn of this.subscribers) {
+    fn();
+  }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/array.ts` around lines 25 - 27, The notify method currently
uses an arrow callback that implicitly returns fn() which is confusing; update
notify (method name notify, property this.subscribers, callback param fn) to use
a statement body or a for...of loop so the subscriber calls are executed without
returning values (e.g., replace this.subscribers.forEach((fn) => fn()) with
this.subscribers.forEach((fn) => { fn(); }) or for (const fn of
this.subscribers) { fn(); }) to clarify intent and avoid implicit returns).

SAIRIN.md (1)

27-32: Add language specifier to the fenced code block.

The path structure example lacks a language specifier. Consider adding text or plaintext to satisfy linters and improve rendering consistency.

-```
+```text
 /user/name       ← signal (inode)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@SAIRIN.md` around lines 27 - 32, The fenced code block containing the path
structure (lines showing "/user/name ← signal (inode)", "/user/age ← signal
(inode)", "/user ← namespace (directory)", "/ui/header ← derived, depends on
/user/name") should include a language specifier (e.g., add ```text or
```plaintext after the opening backticks) so linters and renderers treat it as
plain text; update the opening fence accordingly to include the specifier.

Build/src/store/reactive.ts (1)

38-42: Arrays treated as leaf values — verify this is intentional.

Arrays are excluded from recursive wrapping and become signals directly. If deep array reactivity is needed, ensure there's a separate mechanism (e.g., Build/src/store/array.ts). Otherwise, mutating array elements won't trigger reactivity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/reactive.ts` around lines 38 - 42, The current branch treats
arrays as leaf values (Array.isArray(value) bypasses recursion) so mutations to
elements won't be reactive; update the logic in the reactive function to handle
arrays explicitly by either calling a dedicated array-reactive helper (e.g.,
reactiveArray(value, `${storePath}/${String(key)}`) or by recursively mapping
elements through reactive/signal using the same path convention and storing the
resulting wrapped array in result[key]; keep the existing isObject(...) path for
plain objects, continue using signal(path(storePath, String(key)), value) only
for true primitives, and reference reactive, signal, isObject, storePath, path
and result[key] when making the change.

Build/tests/graph.test.ts (1)

175-186: Consider reordering variable declaration for clarity.

The originalConfig variable is used in beforeEach (line 178) before its declaration appears (line 186). While this works due to JavaScript's execution model (the let declaration is evaluated when the describe block runs synchronously, before any test callbacks execute), placing the declaration before its usage would improve readability.

♻️ Suggested reorder

   describe('configureSairin', () => {
+    let originalConfig: ReturnType<typeof getSairinConfig>;
+
     beforeEach(() => {
       // Capture fresh config snapshot for each test
       originalConfig = getSairinConfig();
     });

     afterEach(() => {
       // Restore full original config
       configureSairin(originalConfig);
     });

-    let originalConfig: ReturnType<typeof getSairinConfig>;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/tests/graph.test.ts` around lines 175 - 186, Move the declaration of
originalConfig above the beforeEach/afterEach hooks to improve readability:
declare let originalConfig: ReturnType<typeof getSairinConfig>; immediately
inside the describe('configureSairin', ...) block before the beforeEach, then
keep beforeEach() that assigns originalConfig = getSairinConfig() and
afterEach() that calls configureSairin(originalConfig); this reorders the
declaration relative to its use without changing behavior.

SAIRIN-UPDATES.md (2)

83-91: Add language specifier to the batching diagram.

📝 Suggested fix

-```
+```text
 tick:
   signal A .set()  → dirty, enqueue flush
   signal B .set()  → dirty, flush already queued → drop
   signal C .set()  → dirty, flush already queued → drop
   
 microtask:
   flush() → runs all dirty effects once

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @SAIRIN-UPDATES.md around lines 83 - 91, The code block showing the batching
diagram is missing a language specifier; update the triple-backtick fence around
the block that starts with "tick:" and ends with "flush() → runs all dirty
effects once" to include the language identifier "text" (i.e., change ``` to

---

12-23: Add language specifiers to code blocks.

The static analysis tool flagged missing language specifiers on fenced code blocks. Adding them improves syntax highlighting and documentation clarity.

📝 Suggested fix

 Most reactive systems (SolidJS, MobX, Preact Signals) model reactivity as:
-```
+```text
 signal → Set<subscriber>

Sairin models it as:
- +text
/user/name ← signal (inode)
/user/age ← signal (inode)
/user ← namespace (directory)
/ui/header ← derived, depends on /user/name

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@SAIRIN-UPDATES.md` around lines 12 - 23, The fenced code blocks around the
examples are missing language specifiers; update the two blocks containing
"signal → Set<subscriber>" and the block listing "/user/name", "/user/age",
"/user", "/ui/header" to use a language tag (e.g., add "text" immediately after
the opening backticks: ```text) so syntax highlighters render them correctly and
match the suggested fix; ensure both opening fences are changed and no other
content is altered.

Build/src/kernel/effect.ts (1)

6-12: Remove unused imports.

subscribe, unsubscribe, getOrCreateNode, PathKey, and EffectNode are imported but not used anywhere in this file. They may be leftover from refactoring.

♻️ Suggested fix

-import {
-  subscribe,
-  unsubscribe,
-  getOrCreateNode,
-  type PathKey,
-  type EffectNode,
-} from "./graph";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 6 - 12, The import statement
currently brings in subscribe, unsubscribe, getOrCreateNode, PathKey, and
EffectNode from "./graph" but those symbols are unused; remove them from the
import list (or replace the whole import with only the exports that are actually
used in this file) so the file no longer imports subscribe, unsubscribe,
getOrCreateNode, PathKey, and EffectNode unnecessarily.

SAZAMI-UPDATES.md (1)

44-55: Add language specifier to architecture diagram.

📝 Suggested fix

-```
+```text
 SazamiComponent
   ├── mount(styles, template)     ← one-time structural stamp
   ├── scheduleRender(...)         ← batched re-render (pre-Sairin fallback)
   ├── _dirty: boolean             ← batching flag
   ├── _pendingTemplate: string    ← latest queued template
   └── _lastTemplate: string       ← template identity cache

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@SAZAMI-UPDATES.md` around lines 44 - 55, The fenced architecture diagram
block lacks a language specifier; update the code fence surrounding the
SazamiComponent diagram so it begins with ```text (not just ```), e.g. change
the opening fence for the block that contains SazamiComponent, mount(...),
scheduleRender(...), _dirty, _pendingTemplate and _lastTemplate to ```text to
ensure the diagram is rendered as plain text.

Build/src/kernel/graph.ts (1)

162-188: Consider tracking subscribed nodes for more efficient cleanup.

The current cleanup iterates all registry nodes to remove handlers. For large registries, tracking which nodes were actually subscribed to would be more efficient.

♻️ Optional optimization

 export function watch(
   pattern: PathKey,
   callback: (path: PathKey, kind: ReactiveKind) => void,
 ): () => void {
-  const registeredCallbacks: (() => void)[] = [];
+  const subscriptions: Array<{ node: ReactiveNode; handler: () => void }> = [];

   const checkAndSubscribe = (node: ReactiveNode) => {
     if (matchesPath(pattern, node.path)) {
       const handler = () => callback(node.path, node.kind);
-      registeredCallbacks.push(handler);
-      node.subscribers.add(handler);
+      subscriptions.push({ node, handler });
+      node.subscribers.add(handler);
     }
   };

   // ...

   return () => {
-    for (const node of nodeRegistry.values()) {
-      for (const handler of registeredCallbacks) {
-        node.subscribers.delete(handler);
-      }
-    }
-    registeredCallbacks.length = 0;
+    for (const { node, handler } of subscriptions) {
+      node.subscribers.delete(handler);
+    }
+    subscriptions.length = 0;
   };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 162 - 188, The cleanup in watch
currently iterates all nodeRegistry entries to remove handlers; instead record
the specific nodes you subscribed to and the handler per node when
checkAndSubscribe runs (e.g., maintain a Map<ReactiveNode, Set<Handler>> or an
array of {node,handler} entries alongside registeredCallbacks in
watch/checkAndSubscribe), push the node reference when you add handler to
node.subscribers, and then in the returned unsubscribe function iterate only
that recorded collection to delete handlers from node.subscribers and clear the
tracking structure and registeredCallbacks; update symbols: watch,
checkAndSubscribe, registeredCallbacks, node.subscribers to use the new per-node
tracking for efficient cleanup.

Build/src/flow/index.ts (1)

83-122: Pipeline lacks re-entrancy protection unlike flow.

flow.start() waits for currentPromise if already running, but pipeline.start() can be called multiple times concurrently, potentially causing overlapping executions with unpredictable state updates.

If this is intentional (pipelines are meant to be one-shot), consider documenting it. Otherwise:

♻️ Optional: Add re-entrancy protection

 export function pipeline<T, R>(
   fn: (input: T, signal: AbortSignal) => Promise<R>,
 ): Pipeline<T, R> {
   const id = nextFlowId();
   const running = signal(path("pipeline", id, "running"), false);
   const result = signal<R | null>(path("pipeline", id, "result"), null);
   const error = signal<Error | null>(path("pipeline", id, "error"), null);
   let abortController: AbortController | null = null;
+  let currentPromise: Promise<void> | null = null;

   return {
     running,
     result,
     error,
     start: async (input: T) => {
+      if (running.peek()) {
+        if (currentPromise) await currentPromise;
+        return;
+      }
       const runController = new AbortController();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/flow/index.ts` around lines 83 - 122, The pipeline currently allows
concurrent starts; update pipeline to add a currentPromise (or currentRun)
tracker and make start either await the existing currentPromise or reject new
starts to enforce single active run like flow.start; set currentPromise when
starting the run, clear it in the finally block, and ensure abortController
comparisons and running/result/error updates reference the same run (use
abortController === runController) so overlapping calls cannot overwrite state.

Build/src/store/array.ts (1)

1-2: Unused import: trackDependency.

trackDependency is imported but not used anywhere in this file.

🧹 Proposed fix

-import { Signal, signal, Subscriber, trackDependency } from "../kernel";
+import { Signal, signal, Subscriber } from "../kernel";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/array.ts` around lines 1 - 2, Remove the unused import symbol
trackDependency from the import statement that currently reads "import { Signal,
signal, Subscriber, trackDependency } from \"../kernel\""; update the import to
only include the actually used symbols (Signal, signal, Subscriber) so there are
no unused imports in this module (check the top of the file where the import is
declared and remove trackDependency).

Build/src/kernel/graph.ts (2)

126-134: ⚠️ Potential issue | 🟠 Major

Root lookups are broken here.

When prefix.raw === "/", Line 133 only accepts an exact / match or startsWith("//"), so getNodesUnder(path()) misses every normal node. matchesPath() does not rescue this because the root pattern is an exact non-glob path.

Suggested fix

-    const startsWithPrefix = node.path.raw === prefixStr || node.path.raw.startsWith(prefixStr + "/");
+    const startsWithPrefix =
+      prefixStr === "/" ||
+      node.path.raw === prefixStr ||
+      node.path.raw.startsWith(prefixStr + "/");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 126 - 134, getNodesUnder is missing
all non-root nodes when prefix.raw === "/" because startsWith check only matches
exact "/" or "//..."; update the startsWithPrefix calculation in getNodesUnder
to treat the root specially — e.g., compute startsWithPrefix as (prefixStr ===
"/" || node.path.raw === prefixStr || node.path.raw.startsWith(prefixStr + "/"))
— so root lookups return all nodes under "/" while preserving the existing
fallback to matchesPath(prefix, node.path).

---

334-363: ⚠️ Potential issue | 🟠 Major

Cleanup state resets too early and never fully drains multi-chunk batches.

Line 357 flips cleanupScheduled to false before the queued work finishes, so another caller can enqueue a second cleanup pass. Also, the buffer clear at Lines 359-362 only runs after the first chunk; once a batch spans multiple microtasks, scheduledCleanupNodes is left populated and gets reprocessed on the next call.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 334 - 363, The cleanupScheduled flag
is being cleared too early and the scheduledCleanupNodes buffer is only cleared
after the first microtask chunk; change the logic so cleanupScheduled remains
true until all chunks are processed and clear scheduledCleanupNodes when
cleanupChunk finishes the final chunk. Specifically, update cleanupChunk (and
its use of index and scheduledCleanupNodes) to detect when index >=
scheduledCleanupNodes.length and then (1) reset cleanupScheduled = false and (2)
set scheduledCleanupNodes.length = 0, instead of clearing cleanupScheduled at
the start of the queued microtask; ensure queueMicrotask only invokes
cleanupChunk and does not flip the flag prematurely so multiple callers won’t
interleave batches.

Build/src/store/map.ts (2)

51-53: ⚠️ Potential issue | 🟡 Minor

Use a non-returning callback here.

Biome is right about Line 52: the concise arrow still returns fn(). A block body or for...of removes the lint error without changing behavior.

Suggested fix

-    this.subscribers.forEach((fn) => fn());
+    for (const fn of this.subscribers) {
+      fn();
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 51 - 53, notify() currently uses a
concise arrow that returns fn() which triggers the lint about returning a value;
change the implementation to use a non-returning callback (e.g., use a block
arrow body or a for...of loop) so each subscriber is invoked but no value is
returned. Update the notify method that iterates over subscribers
(this.subscribers and the subscriber callback symbol fn) to call each subscriber
with a statement body (e.g., this.subscribers.forEach((fn) => { fn(); }); or for
(const fn of this.subscribers) fn();) ensuring subscribers are typed as () =>
void if not already.

---

15-33: ⚠️ Potential issue | 🟠 Major

Primitive key IDs still collide across distinct keys.

Line 26 stringifies the primitive before indexing primitiveKeyIdMap, so 1 and "1" share a kid, and two different symbols with the same description stringify alike. Those keys then reuse the same signal path under path("map", this.id, kid).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 15 - 33, The bug is that primitive keys
are stringified before lookup so distinct primitives (e.g. 1 vs "1" and
different Symbols with same description) collide; in keyToId you should stop
calling String(key) for primitives and instead use the original primitive value
as the Map key (i.e. let prim = key as string|number|symbol; use
primitiveKeyIdMap.get(prim) and primitiveKeyIdMap.set(prim, id)); keep using
keyIdMap for object/function keys and nextKeyId to generate ids, and ensure
primitiveKeyIdMap's declared key type supports symbol/number/string so symbol
identity is preserved.

Build/src/kernel/path.ts (1)

63-102: ⚠️ Potential issue | 🟠 Major

** matching still misses valid deep-glob paths.

The deep branch still jumps to the first segment equal to p[pi + 1] and treats that follower as a literal. path("**", "*") can never match anything, and path("a", "**", "b") still rejects targets like ["a", "b", "c", "b"]. This needs real segment-wise backtracking for **, not a first-hit scan.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/path.ts` around lines 63 - 102, The current deep-glob
matching in the block handling pattern.globType === "deep" incorrectly advances
ti to the first occurrence of the next literal segment and thus fails on cases
like ["**","*"] or patterns like ["a","**","b"] where later occurrences should
match; modify the algorithm in this block (operating on pattern.segments and
target.segments using pi and ti) to perform true segment-wise backtracking for
"**" instead of a single first-hit scan — e.g., when encountering "**" try
matching the remainder of the pattern against every possible ti position
(including zero segments consumed) (implement via recursion or an explicit
backtrack loop/stack), ensure "*" still consumes exactly one segment, and
preserve the existing end-of-pattern checks (trailing "**" handling).

Build/src/store/array.ts (1)

1-2: Unused import: trackDependency

trackDependency is imported but never used in this file. The reactive tracking is correctly done via itemsSignal.get() calls instead.

♻️ Remove unused import

-import { Signal, signal, Subscriber, trackDependency } from "../kernel";
+import { Signal, signal, Subscriber } from "../kernel";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/array.ts` around lines 1 - 2, The import list in
Build/src/store/array.ts includes an unused symbol trackDependency; remove
trackDependency from the import statement that currently reads "import { Signal,
signal, Subscriber, trackDependency } from \"../kernel\";" so the file imports
only the symbols actually used (e.g., Signal, signal, Subscriber). Verify no
other references to trackDependency exist (reactivity is handled via
itemsSignal.get()), then run a quick lint/compile to ensure the unused-import
warning is resolved.

Build/src/flow/index.ts (3)

148-157: Sequence continues after errors by design.

The loop continues executing subsequent functions even after an error occurs, collecting all errors into allErrors. This is intentional collect-all-errors behavior, but consider documenting this semantic since some users might expect fail-fast.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/flow/index.ts` around lines 148 - 157, The loop over fns
intentionally collects results and errors (pushing successes into allResults and
errors into allErrors) instead of failing fast; add an explicit comment and/or
update the function’s docstring explaining that the behavior of the function
(the loop using fns with abortController.signal) is to continue after errors and
aggregate errors in allErrors rather than short-circuiting, and mention how
abortController.signal interacts with flow termination so callers know to expect
collect-all-errors semantics for functions like the one that contains the fns,
allResults, allErrors and abortController symbols.

---

231-276: winner signal remains -1 on error — document this behavior.

When the race resolves with an error (first promise rejects), winner stays at -1 since line 266 only sets it on success. This is reasonable but should be documented for consumers expecting to identify which promise failed first.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/flow/index.ts` around lines 231 - 276, The race function currently
leaves the winner signal at -1 when the first settled promise is a rejection;
update the implementation documentation (add/extend the JSDoc or an inline
comment on the export function race) to explicitly state that winner will only
be set to a non-negative index on success and remains -1 when the first outcome
is an error, so consumers know this is intentional; reference the race function
and the winner signal to locate where to add the note (or, if you prefer
behavioral change instead of docs, set winner.set(outcome.index) inside the
error branch as an alternative).

---

30-36: Consider documenting concurrent start behavior.

When start() is called while already running, it awaits the current promise and returns without re-executing. This may surprise callers expecting idempotent behavior or re-triggering. Consider adding a JSDoc comment explaining this behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/flow/index.ts` around lines 30 - 36, Add a JSDoc comment to the
start function explaining its concurrent behavior: when running.peek() is true,
start will await currentPromise (if set) and return without re-executing the
operation, so callers should not expect re-triggering and can treat it as an
idempotent no-op during active runs; include notes on currentPromise and
running.peek() semantics and intended usage expectations to avoid confusion.

Build/src/kernel/effect.ts (2)

17-32: Effect pool stores fn but never uses it.

The pool stores { fn, cleanup } objects, but getEffectFromPool creates objects with a placeholder fn: () => undefined, and createEffect (line 49) never reads pooled.fn — it uses the fn parameter directly. The pool effectively only reuses the object shell while clearing cleanup.

Consider simplifying if the fn field isn't needed:

♻️ Simplify pool to only track cleanup

-const effectPool: { fn: () => CleanupFn; cleanup: CleanupFn }[] = [];
+const effectPool: { cleanup: CleanupFn }[] = [];

-function getEffectFromPool(): { fn: () => CleanupFn; cleanup: CleanupFn } {
+function getEffectFromPool(): { cleanup: CleanupFn } {
   if (effectPool.length > 0) {
     return effectPool.pop()!;
   }
-  return { fn: () => undefined, cleanup: undefined };
+  return { cleanup: undefined };
 }

-function returnEffectToPool(effect: { fn: () => CleanupFn; cleanup: CleanupFn }): void {
+function returnEffectToPool(effect: { cleanup: CleanupFn }): void {
   if (effectPool.length < EFFECT_POOL_SIZE) {
     effect.cleanup = undefined;
     effectPool.push(effect);
   }
 }

Then update line 94:

-    returnEffectToPool({ fn, cleanup: cleanupFn });
+    returnEffectToPool({ cleanup: cleanupFn });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 17 - 32, The effect pool currently
stores an unused fn field; simplify the pool to store only cleanup functions by
changing EFFECT_POOL_SIZE and effectPool to hold items of type { cleanup:
CleanupFn | undefined }, update getEffectFromPool to return { cleanup: undefined
} as the placeholder, and update returnEffectToPool to set effect.cleanup =
undefined before pushing; also update any usages (e.g., createEffect) to stop
referencing pooled.fn and instead assign the provided fn directly to the newly
created effect so the pooled object is only reused for its cleanup slot.

---

6-12: Remove unused imports from ./graph.

subscribe, unsubscribe, getOrCreateNode, PathKey, and EffectNode are imported but not used in this file.

♻️ Remove unused imports

-import {
-  subscribe,
-  unsubscribe,
-  getOrCreateNode,
-  type PathKey,
-  type EffectNode,
-} from "./graph";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 6 - 12, The import statement from
"./graph" includes unused symbols (subscribe, unsubscribe, getOrCreateNode,
PathKey, EffectNode); remove those names from the import list (or remove the
entire import if nothing from "./graph" is used in Build/src/kernel/effect.ts)
and leave only the symbols actually referenced in this file to eliminate unused
imports.

Build/src/kernel/index.ts (1)

20-28: Reorganize imports before exports for clarity.

Line 28 re-exports path as createPath using the local binding from the import on line 67. While ES module hoisting makes this work, having imports scattered after exports is unconventional and harder to follow. Consider moving all imports to the top.

♻️ Proposed reorganization

Move the local imports (lines 63-67) to the top of the file, before the re-exports:

+import { Signal } from "./signal";
+import { Derived } from "./derived";
+import { effect, effectSync, onCleanup, untracked } from "./effect";
+import { batch } from "./batch";
+import { type PathKey, path } from "./path";
+
 export { Signal, signal, isSignal } from "./signal";
 // ... other exports ...
 export { path as createPath }; // Now clearly uses the local import above
 // ... remaining exports ...
-
-import { Signal } from "./signal";
-import { Derived } from "./derived";
-import { effect, effectSync, onCleanup, untracked } from "./effect";
-import { batch } from "./batch";
-import { type PathKey, path } from "./path";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/index.ts` around lines 20 - 28, Move the local imports that
provide the `path` binding (and any related local imports used later) to the top
of the module so all imports appear before any exports; specifically, ensure the
module that defines `path`, `matchesPath`, `isPathKey`, `getParentPath`,
`joinPath`, and `PathKey` is imported before you export them and before you
re-export `path` as `createPath` to improve readability and follow conventional
ES module ordering.

Build/src/store/map.ts (2)

61-63: ⚠️ Potential issue | 🟡 Minor

notify() still triggers the Biome callback-return warning.

this.subscribers.forEach((fn) => fn()); implicitly returns the callback result, which is the exact pattern Biome flags on this hunk. Switch to a block body or a for...of loop.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 61 - 63, The notify() method currently
uses a concise arrow callback this.subscribers.forEach((fn) => fn()); which
implicitly returns the callback result and triggers the Biome callback-return
warning; change the invocation to a non-returning form (e.g., use a block-bodied
arrow this.subscribers.forEach((fn) => { fn(); }) or replace the forEach with a
for...of loop iterating over this.subscribers and calling fn() inside the loop)
so the callback does not implicitly return a value; update the notify()
implementation accordingly.

---

65-67: ⚠️ Potential issue | 🟠 Major

Collection membership still isn't tracked for reads or iteration.

These helpers only read per-entry signals. Effects/deriveds that call get() on a missing key, has(), or iterate values(), entriesIterable(), forEach(), or toArray() won't rerun on add/remove because none of them touch sizeSignal. Read the membership signal before the lookup/iteration.

Also applies to: 81-83, 112-142

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/map.ts` around lines 65 - 67, The collection membership
(sizeSignal) must be read before any membership-sensitive operations so effects
re-run on add/remove; update the methods get(key: K), has(key: K), values(),
entriesIterable(), forEach(), and toArray() to first call or access
sizeSignal.get() (or otherwise read the membership signal) before performing the
per-entry lookup/iteration via this.entries, so reads of missing keys and all
iteration helpers observe collection membership changes and will rerun when
items are added/removed.

Build/src/store/reactive.ts (1)

33-39: ⚠️ Potential issue | 🔴 Critical

Nested object paths are still being serialized into a single segment.

Line 39 passes ${storePath}/${String(key)} into the next reactive() call. On the next recursion, Line 33 feeds that string back into path(storePath, "$"), and Build/src/kernel/path.ts rejects segments containing /. Any nested object store will still throw during construction.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/store/reactive.ts` around lines 33 - 39, The bug is passing a
manually concatenated path "${storePath}/${String(key)}" into reactive(), which
injects '/' into a segment and later breaks path(...) in
Build/src/kernel/path.ts; fix by constructing the nested path with the path(...)
helper instead of string interpolation (call reactive(value, path(storePath,
String(key)))) and ensure signal(...) remains using path(storePath, "$") so no
segment contains '/'.

Build/src/kernel/signal.ts (1)

78-89: ⚠️ Potential issue | 🔴 Critical

Repeated signal(path, ...) calls still clobber existing nodes.

The alias branch is guarded, but the non-alias path still returns new Signal(path, initial, true) unconditionally. signal(existingPath) clears the current value to undefined, and signal(existingPath, next) overwrites the existing signal instead of reusing it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/signal.ts` around lines 78 - 89, The signal function
currently creates a new Signal for a path even when a signal node already
exists, which clobbers existing state; change the logic in signal(pathOrInitial)
to call getNode(path) after resolving aliases (use resolveAlias and getNode) and
if existingNode is present and existingNode.kind === "signal" return that
existing node (cast to Signal<T>) instead of constructing a new Signal; only
construct new Signal(path, initial, true) when no existing node is found, and do
the same check in the alias-resolved branch so you never pass undefined to the
Signal constructor to overwrite an existing signal.

Build/src/kernel/derived.ts (1)

36-55: ⚠️ Potential issue | 🔴 Critical

recompute() is still clearing the wrong subscriber set.

trackNode() in Build/src/kernel/graph.ts subscribes the tracker onto source nodes, not onto this._node. Clearing this._node.subscribers therefore removes Derived.subscribe() listeners while stale source subscriptions keep accumulating across recomputes. Preserve the derived's own subscribers and explicitly unsubscribe the previous tracker from prior sources before re-tracking.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/derived.ts` around lines 36 - 55, recompute() is clearing
this._node.subscribers (removing Derived.subscribe listeners) instead of
removing the previous tracker subscriptions on source nodes; remove the
this._node.subscribers.clear() call and instead keep/track the previous tracker
unsubscribe handle (or subscription list) so you can explicitly unsubscribe the
prior tracker from source nodes before re-tracking. Concretely: in recompute(),
do not clear this._node.subscribers; create the tracker as you do, call an
unsubscribe/cleanup for the previous tracker (store that handle on the derived
node), then setGlobalActiveComputation(tracker), run compute, restore the
previous global, and store the new tracker unsubscribe handle returned by
trackNode()/subscription API so future recomputes can remove stale source
subscriptions while preserving Derived.subscribe() listeners.

Build/src/kernel/graph.ts (1)

362-369: ⚠️ Potential issue | 🟠 Major

Race condition in incremental cleanup scheduling persists.

The cleanupScheduled flag is set to false (line 365) before cleanupChunk() runs. If cleanupChunk has more work and re-queues itself (line 354), a concurrent call to scheduleIncrementalCleanup during that time sees cleanupScheduled=false and schedules a duplicate microtask.

🐛 Proposed fix: Keep cleanupScheduled true while processing

   if (!cleanupScheduled) {
     cleanupScheduled = true;
     queueMicrotask(() => {
-      cleanupScheduled = false;
       cleanupChunk();
     });
   }

And reset cleanupScheduled only when processing completes:

     } else {
       // Finished, reset for next batch
       scheduledCleanupNodes.length = 0;
       cleanupIndex = 0;
+      cleanupScheduled = false;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 362 - 369, The race occurs because
cleanupScheduled is cleared before cleanupChunk finishes, allowing a concurrent
scheduleIncrementalCleanup to enqueue a duplicate microtask; change the
microtask callback to keep cleanupScheduled true while cleanupChunk runs and
only set it to false after cleanupChunk completes (handle both sync and async
returns). Concretely, in the queueMicrotask callback call
Promise.resolve(cleanupChunk()).finally(() => { cleanupScheduled = false; })
instead of setting cleanupScheduled = false before invoking cleanupChunk,
referencing the existing cleanupScheduled flag and cleanupChunk() function.

Build/src/kernel/path.ts (1)

121-128: ⚠️ Potential issue | 🟡 Minor

getParentPath() behavior may not match the comment for single-segment globs.

The comment states "parent of /user/** is /user", but for path("user", "**"), nonGlobSegments = ["user"] with length 1, which returns null per line 124-125. This contradicts the documented intent.

🧭 Suggested fix to match documented behavior

 export function getParentPath(p: PathKey): PathKey | null {
   const nonGlobSegments = p.segments.filter((s) => s !== "*" && s !== "**");
+  // For glob paths, return the non-glob base if it exists
+  if (p.isGlob && nonGlobSegments.length > 0) {
+    return path(...nonGlobSegments);
+  }
   if (nonGlobSegments.length <= 1) {
     return null;
   }
   return path(...nonGlobSegments.slice(0, -1));
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/path.ts` around lines 121 - 128, getParentPath currently
strips all glob segments and returns null when nonGlobSegments.length <= 1,
which makes path("user","**") return null instead of "/user"; update
getParentPath so that if nonGlobSegments.length === 0 it returns null, if length
=== 1 it returns path(...nonGlobSegments) (the single-segment parent like
"/user"), and otherwise return path(...nonGlobSegments.slice(0, -1)); reference
function getParentPath, variable nonGlobSegments and the path(...) constructor
to locate and implement the simple conditional fix.

Build/src/kernel/effect.ts (1)

6-12: Unused imports from graph module.

The imports subscribe, unsubscribe, getOrCreateNode, PathKey, and EffectNode from "./graph" are not used anywhere in this file. Consider removing them to avoid confusion.

🧹 Proposed fix

 import {
   getGlobalActiveComputation,
   setGlobalActiveComputation,
 } from "./dependency";
 import { scheduleEffect } from "./batch";
-import {
-  subscribe,
-  unsubscribe,
-  getOrCreateNode,
-  type PathKey,
-  type EffectNode,
-} from "./graph";
 import { getSairinLogger } from "./config";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/effect.ts` around lines 6 - 12, The import statement in
effect.ts includes unused symbols from "./graph" — remove subscribe,
unsubscribe, getOrCreateNode, PathKey, and EffectNode from the import list (or
collapse the import entirely if none of the remaining symbols from "./graph" are
used) so only actually referenced exports are imported; update the import line
that currently names subscribe, unsubscribe, getOrCreateNode, PathKey, and
EffectNode accordingly.

Build/src/kernel/graph.ts (1)

371-380: Unused logger variable.

getSairinLogger() is called at line 372 but logger is never used in the function body.

🧹 Remove unused variable

 export function capRetainedMemory(nodes: ReactiveNode[]): void {
-  const logger = getSairinLogger();
   for (const node of nodes) {
     node.subscribers.clear();
     if (node.kind === "derived") {
       (node as any).cached = undefined;
       (node as any).dirty = true;
     }
   }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/kernel/graph.ts` around lines 371 - 380, The function
capRetainedMemory declares an unused variable logger by calling
getSairinLogger(); remove the unused call/variable (delete the line "const
logger = getSairinLogger();") so capRetainedMemory only iterates nodes, clears
node.subscribers and resets derived node cached/dirty flags; alternatively, if
logging is intended, use the returned logger to emit a relevant message inside
capRetainedMemory instead of leaving it unused.

Build/src/flow/index.ts (1)

83-122: Consider documenting concurrent start behavior.

Unlike flow(), pipeline.start() doesn't guard against concurrent runs. Calling start() while a previous run is active will orphan the first run (its results are silently discarded). If this "last-write-wins" behavior is intentional, consider adding a brief comment.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Build/src/flow/index.ts` around lines 83 - 122, The pipeline function's
start() allows concurrent runs and can orphan earlier runs (their results/errors
are discarded) because it overwrites abortController and only applies writes
when abortController === runController; add a concise comment above pipeline or
the start method (referencing pipeline, start, abortController, running, result,
error) stating this "last-write-wins" behavior and that concurrent calls to
start() will not be queued or merged, so callers must manage concurrency —
alternatively, if intended to prevent this, implement a guard in start() to
no-op or throw when running is true.