test to see if memory doubles when using map on array #1979

ellyxir · 2025-10-29T20:52:35Z

Summary by cubic

Adds an integration test to catch a derive()+array.map() memory leak and makes tests more reliable. Also streams fact selection to reduce memory use.

New Features
- Added derive_array_leak.test.ts/.tsx that increments 50 times per click, clamps array length, and fails if server RSS grows >2.0x baseline (5-minute timeout).
Refactors
- Switched from .all() to stmt.iter() and removed result buffering by yielding rows in selectFacts and getMatchingFacts.
- Finalized prepared statements in selectFact and documented why selectFacts cannot finalize inside a generator.
- Converted integration tests to Deno.test with explicit timeouts and proper timer cleanup; reconnection.test now resolves and is skipped by default.

^{Written for commit 20d20ca. Summary will update automatically on new commits.}

cubic-dev-ai

4 issues found across 8 files

Prompt for AI agents (all 4 issues)


Understand the root cause of the following 4 issues and fix them.


<file name="packages/runner/integration/reconnection.test.ts">

<violation number="1" location="packages/runner/integration/reconnection.test.ts:258">
This `await new Promise(() =&gt; {})` never settles, so the Deno.test can never finish; let the test resolve or reject when the reconnection checks complete instead of awaiting an eternal promise.</violation>
</file>

<file name="packages/runner/integration/derive_array_leak.test.ts">

<violation number="1" location="packages/runner/integration/derive_array_leak.test.ts:84">
The timeout Promise never clears its setTimeout, so the test always waits the full 3 minutes and then hits an unhandled rejection even if runTest completes successfully. Please store the timeout handle and clear it once runTest finishes so the timer doesn’t block the test.</violation>
</file>

<file name="packages/runner/integration/derive_array_leak.test.tsx">

<violation number="1" location="packages/runner/integration/derive_array_leak.test.tsx:62">
`new Array(value)` will throw when the counter goes negative because the decrement handler allows state.value to drop below zero. Clamp the length before allocating the array to keep the UI/test from crashing.</violation>
</file>

<file name="packages/runner/integration/array_push.test.ts">

<violation number="1" location="packages/runner/integration/array_push.test.ts:198">
The module-level timeout promise schedules a setTimeout that never gets cleared after this race resolves, so the successful test still holds a pending timer and keeps the process alive until TIMEOUT_MS (~30s). Please move the timeout inside the test and clear it when runTest finishes so the test can exit promptly.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/runner/integration/reconnection.test.ts

packages/runner/integration/derive_array_leak.test.ts

packages/runner/integration/derive_array_leak.test.tsx

packages/runner/integration/array_push.test.ts

- Fix timeout handling in all tests to properly clear timers and prevent hanging processes and unhandled rejections - Fix reconnection.test.ts to use a properly resolving Promise instead of eternal await - Fix derive_array_leak.test.tsx to clamp array length to prevent negative values from throwing errors - Add 3-minute timeouts to pending-nursery, basic-persistence, and sync-schema-path tests

cubic-dev-ai

Reviewed changes from recent commits (found 2 issues).

2 issues found across 3 files

Prompt for AI agents (all 2 issues)


Understand the root cause of the following 2 issues and fix them.


<file name="packages/runner/integration/pending-nursery.test.ts">

<violation number="1" location="packages/runner/integration/pending-nursery.test.ts:147">
The module-scoped timeout Promise keeps a live timer even after runTest() resolves, so every test run idles for the full 180 s before the timer fires and the promise becomes permanently rejected, causing later runs to fail.</violation>
</file>

<file name="packages/runner/integration/sync-schema-path.test.ts">

<violation number="1" location="packages/runner/integration/sync-schema-path.test.ts:133">
Instantiate the timeout inside the test (or create it right before Promise.race) so that the countdown starts when the test begins; otherwise the pre-start timer can make the race reject immediately if earlier setup exceeds the timeout.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/runner/integration/pending-nursery.test.ts

packages/runner/integration/sync-schema-path.test.ts

cubic-dev-ai

No issues found across 4 files

cubic-dev-ai

Reviewed changes from recent commits (found 1 issue).

1 issue found across 1 file

Prompt for AI agents (all 1 issues)


Understand the root cause of the following 1 issues and fix them.


<file name="packages/runner/integration/reconnection.test.ts">

<violation number="1" location="packages/runner/integration/reconnection.test.ts:16">
Removing the slash between the rewritten host and `/api/storage/memory` produces an invalid WebSocket URL (e.g. `ws://example.comapi/storage/memory`), so the test will connect to the wrong host and fail.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/runner/integration/reconnection.test.ts

