native rebuild changes by jeff-hykin · Pull Request #1780 · dimensionalOS/dimos

jeff-hykin · 2026-04-12T02:39:12Z

Problem

Editing source code doesnt cause native modules to rebuild. Very easy to forget and not friendly for ai edits.

Solution

Some generic utils:

dimos/utils/change_detect.py: Content-hash-based file change detection using xxhash.
NativeModuleConfig.rebuild_on_change: Optional list[str|Path|Glob]

Breaking Changes

None

How to Test

# Change detection tests
pytest dimos/utils/test_change_detect.py -v

# Native module rebuild tests
pytest dimos/core/test_native_rebuild.py -v

# Native module crash/thread leak test
pytest dimos/core/test_native_module.py::test_process_crash_triggers_stop -v

# LCM isolation tests
pytest dimos/protocol/pubsub/test_pattern_sub.py -v
pytest dimos/protocol/pubsub/impl/test_lcmpubsub.py -v

# Full fast suite
pytest -m 'not (tool or slow or mujoco)' dimos/

Contributor License Agreement

I have read and approved the CLA.

greptile-apps · 2026-04-12T02:43:15Z

Greptile Summary

This PR adds content-hash-based file change detection (dimos/utils/change_detect.py) and wires it into NativeModuleConfig.rebuild_on_change so that native modules automatically rebuild when their source files change.

_maybe_build cache-poisoning bug (native_module.py L400–407): did_change is called with the default update=True, writing the new hash to the cache before the build runs. If the build fails with RuntimeError, the cache already reflects the current source hash, so every subsequent _maybe_build() call sees no change and silently skips the rebuild — indefinitely. The fix is update=False on the pre-build check and update_cache(...) after confirmed success.
hash_paths None in f-string (mesh_utils.py L87): hash_paths returns str | None; using it directly in the f-string produces a literal "v3_None_…" cache directory for missing/unreadable URDF files.

Confidence Score: 4/5

Not safe to merge as-is: the cache-poisoning bug in _maybe_build means a single failed build silently suppresses all future rebuild attempts, which is the exact problem this PR is designed to solve.

One clear P1 defect in the primary feature path (_maybe_build updating the cache before build success) and one P1 in mesh_utils.py (None-in-f-string cache key). The change_detect utility itself is well-designed; the bug is in how it's consumed.

dimos/core/native_module.py (cache update ordering) and dimos/manipulation/planning/utils/mesh_utils.py (None guard for hash_paths).

Important Files Changed

Filename	Overview
dimos/core/native_module.py	Adds `rebuild_on_change` to NativeModuleConfig and `_maybe_build` change-detection logic; contains a P1 bug where `did_change` is called with `update=True` before the build runs, meaning a failed build permanently poisons the cache and suppresses all future rebuild attempts.
dimos/utils/change_detect.py	New change-detection utility using xxhash with thread + flock cross-process locking; well-designed API with `update=False` / `update_cache` pattern; minor issue: `clear_cache` doesn't acquire locks before deleting the hash file.
dimos/manipulation/planning/utils/mesh_utils.py	Migrates to `hash_dict`/`hash_paths` from `change_detect`; `hash_paths` return value is used directly in an f-string without a None guard, which silently produces a `"v3_None_…"` cache key for missing URDF files.
dimos/utils/test_change_detect.py	Thorough tests for change_detect covering glob expansion, cache isolation, update=False semantics, relative paths, and the build workflow pattern.
dimos/core/test_native_rebuild.py	Tests rebuild-on-change happy paths; does not cover the failed-build scenario that exposes the cache-poisoning bug in `_maybe_build`.
pyproject.toml	Adds `xxhash>=3.0.0` as a core dependency; appropriate since `change_detect` is used in core modules.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[NativeModule.start] --> B[_maybe_build]
    B --> C{rebuild_on_change\nset AND exe exists?}
    C -- No --> D{exe exists?}
    C -- Yes --> E["did_change(update=True ⚠️)\nWrites new hash to cache\nBEFORE build runs"]
    E -- False\nno change --> D
    E -- True\nchanged --> F[needs_rebuild = True]
    F --> G[Run build_command]
    D -- Yes, no rebuild --> H[return early / no build]
    D -- No, or needs rebuild --> G
    G -- returncode != 0 --> I["raise RuntimeError\n(cache already updated ⚠️)"]
    G -- success --> J[exe verified exists]
    J --> K["did_change again\n(redundant – cache already current)"]
    K --> L[return]
    I --> M["Next _maybe_build call:\ndid_change → False\n→ skips rebuild forever ⚠️"]

    style E fill:#ffcccc,stroke:#cc0000
    style I fill:#ffcccc,stroke:#cc0000
    style M fill:#ffcccc,stroke:#cc0000

_{Reviews (1): Last reviewed commit: "native rebuild changes" | Re-trigger Greptile}

dimos/core/native_module.py

dimos/manipulation/planning/utils/mesh_utils.py

dimos/utils/change_detect.py

dimos/core/native_module.py

leshy · 2026-04-14T08:09:25Z

dimos/utils/change_detect.py

+        try:
+            # Include the path so additions/deletions/renames are detected
+            h.update(str(fpath).encode())
+            h.update(fpath.read_bytes())


this will load a full file into memory, ok for source files, but will for sure cause a weird issue for someone down the line. maybe easy hack for this is to check the file size and simply not hash if larger then 500k-1Mb or something

leshy · 2026-04-14T08:11:19Z

so not sure, haven't looked into details, but is there a reason we aren't relying nix build before each module start, since nix essentially does the above by itself?

leshy · 2026-04-14T08:12:13Z

dimos/core/native_module.py


    # Override in subclasses to exclude fields from CLI arg generation
-    cli_exclude: frozenset[str] = frozenset()
+    cli_exclude: frozenset[str] = frozenset({"rebuild_on_change"})


this breaks subclass overriding right? since now all nativemodules need to explicitly include this value

leshy · 2026-04-14T08:12:58Z

dimos/core/native_module.py

+    _tail_size = 50
+
+    @property
+    def _mod_label(self) -> str:


@cachedproperty better here

leshy · 2026-04-14T08:17:27Z

dimos/core/native_module.py

    _stopping: bool = False
-    _last_stderr_lines: collections.deque[str]
+    _stderr_tail: collections.deque[str]
+    _stdout_tail: collections.deque[str]


every line is already being passed to the logger.

So the tail buffers are keeping a duplicate copy of data that's already been logged just to re-log it again - why? I see also _last_stderr_lines was added previously.

IMO if we want more ergonomics with logging should fix the actual logger to allow you to filter, have per module log files you can tail in another term etc

leshy · 2026-04-14T08:19:51Z

dimos/core/native_module.py

+        # from terminal signals (SIGINT from the tty).  preexec_fn is unsafe
+        # in the presence of threads (subprocess docs), so we only use it on
+        # Linux where prctl(PR_SET_PDEATHSIG) has no alternative.
+        def _child_preexec_linux() -> None:


leshy · 2026-04-14T08:48:03Z

dimos/core/native_module.py

    extra_env: dict[str, str] = Field(default_factory=dict)
    shutdown_timeout: float = 10.0
    log_format: LogFormat = LogFormat.TEXT
+    rebuild_on_change: list[PathEntry] | None = None


btw this is the first actual setting that's different between prod and dev, we def don't want this on deployments, so probably if toggled True should be done not in code but the local config file level or via local env vars.

Whats the difference between prod and dev? I don't follow

jeff-hykin · 2026-04-14T19:43:18Z

so not sure, haven't looked into details, but is there a reason we aren't relying nix build before each module start, since nix essentially does the above by itself?

Unfortunately yeah. I don't care much about speed, but rebuild_on_change is 1980x faster than nix's check on reboot, and merely 280x faster once nix has a "warm" cache. And that's for the mid360, which is absolutely tiny. For a native module like PathFollower being checked onboard the G1, nix takes 1037.89ms just to confirm nothing changed (rebuild_on_change runs in 5ms on the G1).

I'm not thrilled about rebuild_on_change, but it was important for development on the G1.

Now that I think about it though, we should probably have it always rebuild unless there is a rebuild_on_change, since its more or less an optimization

native rebuild changes

817ddfd

jeff-hykin added the PlzReview label Apr 12, 2026

jeff-hykin mentioned this pull request Apr 12, 2026

feat(native): add rebuild-on-change for NativeModule #1599

Closed

1 task

jeff-hykin enabled auto-merge (squash) April 12, 2026 02:42

greptile-apps bot reviewed Apr 12, 2026

View reviewed changes

dimos/core/native_module.py Outdated Show resolved Hide resolved

dimos/manipulation/planning/utils/mesh_utils.py Outdated Show resolved Hide resolved

dimos/utils/change_detect.py Outdated Show resolved Hide resolved

jeff-hykin added 3 commits April 11, 2026 19:45

fix: ruff format and update uv.lock

b07cefb

minor fixes

bf80a6b

-

3e0cdef

leshy reviewed Apr 14, 2026

View reviewed changes

dimos/core/native_module.py Outdated Show resolved Hide resolved

leshy reviewed Apr 14, 2026

View reviewed changes

dimos/core/native_module.py

_tail_size = 50

@property

def _mod_label(self) -> str:

Copy link
Copy Markdown

Contributor

leshy Apr 14, 2026

Choose a reason for hiding this comment

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

@cachedproperty better here

leshy reviewed Apr 14, 2026

View reviewed changes

jeff-hykin added 9 commits April 14, 2026 08:23

fixup signal handling (looks worse but in practice behaves better)

b8b9981

cleanup

2718e73

make streaming

7641683

cleaner cache building logic (update file cache on check time)

46df237

make dependencies work on MacOS

c1743c9

add nix performance check

efc5aa9

get build working on macos

8ed96a6

add rebuild_on_change

3e38209

add perf test

bb69f20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

native rebuild changes#1780

native rebuild changes#1780
jeff-hykin wants to merge 13 commits intodevfrom
jeff/feat/native_rebuild2

jeff-hykin commented Apr 12, 2026

Uh oh!

greptile-apps bot commented Apr 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leshy Apr 14, 2026 •

edited

Loading

Uh oh!

leshy commented Apr 14, 2026

Uh oh!

leshy Apr 14, 2026

Uh oh!

leshy Apr 14, 2026

Uh oh!

leshy Apr 14, 2026 •

edited

Loading

Uh oh!

leshy Apr 14, 2026

Uh oh!

leshy Apr 14, 2026 •

edited

Loading

Uh oh!

jeff-hykin Apr 14, 2026

Uh oh!

jeff-hykin commented Apr 14, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jeff-hykin commented Apr 12, 2026

Problem

Solution

Breaking Changes

How to Test

Contributor License Agreement

Uh oh!

greptile-apps bot commented Apr 12, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leshy Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leshy commented Apr 14, 2026

Uh oh!

leshy Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

leshy Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

leshy Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leshy Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

leshy Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-hykin Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

jeff-hykin commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leshy Apr 14, 2026 •

edited

Loading

leshy Apr 14, 2026 •

edited

Loading

leshy Apr 14, 2026 •

edited

Loading

jeff-hykin commented Apr 14, 2026 •

edited

Loading