-
Notifications
You must be signed in to change notification settings - Fork 487
Implement Sequential Executor tool
#806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
WalkthroughIntroduces a new sequential control-flow executor that chains tools with optional per-tool streaming and type-compatibility checks. Adds core implementation and registration, root entry point, comprehensive unit tests, an end-to-end example package (config, registration, README, tests), and updates examples documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant R as Workflow Runner
participant B as Builder
participant SE as SequentialExecutor
participant T1 as Tool 1
participant T2 as Tool 2
participant T3 as Tool 3
U->>R: Run workflow(input)
R->>B: Resolve tools from config
B-->>R: Tool refs (T1,T2,T3) + settings
R->>SE: Execute sequentially(tool_list, config, input)
SE->>T1: Execute(input) [stream or invoke]
alt T1 streaming
T1-->>SE: chunks...
SE->>T2: Execute(assembled output)
else T1 non-stream
T1-->>SE: result
SE->>T2: Execute(result)
end
alt T2 streaming
T2-->>SE: chunks...
SE->>T3: Execute(assembled output)
else T2 non-stream
T2-->>SE: result
SE->>T3: Execute(result)
end
T3-->>SE: final result
SE-->>R: final result
R-->>U: final result
note over SE: Validates I/O type compatibility (warn or raise)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Yuchen Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (21)
examples/agents/sequential_execution/pyproject.toml (1)
16-16: Clarify package description (matches example purpose).Minor accuracy tweak.
-requires-python = ">=3.11,<3.14" -description = "Router Agent for NeMo Agent toolkit" +requires-python = ">=3.11,<3.14" +description = "Sequential Execution example for NeMo Agent Toolkit"examples/agents/sequential_execution/README.md (6)
24-26: Fix TOC anchor to match section name.Prevents MD051 “invalid fragment”.
-- [Graph Structure](#graph-structure) +- [Pipeline Structure](#pipeline-structure)
48-50: Annotate fenced block language.Satisfy MD040.
-``` +```text Raw Text Input → Text Processor → Data Analyzer → Report Generator → Final Report--- `159-159`: **Use a heading instead of bold.** Satisfy MD036 and improve structure. ```diff -**Expected Workflow Output** +### Expected Workflow Output
203-203: Use a heading instead of bold.Satisfy MD036.
-**Starting the Sequential Execution Example Workflow** +#### Starting the Sequential Execution Example Workflow
215-215: Use a heading instead of bold.Satisfy MD036.
-**Non-Streaming Request to the Sequential Execution Example Workflow** +##### Non-Streaming Request to the Sequential Execution Example Workflow
226-226: Use a heading instead of bold.Satisfy MD036.
-**Streaming Request to the Sequential Execution Example Workflow** +##### Streaming Request to the Sequential Execution Example Workflowtests/nat/tool/test_sequential_execution.py (7)
97-102: PEP 484 Optional style (RUF013).Use
| Nonefor optional types.- def __init__(self, name: str = "streaming_mock_tool", chunks: list[str] = None, **kwargs): + def __init__(self, name: str = "streaming_mock_tool", chunks: list[str] | None = None, **kwargs):
78-85: Prefix unused params (ruff ARG002).Silence linter in mock helpers.
- async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, query: typing.Any | None = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, query: typing.Any | None = None, **_kwargs) -> str:
104-118: Prefix unused params (ruff ARG002).Also avoid shadowing
config.- async def astream(self, input_data, config=None, **kwargs): + async def astream(self, _input_data=None, _config=None, **_kwargs): @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, query: typing.Any | None = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, query: typing.Any | None = None, **_kwargs) -> str:
136-141: Prefix unused params (ruff ARG002).- async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, query: typing.Any | None = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, query: typing.Any | None = None, **_kwargs) -> str:
280-281: Useisfor type identity (E721).- assert input_type == str # First function's input type - assert output_type == int # Last function's output type + assert input_type is str # First function's input type + assert output_type is int # Last function's output type
304-305: Useisfor type identity (E721).- assert input_type == str # First function's input type - assert output_type == int # Last function's streaming_output_type + assert input_type is str # First function's input type + assert output_type is int # Last function's streaming_output_type
481-486: Prefix unusedkwargsin mocks (ruff ARG002).- async def _arun(self, query: str = "", **kwargs) -> str: + async def _arun(self, query: str = "", **_kwargs) -> str: @@ - def _run(self, query: str = "", **kwargs) -> str: + def _run(self, query: str = "", **_kwargs) -> str:examples/agents/sequential_execution/src/nat_sequential_execution/register.py (4)
35-35: Silence Ruff ARG001 by marking unused parameters.These registration functions don’t use
configorbuilder. Prefix them with underscores to satisfy the linter without changing behavior.-async def text_processor_function(config: TextProcessorFunctionConfig, builder: Builder): +async def text_processor_function(_config: TextProcessorFunctionConfig, _builder: Builder): -async def data_analyzer_function(config: DataAnalyzerFunctionConfig, builder: Builder): +async def data_analyzer_function(_config: DataAnalyzerFunctionConfig, _builder: Builder): -async def report_generator_function(config: ReportGeneratorFunctionConfig, builder: Builder): +async def report_generator_function(_config: ReportGeneratorFunctionConfig, _builder: Builder):Also applies to: 89-89, 172-172
157-161: Log and keep stack trace when swallowing JSON errors.Per repo guidelines, when catching without re-raising, use
logger.exception()to capture the traceback.- except json.JSONDecodeError: - # Handle invalid JSON input - error_result = {"error": "Invalid input format", "analysis_status": "failed"} - return json.dumps(error_result) + except json.JSONDecodeError as exc: + # Handle invalid JSON input + logger.exception("Invalid input format passed to data_analyzer_function.") + error_result = {"error": "Invalid input format", "analysis_status": "failed"} + return json.dumps(error_result)
231-232: Also log invalid analysis data in report generator.- except json.JSONDecodeError: - return "Report Generation Failed: Invalid analysis data format" + except json.JSONDecodeError as exc: + logger.exception("Invalid analysis data format passed to report_generator_function.") + return "Report Generation Failed: Invalid analysis data format"
137-144: Top-words counting can be simpler and faster with Counter.Optional tweak for clarity/perf; no behavior change.
- words = text.lower().split() - word_freq = {} - for word in words: - if len(word) > 3: # Only count words longer than 3 characters - word_freq[word] = word_freq.get(word, 0) + 1 - - top_words = sorted(word_freq.items(), key=lambda x: x[1], reverse=True)[:5] + from collections import Counter + words = re.findall(r"\b\w{4,}\b", text.lower()) + top_words = Counter(words).most_common(5)src/nat/tool/sequential_execution.py (3)
16-22: Adopt itertools.pairwise and satisfy Ruff B905/RUF007.Use
pairwisefor successive pairs; avoids zip slicing and clarifies intent.import logging import typing +from itertools import pairwise @@ - for src_fn, target_fn in zip(function_list[0:-1], function_list[1:]): + for src_fn, target_fn in pairwise(function_list): if not _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config): raise ValueError( f"The output type of the {src_fn.instance_name} function is not compatible with the input type of the {target_fn.instance_name} function" )Also applies to: 85-89
126-130: Streaming: don’t assume.content; support str-like chunks and avoid quadratic concat.Buffer to a list and coerce via
getattr(..., "content", str(...)).- output = "" - async for chunk in tool.astream(tool_input): - output += chunk.content - tool_response = output + output_parts: list[str] = [] + async for chunk in tool.astream(tool_input): + output_parts.append(getattr(chunk, "content", str(chunk))) + tool_response = "".join(output_parts)
100-104: Add a short Google-style docstring for the public tool.Public APIs under
src/require docstrings.@register_function(config_type=SequentialExecutionToolConfig, framework_wrappers=[LLMFrameworkEnum.LANGCHAIN]) async def sequential_execution(_config: SequentialExecutionToolConfig, builder: Builder): - + """ + Execute a list of registered functions sequentially, piping each output into the next. + + Args: + _config: Sequential execution configuration. + builder: Builder used to resolve functions and wrappers. + """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/agents/sequential_execution/README.md(1 hunks)examples/agents/sequential_execution/configs/config.yml(1 hunks)examples/agents/sequential_execution/pyproject.toml(1 hunks)examples/agents/sequential_execution/src/nat_sequential_execution/register.py(1 hunks)examples/agents/sequential_execution/tests/test_router_agent.py(1 hunks)src/nat/tool/register.py(1 hunks)src/nat/tool/sequential_execution.py(1 hunks)tests/nat/tool/test_sequential_execution.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/README.{md,ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Each documentation README must follow the NeMo Agent toolkit naming rules and must not use deprecated names
Files:
examples/agents/sequential_execution/README.md
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
examples/agents/sequential_execution/README.mdexamples/agents/sequential_execution/pyproject.tomlsrc/nat/tool/sequential_execution.pyexamples/agents/sequential_execution/configs/config.ymlsrc/nat/tool/register.pyexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.pytests/nat/tool/test_sequential_execution.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
examples/agents/sequential_execution/README.mdsrc/nat/tool/sequential_execution.pysrc/nat/tool/register.pyexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.pytests/nat/tool/test_sequential_execution.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/agents/sequential_execution/README.mdexamples/agents/sequential_execution/pyproject.tomlsrc/nat/tool/sequential_execution.pyexamples/agents/sequential_execution/configs/config.ymlsrc/nat/tool/register.pyexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.pytests/nat/tool/test_sequential_execution.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/agents/sequential_execution/README.mdexamples/agents/sequential_execution/pyproject.tomlexamples/agents/sequential_execution/configs/config.ymlexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/tool/sequential_execution.pysrc/nat/tool/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/tool/sequential_execution.pysrc/nat/tool/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/tool/sequential_execution.pysrc/nat/tool/register.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/tool/sequential_execution.pysrc/nat/tool/register.pyexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.pytests/nat/tool/test_sequential_execution.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/tool/sequential_execution.pyexamples/agents/sequential_execution/configs/config.ymlsrc/nat/tool/register.pyexamples/agents/sequential_execution/tests/test_router_agent.pyexamples/agents/sequential_execution/src/nat_sequential_execution/register.pytests/nat/tool/test_sequential_execution.py
**/configs/**/*.y?(a)ml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code must live in a neighbouring configs/ directory
Files:
examples/agents/sequential_execution/configs/config.yml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
In workflow/config YAML, set llms.._type: nat_test_llm to stub responses.
Files:
examples/agents/sequential_execution/configs/config.yml
**/tests/test_*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Test files must be named test_*.py and placed under a tests/ folder
Files:
examples/agents/sequential_execution/tests/test_router_agent.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/tests/**/*.py: Test functions must use the test_ prefix and snake_case
Extract repeated test code into pytest fixtures; fixtures should set name=... in @pytest.fixture and functions named with fixture_ prefix
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Use pytest with pytest-asyncio for async code; mock external services with pytest_httpserver or unittest.mock
Files:
examples/agents/sequential_execution/tests/test_router_agent.pytests/nat/tool/test_sequential_execution.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests must live under tests/ and use configured markers (e2e, integration, etc.)
Files:
tests/nat/tool/test_sequential_execution.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/tool/test_sequential_execution.py
🧠 Learnings (1)
📚 Learning: 2025-08-28T23:22:41.742Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to packages/*/pyproject.toml : Package pyproject.toml must depend on nvidia-nat or a package starting with nvidia-nat- using ~=<two-digit> versions (e.g., ~=1.0)
Applied to files:
examples/agents/sequential_execution/pyproject.toml
🧬 Code graph analysis (5)
src/nat/tool/sequential_execution.py (2)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/utils/type_utils.py (2)
DecomposedType(60-488)is_type_compatible(445-488)
src/nat/tool/register.py (1)
src/nat/tool/sequential_execution.py (1)
sequential_execution(101-145)
examples/agents/sequential_execution/tests/test_router_agent.py (1)
src/nat/runtime/loader.py (1)
load_workflow(97-117)
examples/agents/sequential_execution/src/nat_sequential_execution/register.py (2)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/cli/type_registry.py (1)
register_function(459-467)
tests/nat/tool/test_sequential_execution.py (3)
src/nat/builder/function_info.py (1)
FunctionInfo(290-625)src/nat/tool/sequential_execution.py (5)
sequential_execution(101-145)SequentialExecutionToolConfig(41-56)ToolExecutionConfig(35-38)_validate_function_type_compatibility(59-71)_validate_sequential_tool_list(75-97)src/nat/utils/type_utils.py (1)
DecomposedType(60-488)
🪛 markdownlint-cli2 (0.17.2)
examples/agents/sequential_execution/README.md
25-25: Link fragments should be valid
(MD051, link-fragments)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
203-203: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
215-215: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
226-226: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.12.2)
src/nat/tool/sequential_execution.py
85-85: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
85-85: Prefer itertools.pairwise() over zip() when iterating over successive pairs
Replace zip() with itertools.pairwise()
(RUF007)
87-89: Avoid specifying long messages outside the exception class
(TRY003)
110-110: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
110-110: Avoid specifying long messages outside the exception class
(TRY003)
135-135: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
examples/agents/sequential_execution/src/nat_sequential_execution/register.py
35-35: Unused function argument: config
(ARG001)
35-35: Unused function argument: builder
(ARG001)
89-89: Unused function argument: config
(ARG001)
89-89: Unused function argument: builder
(ARG001)
172-172: Unused function argument: config
(ARG001)
172-172: Unused function argument: builder
(ARG001)
tests/nat/tool/test_sequential_execution.py
78-78: Unused method argument: query
(ARG002)
78-78: Unused method argument: kwargs
(ARG002)
82-82: Unused method argument: query
(ARG002)
82-82: Unused method argument: kwargs
(ARG002)
97-97: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
104-104: Unused method argument: input_data
(ARG002)
104-104: Unused method argument: config
(ARG002)
104-104: Unused method argument: kwargs
(ARG002)
111-111: Unused method argument: query
(ARG002)
111-111: Unused method argument: kwargs
(ARG002)
115-115: Unused method argument: query
(ARG002)
115-115: Unused method argument: kwargs
(ARG002)
136-136: Unused method argument: query
(ARG002)
136-136: Unused method argument: kwargs
(ARG002)
139-139: Unused method argument: query
(ARG002)
139-139: Unused method argument: kwargs
(ARG002)
280-280: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
281-281: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
304-304: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
305-305: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
481-481: Unused method argument: kwargs
(ARG002)
484-484: Unused method argument: kwargs
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (1)
src/nat/tool/register.py (1)
25-25: LGTM: import to trigger registration.The module import pattern matches existing tools and ensures auto-registration.
examples/agents/sequential_execution/tests/test_router_agent.py
Outdated
Show resolved
Hide resolved
willkill07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level question regarding bikeshedding / organization:
I'd expect the composability of this along with the router agent to be pretty common. However, these are defined in completely different areas (tool vs agent). Would it make sense to align where they live?
Secondary question on name: if this is to be moved into the agent section, would "linear_agent" make more sense?
examples/agents/sequential_execution/tests/test_router_agent.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Yuchen Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/nat/front_ends/mcp/tool_converter.py (1)
95-97: Docstring is misleading for MCP wrapper; revert to accurate, non-prescriptive wording.This inner wrapper isn’t the “entry point agent.” MCP calls it to bridge to the underlying NAT function. Keep the docstring neutral to avoid confusing users and LLM tool descriptions elsewhere.
Apply:
- """This is the entry point agent that MUST be called before calling any other tools.""" + """Internal MCP wrapper invoked to call the underlying NAT function. Not a user-facing entry point."""Note: Per previous learnings, tool descriptions should come from the function/workflow description, not this internal wrapper.
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml (1)
42-43: Rephrase description; don’t instruct users to call this as “the entry point.”The entry-point notion belongs in docs, not config descriptions. Also prefer folded YAML for readability.
- description: "This is a simple calculator agent that can perform basic arithmetic operations. When using as a MCP tool, this - MUST be the entry point agent that should be called before calling any other tools." + description: > + A simple calculator agent for basic arithmetic operations. When used as an MCP tool, invoke + the agent as your primary entry point before other tools.src/nat/tool/sequential_execution.py (2)
91-98: Prefer itertools.pairwise for adjacent validation; cleaner and avoids index slicing.+from itertools import pairwise @@ - if len(function_list) > 1: - for src_fn, target_fn in zip(function_list[0:-1], function_list[1:]): + if len(function_list) > 1: + for src_fn, target_fn in pairwise(function_list): try: _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config) - except ValueError as e: - raise ValueError(f"The sequential tool list has incompatible types: {e}") + except ValueError as err: + logger.error("The sequential tool list has incompatible types: %s", err) + raise
73-76: Shorten exception message construction (TRY003) and rely on logging for context.Optional: keep the raised ValueError simple and put details into logs (already addressed above). No functional change required.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/agents/sequential_execution/pyproject.toml(1 hunks)examples/agents/sequential_execution/tests/test_sequential_executor.py(1 hunks)examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml(2 hunks)src/nat/front_ends/mcp/tool_converter.py(1 hunks)src/nat/tool/sequential_execution.py(1 hunks)tests/nat/tool/test_sequential_execution.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/agents/sequential_execution/pyproject.toml
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/tool/sequential_execution.pysrc/nat/front_ends/mcp/tool_converter.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/tool/sequential_execution.pysrc/nat/front_ends/mcp/tool_converter.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/tool/sequential_execution.pysrc/nat/front_ends/mcp/tool_converter.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/tool/sequential_execution.pysrc/nat/front_ends/mcp/tool_converter.pyexamples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/tool/sequential_execution.pyexamples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.ymlsrc/nat/front_ends/mcp/tool_converter.pyexamples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/tool/sequential_execution.pysrc/nat/front_ends/mcp/tool_converter.pyexamples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/tool/sequential_execution.pyexamples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.ymlsrc/nat/front_ends/mcp/tool_converter.pyexamples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/tool/sequential_execution.pyexamples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.ymlsrc/nat/front_ends/mcp/tool_converter.pyexamples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
**/configs/**/*.y?(a)ml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code must live in a neighbouring configs/ directory
Files:
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
In workflow/config YAML, set llms.._type: nat_test_llm to stub responses.
Files:
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.ymlexamples/agents/sequential_execution/tests/test_sequential_executor.py
**/tests/test_*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Test files must be named test_*.py and placed under a tests/ folder
Files:
examples/agents/sequential_execution/tests/test_sequential_executor.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/tests/**/*.py: Test functions must use the test_ prefix and snake_case
Extract repeated test code into pytest fixtures; fixtures should set name=... in @pytest.fixture and functions named with fixture_ prefix
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Use pytest with pytest-asyncio for async code; mock external services with pytest_httpserver or unittest.mock
Files:
examples/agents/sequential_execution/tests/test_sequential_executor.pytests/nat/tool/test_sequential_execution.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests must live under tests/ and use configured markers (e2e, integration, etc.)
Files:
tests/nat/tool/test_sequential_execution.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/tool/test_sequential_execution.py
🧠 Learnings (2)
📚 Learning: 2025-08-28T23:22:41.742Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to **/*.py : When re-raising exceptions, use bare `raise` and log with logger.error(), not logger.exception()
Applied to files:
src/nat/tool/sequential_execution.py
📚 Learning: 2025-09-15T21:26:29.430Z
Learnt from: saglave
PR: NVIDIA/NeMo-Agent-Toolkit#726
File: examples/frameworks/adk_demo/src/nat_adk_demo/weather_update_tool.py:0-0
Timestamp: 2025-09-15T21:26:29.430Z
Learning: In NAT's ADK integration, the docstring of registered functions serves as the tool description sent to the LLM, not standard Python function documentation. The docstring should describe the logical tool interface (parameters the LLM will provide) rather than the NAT framework wrapper parameters like tool_config and builder.
Applied to files:
src/nat/front_ends/mcp/tool_converter.py
🧬 Code graph analysis (3)
src/nat/tool/sequential_execution.py (3)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/utils/type_utils.py (2)
DecomposedType(60-488)is_type_compatible(445-488)tests/nat/tool/test_sequential_execution.py (1)
astream(104-109)
examples/agents/sequential_execution/tests/test_sequential_executor.py (1)
src/nat/runtime/loader.py (1)
load_workflow(97-117)
tests/nat/tool/test_sequential_execution.py (3)
src/nat/builder/function_info.py (1)
FunctionInfo(290-625)src/nat/tool/sequential_execution.py (5)
sequential_execution(108-151)SequentialExecutorConfig(41-56)ToolExecutionConfig(35-38)_validate_function_type_compatibility(59-75)_validate_tool_list_type_compatibility(79-104)src/nat/utils/type_utils.py (1)
DecomposedType(60-488)
🪛 Ruff (0.12.2)
src/nat/tool/sequential_execution.py
73-75: Avoid specifying long messages outside the exception class
(TRY003)
74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
75-75: Use explicit conversion flag
Replace with conversion flag
(RUF010)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
92-92: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
92-92: Prefer itertools.pairwise() over zip() when iterating over successive pairs
Replace zip() with itertools.pairwise()
(RUF007)
96-96: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
96-96: Avoid specifying long messages outside the exception class
(TRY003)
116-116: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
116-116: Avoid specifying long messages outside the exception class
(TRY003)
141-141: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
tests/nat/tool/test_sequential_execution.py
78-78: Unused method argument: query
(ARG002)
78-78: Unused method argument: kwargs
(ARG002)
82-82: Unused method argument: query
(ARG002)
82-82: Unused method argument: kwargs
(ARG002)
97-97: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
104-104: Unused method argument: input_data
(ARG002)
104-104: Unused method argument: config
(ARG002)
104-104: Unused method argument: kwargs
(ARG002)
111-111: Unused method argument: query
(ARG002)
111-111: Unused method argument: kwargs
(ARG002)
115-115: Unused method argument: query
(ARG002)
115-115: Unused method argument: kwargs
(ARG002)
136-136: Unused method argument: query
(ARG002)
136-136: Unused method argument: kwargs
(ARG002)
139-139: Unused method argument: query
(ARG002)
139-139: Unused method argument: kwargs
(ARG002)
280-280: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
281-281: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
304-304: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
305-305: Use is and is not for type comparisons, or isinstance() for isinstance checks
(E721)
477-477: Unused method argument: kwargs
(ARG002)
480-480: Unused method argument: kwargs
(ARG002)
🪛 Gitleaks (8.27.2)
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml
[high] 54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (2)
tests/nat/tool/test_sequential_execution.py (2)
271-282: Use identity for type assertions per ruff E721.[uggest_nitpick_refactor]
- assert input_type == str # First function's input type - assert output_type == int # Last function's output type + assert input_type is str + assert output_type is int @@ - assert input_type == str # First function's input type - assert output_type == int # Last function's streaming_output_type + assert input_type is str + assert output_type is intAlso applies to: 301-306
78-85: Tidy ruff ARG002/RUF013 in test doubles (rename unused params; fix Optional).[ suggest_nitpick_refactor ]
class MockTool(BaseTool): @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str:class StreamingMockTool(BaseTool): @@ - def __init__(self, name: str = "streaming_mock_tool", chunks: list[str] = None, **kwargs): + def __init__(self, name: str = "streaming_mock_tool", chunks: list[str] | None = None, **kwargs): @@ - async def astream(self, input_data, config=None, **kwargs): + async def astream(self, _input_data, _config=None, **_kwargs): @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str:class ErrorMockTool(BaseTool): @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str:class OrderTestTool(BaseTool): @@ - async def _arun(self, query: str = "", **kwargs) -> str: - return f"{query}->{self.__dict__['_tool_name']}" + async def _arun(self, _query: str = "", **_kwargs) -> str: + return f"{_query}->{self.__dict__['_tool_name']}" @@ - def _run(self, query: str = "", **kwargs) -> str: - return f"{query}->{self.__dict__['_tool_name']}" + def _run(self, _query: str = "", **_kwargs) -> str: + return f"{_query}->{self.__dict__['_tool_name']}"Also applies to: 111-118, 136-141, 477-481, 97-103, 104-111, 115-118
examples/agents/sequential_execution/tests/test_sequential_executor.py
Outdated
Show resolved
Hide resolved
examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: Yuchen Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (19)
pyproject.toml (1)
109-110: Add SPDX header to satisfy CI license checks.Per repo guidelines, this file must start with the Apache‑2.0 SPDX header.
Apply at the very top:
+ # SPDX-FileCopyrightText: Copyright (c) 2024-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + # SPDX-License-Identifier: Apache-2.0 [build-system] build-backend = "setuptools.build_meta"Also applies to: 182-182, 235-235
examples/control_flow/sequential_executor/pyproject.toml (1)
1-3: Missing SPDX header.Add the standard header at the top.
+ # SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + # SPDX-License-Identifier: Apache-2.0 [build-system] build-backend = "setuptools.build_meta"examples/control_flow/sequential_executor/src/nat_sequential_executor/register.py (3)
35-36: Annotate return types and silence ARG001 by marking unused params.Add explicit
AsyncIterator[FunctionInfo]and prefix unused args to satisfy ruff.+ from collections.abc import AsyncIterator @@ -async def text_processor_function(config: TextProcessorFunctionConfig, builder: Builder): +async def text_processor_function(_config: TextProcessorFunctionConfig, _builder: Builder) -> AsyncIterator[FunctionInfo]: @@ -async def data_analyzer_function(config: DataAnalyzerFunctionConfig, builder: Builder): +async def data_analyzer_function(_config: DataAnalyzerFunctionConfig, _builder: Builder) -> AsyncIterator[FunctionInfo]: @@ -async def report_generator_function(config: ReportGeneratorFunctionConfig, builder: Builder): +async def report_generator_function(_config: ReportGeneratorFunctionConfig, _builder: Builder) -> AsyncIterator[FunctionInfo]:Also applies to: 89-90, 172-173
137-144: Tokenization includes trailing punctuation, skewing top_words.Strip punctuation via regex tokens to avoid counting “word.” distinct from “word”.
- words = text.lower().split() - word_freq = {} - for word in words: - if len(word) > 3: # Only count words longer than 3 characters - word_freq[word] = word_freq.get(word, 0) + 1 + tokens = re.findall(r"\b\w+\b", text.lower()) + word_freq: dict[str, int] = {} + for tok in tokens: + if len(tok) > 3: # Only count words longer than 3 characters + word_freq[tok] = word_freq.get(tok, 0) + 1
157-160: Log JSON parse failures per exception-handling guidelines.Capture stack traces when not re‑raising.
- except json.JSONDecodeError: - # Handle invalid JSON input + except json.JSONDecodeError: + logger.exception("Invalid input format for data_analyzer_function") error_result = {"error": "Invalid input format", "analysis_status": "failed"} return json.dumps(error_result) @@ - except json.JSONDecodeError: - return "Report Generation Failed: Invalid analysis data format" + except json.JSONDecodeError: + logger.exception("Invalid analysis data format for report_generator_function") + return "Report Generation Failed: Invalid analysis data format"Also applies to: 231-233
examples/control_flow/sequential_executor/configs/config.yml (1)
16-27: Optional: demonstrate type checking by using structured models and enabling checks.Consider switching the example to Pydantic schemas between steps and set
check_type_compatibility: trueto showcase the feature.examples/control_flow/sequential_executor/README.md (5)
20-21: Fix broken source path.The implementation lives under nat/control_flow/sequential_executor.py, not nat/tool/sequential_execution.py.
-... provides a [`sequential_execution`](../../../src/nat/tool/sequential_execution.py) tool. +... provides a [`sequential_execution`](../../../src/nat/control_flow/sequential_executor.py) tool.
24-26: TOC/section anchor mismatch.Link text says “Graph Structure” but the section is “Pipeline Structure”. Update TOC entry and its fragment.
-- [Graph Structure](#graph-structure) +- [Pipeline Structure](#pipeline-structure)Also applies to: 40-47
48-50: Add a language to the fenced code block.Satisfy MD040 by specifying a language.
-``` +```text Raw Text Input → Text Processor → Data Analyzer → Report Generator → Final Report--- `18-21`: **Brand naming consistency.** Use “NeMo Agent Toolkit” capitalization consistently per repo style. ```diff -... the NeMo Agent toolkit. +... the NeMo Agent Toolkit. -### Starting the NeMo Agent Toolkit Server +### Starting the NeMo Agent Toolkit server -You can start the NeMo Agent toolkit server... +You can start the NeMo Agent Toolkit server...Also applies to: 199-207
159-159: Use headings instead of bold for section titles.Fix MD036 and improve structure.
-**Expected Workflow Output** +### Expected workflow output ... -**Starting the Sequential Execution Example Workflow** +### Start the Sequential Execution example workflow ... -**Non-Streaming Request to the Sequential Execution Example Workflow** +#### Non‑streaming request ... -**Streaming Request to the Sequential Execution Example Workflow** +#### Streaming requestAlso applies to: 203-206, 215-222, 226-233
examples/control_flow/sequential_executor/tests/test_sequential_executor.py (1)
40-44: Prefer pathlib for robust path joins.Minor readability/portability improvement.
-import os +from pathlib import Path ... - current_dir = os.path.dirname(os.path.abspath(__file__)) - config_file = os.path.join(current_dir, "../configs", "config.yml") + current_dir = Path(__file__).resolve().parent + config_file = (current_dir / ".." / "configs" / "config.yml").resolve().as_posix()src/nat/control_flow/sequential_executor.py (4)
67-76: Misleading debug log before compatibility check; use explicit f-string conversions.Log a neutral message and use !s instead of str() per ruff.
- logger.debug( - f"The output type of the {src_fn.instance_name} function is {str(src_output_type)}, is not compatible with" - f"the input type of the {target_fn.instance_name} function, which is {str(target_input_type)}") + logger.debug( + f"Validating compatibility: {src_fn.instance_name} output={src_output_type!s} → " + f"{target_fn.instance_name} input={target_input_type!s}" + )
95-101: Prefer itertools.pairwise and preserve cause when re‑raising.Cleaner adjacent pairs iteration; wrap original ValueError as cause.
+ from itertools import pairwise ... - if len(function_list) > 1: - for src_fn, target_fn in zip(function_list[0:-1], function_list[1:]): - try: - _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config) - except ValueError as e: - raise ValueError(f"The sequential tool list has incompatible types: {e}") + if len(function_list) > 1: + for src_fn, target_fn in pairwise(function_list): + try: + _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config) + except ValueError as e: + raise ValueError("The sequential tool list has incompatible types") from e
116-126: Preserve exception chaining and avoid blind except.Use “from e” and narrow catch where possible.
- try: + try: input_type, output_type = _validate_tool_list_type_compatibility(config, builder) except ValueError as e: if config.raise_type_incompatibility: - raise ValueError(f"The sequential executor tool list has incompatible types: {e}") + raise ValueError("The sequential executor tool list has incompatible types") from e else: - logger.warning(f"The sequential executor tool list has incompatible types: {e}") + logger.warning("The sequential executor tool list has incompatible types: %s", e) input_type = typing.Any output_type = typing.Any - except Exception as e: - raise ValueError(f"Error with the sequential executor tool list: {e}") + except (RuntimeError, IndexError, KeyError, TypeError) as e: + raise ValueError("Error with the sequential executor tool list") from e
141-149: Streaming concat uses quadratic string building; join chunks instead.Aggregate into a list and join once.
- if tool_execution_config.use_streaming: - output = "" - async for chunk in tool.astream(tool_input): - output += chunk.content - tool_response = output + if tool_execution_config.use_streaming: + chunks: list[str] = [] + async for chunk in tool.astream(tool_input): + chunks.append(getattr(chunk, "content", str(chunk))) + tool_response = "".join(chunks)tests/nat/control_flow/test_sequential_executor.py (3)
78-85: Silence ruff ARG002 in test doubles by prefixing unused params with underscores.Keeps tests clean without affecting behavior.
- async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: ... - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str: ... - async def astream(self, input, config=None, **kwargs): + async def astream(self, _input, _config=None, **_kwargs): ... - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: ... - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str: ... - async def _arun(self, query: str = "", **kwargs) -> str: + async def _arun(self, query: str = "", **_kwargs) -> str: ... - def _run(self, query: str = "", **kwargs) -> str: + def _run(self, query: str = "", **_kwargs) -> str:Also applies to: 111-118, 136-141, 486-491
520-526: Actually assert dynamic annotations.Exercise the async generator to obtain the function and validate annotations.
- def test_function_annotations_set_correctly(self, mock_builder): + @pytest.mark.asyncio + async def test_function_annotations_set_correctly(self, mock_builder): """Test that function annotations are set correctly based on type validation.""" config = SequentialExecutorConfig(tool_list=[FunctionRef("tool1")]) - with patch('nat.control_flow.sequential_executor._validate_tool_list_type_compatibility', - return_value=(str, int)): - # Get the generator - gen = sequential_execution(config, mock_builder) - - # Since this is an async generator, we need to test differently - # The actual annotation setting happens inside the generator function - assert gen is not None + with patch('nat.control_flow.sequential_executor._validate_tool_list_type_compatibility', + return_value=(str, int)): + async with sequential_execution(config, mock_builder) as function_info: + fn = function_info.single_fn # type: ignore[attr-defined] + assert fn.__annotations__.get("initial_tool_input") is str + assert fn.__annotations__.get("return") is int
249-252: Fix class docstring to match function name.The helper is _validate_tool_list_type_compatibility.
-"""Test cases for _validate_sequential_tool_list function.""" +"""Test cases for _validate_tool_list_type_compatibility function."""
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
examples/control_flow/sequential_executor/README.md(1 hunks)examples/control_flow/sequential_executor/configs/config.yml(1 hunks)examples/control_flow/sequential_executor/pyproject.toml(1 hunks)examples/control_flow/sequential_executor/src/nat_sequential_executor/register.py(1 hunks)examples/control_flow/sequential_executor/tests/test_sequential_executor.py(1 hunks)pyproject.toml(3 hunks)src/nat/control_flow/register.py(1 hunks)src/nat/control_flow/sequential_executor.py(1 hunks)tests/nat/control_flow/test_sequential_executor.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.pyexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pytests/nat/control_flow/test_sequential_executor.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.pyexamples/control_flow/sequential_executor/configs/config.ymlexamples/control_flow/sequential_executor/pyproject.tomlexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pypyproject.tomltests/nat/control_flow/test_sequential_executor.pyexamples/control_flow/sequential_executor/README.md
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.pyexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pytests/nat/control_flow/test_sequential_executor.pyexamples/control_flow/sequential_executor/README.md
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.pyexamples/control_flow/sequential_executor/configs/config.ymlexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pytests/nat/control_flow/test_sequential_executor.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/control_flow/sequential_executor.pysrc/nat/control_flow/register.pyexamples/control_flow/sequential_executor/configs/config.ymlexamples/control_flow/sequential_executor/pyproject.tomlexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pypyproject.tomltests/nat/control_flow/test_sequential_executor.pyexamples/control_flow/sequential_executor/README.md
**/configs/**/*.y?(a)ml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code must live in a neighbouring configs/ directory
Files:
examples/control_flow/sequential_executor/configs/config.yml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
In workflow/config YAML, set llms.._type: nat_test_llm to stub responses.
Files:
examples/control_flow/sequential_executor/configs/config.yml
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/control_flow/sequential_executor/configs/config.ymlexamples/control_flow/sequential_executor/pyproject.tomlexamples/control_flow/sequential_executor/src/nat_sequential_executor/register.pyexamples/control_flow/sequential_executor/tests/test_sequential_executor.pyexamples/control_flow/sequential_executor/README.md
**/tests/test_*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Test files must be named test_*.py and placed under a tests/ folder
Files:
examples/control_flow/sequential_executor/tests/test_sequential_executor.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/tests/**/*.py: Test functions must use the test_ prefix and snake_case
Extract repeated test code into pytest fixtures; fixtures should set name=... in @pytest.fixture and functions named with fixture_ prefix
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Use pytest with pytest-asyncio for async code; mock external services with pytest_httpserver or unittest.mock
Files:
examples/control_flow/sequential_executor/tests/test_sequential_executor.pytests/nat/control_flow/test_sequential_executor.py
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Add new dependencies to pyproject.toml in alphabetical order
Files:
pyproject.toml
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests must live under tests/ and use configured markers (e2e, integration, etc.)
Files:
tests/nat/control_flow/test_sequential_executor.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/control_flow/test_sequential_executor.py
**/README.{md,ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Each documentation README must follow the NeMo Agent toolkit naming rules and must not use deprecated names
Files:
examples/control_flow/sequential_executor/README.md
🧠 Learnings (2)
📚 Learning: 2025-08-28T23:22:41.742Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to packages/*/pyproject.toml : Package pyproject.toml must depend on nvidia-nat or a package starting with nvidia-nat- using ~=<two-digit> versions (e.g., ~=1.0)
Applied to files:
examples/control_flow/sequential_executor/pyproject.toml
📚 Learning: 2025-08-28T23:22:41.742Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to **/README.{md,ipynb} : Each documentation README must follow the NeMo Agent toolkit naming rules and must not use deprecated names
Applied to files:
examples/control_flow/sequential_executor/README.md
🧬 Code graph analysis (4)
src/nat/control_flow/sequential_executor.py (3)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/utils/type_utils.py (2)
DecomposedType(60-488)is_type_compatible(445-488)tests/nat/control_flow/test_sequential_executor.py (1)
astream(104-109)
examples/control_flow/sequential_executor/src/nat_sequential_executor/register.py (1)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)
examples/control_flow/sequential_executor/tests/test_sequential_executor.py (1)
src/nat/runtime/loader.py (1)
load_workflow(97-117)
tests/nat/control_flow/test_sequential_executor.py (3)
src/nat/builder/function_info.py (1)
FunctionInfo(290-625)src/nat/control_flow/sequential_executor.py (5)
SequentialExecutorConfig(41-56)ToolExecutionConfig(35-38)_validate_function_type_compatibility(67-80)_validate_tool_list_type_compatibility(83-106)sequential_execution(110-166)src/nat/utils/type_utils.py (1)
DecomposedType(60-488)
🪛 Ruff (0.12.2)
src/nat/control_flow/sequential_executor.py
73-73: Use explicit conversion flag
Replace with conversion flag
(RUF010)
74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
78-80: Avoid specifying long messages outside the exception class
(TRY003)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
80-80: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
96-96: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
96-96: Prefer itertools.pairwise() over zip() when iterating over successive pairs
Replace zip() with itertools.pairwise()
(RUF007)
100-100: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-100: Avoid specifying long messages outside the exception class
(TRY003)
103-103: Use explicit conversion flag
Replace with conversion flag
(RUF010)
104-104: Use explicit conversion flag
Replace with conversion flag
(RUF010)
120-120: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
120-120: Avoid specifying long messages outside the exception class
(TRY003)
125-125: Do not catch blind exception: Exception
(BLE001)
126-126: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
126-126: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
examples/control_flow/sequential_executor/src/nat_sequential_executor/register.py
35-35: Unused function argument: config
(ARG001)
35-35: Unused function argument: builder
(ARG001)
89-89: Unused function argument: config
(ARG001)
89-89: Unused function argument: builder
(ARG001)
172-172: Unused function argument: config
(ARG001)
172-172: Unused function argument: builder
(ARG001)
tests/nat/control_flow/test_sequential_executor.py
78-78: Unused method argument: query
(ARG002)
78-78: Unused method argument: kwargs
(ARG002)
82-82: Unused method argument: query
(ARG002)
82-82: Unused method argument: kwargs
(ARG002)
104-104: Unused method argument: input
(ARG002)
104-104: Unused method argument: config
(ARG002)
104-104: Unused method argument: kwargs
(ARG002)
111-111: Unused method argument: query
(ARG002)
111-111: Unused method argument: kwargs
(ARG002)
115-115: Unused method argument: query
(ARG002)
115-115: Unused method argument: kwargs
(ARG002)
136-136: Unused method argument: query
(ARG002)
136-136: Unused method argument: kwargs
(ARG002)
139-139: Unused method argument: query
(ARG002)
139-139: Unused method argument: kwargs
(ARG002)
486-486: Unused method argument: kwargs
(ARG002)
489-489: Unused method argument: kwargs
(ARG002)
🪛 markdownlint-cli2 (0.17.2)
examples/control_flow/sequential_executor/README.md
25-25: Link fragments should be valid
(MD051, link-fragments)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
203-203: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
215-215: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
226-226: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (6)
pyproject.toml (2)
109-110: Examples list update looks correct.Entry added in alphabetical position relative to existing items. No further action.
182-182: Confirm example entry-point target exists or update pyproject.tomlpyproject.toml declares nat_sequential_executor = { path = "examples/control_flow/sequential_executor", editable = true } (line ~182) and the entry-point nat_control_flow = "nat.control_flow.register" (line 235); repo search did not find the referenced source file — verify that examples/control_flow/sequential_executor exists or adjust the path/entrypoint.
examples/control_flow/sequential_executor/pyproject.toml (1)
12-14: Packaging and entry point look good.
- Depends on
nvidia-nat[langchain]~=1.3(meets learned rule).- Registers
nat.components: nat_sequential_executor.Also applies to: 22-23
examples/control_flow/sequential_executor/src/nat_sequential_executor/register.py (1)
80-81: Nice, clear docstrings and simple JSON-based chaining.Readable, self‑contained example functions; registration via
FunctionInfo.from_fnis correct.Also applies to: 162-164, 234-235
src/nat/control_flow/register.py (1)
18-19: Registration import is minimal and correct. Import resolves — src/nat/control_flow/register.py containsfrom . import sequential_executor(line 19) and src/nat/control_flow/sequential_executor.py exists.examples/control_flow/sequential_executor/README.md (1)
66-71: Docs/config mismatch — usetool_listandraise_type_incompatibility(notsequential_tool_list/check_type_compatibility)Config model at src/nat/control_flow/sequential_executor.py defines:
tool_list: list[FunctionRef]tool_execution_config: dict[str, ToolExecutionConfig]raise_type_incompatibility: boolrg returned "No files were searched" — unable to verify occurrences in the repo. Re-run the ripgrep command or confirm README lines; if present, apply the diff below.
File: examples/control_flow/sequential_executor/README.md (lines 66–71; also applies to 76–96, 97–115)
-### Sequential Execution Configuration +### Sequential Execution Configuration - **`_type`**: Set to `sequential_execution` to use the sequential execution tool -- **`sequential_tool_list`**: List of functions to execute in order (e.g., `[text_processor, data_analyzer, report_generator]`) -- **`check_type_compatibility`**: Whether to validate type compatibility between adjacent functions (default: false) +- **`tool_list`**: List of functions to execute in order (e.g., `[text_processor, data_analyzer, report_generator]`) +- **`raise_type_incompatibility`**: If true, type incompatibility raises; if false, logs a warning and proceeds. ... - text_processing_pipeline: - _type: sequential_execution - sequential_tool_list: [text_processor, data_analyzer, report_generator] - check_type_compatibility: false + text_processing_pipeline: + _type: sequential_execution + tool_list: [text_processor, data_analyzer, report_generator] + raise_type_incompatibility: false ... - text_processing_pipeline: - _type: sequential_execution - sequential_tool_list: [text_processor, data_analyzer, report_generator] - check_type_compatibility: true # Enable type compatibility checking + text_processing_pipeline: + _type: sequential_execution + tool_list: [text_processor, data_analyzer, report_generator] + raise_type_incompatibility: true # Raise on type incompatibility
examples/control_flow/sequential_executor/tests/test_sequential_executor.py
Show resolved
Hide resolved
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/nat/control_flow/sequential_executor.py (1)
113-115: Blocker: indexing tools with FunctionRef keys causes KeyError. Normalize to names.
tools_dictkeys are strings (tool.name), but the loop usesFunctionRefobjects.Apply this diff:
- tools: list[BaseTool] = builder.get_tools(tool_names=config.tool_list, wrapper_type=LLMFrameworkEnum.LANGCHAIN) - tools_dict: dict[str, BaseTool] = {tool.name: tool for tool in tools} + tool_names: list[str] = [ref.instance_name for ref in config.tool_list] + tools: list[BaseTool] = builder.get_tools(tool_names=tool_names, wrapper_type=LLMFrameworkEnum.LANGCHAIN) + tools_dict: dict[str, BaseTool] = {tool.name: tool for tool in tools} @@ - for tool_name in tool_list: - tool = tools_dict[tool_name] - tool_execution_config = config.tool_execution_config.get(tool_name, None) - logger.debug(f"Executing tool {tool_name} with input: {tool_input}") + for fn_ref in tool_list: + tool_name = fn_ref.instance_name + try: + tool = tools_dict[tool_name] + except KeyError as e: + logger.error(f"Tool {tool_name!s} not found in tool registry.") + raise + tool_execution_config = config.tool_execution_config.get(tool_name) + logger.debug(f"Executing tool {tool_name!s} with input: {tool_input!r}")Also applies to: 133-151
🧹 Nitpick comments (7)
src/nat/control_flow/sequential_executor.py (7)
72-75: Fix misleading debug message.This logs “is not compatible” before checking; log a neutral message instead.
- logger.debug( - f"The output type of the {src_fn.instance_name} function is {str(src_output_type)}, is not compatible with" - f"the input type of the {target_fn.instance_name} function, which is {str(target_input_type)}") + logger.debug( + f"Checking type compatibility: {src_fn.instance_name!s}({src_output_type!s}) -> " + f"{target_fn.instance_name!s}({target_input_type!s})")
96-101: Prefer itertools.pairwise and chain exception properly.
- Use
pairwise(function_list)over manualzip()slicing.- Chain the error using
from eto preserve cause.+from itertools import pairwise @@ - if len(function_list) > 1: - for src_fn, target_fn in zip(function_list[0:-1], function_list[1:]): - try: - _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config) - except ValueError as e: - raise ValueError(f"The sequential tool list has incompatible types: {e}") + for src_fn, target_fn in pairwise(function_list): + try: + _validate_function_type_compatibility(src_fn, target_fn, tool_execution_config) + except ValueError as e: + raise ValueError(f"The sequential tool list has incompatible types: {e!s}") from e
98-101: Preserve stack traces when re-raising.Use exception chaining; avoid swallowing the original traceback.
- except ValueError as e: - raise ValueError(f"The sequential tool list has incompatible types: {e}") + except ValueError as e: + raise ValueError(f"The sequential tool list has incompatible types: {e!s}") from e @@ - except Exception as e: - raise ValueError(f"Error with the sequential executor tool list: {e}") + except Exception as e: + raise ValueError("Error validating the sequential executor tool list.") from eAlso applies to: 126-128
103-105: Tidy debug text and conversion flags.Add missing space and use explicit conversion flags to satisfy Ruff (RUF010).
- logger.debug(f"The input type of the sequential executor tool list is {str(input_type)}," - f"the output type is {str(output_type)}") + logger.debug( + f"The input type of the sequential executor tool list is {input_type!s}, " + f"the output type is {output_type!s}" + )
109-110: Add return type annotation.Public API should have full type hints per guidelines.
-async def sequential_execution(config: SequentialExecutorConfig, builder: Builder): +async def sequential_execution( + config: SequentialExecutorConfig, builder: Builder +) -> typing.AsyncIterator[FunctionInfo]:
47-56: Fix description string concatenation spaces.Adjacent string literals concatenate without spaces; current text renders as “eachtool”, “compatible,which”, etc.
- tool_execution_config: dict[str, ToolExecutionConfig] = Field(default_factory=dict, - description="Optional configuration for each" - "tool in the sequential execution tool list." - "Keys should match the tool names from the" - "tool_list.") + tool_execution_config: dict[str, ToolExecutionConfig] = Field( + default_factory=dict, + description="Optional configuration for each " + "tool in the sequential execution tool list. " + "Keys should match the tool names from the " + "tool_list." + ) @@ - raise_type_incompatibility: bool = Field( - default=False, - description="Default to False. Check if the adjacent tools are type compatible," - "which means the output type of the previous function is compatible with the input type of the next function." - "If set to True, any incompatibility will raise an exception. If set to false, the incompatibility will only" - "generate a warning message and the sequential execution will continue.") + raise_type_incompatibility: bool = Field( + default=False, + description="Default to False. Check if the adjacent tools are type compatible, " + "which means the output type of the previous function is compatible with the input type of the next function. " + "If set to True, any incompatibility will raise an exception. If set to false, the incompatibility will only " + "generate a warning message and the sequential execution will continue." + )
165-167: Minor doc polish in description string.Add a space between concatenated sentences.
- yield FunctionInfo.from_fn(_sequential_function_execution, - description="Executes a list of functions sequentially." - "The input of the next tool is the response of the previous tool.") + yield FunctionInfo.from_fn( + _sequential_function_execution, + description="Executes a list of functions sequentially. " + "The input of the next tool is the response of the previous tool.", + )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/source/_static/sequential_executor.pngis excluded by!**/*.pnguv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
examples/README.md(2 hunks)examples/control_flow/sequential_executor/README.md(1 hunks)examples/control_flow/sequential_executor/configs/config.yml(1 hunks)examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml(1 hunks)src/nat/control_flow/sequential_executor.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/getting_started/simple_calculator/src/nat_simple_calculator/configs/config.yml
- examples/control_flow/sequential_executor/configs/config.yml
- examples/control_flow/sequential_executor/README.md
🧰 Additional context used
📓 Path-based instructions (9)
**/README.{md,ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Each documentation README must follow the NeMo Agent toolkit naming rules and must not use deprecated names
Files:
examples/README.md
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
examples/README.mdsrc/nat/control_flow/sequential_executor.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
examples/README.mdsrc/nat/control_flow/sequential_executor.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/README.mdsrc/nat/control_flow/sequential_executor.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/README.md
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/control_flow/sequential_executor.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/control_flow/sequential_executor.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/control_flow/sequential_executor.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/control_flow/sequential_executor.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/control_flow/sequential_executor.py
🧬 Code graph analysis (1)
src/nat/control_flow/sequential_executor.py (5)
src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (1)
FunctionRef(94-102)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)src/nat/utils/type_utils.py (2)
DecomposedType(60-488)is_type_compatible(445-488)tests/nat/control_flow/test_sequential_executor.py (1)
astream(104-109)
🪛 Ruff (0.12.2)
src/nat/control_flow/sequential_executor.py
73-73: Use explicit conversion flag
Replace with conversion flag
(RUF010)
74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
78-80: Avoid specifying long messages outside the exception class
(TRY003)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
80-80: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
96-96: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
96-96: Prefer itertools.pairwise() over zip() when iterating over successive pairs
Replace zip() with itertools.pairwise()
(RUF007)
100-100: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-100: Avoid specifying long messages outside the exception class
(TRY003)
103-103: Use explicit conversion flag
Replace with conversion flag
(RUF010)
104-104: Use explicit conversion flag
Replace with conversion flag
(RUF010)
120-120: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
126-126: Do not catch blind exception: Exception
(BLE001)
127-127: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
127-127: Avoid specifying long messages outside the exception class
(TRY003)
153-153: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (4)
examples/README.md (2)
37-37: TOC update looks good.The new "Control Flow" section entry integrates correctly with the anchors.
76-78: Additions read well and follow naming/linking rules.SPDX header present; link path and naming are consistent with examples layout.
src/nat/control_flow/sequential_executor.py (2)
120-121: Keep logger.error on re-raise; ignore TRY400.Project guidelines prefer
logger.error()(notlogger.exception()) when re-raising. Current usage is correct; configure Ruff to ignore TRY400 for these lines if needed.If TRY400 persists, add a targeted ignore in pyproject or a
# noqa: TRY400inline.Also applies to: 153-154
30-31: Blocker: confirm correct API/import for type-compatibility checkrg reported "nat/utils/type_utils.py: No such file or directory" — cannot verify whether DecomposedType or TypeUtils exposes is_type_compatible. Confirm where is_type_compatible is defined and update src/nat/control_flow/sequential_executor.py (lines 30–31 and 76–81). If TypeUtils.is_type_compatible is correct, apply:
-from nat.utils.type_utils import DecomposedType +from nat.utils.type_utils import TypeUtils @@ - is_compatible = DecomposedType.is_type_compatible(src_output_type, target_input_type) + is_compatible = TypeUtils.is_type_compatible(src_output_type, target_input_type)
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
Signed-off-by: Yuchen Zhang <[email protected]>
willkill07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I like the naming.
I'm assuming there will be follow up PRs (before release) for:
- documentation
- moving(?) the branching agent to
control_flow
Signed-off-by: Yuchen Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/nat/control_flow/test_sequential_executor.py (4)
78-81: Silence ARG002 (unused-arg) while keeping signatures compatible.Prefix unused parameters with underscores.
@@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str: @@ - async def astream(self, input, config=None, **kwargs): + async def astream(self, _input, _config=None, **_kwargs): @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str: @@ - async def _arun(self, query: typing.Any = None, **kwargs) -> str: + async def _arun(self, _query: typing.Any = None, **_kwargs) -> str: @@ - def _run(self, query: typing.Any = None, **kwargs) -> str: + def _run(self, _query: typing.Any = None, **_kwargs) -> str: @@ - async def _arun(self, query: str = "", **kwargs) -> str: + async def _arun(self, query: str = "", **_kwargs) -> str: @@ - def _run(self, query: str = "", **kwargs) -> str: + def _run(self, query: str = "", **_kwargs) -> str:Also applies to: 82-85, 104-110, 111-114, 115-118, 136-141, 486-491
143-145: Correct class docstring (typo in config name).-class TestSequentialExecutionToolConfig: - """Test cases for SequentialExecutionToolConfig.""" +class TestSequentialExecutionToolConfig: + """Test cases for SequentialExecutorConfig."""
520-532: Actually assert dynamic annotations; make test async.- def test_function_annotations_set_correctly(self, mock_builder): + @pytest.mark.asyncio + async def test_function_annotations_set_correctly(self, mock_builder): @@ - with patch('nat.control_flow.sequential_executor._validate_tool_list_type_compatibility', - return_value=(str, int)): - # Get the generator - gen = sequential_execution(config, mock_builder) - - # Since this is an async generator, we need to test differently - # The actual annotation setting happens inside the generator function - assert gen is not None + with patch('nat.control_flow.sequential_executor._validate_tool_list_type_compatibility', + return_value=(str, int)): + async with sequential_execution(config, mock_builder) as function_info: + fn = function_info.single_fn # type: ignore + assert fn.__annotations__.get("initial_tool_input") is str + assert fn.__annotations__.get("return") is int
538-545: Avoid brittle checks on wrapped; assert callable or async-gen instead.Relying on decorator internals is fragile; the function being a registered async generator is sufficient.
@@ - # Test that the function is properly decorated (has __wrapped__) - assert hasattr(sequential_execution, '__wrapped__') # Should have register_function decorator - # Test that sequential_execution is callable assert callable(sequential_execution) @@ - assert callable(sequential_execution) - assert hasattr(sequential_execution, '__wrapped__') + assert callable(sequential_execution)Also applies to: 546-552
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/control_flow/sequential_executor/README.md(1 hunks)examples/control_flow/sequential_executor/configs/config.yml(1 hunks)examples/control_flow/sequential_executor/pyproject.toml(1 hunks)examples/control_flow/sequential_executor/tests/test_sequential_executor.py(1 hunks)tests/nat/control_flow/test_sequential_executor.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/control_flow/sequential_executor/README.md
- examples/control_flow/sequential_executor/configs/config.yml
- examples/control_flow/sequential_executor/tests/test_sequential_executor.py
- examples/control_flow/sequential_executor/pyproject.toml
🧰 Additional context used
📓 Path-based instructions (7)
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests must live under tests/ and use configured markers (e2e, integration, etc.)
Files:
tests/nat/control_flow/test_sequential_executor.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/control_flow/test_sequential_executor.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
tests/nat/control_flow/test_sequential_executor.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/tests/**/*.py: Test functions must use the test_ prefix and snake_case
Extract repeated test code into pytest fixtures; fixtures should set name=... in @pytest.fixture and functions named with fixture_ prefix
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Use pytest with pytest-asyncio for async code; mock external services with pytest_httpserver or unittest.mock
Files:
tests/nat/control_flow/test_sequential_executor.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
tests/nat/control_flow/test_sequential_executor.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
tests/nat/control_flow/test_sequential_executor.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
tests/nat/control_flow/test_sequential_executor.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
tests/nat/control_flow/test_sequential_executor.py
🧬 Code graph analysis (1)
tests/nat/control_flow/test_sequential_executor.py (4)
src/nat/builder/builder.py (1)
Builder(64-281)src/nat/control_flow/sequential_executor.py (5)
SequentialExecutorConfig(41-56)ToolExecutionConfig(35-38)_validate_function_type_compatibility(67-80)_validate_tool_list_type_compatibility(83-106)sequential_execution(110-167)src/nat/data_models/component_ref.py (1)
FunctionRef(94-102)src/nat/utils/type_utils.py (1)
DecomposedType(60-488)
🪛 Ruff (0.12.2)
tests/nat/control_flow/test_sequential_executor.py
78-78: Unused method argument: query
(ARG002)
78-78: Unused method argument: kwargs
(ARG002)
82-82: Unused method argument: query
(ARG002)
82-82: Unused method argument: kwargs
(ARG002)
104-104: Unused method argument: input
(ARG002)
104-104: Unused method argument: config
(ARG002)
104-104: Unused method argument: kwargs
(ARG002)
111-111: Unused method argument: query
(ARG002)
111-111: Unused method argument: kwargs
(ARG002)
115-115: Unused method argument: query
(ARG002)
115-115: Unused method argument: kwargs
(ARG002)
136-136: Unused method argument: query
(ARG002)
136-136: Unused method argument: kwargs
(ARG002)
139-139: Unused method argument: query
(ARG002)
139-139: Unused method argument: kwargs
(ARG002)
486-486: Unused method argument: kwargs
(ARG002)
489-489: Unused method argument: kwargs
(ARG002)
🔇 Additional comments (1)
tests/nat/control_flow/test_sequential_executor.py (1)
249-251: Align docstring with function name.-class TestValidateSequentialToolList: - """Test cases for _validate_sequential_tool_list function.""" +class TestValidateSequentialToolList: + """Test cases for _validate_tool_list_type_compatibility function."""[raise_ninor_issue]
|
/merge |
Description
This PR implements a
Sequential Executortool, supporting invoking a series of tool/function calls in a row. It takes in a list of tools and execute each of them in sequential order, passing the output of the previous tool to the input of the next tool. Type compatibility of the input/output is checked before execution, any incompatibility will result in an Exception or warning, depending on theraise_type_incompatibilityoption.Closes AIQ-1859
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Tests
Chores