fix(porcelain): apply config overrides before resolving the blueprint - #4035
fix(porcelain): apply config overrides before resolving the blueprint#4035mustafab0 wants to merge 1 commit into
Conversation
❌ 7 Tests Failed:
View the full list of 7 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Greptile SummaryMoves porcelain configuration overrides ahead of target resolution so first-time blueprint imports observe the requested simulation mode.
Confidence Score: 3/5The PR is not yet safe to merge because failed resolution can leak process-wide configuration and cached blueprint modules can preserve the wrong hardware/simulation composition. The reordered mutation fixes first imports, but it introduces persistent state changes when resolution fails and leaves the reported behavior broken when the target module was previously imported under another configuration. Files Needing Attention: dimos/porcelain/dimos.py, dimos/porcelain/test_dimos.py Important Files Changed
Reviews (1): Last reviewed commit: "fix(porcelain): apply config overrides b..." | Re-trigger Greptile |
94553e8 to
c16a514
Compare
c16a514 to
37ab8c9
Compare
Dimos.run() resolved the target first and applied the instance's config overrides second. Resolving a blueprint name imports its module, and blueprints settle the arm-versus-sim split at import time -- SIMULATED = bool( global_config.simulation) at module scope -- so Dimos(simulation="mujoco").run( "xarm-grasp") captured the default, composed the hardware stack, and then tried to reach an xArm and a RealSense that are not attached. Ten blueprint modules read global_config at import, including the shared mujoco_if_sim helper, so this was every one of them through the Python API. The CLI was never affected: it preparses global config and updates it before resolving any name, which is the order this restores. Two edges around that reorder. global_config is process-wide, so a resolve that raises puts back what it found -- otherwise an unknown blueprint name leaves later independent instances composing simulated stacks. And a blueprint module imported earlier in the process is served from the import cache, where no reordering can reach it: the overrides cannot change a choice the module already made. Rather than compose the stale stack and let the caller find out on the wire, run() refuses, naming the module and the settings that cannot land. The registry holds the module path, so the check costs no import. Nothing in the tree hits any of this today -- the only two Dimos(...) call sites with overrides pass n_workers, which is not read at import -- but dimos shell points people at this API for automation. The tests fail on the old ordering, on the reorder without the unwind, and without the refusal.
37ab8c9 to
927c5fe
Compare
Dimos.run()resolved the target first and applied the instance's config overrides second.Resolving a blueprint name imports its module, and blueprints settle the arm-versus-sim split at import time,
SIMULATED = bool(global_config.simulation)at module scope, soDimos(simulation="mujoco").run("xarm-grasp")captured the default, composed the hardware stack, and then tried to reach an xArm and a RealSense that are not attached.Found by greptile on #3873, which added the tenth such blueprint. The cause is here, not there.