Skip to content

Topic List - #3764

Draft
jeff-hykin wants to merge 16 commits into
mainfrom
jeff/feat/topic_list
Draft

Topic List#3764
jeff-hykin wants to merge 16 commits into
mainfrom
jeff/feat/topic_list

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Aug 29, 2026

Copy link
Copy Markdown
Member

config a list of topics that can be specified at blueprint build time

A port's launch-line value can now be an array of topic names instead of a
single name, so a module can subscribe a group of same-typed topics to one
callback that is told which topic each message arrived on.

Rust: `InputGroup<T>` port with `recv() -> Option<(usize, T)>` and a
`#[input_group(decode = ..., handler = ...)]` derive attribute.
Python: `NativeModuleConfig.topic_groups: dict[str, TopicGroup]`.

Existing single-topic ports are unaffected.
@jeff-hykin jeff-hykin changed the title One handler for N same-typed topics in rust native modules Core: Topic List Support - Config version Aug 29, 2026
@jeff-hykin
jeff-hykin marked this pull request as draft August 29, 2026 19:23
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.93487% with 8 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/core/module.py 90.24% 5 Missing and 3 partials ⚠️
@@            Coverage Diff             @@
##             main    #3764      +/-   ##
==========================================
+ Coverage   77.44%   77.63%   +0.18%     
==========================================
  Files        1315     1318       +3     
  Lines      124238   125011     +773     
  Branches    10830    11046     +216     
==========================================
+ Hits        96220    97056     +836     
+ Misses      24895    24821      -74     
- Partials     3123     3134      +11     
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.29% <96.93%> (+0.22%) ⬆️
OS-ubuntu-latest 74.84% <96.93%> (+0.21%) ⬆️
Py-3.10 74.83% <96.93%> (+0.21%) ⬆️
Py-3.11 74.83% <96.93%> (+0.21%) ⬆️
Py-3.12 74.83% <96.93%> (+0.21%) ⬆️
Py-3.13 74.83% <96.93%> (+0.21%) ⬆️
Py-3.14 74.84% <96.93%> (+0.21%) ⬆️
Py-3.14t 74.83% <96.93%> (+0.21%) ⬆️
SelfHosted-Large 30.22% <29.50%> (-0.07%) ⬇️
SelfHosted-Linux 34.73% <32.56%> (-0.11%) ⬇️

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

Files with missing lines Coverage Δ
dimos/core/baked_host.py 97.22% <100.00%> (ø)
dimos/core/coordination/blueprints.py 89.30% <100.00%> (+0.85%) ⬆️
dimos/core/native_module.py 82.35% <100.00%> (+1.03%) ⬆️
dimos/core/test_async_module_topic_funnels.py 100.00% <100.00%> (ø)
dimos/core/test_native_module.py 97.97% <100.00%> (+0.29%) ⬆️
dimos/core/module.py 79.88% <90.24%> (+1.40%) ⬆️

... and 42 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 Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds grouped native input ports so several same-typed topics can feed one indexed Rust handler. Group handling works in the Rust runtime, but baked module metadata still represents an input group as a scalar topic. A focused host fixture verified that this produces an empty InputGroup, preventing grouped handlers from receiving messages. This needs correction before merge.

Confidence Score: 4/5

Not merge-safe: baked modules using #[input_group] silently start without any group members.

There is one independent, verified P1 finding and it is not security-related, which maps to a score of 4.

Files Needing Attention: Update native/rust/dimos-module-macros/src/lib.rs together with the bake graph and host metadata path so grouped ports retain their array-shaped topic configuration.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for the posted P1 finding.
  • The proof was validated against the review comment for the finding to confirm alignment.
  • The focused Rust host fixture source was inspected to verify the test setup used for the proof.
  • The focused Rust host fixture observed output was reviewed to confirm the observed behavior matches the proof expectations.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat: one handler for N same-typed topic..." | Re-trigger Greptile

