Fix nsys subfield merging behavior

[juntaowww]

Summary

When specifying [Tests.nsys] in a scenario file to override only specific nsys configuration fields (like output), the entire nsys object was being overwritten instead of just the specified fields.

Fix:

"nsys": self.nsys.model_dump() if self.nsys else None,

to:

"nsys": self.nsys.model_dump(exclude_unset=True) if self.nsys else None,

Using exclude_unset=True ensures that only fields explicitly set by the user in the scenario file are included in the dump. The deep_merge function then correctly merges only those specified fields with the test configuration, preserving all other nsys settings from the test definition.

Test Plan

TestNsysMerging is added to tests/test_test_scenario.py for testing the correctness of overwriting of nsys fields from scenario configuration.

[coderabbitai]

Warning

Rate limit exceeded

@juntaowww has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

This PR changes nsys serialization to exclude unset fields during TestRunModel.tdef_model_dump and adds a new TestNsysMerging test suite (four tests) validating merging behavior of NsysConfiguration between base NCCLTestDefinition and scenario overrides.

Changes

Cohort / File(s)	Summary
Model Serialization src/cloudai/models/scenario.py	Serialize nsys with model_dump(exclude_unset=True) when present (previously default serialization); minor copyright year update.
Test Coverage tests/test_test_scenario.py	Added TestNsysMerging with four tests: partial override, multiple-field override, scenario-only addition, and disable-override behavior for NsysConfiguration merging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibble at code under moonlit beams,
I tuck away unset fields like secret dreams.
Four tests I plant in soft loamy ground,
Merging configs — tidy, safe, and sound.
Hop, patch, repeat — a rabbit's small cheer!

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix nsys subfield merging behavior' clearly and specifically summarizes the main bug fix in the changeset.
Description check	✅ Passed	The description comprehensively explains the bug, provides the specific code fix with before/after comparison, and documents the test plan for validation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Overview

Greptile Summary

Fixed nsys configuration merging behavior by using exclude_unset=True in model_dump(). Previously, when scenario files specified partial nsys overrides (e.g., only output), the entire nsys object was replaced, losing all other base configuration fields. Now only explicitly set fields are merged.

Key Changes:

• Modified TestRunModel.tdef_model_dump() in src/cloudai/models/scenario.py:111 to use exclude_unset=True
• Added comprehensive test suite TestNsysMerging with 4 test cases covering partial overrides, multiple field overrides, adding to empty base, and disable scenarios

Impact:
This fix ensures that users can override individual nsys fields in scenario files while preserving other configuration from the base test definition, enabling more flexible profiling configurations.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix is a minimal, targeted change (single parameter addition) with clear intent and comprehensive test coverage. The solution correctly leverages Pydantic's exclude_unset=True feature to solve the merging issue, and the 4 new test cases thoroughly validate the behavior across different scenarios
• No files require special attention

Important Files Changed

Filename	Overview
src/cloudai/models/scenario.py	Added exclude_unset=True to nsys model dump to enable proper field-level merging instead of full object replacement
tests/test_test_scenario.py	Added comprehensive test suite (160 lines, 4 test cases) covering partial override, multiple fields, adding to empty base, and disable scenarios

Sequence Diagram

sequenceDiagram
    participant User
    participant TestScenarioParser
    participant TestRunModel
    participant deep_merge
    participant TestParser
    participant NsysConfiguration

    User->>TestScenarioParser: Parse scenario with [Tests.nsys] override
    TestScenarioParser->>TestScenarioParser: _prepare_tdef(test_info)
    TestScenarioParser->>TestRunModel: test_info.tdef_model_dump(by_alias=True)
    TestRunModel->>NsysConfiguration: nsys.model_dump(exclude_unset=True)
    Note right of NsysConfiguration: Only includes fields<br/>explicitly set by user<br/>(e.g., only "output")
    NsysConfiguration-->>TestRunModel: {output: "/scenario/output"}
    TestRunModel-->>TestScenarioParser: tc_defined (scenario overrides)
    TestScenarioParser->>TestScenarioParser: test.model_dump(by_alias=True)
    Note right of TestScenarioParser: Base test definition with<br/>all nsys fields populated
    TestScenarioParser->>deep_merge: deep_merge(test_defined, tc_defined)
    Note right of deep_merge: Merges only specified fields,<br/>preserves base nsys fields<br/>like nsys_binary, trace, etc.
    deep_merge-->>TestScenarioParser: merged_data
    TestScenarioParser->>TestParser: load_test_definition(merged_data)
    TestParser-->>TestScenarioParser: TestDefinition with merged nsys
    TestScenarioParser-->>User: Test with partial nsys override applied

Loading

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/test_test_scenario.py`:
- Around line 520-653: Remove the behavioral docstrings inside the
TestNsysMerging test methods: delete the triple-quoted strings at the top of
test_nsys_partial_override_preserves_base_config,
test_nsys_multiple_fields_override, test_nsys_scenario_adds_to_base_without_nsys
(and any similar method docstrings such as test_nsys_disable_override) so the
tests only contain assertions and setup; leave any high-level
interface/class-level documentation if needed but strip per-test behavior
descriptions that duplicate the assertions.

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}