[ac-pplx]

Summary

observed_demand_release_matching_characterization (cluster-pool) cloned a fresh test database for each of its ten cases. On CI it took 2.7 to 4.7 s: over the 2 s limit on every recent Rust tests run, and once over the 4 s failure bound from #1408. A run over that bound fails the Rust tests job of whichever pull request is running, whether or not it touches cluster-pool.

The cases now share one database. The releases and the policy are seeded once. Each case first deletes the template's sandboxes and every pool config, so it starts from the state a fresh database gave it. The cases and their expectations are unchanged.

Validation

At this head, on a dev box with Docker Postgres:

• cargo fmt and cluster-pool clippy with all features and -D warnings pass. nextest passes all 75 cluster-pool tests.
• This test, 20 runs each: before (main 95e0e7348), min 1.528 s, median 1.533 s, max 1.540 s; after, min 0.229 s, median 0.232 s, max 0.234 s. With the whole crate running in parallel, it took 0.16 s in each of 3 runs.
• Sensitivity:
  • Changing the sha256:99… case's expected release fails the test on that case.
  • Dropping the sandbox delete fails it: demand from earlier cases accumulates.
  • Dropping the config delete does not fail it, because each autoscale step already retires the previous case's configs. The delete stays, so each case starts as on a fresh database rather than relying on that.

The CI timing at this head is in this PR's Rust tests job.

Severity and risk

Severity: low to medium. The test does not fail on its own merits, but an over-bound run fails unrelated pull requests at random.

Risk: test-only. A case could now see state an earlier case left. The deletes cover the rows the step reads, and the sensitivity checks above show the sandbox delete is what keeps cases apart. Roll back by reverting.

AI assistance: Claude Code drafted the change and this description, and ran the timing and sensitivity checks.

[github-actions]

Thanks for opening this pull request! A SPACE maintainer will take a look.

CONTRIBUTING.md explains how we review outside contributions.

[coderabbitai]

Automatic approval withheld

Assessment	Current risk	Maximum for approval
Merge risk	Moderate	Low

Why: The shared test fixture is isolated correctly, but its destructive sandbox cleanup warrants explicit owner awareness before merge.

Commit: 744601e36612.

No actionable comments were generated in the recent review.

Walkthrough

The characterization test now reuses one fixture and resolver-release set across its cases. It upserts the policy once before the loop. Before each case, it deletes the template’s sandbox rows and all pool configs, then updates the template’s desired state.

Possibly related PRs

• perplexityai/SPACE#1219: Added the characterization test that this change updates.
• perplexityai/SPACE#1426: Addresses test duration in other slow tests and their 2-second budget.

Merge Risk: Moderate · up to 74460

The shared test fixture is isolated correctly, but its destructive sandbox cleanup warrants explicit owner awareness before merge.

Architecture risk: Low · up to 74460

The change affects 1 system.

Changed systems: crates

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — crates (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in crates/l30_storage/cluster/pool/src/tests.rs: The test now reuses one fixture and resolver-release set across all cases, with a single policy upsert before the loop. Each iteration clears sandbox rows for the resolver template and all pool configs before setting the case’s desired state; previously, fixture and release seeding and policy upsert occurred separately for each case.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "test: share one database across the demand-matching cases" uses the lowercase area prefix "test:" followed by a lowercase summary, matching the required format. It names the main change: th…
Description Follows The Pr Template		The description contains all required template content: a substantive “## Summary” section, a “## Validation” section with commands and results, a “## Severity and risk” section with severity and risk…
Bug Fixes Name A Regression Test		PASS. The pull request changes only crates/l30_storage/cluster/pool/src/tests.rs. It reuses test fixtures and resets database rows between characterization cases. It does not fix an application beha…
Weakened Safeguards Are Justified		The diff changes only crates/l30_storage/cluster/pool/src/tests.rs. It keeps all 10 cases and the assert_eq! expectation. It adds per-case deletion of sandbox and pool-config rows while sharing th…
Public-Safe Content		The changed Rust test and commit metadata contain no credentials, tokens, private keys, internal URLs or hostnames, real cloud identifiers, personal workstation paths, incident details, or raw product…

Full details: Title check

Explanation

The title "test: share one database across the demand-matching cases" uses the lowercase area prefix "test:" followed by a lowercase summary, matching the required format. It names the main change: the demand-matching test now reuses one database across its cases, which the raw summary and PR objectives both confirm. No trailing period, ticket ID, or link is present.

Full details: Description Follows The Pr Template

Explanation

The description contains all required template content: a substantive “## Summary” section, a “## Validation” section with commands and results, a “## Severity and risk” section with severity and risk, and an “AI assistance:” line that identifies Claude Code and the drafted description and timing/sensitivity work.

Full details: Bug Fixes Name A Regression Test

Explanation

PASS. The pull request changes only crates/l30_storage/cluster/pool/src/tests.rs. It reuses test fixtures and resets database rows between characterization cases. It does not fix an application behavior defect, so the regression-test requirement does not apply.

Full details: Weakened Safeguards Are Justified

Explanation

The diff changes only crates/l30_storage/cluster/pool/src/tests.rs. It keeps all 10 cases and the assert_eq! expectation. It adds per-case deletion of sandbox and pool-config rows while sharing the fixture. The description gives a specific reason: reduce test time and avoid CI timeout failures. No test, assertion, lint, CI check, or repository guard is removed or weakened without justification.

Full details: Public-Safe Content

Explanation

The changed Rust test and commit metadata contain no credentials, tokens, private keys, internal URLs or hostnames, real cloud identifiers, personal workstation paths, incident details, or raw production logs. The description reports test and CI timing measurements, not production measurements, and contains no private links or sensitive values.

Recent review info

Automatic approval policy

Commit: 744601e36612.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Low	Override 2
Security risk	Not set	Global configuration

Matching overrides (numbered as in the resolved configuration):

• Override 2 — any author; paths: **, !ui/**; merge risk: Low.

Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: perplexityai/SPACE
• Branch: main
• File: .coderabbit.yaml
• Commit: 95e0e73
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: f3bfd705-6a94-4f98-9893-5da949862261

Commits

Reviewing files that changed from the base of the PR and between 95e0e73 and 744601e.

Files selected for processing: 1

• crates/l30_storage/cluster/pool/src/tests.rs

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

[the-gigi-pplx]

Review

No findings.

Verified against 744601e36612bd14802e9e51ebe6f2df7c68e3f6. Reproduced locally with Docker Postgres:

• The test passes at this head, 0.233 to 0.236 s across three runs. On the parent commit it takes 1.545 s.
• Removing the per-case DELETE FROM sandboxes fails the test at the third case (demand=sha256:bbbb… present), so the delete is what keeps the cases apart.
• autoscale_configs writes only kubernetes_pool_configs (store.rs insert, update and delete paths), and nothing in the loop changes the policy or the releases. So the two deletes plus the per-case desired_state update reset everything a case reads, and moving the policy upsert out of the loop is sound.

I didn't run the full crate or workspace suite; the change touches only this one test.

[the-gigi-pplx]

Approved. No findings; details in the review comment above.