Skip to content

Build/cache native modules#2470

Merged
paul-nechifor merged 47 commits into
mainfrom
sam/native-modules-test
Jul 1, 2026
Merged

Build/cache native modules#2470
paul-nechifor merged 47 commits into
mainfrom
sam/native-modules-test

Conversation

@Dreamsorcerer

Copy link
Copy Markdown
Collaborator

Problem

nav_stack tests are being skipped due to lack of native modules.

Solution

Build the modules in CI, cache the nix build in Cachix.

@Dreamsorcerer
Dreamsorcerer marked this pull request as draft June 11, 2026 16:56
@codecov

codecov Bot commented Jun 11, 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    #2470      +/-   ##
==========================================
+ Coverage   71.10%   71.53%   +0.43%     
==========================================
  Files         897      897              
  Lines       80290    80290              
  Branches     7183     7186       +3     
==========================================
+ Hits        57089    57435     +346     
+ Misses      21319    20967     -352     
- Partials     1882     1888       +6     
Flag Coverage Δ
OS-ubuntu-24.04-arm 63.51% <ø> (+<0.01%) ⬆️
OS-ubuntu-latest 66.23% <ø> (+<0.01%) ⬆️
Py-3.10 66.22% <ø> (-0.01%) ⬇️
Py-3.11 66.22% <ø> (+<0.01%) ⬆️
Py-3.12 66.22% <ø> (-0.02%) ⬇️
Py-3.13 66.22% <ø> (+<0.01%) ⬆️
Py-3.14 66.24% <ø> (+<0.01%) ⬆️
Py-3.14t 66.23% <ø> (+<0.01%) ⬆️
SelfHosted-Large 30.12% <ø> (+0.02%) ⬆️
SelfHosted-Linux 38.05% <ø> (+0.57%) ⬆️
SelfHosted-macOS 36.31% <ø> (-0.04%) ⬇️

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

Files with missing lines Coverage Δ
...igation/cmu_nav/modules/far_planner/far_planner.py 97.33% <ø> (ø)
...ion/cmu_nav/modules/path_follower/path_follower.py 95.45% <ø> (ø)
...u_nav/modules/terrain_analysis/terrain_analysis.py 96.15% <ø> (ø)

... and 8 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.

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a CI job that builds the cmu_nav native modules via Nix, pushes the resulting store paths to Cachix, and then fetches them in self-hosted-tests so the previously-skipped nav-stack tests can run. A lightweight marker-cache job gates the build to avoid redundant work on unchanged inputs.

  • Two new CI jobs (cmu-nav-natives-marker + cmu-nav-natives) wire together a skip-if-cached pattern: the marker job does a lookup-only cache check, and the build job only runs when the marker is absent, then saves the marker only after a successful Cachix push to prevent false-positive skips.
  • bin/build-cmu-nav-natives discovers NativeModuleConfig subclasses via import-time reflection and builds them in parallel with ProcessPoolExecutor, checking for an existing executable before invoking Nix.
  • Version bumps for far_planner (v0.5.0 → v0.7.1), path_follower (v0.2.0 → v0.2.1), and terrain_analysis (v0.1.1 → v0.1.2), plus install-nix.sh gains INPUT_* defaults for standalone invocation.

Confidence Score: 5/5

Safe to merge — the workflow logic is sound, the fork-PR guard is correctly applied to both the marker and build jobs, and the previously flagged placeholder key has been replaced with a real Ed25519 public key.

No functional defects found. The marker-cache skip pattern is correctly implemented (marker saved only on build success, looked up before building, consumed correctly by self-hosted-tests). The build script's discovery, parallelism, and early-exit logic are all correct. Version bumps are straightforward. The install-nix.sh defaults are safe no-ops when the variables are already set.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds cmu-nav-natives-marker and cmu-nav-natives jobs with correct fork-PR guard, marker-based skip logic, and Cachix push; wires self-hosted-tests to depend on cmu-nav-natives with proper skipped/success check; ci-complete updated with cmu-nav-natives in needs and allowed-skips.
bin/build-cmu-nav-natives New discovery-and-build script; correctly finds NativeModuleConfig subclasses via regex pre-filter + import, builds in parallel via ProcessPoolExecutor, skips already-built executables, and exits non-zero on any failure.
docker/ros/install-nix.sh Added default assignments for INPUT_ENABLE_KVM, INPUT_SET_AS_TRUSTED_USER, INPUT_EXTRA_NIX_CONFIG, and INPUT_INSTALL_OPTIONS so the script works when called directly from a workflow run: step, not only through the composite action.
dimos/navigation/cmu_nav/modules/far_planner/far_planner.py Bumps far_planner build_command from v0.5.0 to v0.7.1.
dimos/navigation/cmu_nav/modules/path_follower/path_follower.py Bumps path_follower build_command from v0.2.0 to v0.2.1.
dimos/navigation/cmu_nav/modules/terrain_analysis/terrain_analysis.py Bumps terrain_analysis build_command from v0.1.1 to v0.1.2.
docs/usage/native_modules.md Adds a Faster builds via the Cachix substituter section with the real public key and nix.conf snippet; no placeholder values remain.

Reviews (8): Last reviewed commit: "Merge branch 'main' into sam/native-modu..." | Re-trigger Greptile

Comment thread .github/workflows/ci.yml Outdated
Comment thread docs/usage/native_modules.md Outdated
Comment thread bin/build-nav-stack-natives Outdated
Dreamsorcerer and others added 3 commits June 11, 2026 18:20
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@Dreamsorcerer
Dreamsorcerer marked this pull request as ready for review June 15, 2026 20:19
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 15, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Jun 15, 2026
@Dreamsorcerer
Dreamsorcerer marked this pull request as draft June 15, 2026 21:45
@Dreamsorcerer
Dreamsorcerer marked this pull request as ready for review June 22, 2026 14:51
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 22, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Jun 22, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 23, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Jun 29, 2026
@Dreamsorcerer Dreamsorcerer added the backport:skip Skip creating a backport to any release branches label Jun 29, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 29, 2026
@paul-nechifor
paul-nechifor merged commit cfb0bce into main Jul 1, 2026
32 of 33 checks passed
@paul-nechifor
paul-nechifor deleted the sam/native-modules-test branch July 1, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip Skip creating a backport to any release branches ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants