Bounded query execution: heap-growth budget + scanColumn wiring (LLP 0097)

[philcunliffe]

Queries over the ~202k-row / 931MB ai_gateway_messages cache could OOM-kill the daemon/server (hyparam/hypaware-server#9 class). This lands the kernel side of the bounded-query-execution corpus (LLP 0054/0055/0056/0057) with a new decision, LLP 0097, sized from a fresh measurement pass.

Before / after (median of 3, fresh process per run, production cache)

Query	Wall	Peak RSS
COUNT(DISTINCT session_id)	759 → 563ms	510 → 400MB
GROUP BY provider	776 → 677ms	469 → 486MB
high-card GROUP BY session_id top-20	792 → 677ms	534 → 475MB
ORDER BY ... LIMIT 20	888 → 791ms	444 → 434MB
full sort, narrow, no LIMIT	987 → 914ms	743 → 654MB
COUNT(DISTINCT content_text)	1085 → 843ms	685 → 622MB
SELECT * ORDER BY (crasher)	18.3s, died at 6.1GB	refuses in ~1.2s at 1.4GB, exit 1
hyp graph neighbors depth 1/3	173 → 151ms	~147 → ~135MB

Every measured query got faster; none regressed.

What changed

• src/core/query/sql.js — threads an AbortSignal into squirrelExecuteSql (LLP 0054 #signal-threading) and enforces a per-query heap-growth budget: an inline check on the scan path (every 4096 rows / per column chunk) plus a 100ms interval for post-scan phases. The inline layer matters: a timer-only watchdog measurably never fires during a hot query (event loop starved; zero samples in 8s). Default 1GiB growth (2x headroom over the worst legitimate measured query), overridable via HYP_QUERY_MAX_HEAP_MB or maxHeapBytes on ExecuteSqlOptions (0 disables). Refusal is a typed QueryExecutionBudgetError (exported from hypaware/core/query) per LLP 0056 - never a truncated result.
• src/core/cache/storage.js — internal cache fields are now hidden by scan-level projection instead of rebuilding every row object (~5 allocations/row removed on every query); forwards scanColumn.
• src/core/query/union-source.js — forwards scanColumn by concatenating per-partition column streams, owning limit/offset over the merged stream (LLP 0055; offered only when every partition can stream).
• ai-gateway dataset.js — withSchemaColumns forwards scanColumn, null-filling partitions that physically lack the column (same additive schema-drift rule as row reads).

No engine changes required: squirreling 0.14.0 already streams aggregates and honors signal; icebird 0.8.13 already implements scanColumn at the leaf - the missing pieces were all kernel wrappers. squirreling's per-operator ExecutionBudget (PR hyparam/squirreling#42, still unmerged) remains the precise refinement; LLP 0097 keeps this guard as defense-in-depth when it lands.

Tests

• 8 new tests: budget refusal (typed error shape), budget disable, pre-aborted signal, scanColumn fast path through the budget decoration, union scanColumn concatenation/limit/offset/partition-skip semantics, ai-gateway scanColumn null-fill on the real storage stack.
• Full suite: 2151 pass / 0 fail; npm run build:types clean; core_boot_noop smoke ok. ref-check clean for the new refs (the 95 pre-existing 0040/0086 anchor errors are untouched).

🤖 Generated with Claude Code

[philcunliffe]

neutral dual-review round — head a012128

Verdict: approve (all findings fixed and pushed). Review path: dual (Codex + Claude) — both reviewers ran; Codex produced a full review. Risk class: low. Full test suite green (2152 pass, 1 pre-existing skip); tsc (build:types) clean.

Actionable findings — all fixed in 57fdd76

1. No terminal budget check after collect() — major (Codex, high confidence). src/core/query/sql.js, success path after collect(results). The inline guard samples only every 4096 rows (and per column chunk), and the interval watchdog cannot fire during a fully synchronous run, so heap growth concentrated in a sub-stride tail or in finalization could return a wrongly-successful result. Fix: one guard.check() after materialization, before recording success.
2. Upstream-signal abort-listener leak — minor (Codex, high confidence). src/core/query/sql.js, args.signal linking. A long-lived upstream AbortSignal shared across many queries retained the per-call controller closure via a { once: true } listener that never fires on a non-aborting signal. Fix: capture the handler and removeEventListener in the finally.
3. Blank HYP_QUERY_MAX_HEAP_MB silently disables the guard — minor (Claude bug-scan, 85%). resolveHeapBudgetBytes. Number('') / Number(' ') are 0 (finite), so a set-but-empty var (how many config systems render an unset optional, e.g. export HYP_QUERY_MAX_HEAP_MB=) resolved to 0 = disabled, turning OFF the very OOM guard this PR adds. Fix: gate on ?.trim() being truthy; blank/whitespace now falls through to the measured default. Covered by a new deterministic resolveHeapBudgetBytes unit test.

Clean

• Guidance compliance: no semicolons / em dashes / @typedef / inline import(...) types; @ref conventions honest and correctly attached; LLP doc conventions (filename type matches Type, and the 0054/0057 edits are the allowed trivial forward-refs).
• Callers of executeQuerySql (CLI/MCP verb, clients, context-graph project/query): compatible — signal / maxHeapBytes are optional additions, no signature break. Server 4xx mapping is deferred to HypAware Server LLP 0020 by design (not a finding).
• scanColumn forwarding across storage / union-source / ai-gateway wrappers: consistent and actually consumed by the pinned squirreling 0.14.0 engine; the union limit/offset-over-concatenated-stream math was hand-traced correct; storage passthrough can't leak an internal field (validated column set).
• Existing tests in the three touched suites: 25/25 pass.

Note on coverage

The terminal-check fix is correct by construction (it can only add refusals; no existing test regressed, confirming it introduces no false refusals) but a heap-growth behavioral assertion is not deterministic in a shared process (heapUsed - baseline is GC-dependent), so it is covered by inspection rather than a flaky memory test. The env-resolution fix is covered by a new deterministic unit test.

Head moved to 57fdd76 by the fix commit. Held for the maintainer to merge (never merged or readied by neutral).

[philcunliffe]

Approved — the one major (missing terminal budget check) and two minors (upstream-signal listener leak, blank HYP_QUERY_MAX_HEAP_MB disabling the guard) surfaced by the dual-review were fixed and pushed as 57fdd76. Full suite green (2152 pass), tsc build:types clean. Mergeable and green. Held for the maintainer to merge.

[philcunliffe]

Approval label withdrawn: the review fixes for the three findings were pushed as 57fdd76, which moves the head past the approved SHA a012128. The current head is unreviewed; neutral will re-review 57fdd76 (round 2) on its next tick and re-post the verdict against it. CI is green and the PR is mergeable, but held for the maintainer either way.

[philcunliffe]

neutral round-2 review — 57fdd76 (dual-review: Codex + Claude)

Re-review of the fixed head after round 1's three findings. Verdict: approve / clean. Green, mergeable. Held for the maintainer to merge.

Round-1 fixes: all three confirmed sound and complete

1. Terminal guard.check() after collect() (src/core/query/sql.js:302). Correct. The inline guard samples only every BUDGET_CHECK_ROW_STRIDE (4096) rows / per column chunk, and the unref()'d interval watchdog cannot fire during a fully synchronous run, so a sub-stride tail or finalization could otherwise return a wrongly-successful over-budget result. The terminal check closes that window before success is recorded; on trip it throws budgetError, is re-mapped by the inner catch (if (budgetError) throw budgetError), and the finally still clears the watchdog. No false-refusal risk introduced: the check only refuses growth already over budgetBytes (default 1GiB), which is by definition the crasher class, not a legitimately large result.

2. Upstream-signal listener cleanup (sql.js:247-256, :315-318). Correct. onUpstreamAbort is captured and removeUpstreamAbort calls removeEventListener in finally, so a long-lived shared args.signal no longer retains the per-call controller closure. { once: true } + explicit removal is safe (idempotent if the listener already fired). Verified by the pre-aborted-signal test.

3. resolveHeapBudgetBytes blank-env gate (sql.js:343-356). Correct. HYP_QUERY_MAX_HEAP_MB?.trim() truthiness gate means a set-but-blank/whitespace var no longer resolves via Number('')===0 to a silently-disabled guard; blank falls through to the measured default. The new deterministic unit test covers explicit-option, blank, whitespace, unset, garbage, real-numeric, and explicit-0-disables. Negative/0 overrides still disable, which the decision doc documents as intended.

Independently ran the affected suites in an isolated worktree at 57fdd76: query-sql-budget.test.js (5/5), union-source.test.js + ai-gateway-dataset.test.js (21/21) all pass.

Codex finding (major, security surface) — investigated, non-blocking false positive

Codex flagged that storage.js dataSourceForTable().scanColumn delegates straight to source.scanColumn(options) without the INTERNAL_FIELDS stripping that scan() applies, so an internal _hyp_* column could in principle be streamed out, bypassing the hidden-field invariant.

Investigated and not exploitable:

• The wrapper advertises only publicColumns (internal fields excluded). The squirreling engine is the only caller of scanColumn, and it can only name advertised columns.
• Both dispatch paths validate referenced columns against the advertised set before any scanColumn call: the direct single-column scan path (squirreling/src/execute/execute.js:277 validateScan) and the aggregate/plan path (squirreling/src/plan/plan.js:408 validateScan). An unadvertised column throws ColumnNotFoundError at plan time.
• Empirical probe (source advertising only a but whose scanColumn would leak _hyp_ingest_seq if asked): SELECT _hyp_ingest_seq …, COUNT(DISTINCT _hyp_ingest_seq), and MAX(_hyp_ingest_seq) (the scanColumn aggregate fast path) are all rejected with ColumnNotFoundError: … Available columns: a. No leak.
• The only scanColumn callers in the repo are the three wrappers themselves (each forwarding to its inner source) plus the engine. No direct external caller passes an arbitrary column name. The same reasoning holds for the union-source and ai-gateway withSchemaColumns wrappers (both advertise public-only column sets).

So the invariant is preserved by construction. A defensive INTERNAL_FIELDS guard on scanColumn mirroring scan() would be reasonable belt-and-suspenders symmetry, but it is optional hardening, not a fix for a live leak, and does not block this PR.

Fresh diff scan

No other actionable findings. The union-source scanColumn limit/offset arithmetic (offset non-distributive, applied over the concatenated stream; only the remaining-need upper bound pushed per partition; whole-partition skip via numRows) is correct and covered by the two new union tests. LLP docs (0097 decision, 0054/0057 forward-refs) and @ref annotations are consistent with the code.

[philcunliffe]

Approved - mergeable, green, reviewed at 57fdd76 (dual-review Codex+Claude). The three round-1 fixes are confirmed sound and complete; the one Codex major (scanColumn internal-field surface) was investigated and is a non-exploitable false positive (engine only names advertised columns; every dispatch path validates against them; empirical probe rejects internal-column queries with ColumnNotFoundError). Held for the maintainer to merge.