cubic-dev-ai

Reviewed changes from recent commits (found 2 issues).

2 issues found across 1 file

Prompt for AI agents (all 2 issues)


Understand the root cause of the following 2 issues and fix them.


<file name="packages/memory/space.ts">

<violation number="1" location="packages/memory/space.ts:575">
Avoid buffering the entire result set in `selectFacts`; continue streaming rows directly from `stmt.iter(...)` to keep memory usage bounded.</violation>

<violation number="2" location="packages/memory/space.ts:588">
Restore a try/finally around the iteration so the prepared statement is finalized even when `stmt.iter(...)` or `toFact(row)` throws; otherwise the statement leaks resources.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/memory/space.ts

cubic-dev-ai

Reviewed changes from recent commits (found 2 issues).

2 issues found across 2 files

Prompt for AI agents (all 2 issues)


Understand the root cause of the following 2 issues and fix them.


<file name="packages/runner/integration/reconnection.test.ts">

<violation number="1" location="packages/runner/integration/reconnection.test.ts:21">
Setting `ignore: true` skips this reconnection integration test, so it will no longer run and cannot catch regressions. Please keep the test enabled.</violation>
</file>

<file name="scripts/run_reconnection_test.sh">

<violation number="1" location="scripts/run_reconnection_test.sh:15">
start-local-dev.sh launches the dev servers in the background, but this script never stops them, so they remain running (and hold ports) after the test or on failures. Please add an EXIT trap or other cleanup to call stop-local-dev.sh once the test finishes.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-10-30T19:12:11Z

packages/runner/integration/reconnection.test.ts

-await provider2.sync(uri, testSelector);
+Deno.test({
+  name: "schema query reconnection test",
+  ignore: true,


Setting ignore: true skips this reconnection integration test, so it will no longer run and cannot catch regressions. Please keep the test enabled.

Prompt for AI agents

Address the following comment on packages/runner/integration/reconnection.test.ts at line 21: <comment>Setting `ignore: true` skips this reconnection integration test, so it will no longer run and cannot catch regressions. Please keep the test enabled.</comment> <file context> @@ -18,6 +18,7 @@ const TEST_DOC_ID = "test-reconnection-counter"; Deno.test({ name: "schema query reconnection test", + ignore: true, fn: async () => { console.log("Schema Query Reconnection Integration Test"); </file context>

cubic-dev-ai · 2025-10-30T19:12:11Z

scripts/run_reconnection_test.sh

+rm -f "$REPO_ROOT/packages/toolshed/cache/memory/did:key:z6Mk"*.sqlite
+
+echo "Starting dev servers..."
+"$SCRIPT_DIR/start-local-dev.sh"


start-local-dev.sh launches the dev servers in the background, but this script never stops them, so they remain running (and hold ports) after the test or on failures. Please add an EXIT trap or other cleanup to call stop-local-dev.sh once the test finishes.

Prompt for AI agents

Address the following comment on scripts/run_reconnection_test.sh at line 15: <comment>start-local-dev.sh launches the dev servers in the background, but this script never stops them, so they remain running (and hold ports) after the test or on failures. Please add an EXIT trap or other cleanup to call stop-local-dev.sh once the test finishes.</comment> <file context> @@ -0,0 +1,19 @@ +rm -f "$REPO_ROOT/packages/toolshed/cache/memory/did:key:z6Mk"*.sqlite + +echo "Starting dev servers..." +"$SCRIPT_DIR/start-local-dev.sh" + +echo "Running reconnection test..." </file context>

cubic identified that selectFacts was buffering all results before yielding, which defeats the purpose of using a generator and reintroduces the memory problem that bc0c716 originally fixed by switching from .all() to .iter(). this change: - removes the results array buffering - streams rows directly with yield toFact(row) - documents why finalize() cannot be used in a generator's finally block (finally blocks execute when generator returns, not when exhausted) the statement will be finalized when the store is closed. this is an acceptable trade-off given javascript generator limitations. addresses cubic's feedback on PR #1979

cubic-dev-ai

Reviewed changes from recent commits (found 1 issue).

1 issue found across 1 file

Prompt for AI agents (all 1 issues)


Understand the root cause of the following 1 issues and fix them.


<file name="packages/memory/space.ts">

<violation number="1" location="packages/memory/space.ts:574">
Leaving the prepared statement unfinalized means every selectFacts call keeps a SQLite statement open until the whole store closes, leaking resources and risking SQLITE_BUSY conflicts. Please wrap the loop in a try/finally and finalize the statement once iteration finishes.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/memory/space.ts

cubic-dev-ai

3 issues found across 11 files

Prompt for AI agents (all 3 issues)


Understand the root cause of the following 3 issues and fix them.


<file name="packages/memory/space.ts">

<violation number="1" location="packages/memory/space.ts:573">
`selectFacts` prepares `EXPORT` but never finalizes the statement. Prepared statements in `@db/sqlite` must be finalized after iteration to release locks, otherwise repeated calls leak resources and can exhaust statement handles or keep tables locked. The generator can wrap the loop in a try/finally so `stmt.finalize()` runs when iteration completes. Please add that finalization.</violation>
</file>

<file name="scripts/run_reconnection_test.sh">

<violation number="1" location="scripts/run_reconnection_test.sh:15">
`start-local-dev.sh` relies on running from the repo root, but this script calls it without first switching directories, so invoking run_reconnection_test.sh from scripts/ (or any non-root cwd) fails before the test can run. Please `cd` to the repo root before starting dev servers.</violation>
</file>

<file name="packages/runner/integration/reconnection.test.ts">

<violation number="1" location="packages/runner/integration/reconnection.test.ts:16">
Missing slash between host and path makes MEMORY_WS_URL invalid, causing `new URL(MEMORY_WS_URL)` to throw before the test can run.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-10-30T19:40:51Z

packages/memory/space.ts

-  }) as StateRow[];
-
-  for (const row of rows) {
+  const stmt = store.prepare(EXPORT);


selectFacts prepares EXPORT but never finalizes the statement. Prepared statements in @db/sqlite must be finalized after iteration to release locks, otherwise repeated calls leak resources and can exhaust statement handles or keep tables locked. The generator can wrap the loop in a try/finally so stmt.finalize() runs when iteration completes. Please add that finalization.

Prompt for AI agents

Address the following comment on packages/memory/space.ts at line 573: <comment>`selectFacts` prepares `EXPORT` but never finalizes the statement. Prepared statements in `@db/sqlite` must be finalized after iteration to release locks, otherwise repeated calls leak resources and can exhaust statement handles or keep tables locked. The generator can wrap the loop in a try/finally so `stmt.finalize()` runs when iteration completes. Please add that finalization.</comment> <file context> @@ -570,15 +570,19 @@ export const selectFacts = function* <Space extends MemorySpace>( - }) as StateRow[]; - - for (const row of rows) { + const stmt = store.prepare(EXPORT); + // Note: Cannot finalize() in a generator function's finally block because + // the finally block runs when the generator returns (immediately), not when </file context>

cubic-dev-ai · 2025-10-30T19:40:51Z

scripts/run_reconnection_test.sh

+rm -f "$REPO_ROOT/packages/toolshed/cache/memory/did:key:z6Mk"*.sqlite
+
+echo "Starting dev servers..."
+"$SCRIPT_DIR/start-local-dev.sh"


start-local-dev.sh relies on running from the repo root, but this script calls it without first switching directories, so invoking run_reconnection_test.sh from scripts/ (or any non-root cwd) fails before the test can run. Please cd to the repo root before starting dev servers.

Prompt for AI agents

Address the following comment on scripts/run_reconnection_test.sh at line 15: <comment>`start-local-dev.sh` relies on running from the repo root, but this script calls it without first switching directories, so invoking run_reconnection_test.sh from scripts/ (or any non-root cwd) fails before the test can run. Please `cd` to the repo root before starting dev servers.</comment> <file context> @@ -0,0 +1,19 @@ +rm -f "$REPO_ROOT/packages/toolshed/cache/memory/did:key:z6Mk"*.sqlite + +echo "Starting dev servers..." +"$SCRIPT_DIR/start-local-dev.sh" + +echo "Running reconnection test..." </file context>

Suggested change

"$SCRIPT_DIR/start-local-dev.sh"

cd "$REPO_ROOT" && "$SCRIPT_DIR/start-local-dev.sh"

cubic-dev-ai · 2025-10-30T19:40:52Z

packages/runner/integration/reconnection.test.ts

 const MEMORY_WS_URL = `${
  API_URL.replace("http://", "ws://")
-}/api/storage/memory`;
+}api/storage/memory`;


Missing slash between host and path makes MEMORY_WS_URL invalid, causing new URL(MEMORY_WS_URL) to throw before the test can run.

Prompt for AI agents

Address the following comment on packages/runner/integration/reconnection.test.ts at line 16: <comment>Missing slash between host and path makes MEMORY_WS_URL invalid, causing `new URL(MEMORY_WS_URL)` to throw before the test can run.</comment> <file context> @@ -13,230 +13,256 @@ const { API_URL } = env; const MEMORY_WS_URL = `${ API_URL.replace("http://", "ws://") -}/api/storage/memory`; +}api/storage/memory`; const TEST_DOC_ID = "test-reconnection-counter"; </file context>

Suggested change

}api/storage/memory`;

}/api/storage/memory`;

ellyxir self-assigned this Oct 29, 2025

ellyxir force-pushed the ellyse/memory-leak-rd branch 2 times, most recently from 160a860 to c247b0d Compare October 29, 2025 20:58

ellyxir temporarily deployed to ci October 29, 2025 21:02 — with GitHub Actions Inactive

test to see if memory doubles when using map on array

3a00ff7

ellyxir force-pushed the ellyse/memory-leak-rd branch from c247b0d to 3a00ff7 Compare October 29, 2025 21:22

ellyxir temporarily deployed to ci October 29, 2025 21:26 — with GitHub Actions Inactive

use Deno.test()

5daaa55

ellyxir temporarily deployed to ci October 29, 2025 22:58 — with GitHub Actions Inactive

ellyxir had a problem deploying to ci October 29, 2025 22:58 — with GitHub Actions Error

ellyxir temporarily deployed to ci October 29, 2025 22:58 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 29, 2025

View reviewed changes

adding timeouts to tests

41a8f22

ellyxir temporarily deployed to ci October 29, 2025 23:21 — with GitHub Actions Inactive

ellyxir had a problem deploying to ci October 29, 2025 23:21 — with GitHub Actions Error

ellyxir temporarily deployed to ci October 29, 2025 23:21 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 29, 2025

View reviewed changes

packages/runner/integration/pending-nursery.test.ts Outdated Show resolved Hide resolved

packages/runner/integration/sync-schema-path.test.ts Outdated Show resolved Hide resolved

increase derive_array_leak test to 100 increments per click

0d89be6

ellyxir temporarily deployed to ci October 29, 2025 23:31 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

fix malformed url

07e2249

ellyxir temporarily deployed to ci October 30, 2025 16:13 — with GitHub Actions Inactive

ellyxir had a problem deploying to ci October 30, 2025 16:13 — with GitHub Actions Failure

ellyxir temporarily deployed to ci October 30, 2025 16:13 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

packages/runner/integration/reconnection.test.ts Show resolved Hide resolved

properly finalize prepared statements in

a818566

ellyxir temporarily deployed to ci October 30, 2025 16:47 — with GitHub Actions Inactive

ellyxir had a problem deploying to ci October 30, 2025 16:47 — with GitHub Actions Failure

ellyxir temporarily deployed to ci October 30, 2025 16:47 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

packages/memory/space.ts Outdated Show resolved Hide resolved

packages/memory/space.ts Outdated Show resolved Hide resolved

ellyxir added 2 commits October 30, 2025 17:16

temp script to run test easily

b225bd7

skipping reconnection test for now

c918022

ellyxir temporarily deployed to ci October 30, 2025 19:03 — with GitHub Actions Inactive

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

ellyxir temporarily deployed to ci October 30, 2025 19:27 — with GitHub Actions Inactive

ellyxir marked this pull request as ready for review October 30, 2025 19:31

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

packages/memory/space.ts Show resolved Hide resolved

ellyxir merged commit ea62cd9 into main Oct 30, 2025
9 checks passed

ellyxir deleted the ellyse/memory-leak-rd branch October 30, 2025 19:38

cubic-dev-ai bot reviewed Oct 30, 2025

View reviewed changes

	"$SCRIPT_DIR/start-local-dev.sh"
	cd "$REPO_ROOT" && "$SCRIPT_DIR/start-local-dev.sh"

test to see if memory doubles when using map on array #1979

test to see if memory doubles when using map on array #1979

Uh oh!

Conversation

ellyxir commented Oct 29, 2025 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ellyxir commented Oct 29, 2025 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot Oct 30, 2025 •

edited

Loading

cubic-dev-ai bot Oct 30, 2025 •

edited

Loading

cubic-dev-ai bot Oct 30, 2025 •

edited

Loading

cubic-dev-ai bot Oct 30, 2025 •

edited

Loading

cubic-dev-ai bot Oct 30, 2025 •

edited

Loading