Skip to content

refactor(stella-core): take the two shared sleeper doubles from stella-time - #6499

Merged
macanderson merged 2 commits into
mainfrom
fix/6484-shared-sleeper-doubles
Sep 11, 2026
Merged

refactor(stella-core): take the two shared sleeper doubles from stella-time#6499
macanderson merged 2 commits into
mainfrom
fix/6484-shared-sleeper-doubles

Conversation

@macanderson

@macanderson macanderson commented Sep 11, 2026

Copy link
Copy Markdown
Owner

What & why

The test-double half of #6484, on top of the merged stella-time PR (#6488). Re-opened: the first copy (#6491) was closed by GitHub when #6488's branch was deleted on merge. About thirty test files across stella-core, stella-engine and their tests/ directories each wrote the same Sleeper double — a no-op one under three names, and one on tokio's paused clock — and each copy grew a now() body in #6486. Every one of them is deleted here.

  • Integration tests and stella-engine take PausedSleeper and NoopSleeper from stella_time::test_util through a dev-dependency on stella-time with test-util on. For stella-core that is a dependency cycle, which cargo allows; it is the tokio / tokio-test shape.
  • stella-core's own unit tests keep one copy, src/tests.rs, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. The compiler forces that copy (the first attempt at taking the crate's doubles from stella-time failed on every unit test with "multiple different versions of crate stella_core"). It is named tests.rs so make core-no-io reads it as test code, which is how that guard tells test code from shipping code.
  • The one_home witness in stella-time now scans test files too. Its kept list shrinks to the four doubles with a shape a shared one cannot have — the retry tests' recording sleeper, the monitor's advancing sleeper, the hanging sleeper, and step/tests.rs's unpaused-time sleeper — plus the forced copy.

Closes #6484

The witness

  • This PR includes a witness test (fails on main, passes here)

crates/stella-time/tests/one_home.rs reads every .rs under crates/*/src and crates/*/tests. On the base branch it fails on twenty-three files that each define a sleeper double; here it passes, and a second test fails if a kept one disappears. Locally: stella-core's 893 lib tests and every integration test, stella-engine's tests and the time crate's 6 + 3 tests pass.

The gate

  • cargo fmt --check
  • clippy over the workspace at -D warnings — CI; ran locally on stella-core, stella-engine, stella-time
  • the workspace test suite — CI
  • Docs updated where behavior/flags changed (README, --help, doc comments)
  • CLA signed
  • Closes #N appears both above and as a commit trailer

Fix over file

  • Extra fixes in this PR: none beyond the sweep
  • Nothing was deferred

Ground-rule check

  • No I/O added to stella-core; src/tests.rs is #[cfg(test)] and make core-no-io is green
  • No new outbound network calls
  • No new cross-boundary serde types

Anything reviewers should know?

Summary by Sourcery

Consolidate test sleeper doubles in stella-time and enforce a single ownership location across the workspace.

Enhancements:

  • Centralize the shared no-op and paused-clock sleeper doubles in stella-time and update stella-core and stella-engine tests to use them.
  • Retain only the specialized sleeper doubles that require unique behavior, including the compiler-required stella-core unit-test copy.
  • Extend the sleeper ownership witness to scan test sources and document the shared test utility arrangement.

Build:

  • Add stella-time as a test-only dependency with the test-util feature where shared sleeper doubles are used.

Documentation:

  • Update architecture and crate documentation to describe the shared sleeper test utilities and the stella-core unit-test exception.

Tests:

  • Update the affected unit and integration tests to use the centralized sleeper doubles.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @macanderson, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.

…a-time

About thirty test files across stella-core, stella-engine and their
tests/ directories each wrote the same Sleeper double, a no-op one under
three names and one on tokio's paused clock, and each copy grew a now()
body in #6486. They are deleted. Integration tests and stella-engine take
PausedSleeper and NoopSleeper from stella_time::test_util through a
dev-dependency; stella-core's own unit tests keep one copy in
src/tests.rs, because a lib's unit tests are a second build of the lib
and a dev-dependency that links the lib implements the trait for the
first. stella-time's one_home witness now scans test files too.

Closes #6484
@macanderson
macanderson force-pushed the fix/6484-shared-sleeper-doubles branch from fd17d35 to ef34918 Compare September 11, 2026 18:29
@sourcery-ai

sourcery-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This refactor removes duplicated sleeper test doubles from stella-core, stella-engine, and their test suites, uses the shared stella-time::test_util implementations wherever Cargo permits, keeps one stella-core unit-test copy required by Rust’s separate library test build, and adds a source-scanning witness to prevent future duplication.

Flow diagram for the sleeper duplication witness

flowchart TD
    Scan["one_home.rs scans crate src and tests .rs files"] --> Detect["Find sleeper double definitions"]
    Detect --> Allowed["Compare against kept-list"]
    Allowed --> Pass["Pass when shared doubles are used and required exceptions remain"]
    Allowed --> Fail["Fail on duplicated or missing sleeper doubles"]
Loading

File-Level Changes

Change Details Files
Centralize the standard no-op and paused-clock sleeper doubles in stella-time and migrate tests to use them.
  • Enable the test-util feature through dev-dependencies in stella-core and stella-engine.
  • Replace locally defined sleeper implementations across unit and integration tests with stella_time::test_util::{NoopSleeper, PausedSleeper}.
  • Rename paused-clock test usage from TokioSleeper to PausedSleeper to distinguish it from the production sleeper.
crates/stella-core/Cargo.toml
crates/stella-core/tests/engine_emits_no_stage.rs
crates/stella-core/tests/hard_drop_write_back.rs
crates/stella-core/tests/parallel_dispatch.rs
crates/stella-core/tests/spend_gate.rs
crates/stella-core/tests/tool_wall_clock.rs
crates/stella-engine/Cargo.toml
crates/stella-engine/src/tests.rs
crates/stella-engine/tests/embedding.rs
crates/stella-core/src/accounted_call.rs
crates/stella-core/src/driver/
crates/stella-core/src/goal.rs
crates/stella-core/src/subagent/
Retain the compiler-required sleeper implementations for stella-core library unit tests in a dedicated test-only module.
  • Add src/tests.rs with PausedSleeper and NoopSleeper implementations matching the shared test utilities.
  • Expose the module only under #[cfg(test)] and update unit-test imports and call sites.
  • Document why the unit-test build cannot use the dev-dependency's implementations.
crates/stella-core/src/lib.rs
crates/stella-core/src/tests.rs
crates/stella-core/src/subagent/tests.rs
crates/stella-core/src/subagent/tests/*.rs
Expand the sleeper ownership witness to cover test sources and enforce the remaining intentional exceptions.
  • Scan both src and tests Rust files across workspace crates.
  • Remove retired sleeper copies from the allowlist and retain only the forced unit-test copy and doubles with specialized behavior.
  • Update witness documentation and the expected kept paths.
crates/stella-time/tests/one_home.rs
Update architecture and crate documentation to describe shared test utilities and the allowed dependency-cycle arrangement.
  • Document stella-core and stella-engine test usage of stella-time::test_util.
  • Clarify the unit-test exception and dev-dependency cycle in the crate README, ADR, and repository guidance.
  • Refresh test comments and nearby prose without changing production behavior.
AGENTS.md
crates/stella-core/README.md
crates/stella-time/README.md
docs/adr/0042-the-engine-reads-time-through-the-sleeper-port.md
crates/stella-core/src/accounted_call.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#6484 Provide a single shared home for the sleeper test doubles, with PausedSleeper and NoopSleeper behind stella-time's test-util feature, and remove the duplicated doubles from stella-core, stella-engine, and their integration tests except for necessary specialized or compiler-forced cases.
#6484 Add an automated witness that scans the workspace for duplicate sleeper implementations and documents the specific exceptions that are intentionally retained.
#6484 Complete the time-source consolidation by using stella-time as the shared home for production clock and sleeper implementations, while preserving the no-I/O constraint of stella-core and documenting the design.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@macanderson
macanderson enabled auto-merge (squash) September 11, 2026 18:42
@macanderson
macanderson merged commit cc3b670 into main Sep 11, 2026
28 of 29 checks passed
@macanderson
macanderson deleted the fix/6484-shared-sleeper-doubles branch September 11, 2026 19:36
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.

The engine reads the clock itself in nineteen places, and three crates keep their own copy of the wall clock

1 participant