Skip to content

refactor: create top level rust workspace - #3666

Open
aclauer wants to merge 9 commits into
mainfrom
andrew/chore/speed-up-rust-ci
Open

refactor: create top level rust workspace#3666
aclauer wants to merge 9 commits into
mainfrom
andrew/chore/speed-up-rust-ci

Conversation

@aclauer

@aclauer aclauer commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Contribution path

  • Small, safe change that does not need a tracking issue
  • Linked issue or discussion: DIM-XXX / #XXX / URL

Problem

Past me thought it would be a good idea to put each rust module in its own workspace. Past me also didn't care as much as current me about CI runtime and code organization. Even though there is a ton of dependency overlap, each crate would rebuild all dependencies itself.

Solution

  • created a top level rust workspace
  • moved current FFI (ray tracer and mls planner) to their own crate so we can define the main implementation as rlib and the ffi crate as cdylib instead of doing both in one

Now building is a lot faster (especially important in CI) because each module can share the appropriate build artifacts.

this was also making cargo clippy take a horrible amount of time during precommit

How to Test

AI assistance

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@           Coverage Diff           @@
##             main    #3666   +/-   ##
=======================================
  Coverage   77.78%   77.79%           
=======================================
  Files        1287     1287           
  Lines      123467   123480   +13     
  Branches    10822    10822           
=======================================
+ Hits        96044    96059   +15     
  Misses      24281    24281           
+ Partials     3142     3140    -2     
Flag Coverage Δ
OS-ubuntu-24.04-arm 72.77% <100.00%> (+<0.01%) ⬆️
OS-ubuntu-latest 74.61% <100.00%> (-0.01%) ⬇️
Py-3.10 74.60% <100.00%> (+<0.01%) ⬆️
Py-3.11 74.59% <100.00%> (-0.02%) ⬇️
Py-3.12 74.60% <100.00%> (+<0.01%) ⬆️
Py-3.13 74.60% <100.00%> (+<0.01%) ⬆️
Py-3.14 74.61% <100.00%> (+<0.01%) ⬆️
Py-3.14t 74.60% <100.00%> (-0.01%) ⬇️
SelfHosted-Large 29.99% <31.25%> (+<0.01%) ⬆️
SelfHosted-Linux 35.21% <31.25%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/cli/bake/build.py 95.23% <100.00%> (+0.23%) ⬆️
dimos/cli/bake/codegen.py 100.00% <100.00%> (ø)
dimos/cli/bake/test_build.py 100.00% <100.00%> (ø)
dimos/cli/bake/test_codegen.py 100.00% <100.00%> (ø)
...avigation/nav_3d/mls_planner/mls_planner_native.py 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aclauer aclauer changed the title Consolidate rust to a single workspace refactor: consolidate rust to single workspace Aug 25, 2026
@aclauer aclauer changed the title refactor: consolidate rust to single workspace refactor: create top level rust workspace Aug 25, 2026
@aclauer
aclauer marked this pull request as ready for review August 25, 2026 02:16
@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change consolidates the Rust projects into a root Cargo workspace, separates the ray-tracing and MLS-planner Python bindings into dedicated cdylib crates, and updates bake/native-module builds to use the shared repository target directory.

Verified that locked workspace metadata resolves both binding crates, cargo test --workspace --all-features --locked passes, both extracted native modules build and import under their public Python names, and the MLS planner release binary is produced at the configured repository-root path.

T-Rex validation blocked

  • Tool: The generated bake host’s explicit shared-target build did not finish before the validation execution limit during concurrent Cargo compilation, so its final artifact-location comparison could not be completed.
  • Tool: The Rust native examples release build did not finish before the validation execution limit. The MLS planner path completed successfully, but the example executable paths were not fully exercised.

Confidence Score: 5/5

No actionable defect was found in the exercised workspace, native-extension, or MLS planner build flows.

The root workspace, complete Rust test suite, extracted binding builds, Python imports, public constructors, and MLS planner output path were exercised successfully. The incomplete bake and example build checks did not demonstrate a code defect.

Files Needing Attention: No files require changes from this review. If additional assurance is desired, rerun the focused checks for dimos/cli/bake/build.py, examples/native-modules/rust_ping_pong.py, and examples/native-modules/rust_tf.py without concurrent Cargo compilation.

T-Rex T-Rex Logs

What T-Rex did

  • Verified that the workspace root now resolves both Python binding crates as cdylib members, that all workspace tests pass, that the PyO3 binding crates build successfully, and that the Python native modules import and expose the public classes.
  • T-Rex produced proof for a posted P2 finding.
  • Validated the MLS planner path by confirming the executable path target/release/mls_planner was produced from the configured Rust working directory, while the focused Cargo build did not complete before the step limit.
  • Compared base versus PR workspace resolution and confirmed PR refactor: create top level rust workspace #3666 sets the repository root and resolves both bindings; command transcripts were uploaded and import evidence confirms the PyInit/init symbols match the expected public module names.
  • Analyzed bake-target-dir experiments showing the reproduction harness runs but could not complete due to the step limit, and that the before/after checks document the inherited-target versus explicit shared-target behavior.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P2 Validation remains incomplete because the PR-path build did not finish

    • Bug
      • The control build demonstrated the claimed pre-PR failure mode, but the current build command with explicit repository target directory was not allowed to complete before the execution limit. Therefore this review cannot make a final pass/fail claim for PR refactor: create top level rust workspace #3666.
    • Cause
      • The runtime test was interrupted by the maximum agent-step limit after long concurrent Cargo compilation and prior setup failures caused by unavailable Python test dependencies.
    • Fix
      • Re-run trex-artifacts/bake-target-dir-check.sh in a clean process state, wait for both captures to finish, and verify that target/debug/trex-bake-target-probe exists while the inherited target contains no host binary. Then run the focused bake unit tests in an environment with test dependencies installed.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Merge branch 'main' into andrew/chore/sp..." | Re-trigger Greptile

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant