Yet another attempt on the right copyright#815
Merged
podkidyshev merged 6 commits intomainfrom Feb 24, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR updates copyright header years across many TOML and Python files and revises the copyright-header test to use a new Git-based per-file year collection (with rename tracking and record-separated parsing). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
podkidyshev
force-pushed
the
ipod/broken-copyright
branch
from
24f8e97 to
897948e
Compare
podkidyshev
commented
Feb 23, 2026
podkidyshev
force-pushed
the
ipod/broken-copyright
branch
from
897948e to
f85008c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cloudai/workloads/slurm_container/__init__.py`:
- Line 2: Update the copyright header in this module to use "2026" only (remove
the "2025-" prefix so the header reads "2026 NVIDIA CORPORATION & AFFILIATES.
All rights reserved.")—edit the top-of-file comment in
src/cloudai/workloads/slurm_container/__init__.py to reflect the single year.
In `@tests/test_check_copyright_headers.py`:
- Around line 103-111: The comment in collect_years_same_file incorrectly states
"Exact-content rename only" while the git flag "--find-renames=92%" allows
renames with ≥92% similarity; update the comment to accurately describe the
threshold (e.g., "Treat renames with ≥92% similarity as renames") or change the
flag to "--find-renames=100%" if you want true exact-content-only behavior;
adjust the comment accordingly to match whichever choice you make.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (53)
conf/common/test/sleep.tomlconf/common/test/ucc_test.tomlconf/common/test_scenario/dse_nemo_run_llama3_8b.tomlconf/experimental/aiconfigurator/test_scenario/dse_aiconfigurator_disagg.tomlconf/experimental/test/deepep_standard.tomlconf/experimental/test_scenario/ddlb_test.tomlconf/experimental/test_scenario/deepep.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_128_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_16_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_256_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_2_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_32_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_4_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_64_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_8_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_128_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_16_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_256_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_2_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_32_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_4_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_64_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_8_node.tomlconf/hook/test/nccl_test_all_gather.tomlconf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.tomlconf/release/nemo_acceptance/test/nemo_run_llama3_8b.tomlconf/release/nemo_acceptance/test_scenario/gpt3_126m_mock.tomlconf/release/nemo_acceptance/test_scenario/gpt3_126m_pile.tomlconf/release/nemo_acceptance/test_scenario/nccl_test_loopback.tomlconf/release/nemo_acceptance/test_scenario/nemo_run_llama3_8b.tomlsrc/cloudai/models/workload.pysrc/cloudai/report_generator/__init__.pysrc/cloudai/reporter.pysrc/cloudai/systems/kubernetes/kubernetes_runner.pysrc/cloudai/systems/lsf/__init__.pysrc/cloudai/systems/runai/__init__.pysrc/cloudai/systems/runai/runai_installer.pysrc/cloudai/systems/standalone/__init__.pysrc/cloudai/workloads/chakra_replay/__init__.pysrc/cloudai/workloads/deepep/__init__.pysrc/cloudai/workloads/jax_toolbox/__init__.pysrc/cloudai/workloads/megatron_run/__init__.pysrc/cloudai/workloads/megatron_run/slurm_command_gen_strategy.pysrc/cloudai/workloads/nccl_test/__init__.pysrc/cloudai/workloads/nccl_test/report_generation_strategy.pysrc/cloudai/workloads/nemo_launcher/__init__.pysrc/cloudai/workloads/nemo_run/__init__.pysrc/cloudai/workloads/slurm_container/__init__.pytests/systems/standalone/test_installer.pytests/test_base_runner.pytests/test_check_copyright_headers.pytests/test_cli.pytests/test_get_job_id.py
podkidyshev
requested review from
amaslenn,
jeffnvidia and
srivatsankrishnan
as code owners
Contributor
Greptile SummaryRewrites the copyright year detection in
Confidence Score: 4/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
conf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_4_node.toml (1)
21-21:⚠️ Potential issue | 🟡 MinorPre-existing:
test_namereferences the wrong variant.
test_name = "nemo_launcher_nemotron_15b_bf16_2_node"whilenum_nodes = "4"andid = "nemo_launcher_nemotron_15b_bf16_4_node". This is a copy-paste artefact that will select the wrong test template at runtime.🐛 Proposed fix
-test_name = "nemo_launcher_nemotron_15b_bf16_2_node" +test_name = "nemo_launcher_nemotron_15b_bf16_4_node"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_4_node.toml` at line 21, The test_name field is a copy-paste mismatch: update test_name (currently "nemo_launcher_nemotron_15b_bf16_2_node") to match the id/num_nodes variant by setting test_name to "nemo_launcher_nemotron_15b_bf16_4_node" so the test selector uses the correct 4-node template; change the test_name value in the same TOML block where id and num_nodes are defined.
♻️ Duplicate comments (2)
src/cloudai/models/workload.py (1)
2-2:⚠️ Potential issue | 🟡 MinorSame copyright start-year concern — core model file likely predates 2025.
workload.pydefines foundational types (TestDefinition,CmdArgs,NsysConfiguration) that almost certainly existed at project inception. The verification script above also covers this file; if git history shows anAcommit in 2024, the correct header is2024-2026.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cloudai/models/workload.py` at line 2, Update the file-level copyright header to reflect the original creation year by changing the start year from 2025 to 2024; specifically edit the top-of-file comment so the range reads "2024-2026" (this is the only change needed), then save—no code changes to types TestDefinition, CmdArgs, or NsysConfiguration are required.conf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.toml (1)
2-2:⚠️ Potential issue | 🟡 MinorSame copyright start-year concern applies here.
As a release-tier NCCL test, this file likely predates 2025. The same verification script above (covering
conf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.toml) will confirm whether the 2024 start year is being incorrectly dropped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.toml` at line 2, The copyright header on the file currently starts at 2025 but should retain the original start year (likely 2024); update the copyright range in the header string (the line beginning with "# Copyright") to include the correct start year (e.g., "2024-2026") so the release-tier NCCL test preserves the original start-year rather than dropping it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_check_copyright_headers.py`:
- Around line 130-133: After building lines = [ln.strip() for ln in
rec.splitlines() if ln.strip()], add a bounds check before accessing lines[1]:
verify len(lines) >= 2 and handle the unexpected case (e.g., treat the record as
a broken/empty diff by skipping it or failing the test with a clear message) so
accessing parts = lines[1].split("\t") cannot raise IndexError; update the test
logic around the variables lines, rec, year and parts to either continue on
malformed records or raise a descriptive AssertionError.
- Around line 96-100: The comment and code in run_git incorrectly treat exit
code 128 as a benign "no commits match" case; instead, change run_git to treat
any non-zero returncode as an error: remove the special-case for 128, raise a
RuntimeError including the git command, res.returncode and res.stderr when
res.returncode != 0, and rely on empty stdout (not exit code) to represent no
matching commits; keep the function name run_git and the subprocess.run call
unchanged other than the error handling and message.
---
Outside diff comments:
In `@conf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_4_node.toml`:
- Line 21: The test_name field is a copy-paste mismatch: update test_name
(currently "nemo_launcher_nemotron_15b_bf16_2_node") to match the id/num_nodes
variant by setting test_name to "nemo_launcher_nemotron_15b_bf16_4_node" so the
test selector uses the correct 4-node template; change the test_name value in
the same TOML block where id and num_nodes are defined.
---
Duplicate comments:
In `@conf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.toml`:
- Line 2: The copyright header on the file currently starts at 2025 but should
retain the original start year (likely 2024); update the copyright range in the
header string (the line beginning with "# Copyright") to include the correct
start year (e.g., "2024-2026") so the release-tier NCCL test preserves the
original start-year rather than dropping it.
In `@src/cloudai/models/workload.py`:
- Line 2: Update the file-level copyright header to reflect the original
creation year by changing the start year from 2025 to 2024; specifically edit
the top-of-file comment so the range reads "2024-2026" (this is the only change
needed), then save—no code changes to types TestDefinition, CmdArgs, or
NsysConfiguration are required.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (53)
conf/common/test/sleep.tomlconf/common/test/ucc_test.tomlconf/common/test_scenario/dse_nemo_run_llama3_8b.tomlconf/experimental/aiconfigurator/test_scenario/dse_aiconfigurator_disagg.tomlconf/experimental/test/deepep_standard.tomlconf/experimental/test_scenario/ddlb_test.tomlconf/experimental/test_scenario/deepep.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_128_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_16_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_256_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_2_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_32_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_4_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_64_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_bf16_8_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_128_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_16_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_256_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_2_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_32_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_4_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_64_node.tomlconf/experimental/test_scenario/nemo_launcher_nemotron_15b_fp8_8_node.tomlconf/hook/test/nccl_test_all_gather.tomlconf/release/nemo_acceptance/test/nccl_test_all_reduce_loopback.tomlconf/release/nemo_acceptance/test/nemo_run_llama3_8b.tomlconf/release/nemo_acceptance/test_scenario/gpt3_126m_mock.tomlconf/release/nemo_acceptance/test_scenario/gpt3_126m_pile.tomlconf/release/nemo_acceptance/test_scenario/nccl_test_loopback.tomlconf/release/nemo_acceptance/test_scenario/nemo_run_llama3_8b.tomlsrc/cloudai/models/workload.pysrc/cloudai/report_generator/__init__.pysrc/cloudai/reporter.pysrc/cloudai/systems/kubernetes/kubernetes_runner.pysrc/cloudai/systems/lsf/__init__.pysrc/cloudai/systems/runai/__init__.pysrc/cloudai/systems/runai/runai_installer.pysrc/cloudai/systems/standalone/__init__.pysrc/cloudai/workloads/chakra_replay/__init__.pysrc/cloudai/workloads/deepep/__init__.pysrc/cloudai/workloads/jax_toolbox/__init__.pysrc/cloudai/workloads/megatron_run/__init__.pysrc/cloudai/workloads/megatron_run/slurm_command_gen_strategy.pysrc/cloudai/workloads/nccl_test/__init__.pysrc/cloudai/workloads/nccl_test/report_generation_strategy.pysrc/cloudai/workloads/nemo_launcher/__init__.pysrc/cloudai/workloads/nemo_run/__init__.pysrc/cloudai/workloads/slurm_container/__init__.pytests/systems/standalone/test_installer.pytests/test_base_runner.pytests/test_check_copyright_headers.pytests/test_cli.pytests/test_get_job_id.py
podkidyshev
commented
Feb 23, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_check_copyright_headers.py`:
- Around line 96-100: The RuntimeError in run_git currently only mentions
cmd[0], which loses the git exit code and stderr; update run_git (the function
using subprocess.run) so when res.returncode != 0 it raises a RuntimeError that
includes the full cmd (or joined cmd), res.returncode, and res.stderr (or
res.stderr.strip()) to make CI failures self-explanatory; keep the
subprocess.run call and return res.stdout on success.
- Around line 137-143: The len(commit_status) == 4 check wrongly restricts
rename handling to only "R100"; change the branch to detect rename tokens by
checking commit_status starts with "R" (e.g., commit_status.startswith("R")),
parse percentage = int(commit_status[1:]) to handle "R92"–"R100", and keep the
existing percentage >= 92 guard so renames with scores 92–99 are followed;
update the logic around current_path, old_path/new_path (from parts[1],
parts[2]) and the years.add(year)/continue flow accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/test_check_copyright_headers.py (1)
136-142:⚠️ Potential issue | 🟠 MajorRename handling only matches
R100tokens (R92–R99 are skipped).
len(commit_status) == 4filters outR92–R99(length 3), so those renames are never followed and history stops too early.🐛 Proposed fix
- if len(commit_status) == 4 and commit_status[0] == "R" and len(parts) == 3: - percentage = int(commit_status[1:]) + if commit_status.startswith("R") and commit_status[1:].isdigit() and len(parts) == 3: + percentage = int(commit_status[1:])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_check_copyright_headers.py` around lines 136 - 142, The rename handling skips R92–R99 because the code checks len(commit_status) == 4; update the logic in the block that inspects commit_status (the variables commit_status, parts, current_path, percentage, years) to accept any Rxx token >=92: ensure you check commit_status startswith 'R' and that the numeric suffix (commit_status[1:]) is parsed safely (e.g., isdigit()) then set percentage = int(commit_status[1:]) and proceed if percentage >= 92 and parts has the expected elements (use len(parts) >= 3 or similar) before assigning old_path/new_path and updating current_path and years so R92–R99 are followed correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_check_copyright_headers.py`:
- Around line 109-124: Git paths are POSIX so normalize Path objects to POSIX
before using them: change the run_git call to pass path.as_posix() (or
str(path.as_posix())) and set current_path = path.as_posix() (instead of
str(path)), and ensure any comparisons like parts[1] == current_path use the
POSIX-normalized current_path (also apply the same fix around the other
occurrence at the code near lines referenced for parts/years traversal).
---
Duplicate comments:
In `@tests/test_check_copyright_headers.py`:
- Around line 136-142: The rename handling skips R92–R99 because the code checks
len(commit_status) == 4; update the logic in the block that inspects
commit_status (the variables commit_status, parts, current_path, percentage,
years) to accept any Rxx token >=92: ensure you check commit_status startswith
'R' and that the numeric suffix (commit_status[1:]) is parsed safely (e.g.,
isdigit()) then set percentage = int(commit_status[1:]) and proceed if
percentage >= 92 and parts has the expected elements (use len(parts) >= 3 or
similar) before assigning old_path/new_path and updating current_path and years
so R92–R99 are followed correctly.
amaslenn
approved these changes
Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
No more relying on what's in the file. Traversing the history of each file including R100 (100% file move) and stopping with any other copy/move status
Test Plan
git --no-pager log --follow --name-status --format=%x1e%ad --date=format:%Y --find-renames=92% -- <path>for manual testingAdditional Notes