Spinel refreshes prepared statement recency on cache hits

[bunnykong]

Part of #12, independent of #378.

Spinel's prepared-statement cache retains entries by insertion order: a frequently reused statement can still be trimmed because it was prepared early. Refreshing hits makes replacement follow access order.

Mechanism

DbConn#prepare_cached searches from the most-recent end of the existing concrete Stmt array. A hit moves that same entry to the tail in place; an already-most-recent hit needs no array writes. Misses still append. This preserves statement pointers and any live cursor position (cache).

The existing trim keeps the last 128 entries at successful lease boundaries. Nothing evicts during prepare, so overflow cannot close an in-use cursor within its lease (trim, lease).

Evidence

Native Lobsters counts, condition lru-fix-lobsters-counts. Lobsters d771f81f, Spinel 62b01c7fc, SQLite 3.45.1, eight pooled connections, capacity 128; 15 warmup sequences, then 100 measured sequences of 106 visits. Hit rate is hits/(hits+prepares), excluding result replay.

Binds	Prepares, base → LRU	Hit rate, base → LRU
OFF	34,901 → 28,700	65.27% → 71.44%
ON	31,198 → 25,200	69.11% → 75.05%

This rerun uses x2's emitted Lobsters trees and Docker recipe, replacing only the instrumented runtime with this main/branch pair. It exactly reproduces x2-lobsters-lru's counts: 17.77% fewer prepares OFF and 19.23% fewer ON. Emission inputs retain that condition's three raising compile exclusions; neither runtime includes the separate text-binding repair. Executions and replay counts are unchanged within each bind mode; prepare failures and retained entries after close are zero.

The wrapper still exits 1 for the existing /u/michell_wiegand 500: 25/26 representative routes and all 106 verification visits pass. These counts are not timing or untouched-corpus parity claims. The prior x2-lobsters-lru condition matched 132/132 bodies byte-for-byte in each controlled pair; body equality was not rerun for this counts-only condition.

Native warm-hit microbenchmark, condition lru-fix-warm-hit. The actual base and LRU runtimes, compiled by Spinel at its default -O2, each start with 128 entries. Each sample times 10 million prepare_cached lookups after 100,000 warmups; initial promotion is outside the timed loop. There are 21 alternating base/LRU and LRU/base pairs per original entry position, with case order alternated too. Values are nanoseconds per lookup, median [p10–p90].

Repeated entry	Base	LRU
Original oldest	8.89 [8.65–9.31]	9.07 [8.84–9.68]
Original newest	729.29 [702.11–764.98]	9.56 [9.35–9.99]

For oldest hits, the paired LRU/base ratio is 1.013 [0.993–1.056]: a small positive point estimate, with overlapping sample ranges, not evidence of zero overhead. MRU-first search removes the long scan for repeated newest hits. These are lookup-only results, excluding stepping and request work. On the 18-core host, load averages (1/5/15 minutes) were 1.91/5.79/11.44 before and 3.48/4.79/10.01 after; one-minute load ranged 1.70–4.02. No worker-owned builds or Docker runs overlapped these samples.

Tests and CI

Three probes compile the real Spinel runtime and check:

• Repeated hits refresh recency without growing the cache.
• Reversing access order changes both eviction victims.
• Promotion retains a live statement's pointer and next row; overflow waits until lease exit, then trims the idle LRU entry while retaining the recently promoted statement.

Every probe fails on main and passes with the change. Additional mutations that reset hits or trim during prepare fail at their cursor-position and mid-lease assertions (driver, harness).

The suite joins SPINEL_TESTS; database runtime paths and both test files select it through the existing framework-tests-spinel loop. Planner tests cover those owners and combined selections. The native job remains advisory (planner, workflow).

Not covered

• CRuby's shim already refreshes recency; JRuby's cache stays exactly as it is on main.
• Bind lowering, bind defaults, text binding and the separate bind-correctness gate.
• Nested readers reusing identical SQL, exception recovery, or handles retained beyond a successful lease. Callers must still finalize their cursors before that lease ends.
• A hard within-lease capacity bound or trimming direct, unleased calls. Both keep their existing behavior.
• General request throughput or latency. Non-MRU promotion still shifts an array suffix; this is not an O(1) cache, and the warm-hit microbenchmark does not cover arbitrary access patterns.

Validation

Base: upstream main 65cc85c1e0f2ddd5ac79e2694703589e178f0679, fetched for this branch. macOS arm64; rustc/cargo 1.98.1; Spinel 62b01c7fc.

• SPINEL=/path/to/spinel cargo test --test spinel_stmt_cache_lru -- --ignored: 3 passed.
• cargo test --lib: 923 passed, 1 ignored, 0 failed.
• cargo test --no-run: all 440 test executables build; this is build coverage, not full integration execution.
• python3 -m unittest discover -s tests -p ci_plan_test.py: 35 passed on main and the branch, with TMPDIR set to the worker's canonical path.
• Emission with the bind environment gate unset: all six fixtures × all 14 transpile targets, 84 pairs / 168 invocations. Exit statuses and normalized stdout/stderr agree for every pair.

Fixture	Trees per side	Differences
real-blog	14	Spinel runtime only
tiny-blog	4	Spinel runtime only
tiny-blog-uuid	0	Same rejection
tiny-api	3	Spinel runtime only
roda-blog	14	Spinel runtime only
gem-capabilities	14	Spinel runtime only

Across 49 trees / 5,765 files per side, only five Spinel runtime/db.rb files differ; the other 44 trees are byte-identical. The other 35 combinations reject without output on both sides. Current main rejects tiny-blog-uuid for its undefined authenticate_user filter target.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Prepared-statement caching now refreshes a statement’s recency when it is reused, so less-recently used idle statements are evicted first when the cache is full.
  • Statements that are in active use remain available during cache activity, including partially consumed results. This improves cache behavior without changing how queries are used.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 91eae44c-6af2-4211-8103-49be22b381f9

📥 Commits

Reviewing files that changed from the base of the PR and between 26365fa and 5507016.

📒 Files selected for processing (5)

• runtime/spinel/db.rb
• scripts/ci-plan.py
• tests/ci_plan_test.py
• tests/spinel_stmt_cache_lru.rb
• tests/spinel_stmt_cache_lru.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The prepared-statement cache now refreshes entry order on hits, so trimming uses recent access order. New Spinel probes test cache recency, eviction order, and live-cursor preservation. CI planning selects the focused suite for related Spinel and Ruby runtime paths.

Changes

Spinel statement-cache LRU

Layer / File(s)	Summary
Refresh cache order on hits runtime/spinel/db.rb	prepare_cached searches from newest to oldest and moves a matching entry to the tail. The comment describes recency as a reuse proxy for per-value keys.
Probe LRU behavior and route focused tests tests/spinel_stmt_cache_lru.rb, tests/spinel_stmt_cache_lru.rs, scripts/ci-plan.py, tests/ci_plan_test.py	The Spinel probes cover cache recency, eviction order, and live-cursor preservation. The Rust harness runs each probe. CI planning selects the focused suite for related runtime paths, and routing tests check those selections.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: thomasklemm

Merge Risk: ⚪ Minimal · up to 55070

No actionable merge-blocking issue is identified; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 55070

Normal request and background processing retain exclusive database-connection ownership, and promotion preserves active statement identity. No supported security bypass was established. Concurrent direct access and interruption behavior remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected state is the prepared-statement cache of the selected DbConn. Connection leasing is the failure-containment boundary for normal parallel processing; broader exposure would require concurrent callers to share that connection outside the documented ownership contract.

Trust Boundaries and Controls

• observed — The checked HTTP dispatcher, Rails executor, cable event handlers and background-job drain establish or reuse a connection lease before database work. These paths provide concrete counterevidence against attacker-driven concurrent promotion on an unleased fallback connection.

Resilience and Maintainability Implications

• inferred — The existing unleased fallback is documented for single-fiber use. Promotion now performs several array writes without a cache-level lock, increasing race exposure under concurrent misuse. No supported caller exercising that misuse was established. The promotion loop has no explicit suspension point, but externally imposed interruption between assignments remains unverified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: prepared-statement cache hits refresh recency in Spinel.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[webit-wagner]

This conflicts with main now that #378 has landed, and the conflict is a real
one rather than a textual one — worth flagging because the mechanical union is
wrong.

Both PRs claim runtime/spinel/db.rb (and sqlite_adapter.rb) for their own
suite in scripts/ci-plan.py:

• main, from A varying-bind gate checks bound reads, and Spinel's bind_text keeps NUL bytes #378: owned_tests.update(("spinel_db_lease", "param_binds"))
• here: owned_tests.add("spinel_db_lease") + owned_tests.add("spinel_stmt_cache_lru")

I tried the obvious merge — all three in one update — and
tests/ci_plan_test.py then fails five assertions, because each PR's own test
pins the list it expects:

test_param_binds_owns_lowering_drivers_and_database_runtime
  ['spinel_db_lease', 'param_binds', 'spinel_stmt_cache_lru']
    != ['spinel_db_lease', 'param_binds']

test_shared_runtime_and_driver_inputs_select_real_harnesses
  ['spinel_db_lease', 'param_binds', 'spinel_stmt_cache_lru']
    != ['spinel_db_lease', 'spinel_stmt_cache_lru']

main passes all 47 of those tests; only the merged version fails. So the
question the merge has to answer is what touching db.rb should select now
that two suites depend on it — presumably both, with both expectations
updated — and that is a decision about the plan rather than a conflict
resolution. Both tests are yours, so you are the one who can say.

The rest rebased cleanly, and the Rust side compiles. I did not push anything.

The change itself reads well, by the way —
promotion_preserves_live_cursors_until_the_lease_ends is the test I would
have asked for, since that is where an access-order cache usually goes wrong.

(Not a maintainer, just reviewing.)