Address ManagerWorkers review feedback from oracle#211 - #40
Merged
spichen merged 1 commit intoAug 18, 2026
Merged
Conversation
- Remove the SKIP_LLM_TESTS opt-out fixture from tests/adapters/conftest.py and its usages; the tests now skip like every other LLM-config test. - Rename the delegation tool prefix to __delegate_to__ so it cannot collide with a real tool name, and route a delegation only when its suffix is an actual worker node. - Move flows/test_managerworkers_node.py into the main test_managerworkers.py since ManagerWorkers is not a Node. - Enforce that the I/Os of a ManagerWorkers match the I/Os of its group manager (same name and type), per the language spec decision. - Reject unsupported ManagerWorkers flow-step output shapes when the flow is converted instead of when the step runs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Addresses the review comments from oracle#211.
Changes
allow_llm_config_constructionfixture together with the_resolvehelper and_REAL_LLM_INITS, plus their usages. The ManagerWorkers tests now skip underSKIP_LLM_TESTS=1like every other LLM-config test.delegate_to_to__delegate_to__so it cannot collide with a real tool name. Routing is now built per-graph by_make_manager_router(worker_node_names)and only emits aSendwhen the tool-name suffix is an actual worker node; a real tool that merely matches the prefix is left to the react loop.flows/test_managerworkers_node.pymerged into the maintest_managerworkers.py, sinceManagerWorkersis not a Node.ManagerWorkersmust be the I/Os of its group manager (same name and type). The baseComponentWithIOvalidators already enforce identical titles; a new_validate_ios_match_group_manager_iosvalidator enforces identical types, with validation tests for the matching, mismatching, and extra-title cases._format_agent_result(runtime) intoAgentNodeExecutor.__init__, so conversion (load_component) raises theNotImplementedErrorbefore anything runs.Testing
pytest tests/adapters/langgraph/test_managerworkers.py tests/validation/ tests/serialization/test_managerworkers.py— 138 passedSKIP_LLM_TESTS=1 pytest tests/adapters/langgraph/test_managerworkers.py tests/adapters/langgraph/flows/— 34 passed, 16 skipped