docs: update function group documentation and object store example#1065
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
WalkthroughDocumentation updated to describe function groups with new rhs configuration field and revised implementation examples. User report example reworded to emphasize function group features and updated to reference group-level configuration instead of individual function references. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/object_store/user_report/README.md (1)
20-20: Fix typo: "And" should be "An".Line 20 reads: "And example tool in the NeMo Agent toolkit" but should be "An example tool in the NeMo Agent toolkit".
Apply this diff to fix the typo:
-And example tool in the NeMo Agent toolkit that makes use of an Object Store to retrieve data. +An example tool in the NeMo Agent toolkit that makes use of an Object Store to retrieve data.
🧹 Nitpick comments (2)
docs/source/workflows/function-groups.md (1)
215-235: Clarify config class naming in "without function groups" example.Lines 220-228 show individual config classes (AddConfig, MultiplyConfig, DivideConfig) each with distinct
nameparameters. This could confuse readers about whether these are valid independent config types or just pedagogical contrasts. Consider adding a brief comment (e.g.,# Without function groups, each operation needs its own config type) to clarify the intent.examples/object_store/user_report/README.md (1)
88-103: Function references section is helpful but needs minor clarity improvement.The section shows both individual function references (line 95) and the group reference (line 102) as alternatives. However, the two YAML blocks are presented sequentially without clear indication that they are mutually exclusive options. Consider adding a brief comment or restructuring to make it clear these are alternative approaches, not both to be used together.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/workflows/function-groups.md(3 hunks)examples/object_store/user_report/README.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Use the official naming throughout documentation: first use “NVIDIA NeMo Agent toolkit”, subsequent “NeMo Agent toolkit”; never use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq)
Documentation sources are Markdown files under docs/source; images belong in docs/source/_static
Keep docs in sync with code; documentation pipeline must pass Sphinx and link checks; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling correctness
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted terms in accept.txt are allowed
Files:
docs/source/workflows/function-groups.md
**/*
⚙️ 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:
docs/source/workflows/function-groups.mdexamples/object_store/user_report/README.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/function-groups.md
**/README.@(md|ipynb)
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Ensure READMEs follow the naming convention; avoid deprecated names; use “NeMo Agent Toolkit” (capital T) in headings
Files:
examples/object_store/user_report/README.md
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/object_store/user_report/README.md
🧠 Learnings (1)
📚 Learning: 2025-08-28T20:03:37.650Z
Learnt from: willkill07
PR: NVIDIA/NeMo-Agent-Toolkit#684
File: src/nat/front_ends/mcp/mcp_front_end_plugin_worker.py:0-0
Timestamp: 2025-08-28T20:03:37.650Z
Learning: Function groups in NAT automatically namespace their function names with the group name (e.g., "user_report.get", "user_report.put"), making function name collisions impossible by design. No collision detection is needed when merging functions from function groups.
Applied to files:
examples/object_store/user_report/README.md
🔇 Additional comments (7)
docs/source/workflows/function-groups.md (3)
268-296: Code example is clear and correct.The function group implementation effectively demonstrates resource sharing and configuration consolidation. The pattern of creating a single
FunctionGroup, defining functions, and usinggroup.add_function()is well-documented and the math operations correctly implement the logic.
559-559: Verify the programmatic usage example is consistent with API.Line 559 creates a
MathGroupConfigwithinclude=["add", "multiply"]and line 563 invokes theaddfunction withainvoke(3.0), expecting8.0(3.0 + 5.0). The math is correct. Verify that theincludeparameter correctly gates individual function access and thatainvokeis the correct method name for the function type being returned.Can you confirm that:
- The
includeparameter onMathGroupConfigcorrectly restricts function visibility?ainvoke()is the correct async invocation method for functions returned bybuilder.get_function()?Also applies to: 563-563
884-884: Testing example usage looks appropriate.The testing pattern correctly instantiates
MathGroupConfig(rhs=5.0)and iterates over functions to validate outputs. This demonstrates a practical testing approach for function groups.examples/object_store/user_report/README.md (4)
18-18: Verify naming convention consistency.Line 18 correctly uses "NVIDIA NeMo Agent Toolkit" (first mention, capitals on all key terms). Line 55 and subsequent uses correctly reference "NeMo Agent toolkit" (lowercase "toolkit"). Naming convention appears consistent with guidelines.
Also applies to: 55-55
55-71: Function Groups Overview section is well-structured.The overview clearly explains the benefits of function groups (shared configuration, shared resources, reduced duplication) and adds an explicit reference to the function-groups.md documentation for deeper context. The shift from individual function references to group-oriented configuration is appropriately emphasized.
76-85: Configuration structure example is clear.The YAML configuration block correctly shows the function_groups section with the
user_reportgroup including all four functions (get, put, update, delete) and demonstrating theincludeparameter and configuration fields. This effectively contrasts with per-function configuration from the old approach.
71-71: Verify relative link to function-groups documentation.Line 71 contains a relative path link:
../../../docs/source/workflows/function-groups.md. Since this README is atexamples/object_store/user_report/README.md, the relative path should navigate to the correct location. Verify this link resolves correctly in the documentation build or use a Sphinx cross-reference if available.Can you verify that the link on line 71 resolves correctly during the documentation build?
|
line 326: sentence could be improved by calling it an "instance name" rather than just "name" |
|
/merge |
1 similar comment
|
/merge |
Description
Address missing VDR feedback related to function groups.
Closes AIQ-2139, AIQ-2140
By Submitting this PR I confirm:
Summary by CodeRabbit
Release Notes