Skip to content

api: move six groups out of controllers/substrate/mod.rs (pure move) - #569

Merged
ac-pplx merged 2 commits into
mainfrom
ac/split-api-substrate-mod-1
Oct 16, 2026
Merged

ac-pplx merged 2 commits into
mainfrom
ac/split-api-substrate-mod-1

Conversation

@ac-pplx

@ac-pplx ac-pplx commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

crates/libs/api/src/controllers/substrate/mod.rs (2,964 code lines) holds the SubstrateAdapter trait and its dispatch, plus about a dozen unrelated groups of types and helpers. This first step moves six of those groups into child modules. No open PR edits them. mod.rs drops to 2,042 code lines; the largest new file is execution_fence.rs at 275.

A later step moves the trait, the dispatch impl, errors, capabilities and request types, once the open PRs that edit them (#559, #554, #545, #350, #336, #168) have landed or rebased.

This is a pure move, generated by scripts/split_rust_module.py (#562) from the spec below and proved by its verify. It is part of splitting the largest production modules into smaller ones with one job each.

What moved where

Module Holds Code lines
node_api_client Clients for a node's node-api (worker, runtime, builder) and the RPC budgets they share. 263
pool_claims Warm-pool eligibility and the request, claim and refusal types of a pooled execution claim. 163
local_artifact The local artifact a sandbox publishes when it suspends, and the guard that refuses to delete the current one. 89
execution_fence The identity fence that decides whether a recorded execution is the one a retire or teardown may act on. 275
recovery_charge Activation admission and the recovery charge a cold or restored activation pays. 65
sandbox_state_probe Reads of the sandbox states a node reports, used to verify before destroying. 89
mod.rs (parent) the SubstrateAdapter trait and its dispatch, Substrate, errors, capabilities, lifecycle request types, node targets, artifact-backup enqueue, and the artifact persistence a test pins by locator 2042

How to check it

The spec (item names, not lines):

file = "crates/libs/api/src/controllers/substrate/mod.rs"

[[module]]
name = "node_api_client"
doc = "Clients for a node's node-api (worker, runtime, builder) and the RPC budgets they share."
items = [
  "type RuntimeTransport", "struct RuntimeClient", "impl RuntimeClient",
  "impl std::ops::Deref for RuntimeClient", "impl std::ops::DerefMut for RuntimeClient",
  "type RuntimeInternalClient", "type BuilderClient",
  "const NODE_RPC_TIMEOUT", "const NODE_RPC_TIMEOUT_SECS", "const NODE_API_HTTP2_KEEP_ALIVE_INTERVAL",
  "const NODE_API_HTTP2_KEEP_ALIVE_TIMEOUT", "const BLOCKING_WAIT_MARGIN_SECS", "const NODE_RPC_BLOCKING_BUDGET",
  "const ALB_IDLE_TIMEOUT_SECS", "const _",
  "fn node_api_endpoint", "fn configure_node_api_endpoint", "fn retrying_worker_node_api",
  "fn connect_node_api_client_for_target", "fn connect_retrying_node_api_client_for_target",
  "fn node_api_channel", "fn node_api_connect_endpoint", "fn require_activation_policy_support",
  "fn connect_worker", "const KEPT_WORKER_CONNECTION_WINDOW_BYTES", "fn connect_worker_watched",
  "fn connect_worker_internal", "fn connect_builder",
]

[[module]]
name = "pool_claims"
doc = "Warm-pool eligibility and the request, claim and refusal types of a pooled execution claim."
items = [
  "fn default_pool_eligible_for_request", "fn pool_eligible_with_env",
  "struct PooledExecutionClaimRequest", "struct PooledExecutionPoolShape", "impl PooledExecutionPoolShape",
  "struct PooledExecutionClaim", "struct AdmittedPoolClaim", "struct AdmissionPoolClaim",
  "enum AdmittedPoolClaimLookup", "struct PooledExecutionClaimAttempt", "enum PooledClaimRefusal",
  "impl std::fmt::Debug for PooledExecutionClaimAttempt", "impl PooledExecutionClaimAttempt",
  "enum PooledPrepareFailureCleanup",
]

[[module]]
name = "local_artifact"
doc = "The local artifact a sandbox publishes when it suspends, and the guard that refuses to delete the current one."
items = [
  "struct PublishedLocalArtifact", "impl PublishedLocalArtifact", "fn reject_deleting_current_local_artifact",
]

[[module]]
name = "execution_fence"
doc = "The identity fence that decides whether a recorded execution is the one a retire or teardown may act on."
items = [
  "struct PodIdentityFence", "enum RecordedExecution", "impl RecordedExecution", "enum ObservedPod",
  "struct ExecutionIdentity", "impl ExecutionIdentity", "impl PodIdentityFence", "enum RetireFence",
  "struct ObservedPodIdentity", "struct RetireExecutionRequest", "impl RetireExecutionRequest",
  "enum PodFenceMismatch", "impl PodFenceMismatch", "impl std::fmt::Display for PodFenceMismatch",
  "enum RetireExecutionOutcome", "impl RetireExecutionOutcome", "enum TeardownProgress", "impl TeardownProgress",
]

[[module]]
name = "recovery_charge"
doc = "Activation admission and the recovery charge a cold or restored activation pays."
items = [
  "enum ActivationAdmissionClass", "struct ActivationAdmissionRejection", "impl ActivationAdmissionRejection",
  "type ActivationAdmissionFuture", "trait ActivationAdmission", "enum RecoveryCharge", "impl RecoveryCharge",
]

[[module]]
name = "sandbox_state_probe"
doc = "Reads of the sandbox states a node reports, used to verify before destroying."
items = [
  "enum ObservedSandboxState", "impl ObservedSandboxState", "impl std::fmt::Display for ObservedSandboxState",
  "fn sandbox_states_with_client", "fn observed_sandbox_states",
]

The tool is scripts/split_rust_module.py from #562 (not on main yet).

  • Regenerate: on main at 9d6f4f65d, run split_rust_module.py apply spec.toml && cargo fmt --all. This reproduces the first commit.
  • Proof: split_rust_module.py verify origin/main → regenerated and compared 7 files from 1 split parents: identical. verify derives the spec from the diff, regenerates the split from the merge base, and requires HEAD's Rust files to equal the rustfmt'd result byte for byte.
  • Edits inside moved text: 20 private items, fields or methods → pub(super); 2 pub(super) → pub(in crate::controllers). Each keeps the reach the item had: one module deeper, pub(super) reaches exactly the old module.

Observable changes

  • Log targets: 2 tracing events without an explicit target: now log under the new module paths (pplx_space_api::controllers::substrate::<module> instead of pplx_space_api::controllers::substrate). Nothing in the repository selects on these targets exactly.

  • sql! locators: the leading /*module:line*/ comment of 1 moved statement now names the new module path.

  • A test pins a locator: controllers::sandbox::lifecycle::characterization_tests::completed_actions_do_not_wait_for_a_key_share_lock matches persist_optional_current_local_artifact's statement in pg_stat_activity by its /*pplx_space_api::controllers::substrate:N*/ locator. Moving that function made the test fail, so it stays in mod.rs. Only PublishedLocalArtifact and reject_deleting_current_local_artifact move to local_artifact; EnqueuedArtifactBackup and PublishedDurableSnapshot stay with the parent code that uses them.

  • Comment: space-node-api-local/values.yaml cited internal_client in substrate/mod.rs; a docs commit points it at substrate/node_api_client.rs.

  • Diagnostics name new locations:

    • panic and backtrace locations of the 1 panic site (expect, unwrap, unreachable!, asserts) in moved code name the new file and line;
    • the 2 tracing callsites in moved code report the new file and line in their metadata, including OpenTelemetry's code.filepath, code.lineno and code.namespace;
    • std::any::type_name of a moved type names its new module;
    • the 2 sql! locators that stay in the parent shift line numbers;

    The reviews found nothing in the repository that depends on these.

Validation

Run on a Linux development machine on the previous base (b9fc395). After #552 merged, the branch was regenerated from the same spec on the new main, and verify passes there; this PR's CI validates the regenerated head.

  • cargo check --workspace --all-targets --all-features --locked: pass, no warnings.
  • Every clippy invocation in run_clippy_checks with -D warnings: pass.
  • cargo check -p pplx-space-api --no-default-features --all-targets: pass (the crate's default feature set is empty, so this is also the default build).
  • cargo nextest run -p pplx-space-api --all-features with Postgres, Redis and SPACE_CI_REQUIRE_TEST_DEPS=1: 1,826 passed, 1 skipped (the #[ignore] occupancy benchmark). This includes the Kubernetes adapter tests, which compile only with --features kubernetes.
  • cargo doc --document-private-items: the same 19 warnings as main.
  • archcheck as CI runs it: no required findings, and the same advisory count as main.

Interactions

Open PRs touching substrate/mod.rs: #559, #554, #545 (draft), #350 (draft), #336 (draft) and #168. This branch is regenerated on main after #552 merged. Their hunks were mapped onto current main by content. None edits an item this PR moves, and every item they edit stays in mod.rs: the trait, the dispatch impl, SubstrateError, capabilities and lifecycle request types.

Follow-ups, kept out of this PR because verify allows no other Rust edits: the moved pool_claims comment that says "only this module knows which condition occurred"; pool_eligibility_tests placement; parent imports used only by children; and deleting the uncalled connect_worker_internal.

The archcheck file_size baseline entry for this file is now stale (an advisory finding). Split PRs leave that table alone, so they don't conflict with each other there; one follow-up prunes the stale entries after they land.

Rollback

Revert the commit(s). No data, configuration, wire or public API change.

🤖 Generated with Claude Code

substrate/mod.rs holds the SubstrateAdapter trait and its dispatch, and
also a dozen unrelated groups of types and helpers. Move six of them,
which no open pull request edits, into child modules: node_api_client,
pool_claims, local_artifact, execution_fence, recovery_charge and
sandbox_state_probe. The trait, dispatch, errors, capabilities and
request types stay for a later step.

Generated by scripts/split_rust_module.py (#562) from the spec in the
pull request. Items move verbatim. The only edits keep each item's
reach: 20 private items, fields or methods become pub(super); 2
pub(super) ones become pub(in crate::controllers). The parent re-exports
the moved items, so no caller changes.
@ac-pplx
ac-pplx force-pushed the ac/split-api-substrate-mod-1 branch from 58318f6 to 15d51e0 Compare October 16, 2026 14:47
@ac-pplx
ac-pplx marked this pull request as ready for review October 16, 2026 14:53
@the-gigi-pplx

Copy link
Copy Markdown
Collaborator

Review

No findings.

Verified against 15d51e0b90ce06edb3cb864019fe559d4391a343.

What I ran:

  • scripts/split_rust_module.py verify origin/main, using the script from PR scripts: add split_rust_module.py for mechanical, verifiable Rust module splits #562's head: identical, exit 0.
  • apply with the spec in this PR's body, run on the merge base and rustfmt'd: it reproduces every .rs file at the head byte for byte. So the glob patterns in the published spec select the same items as the expanded spec verify --print-spec derives.
  • Each moved child file, grepped for self::/super:: in doc comments. The tool doesn't rewrite comments, so a link like that would point one module off after the move. There are none.
  • The non-split files, grepped for comments that name the old file next to a moved item.
  • Whether main has touched the split file since the merge base: it hasn't.

The non-Rust edit checks out: RuntimeClient::internal_client is at substrate/node_api_client.rs:57. connect_worker_internal has no caller, as the body says.

What I didn't check: whether the grouping is the right one. That's a design call, and I took the body's word for it. I also didn't rebuild or retest the crate; CI is green on this head.

@the-gigi-pplx the-gigi-pplx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ac-pplx
ac-pplx added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit c40398b Oct 16, 2026
25 checks passed
@ac-pplx
ac-pplx deleted the ac/split-api-substrate-mod-1 branch October 16, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants