RPC Rework: With Var Renames #1143
Open
+726
−172
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lueprint to be consistent with docs
…o jeff/blueprint/3
…imos into jeff/blueprint/6
Greptile OverviewGreptile SummaryThis PR implements a major refactoring of the RPC system with support for Spec-based module references. The core changes enable modules to declare dependencies on other modules via Protocol-based specifications rather than concrete types. Major Changes:
Critical Issue:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client Module
participant BP as Blueprint
participant MC as ModuleCoordinator
participant Calc as Calculator Module
participant RPC as LCMRPC
Note over Client,Calc: Module Definition Phase
Client->>Client: Define calc: ComputeSpec
Calc->>Calc: Implement @rpc methods
Note over BP,MC: Blueprint Build Phase
BP->>BP: Create _BlueprintAtom for each module
BP->>BP: Extract StreamRefs (In/Out)
BP->>BP: Extract ModuleRefs (Spec/Module)
BP->>MC: build() - deploy modules
MC->>Calc: Instantiate Calculator
MC->>Client: Instantiate Client
Note over BP,MC: Connection Phase
BP->>BP: _connect_module_refs()
BP->>BP: Find module matching ComputeSpec
BP->>BP: Check spec_structural_compliance(Calculator, ComputeSpec)
BP->>BP: Check spec_annotation_compliance(Calculator, ComputeSpec)
BP->>Client: setattr(calc, calculator_instance)
BP->>Client: set_module_ref("calc", calculator_instance)
BP->>BP: _connect_rpc_methods()
BP->>RPC: Register Calculator.compute1
BP->>RPC: Register Calculator.compute2
BP->>Client: set_rpc_method("Calculator.compute1", callable)
BP->>Client: set_rpc_method("Calculator.compute2", callable)
Note over Client,RPC: Runtime RPC Call
Client->>Client: start()
Client->>Calc: self.calc.compute1(2, 3)
Calc->>RPC: call_sync("Calculator/compute1", args)
RPC->>Calc: Execute compute1(2, 3)
Calc-->>RPC: Return result: 5
RPC-->>Client: Return result: 5
|
jeff-hykin
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) | ||
|
|
||
| # if the spec is actually module, use that (basically a user override) | ||
| if inspect.isclass(spec, type) and issubclass(spec, Module): |
There was a problem hiding this comment.
inspect.isclass() only takes one argument, not two
Suggested change
| if inspect.isclass(spec, type) and issubclass(spec, Module): | |
| if inspect.isclass(spec) and issubclass(spec, Module): |
| @@ -0,0 +1,56 @@ | |||
| # Copyright 2026 Dimensional Inc. | |||
Contributor
There was a problem hiding this comment.
Should this example be in the docs? I don't think anyone will look at it here.
Member
Author
There was a problem hiding this comment.
I should have updated blueprints.md for the docs and the had this as a standalone example for people who jump into the code. (I believe this dir is linked to somewhere in the docs)
Comment on lines
268
to
294
| def _connect_transports(self, module_coordinator: ModuleCoordinator) -> None: | ||
| # Gather all the In/Out connections with remapping applied. | ||
| # dict when given (final/remapped) connection name+type, provides a list of modules + original (non-remapped) connection names | ||
| connections = defaultdict(list) | ||
| # Track original name -> remapped name for each module | ||
| module_conn_mapping = defaultdict(dict) # type: ignore[var-annotated] | ||
|
|
||
| for blueprint in self.blueprints: | ||
| for conn in blueprint.connections: | ||
| # Check if this connection should be remapped | ||
| remapped_name = self.remapping_map.get((blueprint.module, conn.name), conn.name) | ||
| # Store the mapping for later use | ||
| module_conn_mapping[blueprint.module][conn.name] = remapped_name | ||
| # Group by remapped name and type | ||
| connections[remapped_name, conn.type].append((blueprint.module, conn.name)) | ||
| if isinstance(remapped_name, str): | ||
| # Group by remapped name and type | ||
| connections[remapped_name, conn.type].append((blueprint.module, conn.name)) | ||
|
|
||
| # Connect all In/Out connections by remapped name and type. | ||
| for remapped_name, type in connections.keys(): | ||
| transport = self._get_transport_for(remapped_name, type) | ||
| for module, original_name in connections[(remapped_name, type)]: | ||
| instance = module_coordinator.get_instance(module) | ||
| for remapped_name, stream_type in connections.keys(): | ||
| transport = self._get_transport_for(remapped_name, stream_type) | ||
| for module, original_name in connections[(remapped_name, stream_type)]: | ||
| instance = module_coordinator.get_instance(module) # type: ignore[assignment] | ||
| instance.set_transport(original_name, transport) # type: ignore[union-attr] | ||
| logger.info( | ||
| "Transport", | ||
| name=remapped_name, | ||
| original_name=original_name, | ||
| topic=str(getattr(transport, "topic", None)), | ||
| type=f"{type.__module__}.{type.__qualname__}", | ||
| type=f"{stream_type.__module__}.{stream_type.__qualname__}", | ||
| module=module.__name__, | ||
| transport=transport.__class__.__name__, | ||
| ) |
Contributor
There was a problem hiding this comment.
Maybe this should be called _connect_streams() and connection renamed to stream everywhere in this func to match the rest of the renames.
paul-nechifor
approved these changes
Jan 29, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'll fix the test in just a bit