.iter()
.filter(|f| match f.kind {
FieldKind::Input { .. } => want_input,
FieldKind::Input { .. } | FieldKind::InputGroup { .. } => want_input,

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.

P1 Baked InputGroup wiring loses shape

InputGroup fields are registered as ordinary scalar inputs here, so baked metadata emits a single topic value instead of the topic_groups array required by grouped ports. The host parses that value into Topics.single, while Builder::input_group reads only Topics.grouped; consequently, a baked module starts with an empty group and its indexed handler receives no input. Preserve grouped-port metadata through bake generation and host configuration rather than representing the group as a scalar input.

Artifacts

Focused Rust host fixture source

  • The uploaded host test fixture constructs an InputGroup from the scalar baked metadata shape and asserts that its group length is zero, confirming the metadata-shape loss takeaway.

Focused Rust host fixture observed output

  • The captured cargo test output records `scalar baked cams metadata -> InputGroup length: Some(0)` and a passing assertion over the executed host construction path, confirming the empty InputGroup takeaway.

View artifacts

T-Rex Ran code and verified through T-Rex

Group entries were written as backend topic strings (`/cam0/color`),
which reads as LCM-specific and bypasses the stream-name vocabulary the
rest of a blueprint uses. They are now stream names as they read after
remapping and namespacing, resolved to wire channels the same way a
declared stream name is, with a leading `/` rejected outright.
Group entries used to be resolved to wire channels directly, so
`.remappings()`, `.namespace()` and transport pins silently did not
apply to them. Each entry now becomes a synthetic `In` stream: the
blueprint atom declares it, the coordinator wires it like a declared
port, and the module (python or native) reads the wired transport
instead of computing a channel by convention.
`StreamGroup`/`stream_groups` becomes `TopicFunnel`/`topic_funnels` in
python, and rust's `InputGroup`/`#[input_group]` becomes
`TopicFunnel`/`#[topic_funnel]` to match. Naming only, no behavior
change.
Rust: fold TopicFunnel<T> into Input<T> — every port takes one topic or an
array from the launch line. Handlers keep their (msg) signature; a handler
that needs the source opts in with #[input(meta)] and receives
(msg, Metadata { index, topic }).

Python: opt in by signature instead — a two-parameter handle_<port> or
handle_<input> receives (msg, Metadata(index, name)), where name is the
declared stream name pre-remapping; a one-parameter handler just gets the
message.
TopicFunnel.names may be a dict mapping each stream name to arbitrary info
(is this camera rectified, what are the depth units) that the handler
receives on Metadata.info. On the launch line such an entry becomes a
{"topic", "info"} object instead of a plain string; rust exposes it as
serde_json::Value, python as a dict. Entries without info stay plain
strings, so existing wiring is untouched.
The handler captured as a closure default was still typed Any | None from the getattr, and baked_host's _argv/_stdin_blob overrides never widened to the funnel-aware topics type.
Fanning several same-typed streams into one handler is stream routing, so it
belongs alongside the other routing rewrites rather than in a module kwarg. A
list of names (or a dict of name -> info) as a remapping value now fans that
port in; a string still renames a single stream.

.remappings() expands the fan-in into the blueprint atom right away: the port's
StreamRef is replaced by one In per entry, typed from the declaration.
remapping_map stays string-valued, so autoconnect, namespacing and transport
pins are untouched. The declared port also supplies the message type, so
TopicFunnel.msg_type is gone, and naming a port that does not exist is now an
error instead of a silent no-op.
@jeff-hykin jeff-hykin changed the title Core: Topic List Support - Config version Topic List Aug 31, 2026
Comment thread dimos/core/coordination/blueprints.py Outdated
The topics map was typed loosely enough that the baked host had to widen its
override to Any. Name the info-carrying entry after the native TopicEntry it
serializes to, and give the port's value its own alias so the host can state
what it passes through. The remaining Any is the info payload itself, which is
arbitrary by design.
Comment thread dimos/core/module.py Outdated
Comment thread dimos/core/module.py Outdated
"(use a manual self.<input>.subscribe(...) for sync handlers)"
)
handler: Callable[..., Any] = declared_handler
if _handler_wants_metadata(handler, f"{type(self).__name__}.handle_{input_name}"):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

# doesn't get metadata
def callback(msg): pass

# gets metadata
def callback(msg, metadata): pass

Comment thread dimos/core/native_module.py Outdated
Metadata is a word this codebase already uses for several unrelated things, so
the bare name says nothing about what the handler is being handed. Name it for
what it describes: which topic the message arrived on.
Namespacing walks a blueprint's streams uniformly, so prefixing a funnel entry
was already covered by the generic namespace tests once the entries became
streams. The port's absence from that same list and the leading-slash rejection
were each one assertion about an internal structure, not about anything a
module does.
Filtering the signature down to positional parameters made a keyword-only or
variadic handler quietly count as one-argument. A handler takes the message and
optionally the metadata, so count what it declares and reject anything else.
Review feedback: remappings needs no prose, TopicFunnel is better shown than
described, and _argv's existing one-liner was fine as it was.
A pydantic model is keyword-only, so building one meant naming both stored
fields at every call site even though authoring takes a single list-or-dict.
A plain dataclass takes that one argument positionally and splits it itself.

Also name the funnel tests' delivery timeout rather than repeating the literal.
A plain port is a funnel of one, so the two bind routines were the same
thirty lines twice. Treating a declared port as a single-stream funnel
folds them together and drops the second one, along with the extra rx
layer the funnel path was going through to reach the same dispatcher.

Funnel streams now get their default transport when they are built,
which is what makes the merged path safe standalone; a coordinator
overwrites it as before. That also leaves channel_for with no callers.
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.

1 participant