Skip to content

Yimo/feat/graphviz rpc - #3693

Open
poorwym wants to merge 3 commits into
mainfrom
yimo/feat/graphviz-rpc
Open

Yimo/feat/graphviz rpc#3693
poorwym wants to merge 3 commits into
mainfrom
yimo/feat/graphviz-rpc

Conversation

@poorwym

@poorwym poorwym commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Problem

Blueprint topology was difficult to inspect from the CLI. The existing Graphviz renderer only showed stream connections and did not visualize RPC dependencies.

Solution

  • Added dimos graph to generate a Blueprint topology SVG.
  • Added RPC Spec/function relationships using dashed edges.
  • Added a dark SVG background for standalone viewing. This needs more discussions. I'm not sure whether should add a background instead of transparent background.
  • Kept static rendering isolated from runtime transports.
  • Updated Rerun parsing to support long RPC labels.

How to Test

dimos graph unitree-go2-agentic --output unitree-go2-agentic.svg
uv run pytest dimos/cli/commands/test_graph.py -q
image

@mintlify

mintlify Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
dimensional 🔴 Failed Aug 26, 2026, 4:58 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@github-actions github-actions Bot added the first-time-contributor PR opened by an author who had not previously committed to this repository label Aug 26, 2026
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds a dimos graph command for rendering Blueprint stream and RPC relationships. A runtime reproduction with two namespaced instances of the same caller and provider classes showed that the graph merges their separate RPC relationships into a single class-name-based topology. Update the DOT RPC identities to retain Blueprint atom names before merging.

Confidence Score: 4/5

The graph command is not safe to merge as-is for deployments that contain multiple named instances of the same module class, because it can display incorrect RPC provider relationships.

A focused executed reproduction exercised the exact multi-instance resolution path and directly observed the renderer discard the instance identities in its emitted DOT.

Files Needing Attention: dimos/core/introspection/blueprint/dot.py needs to use Blueprint atom names for RPC caller and provider identities, with a regression test for same-class namespaced instances.

T-Rex T-Rex Logs

What T-Rex did

  • Produced a proof for the first posted P1 finding and linked it to the review comment.
  • Produced a proof for the second posted P1 finding and linked it to the review comment.
  • Before capture, per-instance RPC dependencies were resolved separately, with robot0/caller --service--> robot0/provider and robot1/caller --service--> robot1/provider.
  • After capture, both RpcConnection records became identical (caller='Caller', provider='Provider'), and the DOT output collapsed the two relationships into a single Caller/Provider module.
  • Uploaded artifacts including the executable reproduction source and the before/after RPC logs and DOT output to support the proofs.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 DOT RPC renderer collapses distinct same-class Blueprint instances

    • Bug
      • Two independent namespaced RPC relationships (robot0/caller to robot0/provider and robot1/caller to robot1/provider) render as duplicate edges through one rpc_Caller_service contract between one Caller and one Provider DOT node.
    • Cause
      • _resolve_rpc_connections stores atom.module.__name__ and provider.module.__name__ in RpcConnection, discarding BlueprintAtom.name, which is the unique instance identity used for ModuleRef resolution.
    • Fix
      • At dimos/core/introspection/blueprint/dot.py:106-107, use caller=atom.name and provider=provider.name. The renderer's existing RPC node ID and edge construction will then retain unique namespaced identities; add a regression test with two namespaced same-class caller/provider pairs.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "add rpc connections" | Re-trigger Greptile

Comment on lines +106 to +107
caller=atom.module.__name__,
provider=provider.module.__name__,

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 RPC instance identities collapse

_resolve_rpc_connections stores atom.module.__name__ and provider.module.__name__, discarding the Blueprint atom names that distinguish instances. Two independently resolved same-class relationships, such as robot0/caller → robot0/provider and robot1/caller → robot1/provider, therefore render as one Caller → rpc_Caller_service → Provider topology with duplicate edges. Use atom.name and provider.name for the connection identities, and add a regression case with multiple namespaced instances of the same caller/provider classes.

Artifacts

Executable reproduction source for two namespaced same-class RPC pairs

  • Creates two independent same-class Blueprint instance pairs, resolves their ModuleRef providers, renders DOT, and asserts the observed collapse.

Resolved per-instance RPC dependencies before DOT rendering

  • The executed script shows `robot0/caller` resolves to `robot0/provider` and `robot1/caller` resolves to `robot1/provider`, establishing the distinct relationships the renderer must preserve.

DOT output with same-class RPC instances collapsed

  • The executed renderer produces two identical class-name RpcConnections and one repeated `rpc_Caller_service` DOT identity, confirming the topology collapse.

View artifacts

T-Rex Ran code and verified through T-Rex

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4912 1 4911 74
View the full list of 1 ❄️ flaky test(s)
dimos.e2e_tests.test_dimsim_spatial_memory::test_go_to_the_bed

Flake rate in main: 15.43% (Passed 137 times, Failed 25 times)

Stack Traces | 318s run time
lcm_spy = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x724ae07355b0>
start_blueprint = <function start_blueprint.<locals>.set_name_and_start at 0x724ae0450180>
human_input = <function human_input.<locals>.send_human_input at 0x724ae0450900>
dim_sim = <dimos.e2e_tests.dim_sim_client.DimSimClient object at 0x724be8e03ef0>
explore_house = <function explore_house.<locals>.explore at 0x724ae0450fe0>

    @pytest.mark.self_hosted_large
    def test_go_to_the_bed(lcm_spy, start_blueprint, human_input, dim_sim, explore_house) -> None:
        start_blueprint(
            "run",
            "unitree-go2-agentic",
            simulator="dimsim",
        )
        lcm_spy.save_topic(".../McpClient/on_system_modules/res")
        lcm_spy.wait_for_saved_topic(".../McpClient/on_system_modules/res", timeout=1200.0)
    
        explore_house()
    
        human_input("go to the bed")
    
>       lcm_spy.wait_until_odom_position(-3.567, -1.332, threshold=2, timeout=180)

dim_sim    = <dimos.e2e_tests.dim_sim_client.DimSimClient object at 0x724be8e03ef0>
explore_house = <function explore_house.<locals>.explore at 0x724ae0450fe0>
human_input = <function human_input.<locals>.send_human_input at 0x724ae0450900>
lcm_spy    = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x724ae07355b0>
start_blueprint = <function start_blueprint.<locals>.set_name_and_start at 0x724ae0450180>

dimos/e2e_tests/test_dimsim_spatial_memory.py:32: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
dimos/e2e_tests/lcm_spy.py:167: in wait_until_odom_position
    self.wait_for_message_result(
        predicate  = <function LcmSpy.wait_until_odom_position.<locals>.predicate at 0x724ae0467740>
        self       = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x724ae07355b0>
        threshold  = 2
        timeout    = 180
        x          = -3.567
        y          = -1.332
dimos/e2e_tests/lcm_spy.py:153: in wait_for_message_result
    wait_until(
        event      = <threading.Event at 0x724ae12b9730: unset>
        fail_message = 'Failed to get to position x=-3.567, y=-1.332'
        listener   = <function LcmSpy.wait_for_message_result.<locals>.listener at 0x724ae0451300>
        predicate  = <function LcmSpy.wait_until_odom_position.<locals>.predicate at 0x724ae0467740>
        self       = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x724ae07355b0>
        timeout    = 180
        topic      = '/odom#geometry_msgs.PoseStamped'
        type       = <class 'dimos.msgs.geometry_msgs.PoseStamped.PoseStamped'>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

predicate = <bound method Event.is_set of <threading.Event at 0x724ae12b9730: unset>>

    def wait_until(
        predicate: Callable[[], bool],
        *,
        timeout: float,
        interval: float = 0.1,
        message: str | None = None,
    ) -> None:
        """Poll ``predicate`` until it returns truthy or ``timeout`` elapses."""
        deadline = time.monotonic() + timeout
        while time.monotonic() < deadline:
            if predicate():
                return
            time.sleep(interval)
>       raise TimeoutError(message or f"Timed out after {timeout}s waiting for condition")
E       TimeoutError: Failed to get to position x=-3.567, y=-1.332

deadline   = 4218138.460349792
interval   = 0.1
message    = 'Failed to get to position x=-3.567, y=-1.332'
predicate  = <bound method Event.is_set of <threading.Event at 0x724ae12b9730: unset>>
timeout    = 180

.../utils/testing/waiting.py:35: TimeoutError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 26, 2026
@poorwym
poorwym force-pushed the yimo/feat/graphviz-rpc branch from 04d1aa6 to 6570872 Compare August 26, 2026 07:22
@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 26, 2026
@leshy

leshy commented Aug 26, 2026

Copy link
Copy Markdown
Member

looks great can we make rpc render an optional boolean then ready to merge

@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 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository 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