Skip to content

Conversation

@zhongxuanwang-nv
Copy link
Member

@zhongxuanwang-nv zhongxuanwang-nv commented Sep 9, 2025

Description

Close

  • Added: .cursor/rules/nat-test-llm.mdc with usage rules for nat_test_llm (YAML example, fields response_seq/delay_ms, builder pattern, wrapper hints). 

I separated this code from the original big PR because I want to make sure I don't mess things up for cursor rules :)

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features

    • Introduced a deterministic Test LLM for workflows/tests with configurable cyclic response sequences and optional per-call latency; compatible with common wrapper types and returns plain strings.
  • Documentation

    • Added YAML and programmatic examples, wrapper integration and registration/install guidance (package: nvidia-nat-test), notes on cycle-reset behavior on reload, and that no retry/thinking patches are applied.

Signed-off-by: Daniel Wang <daniewang@nvidia.com>
@zhongxuanwang-nv zhongxuanwang-nv self-assigned this Sep 9, 2025
@zhongxuanwang-nv zhongxuanwang-nv marked this pull request as draft September 9, 2025 00:37
@coderabbitai
Copy link

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds a new rule file .cursor/rules/nat-test-llm.mdc documenting the deterministic NAT test LLM provider nat_test_llm, its config fields (response_seq, delay_ms), YAML and programmatic usage, wrapper retrieval guidance, and behavior details (cycling, reset, plain strings, no retries).

Changes

Cohort / File(s) Summary
Rules and documentation
.cursor/rules/nat-test-llm.mdc
New rule describing nat_test_llm provider: front matter, response_seq and delay_ms config fields, YAML example (llms.main), programmatic builder example (TestLLMConfig, add_llm, get_llm), wrapper usage across frameworks, registration/import note (nvidia-nat-test package), and behavior notes (cycle persists per workflow instance, resets on reload, returns plain strings, no NAT retry/thinking patches).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Workflow
  participant NATTestLLM as nat_test_llm
  note over NATTestLLM: Config: response_seq, delay_ms

  Workflow->>NATTestLLM: request(prompt, llm_name="main")
  activate NATTestLLM
  NATTestLLM-->>Workflow: (after delay_ms) response = next(response_seq)
  deactivate NATTestLLM
  note right of Workflow: Subsequent calls cycle through response_seq\nCycle resets only when workflow reloads
Loading

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The current title “Add cursor rules for test_llm from nvidia-nat-test package” uses imperative mood, stays under the 72-character limit, and concisely highlights the primary change of adding new cursor rules from the specified package, making it a clear and specific summary of the main change.
Description Check ✅ Passed The description directly relates to the changeset by summarizing the addition of the .cursor/rules/nat-test-llm.mdc file along with its content (YAML examples, response_seq/delay_ms fields, builder pattern, and wrapper hints), and it confirms contributor compliance, making it on-topic and relevant.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c977e7e and 273567d.

📒 Files selected for processing (1)
  • .cursor/rules/nat-test-llm.mdc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .cursor/rules/nat-test-llm.mdc
⏰ 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
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zhongxuanwang-nv zhongxuanwang-nv added feature request New feature or request non-breaking Non-breaking change labels Sep 9, 2025
@zhongxuanwang-nv zhongxuanwang-nv marked this pull request as ready for review September 9, 2025 00:53
@coderabbitai coderabbitai bot added the doc Improvements or additions to documentation label Sep 9, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
.cursor/rules/nat-test-llm.mdc (4)

6-12: Minor clarity: document defaults explicitly.

State that delay_ms defaults to 0 if omitted, and clarify behavior when response_seq is omitted vs. empty.


13-22: Quote YAML strings to avoid YAML edge cases.

Unquoted scalars like yes/no/null can misparse. Quote sample values.

     _type: nat_test_llm
-    response_seq: [alpha, beta, gamma]
+    response_seq: ["alpha", "beta", "gamma"]
     delay_ms: 0

23-24: Provide a minimal code snippet for programmatic usage.

A short code block (Python) showing builder + wrapper call will improve copy/paste-ability.

Happy to draft a concrete snippet targeting one wrapper (e.g., LangChain) if you confirm preferred wrapper and method name.


6-6: Optional: group related rules under a subdirectory.

For future NAT test rules, consider .cursor/rules/nat-test/llm.mdc to follow the "kebab-case subdirectory" learning.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5836c0 and 3170f00.

📒 Files selected for processing (1)
  • .cursor/rules/nat-test-llm.mdc (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.cursor/rules/**/*.mdc

📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)

.cursor/rules/**/*.mdc: Place all Cursor rule files under PROJECT_ROOT/.cursor/rules/
Name rule files in kebab-case, always using the .mdc extension, with descriptive filenames
Rule descriptions must start with the phrase: 'Follow these rules when'
Descriptions should specify clear trigger conditions (e.g., when the user's request meets certain criteria)
Use precise action verbs in descriptions (e.g., creating, modifying, implementing, configuring, adding, installing, evaluating)
Descriptions should be comprehensive but concise
Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Proofread descriptions for typos and grammar
Avoid overly narrow descriptions when rules cover multiple related scenarios
Prefer the 'user's request involves' phrasing pattern in descriptions
Rule files must include the specified frontmatter structure: description (string), optional globs, and alwaysApply (boolean), followed by markdown content

Files:

  • .cursor/rules/nat-test-llm.mdc
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/*/ : For related rules on the same topic, create a kebab-case subdirectory under .cursor/rules/
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Rule descriptions must start with the phrase: 'Follow these rules when'
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)

Applied to files:

  • .cursor/rules/nat-test-llm.mdc
🔇 Additional comments (1)
.cursor/rules/nat-test-llm.mdc (1)

27-29: LGTM: behavior notes are clear and actionable.

Cycle/reset semantics and "plain string" return are well-specified.

@zhongxuanwang-nv zhongxuanwang-nv removed the feature request New feature or request label Sep 9, 2025
Signed-off-by: Daniel Wang <daniewang@nvidia.com>
Copy link
Contributor

@yczhang-nv yczhang-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Only one small suggestion.

Copy link
Member

@willkill07 willkill07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor feedback. Feel free to ignore

@zhongxuanwang-nv zhongxuanwang-nv merged commit d26e50d into NVIDIA:develop Sep 9, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Improvements or additions to documentation non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants