diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 65c2fa9..0cf184d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -13,3 +13,17 @@ /LICENSE @NVIDIA-NeMo/user_sim_maintainers /DCO @NVIDIA-NeMo/user_sim_maintainers /pyproject.toml @NVIDIA-NeMo/user_sim_maintainers + +# These define or validate the acceptance gates and their exclusions. +/Makefile @NVIDIA-NeMo/user_sim_maintainers +/.pre-commit-config.yaml @NVIDIA-NeMo/user_sim_maintainers +/.gitignore @NVIDIA-NeMo/user_sim_maintainers +/.gitattributes @NVIDIA-NeMo/user_sim_maintainers +/MANIFEST.in @NVIDIA-NeMo/user_sim_maintainers +/uv.lock @NVIDIA-NeMo/user_sim_maintainers +/scripts/ @NVIDIA-NeMo/user_sim_maintainers +/tests/test_ci_workflows.py @NVIDIA-NeMo/user_sim_maintainers +/tests/test_ignore_policy.py @NVIDIA-NeMo/user_sim_maintainers +/tests/test_update_license_headers.py @NVIDIA-NeMo/user_sim_maintainers +/docs/engineering/ @NVIDIA-NeMo/user_sim_maintainers +/tests/fixtures/exclusion-policy.json @NVIDIA-NeMo/user_sim_maintainers diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4dfeaec..2aaa0eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,23 +39,12 @@ jobs: - run: make install-ci - - name: Lint and formatting + - name: Static checks and all tracked-file hooks run: make check-all - - name: SPDX headers - run: make check-license-headers - - - name: Relative links in markdown - run: make check-doc-links - - name: Dependency licences run: make check-dependency-licenses - - name: Notebook outputs are stripped - # nbstripout runs from a pre-commit hook that only exists if a - # contributor ran `make install-dev`. A fresh clone has no hooks, so - # this is the only thing that actually keeps outputs out of the repo. - run: make check-notebooks-clean test: name: Tests (Python ${{ matrix.python-version }}) diff --git a/.gitignore b/.gitignore index 85cdf2d..fb305ec 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,12 @@ env/ coverage.xml htmlcov/ .pytest_cache/ +.ruff_cache/ +.mypy_cache/ +.ty_cache/ +.coverage.* +junit*.xml +test-results/ # Jupyter .ipynb_checkpoints/ @@ -39,9 +45,9 @@ models.local.toml *.local.yaml # Data artifacts -data/ -artifacts/ -output/ +/data/ +/artifacts/ +/output/ # Dense-retrieval vectors `gen-assets` writes beside a bank stay local. src/usersim/engine/assets/financial_services/**/embeddings.parquet @@ -58,11 +64,15 @@ Thumbs.db # Local DataDesigner branch checkout .dd-branch/ -# uv lockfiles. The lock is COMMITTED: CI installs from it with --locked so -# dependency drift is a build failure rather than a silent re-resolution. - # Cursor .cursor/ +# Private agent overrides and session artifacts; shared guidance stays trackable. +AGENTS.local.md +CLAUDE.local.md +.claude/settings.local.json +.codex/config.local.toml +.worktrees/ + # Local agent working notes /.agent-work/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 55097ce..4175b6f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,14 +1,8 @@ # Local mirror of the CI lint and format gate, so failures surface before push # rather than in the pipeline. Install with `make install-dev`. # -# The global exclude covers files no hook should touch at all. A top-level -# directory whose name starts with `_` holds material that is not part of the -# distribution, and uv owns uv.lock's format. Notebooks are NOT global: they -# are exempt per-hook from the rewriting checks (a whitespace fix on notebook -# JSON is a large unreviewable diff) while staying subject to the size check. -# Global and per-hook excludes are cumulative, so a globally excluded path -# cannot be re-included by a single hook. - +# Top-level underscore directories contain private, non-distribution material. +# uv owns lock formatting; notebooks skip rewriting text hooks individually. exclude: '^_[^/]+/|^uv\.lock$' repos: @@ -22,6 +16,7 @@ repos: - id: mixed-line-ending exclude: '\.ipynb$' - id: check-merge-conflict + args: ["--assume-in-merge"] - id: check-yaml - id: check-json - id: check-toml @@ -30,7 +25,10 @@ repos: # Notebooks stay in scope, so one committed with its outputs fails # here. uv.lock is exempt via the global exclude: it is a committed # resolved graph that grows with the dependency set. - args: ["--maxkb=600"] + args: ["--maxkb=600", "--enforce-all"] + # Existing shipped corpora exceed the budget; only these two files + # are exempt. They still receive syntax and conflict checks. + exclude: '^src/usersim/engine/assets/(safety_chat_pressure|safety_agentic)/sample\.yaml$' # Ruff runs through `uv run` rather than pre-commit's own mirror so the # version always matches the pin in pyproject.toml. A mirrored rev drifts @@ -39,6 +37,17 @@ repos: # identify tags .ipynb as `jupyter`, not `python`. - repo: local hooks: + - id: check-lock + name: dependency lock consistency + entry: make check-lock + language: system + pass_filenames: false + always_run: true + - id: spdx-headers + name: SPDX headers (staged files) + entry: uv run python scripts/update_license_headers.py + language: system + files: '^(src/|tests/|scripts/|templates/|Makefile$|conftest\.py$)' - id: ruff name: ruff (lint) entry: uv run ruff check --fix diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e951c9c..c5ff069 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -204,10 +204,8 @@ route it to raises `ConfigError` rather than guessing an endpoint. ## Before you open a pull request ```bash -make check-all # ruff lint +make check-all # lock, lint, format, SPDX, docs, notebooks, and all tracked-file hooks make test # the full suite -make check-license-headers # SPDX header on every source file -make check-doc-links # every relative link in markdown resolves ``` Licensing has its own gate, worth running if you added a dependency: @@ -224,7 +222,21 @@ make check-dist # builds and checks the sdist and wheel, installs the wh make check-extensions # installs an out-of-tree package, asserts all six seams work ``` -`make update-license-headers` adds the SPDX header to new files. +`make check-all` includes `make check-hooks`, which runs pre-commit against all +tracked files. Hooks may rewrite files and fail: review those changes and rerun +`make check-all`. Use `make install-dev` to install local pre-commit hooks; +CI runs the same checks even when local hooks are bypassed. + +The SPDX hook fixes only the staged paths passed to it. `make update-license-headers` +adds or refreshes headers throughout `src/`, `tests/`, `scripts/`, `templates/`, +`Makefile`, and root `conftest.py`; `make check-license-headers` checks those +roots without writing. Python, shell, and `.template` files are covered; Markdown, +YAML, and TOML do not require SPDX headers under this policy. + +Keep credentials and generated output local; keep required source and fixtures +trackable. Git ignores do not control archive or Docker inclusion. +Maintainers own gate scripts, tool declarations, hook configuration, and exclusion +policy through `.github/CODEOWNERS`. Your PR title must follow [Conventional Commits](https://www.conventionalcommits.org/) (`feat:`, `fix:`, `docs:`, `ci:`, `chore:`, `refactor:`, `test:`, `style:`, diff --git a/Makefile b/Makefile index bb54d64..2f12a66 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,8 @@ COV_FAIL_UNDER ?= 84 .DEFAULT_GOAL := help .PHONY: help bootstrap check-tools check-lock install-dev install-ci install-kernel health-check check-notebooks-clean lint lint-fix format format-check check-all \ - check-all-fix test test-fast coverage smoke dist check-dist check-release-tag check-extensions check-doc-links clean-notebooks check-license-headers update-license-headers check-dependency-licenses clean + check-hooks check-all-fix test test-fast coverage smoke dist check-dist check-release-tag check-extensions check-doc-links clean-notebooks check-license-headers update-license-headers check-dependency-licenses clean + help: ## Show the available targets @echo "usersim make targets:" @@ -115,7 +116,10 @@ format-check: ## Check formatting (no changes written) format: ## Reformat in place $(UV) run ruff format $(PY_ALL) -check-all: lint format-check ## Run every static check CI enforces +check-all: check-tools check-lock lint format-check check-license-headers check-doc-links check-notebooks-clean check-hooks ## Run ordinary static gates (hooks may rewrite files) + +check-hooks: ## Run hooks on every tracked file; review rewrites and rerun after failure + $(UV) run pre-commit run --all-files --show-diff-on-failure check-all-fix: lint-fix format ## Fix everything fixable, then you re-review diff --git a/scripts/check_doc_links.py b/scripts/check_doc_links.py index b732aad..20e5146 100644 --- a/scripts/check_doc_links.py +++ b/scripts/check_doc_links.py @@ -34,21 +34,18 @@ "https://raw.githubusercontent.com/NVIDIA-NeMo/UserSim/main/", ) -#: Paths are repo-relative, so a leading `_` marks a top-level directory that -#: is not part of the distribution. -SKIP_PREFIXES = ("_",) SKIP_SCHEMES = ("http://", "https://", "mailto:", "tel:", "#") def tracked_markdown() -> list[Path]: out = subprocess.run( - ["git", "ls-files", "*.md"], + ["git", "ls-files", "-z", "*.md"], cwd=REPO, capture_output=True, text=True, check=True, - ).stdout.split() - return [REPO / f for f in out if not f.startswith(SKIP_PREFIXES)] + ).stdout.split("\0") + return [REPO / f for f in out if f and not f.startswith("_")] def link_path(path: Path, target: str) -> Path | None: diff --git a/scripts/update_license_headers.py b/scripts/update_license_headers.py index 921586a..d76b0aa 100644 --- a/scripts/update_license_headers.py +++ b/scripts/update_license_headers.py @@ -33,15 +33,6 @@ ] ) -#: A top-level directory whose name starts with `_` holds material that is not -#: part of the distribution, so its files are never stamped. This is matched on -#: the first path component alone: `_private.py` modules inside the package are -#: ordinary source and do get stamped. -PRIVATE_TOP_LEVEL_PREFIX = "_" - -# Skip auto-generated version files (hatch-vcs generates these at build time) -SKIP_FILES = frozenset(["_version.py"]) - # File extensions to process for license headers SUPPORTED_EXTENSIONS = frozenset([".py", ".sh", ".template"]) @@ -269,18 +260,13 @@ def should_process_file(file_path: Path) -> bool: if file_path.name not in ROOT_FILES and file_path.suffix not in SUPPORTED_EXTENSIONS: return False - if file_path.name in SKIP_FILES: - return False - try: relative = file_path.resolve().relative_to(REPO_ROOT) except ValueError: - relative = None - if relative and relative.parts[0].startswith(PRIVATE_TOP_LEVEL_PREFIX): + relative = file_path + if relative.parts and relative.parts[0].startswith("_"): return False - - file_str = str(file_path) - return not any(pattern in file_str for pattern in SKIP_PATTERNS) + return not any(part in SKIP_PATTERNS for part in file_path.parts) def get_file_creation_year(file_path: Path) -> int | None: @@ -408,14 +394,16 @@ def main( action="store_true", help="Check if all files have correct license headers without modifying files", ) + parser.add_argument("files", nargs="*", type=Path, help="Only process these paths (for staged-file hooks)") args = parser.parse_args() + args.files = [path.resolve() for path in args.files] repo_path = Path(__file__).parent.parent all_files_needing_update: list[Path] = [] total_processed = total_updated = total_skipped = 0 # This project is one distribution under src/, so the roots are flat. - for folder in ["src", "tests", "scripts", "templates"]: + for folder in [] if args.files else ["src", "tests", "scripts", "templates"]: folder_path = repo_path / folder if not folder_path.exists(): continue @@ -437,7 +425,7 @@ def main( print(f" ✏️ Updated: {updated}") print(f" ⏭️ Skipped: {skipped}") - root_files = [repo_path / name for name in sorted(ROOT_FILES) if (repo_path / name).is_file()] + root_files = args.files or [repo_path / name for name in sorted(ROOT_FILES) if (repo_path / name).is_file()] if root_files: action = "Checking" if args.check else "Processing" print(f"\n📂 {action} repository root") diff --git a/tests/fixtures/exclusion-policy.json b/tests/fixtures/exclusion-policy.json new file mode 100644 index 0000000..97047a8 --- /dev/null +++ b/tests/fixtures/exclusion-policy.json @@ -0,0 +1,42 @@ +[ + { + "path": ".env.local", + "git_ignored": true + }, + { + "path": "models.local.toml", + "git_ignored": true + }, + { + "path": "spec.local.yaml", + "git_ignored": true + }, + { + "path": "output/run=abc/data.parquet", + "git_ignored": true + }, + { + "path": "debug_llm_responses.jsonl", + "git_ignored": true + }, + { + "path": ".agent-work/plans/engineering-workflows.md", + "git_ignored": true + }, + { + "path": "src/usersim/cli/models_default.toml", + "git_ignored": false + }, + { + "path": "src/usersim/engine/assets/example/data/required.json", + "git_ignored": false + }, + { + "path": "tests/fixtures/output/expected.json", + "git_ignored": false + }, + { + "path": "data/generated.json", + "git_ignored": true + } +] diff --git a/tests/test_check_doc_links.py b/tests/test_check_doc_links.py index ae1f14d..0435ece 100644 --- a/tests/test_check_doc_links.py +++ b/tests/test_check_doc_links.py @@ -35,3 +35,16 @@ def test_links_into_the_main_branch_resolve_against_the_checkout(tmp_path: Path) def test_relative_links_resolve_against_the_file(tmp_path: Path) -> None: (tmp_path / "there.md").write_text("") assert _broken(tmp_path / "doc.md", "[ok](there.md)\n[gone](missing.md#anchor)\n") == ["missing.md#anchor"] + + +def test_tracked_markdown_preserves_spaces_and_private_scope(tmp_path: Path, monkeypatch) -> None: + import subprocess + + subprocess.run(["git", "init", "-q", str(tmp_path)], check=True) + for name in ["guide with spaces.md", "_private/notes.md"]: + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("documentation\n") + subprocess.run(["git", "add", "."], cwd=tmp_path, check=True) + monkeypatch.setattr(check_doc_links, "REPO", tmp_path) + assert check_doc_links.tracked_markdown() == [tmp_path / "guide with spaces.md"] diff --git a/tests/test_ignore_policy.py b/tests/test_ignore_policy.py new file mode 100644 index 0000000..d471b82 --- /dev/null +++ b/tests/test_ignore_policy.py @@ -0,0 +1,34 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Git exclusion behavior for private output and required repository inputs.""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[1] + + +@pytest.fixture(scope="module") +def ignore_repo(tmp_path_factory): + """Exercise the committed policy without requiring checkout Git metadata.""" + repo = tmp_path_factory.mktemp("ignore-policy") + subprocess.run(["git", "-c", "init.templateDir=", "init", "-q", str(repo)], check=True) + shutil.copyfile(REPO / ".gitignore", repo / ".gitignore") + return repo + + +CASES = json.loads((REPO / "tests/fixtures/exclusion-policy.json").read_text()) + + +@pytest.mark.parametrize("case", CASES, ids=lambda case: case["path"]) +def test_git_exclusion_policy(case, ignore_repo: Path) -> None: + result = subprocess.run( + ["git", "-c", "core.excludesFile=/dev/null", "check-ignore", "--no-index", "-q", case["path"]], + cwd=ignore_repo, + ) + assert result.returncode == (0 if case["git_ignored"] else 1), case["path"] diff --git a/tests/test_update_license_headers.py b/tests/test_update_license_headers.py new file mode 100644 index 0000000..6cd8ebc --- /dev/null +++ b/tests/test_update_license_headers.py @@ -0,0 +1,38 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Staged SPDX fixes preserve executable content and only touch named files.""" + +import subprocess +import sys +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts/update_license_headers.py" + + +def test_explicit_paths_fix_only_named_file_and_preserve_shebang(tmp_path: Path) -> None: + selected = tmp_path / "selected.py" + untouched = tmp_path / "untouched.py" + body = '#!/usr/bin/env python3\nprint("example")\n' + selected.write_text(body) + untouched.write_text(body) + selected.chmod(0o755) + result = subprocess.run([sys.executable, str(SCRIPT), "--check", str(selected)], capture_output=True) + assert result.returncode == 1 + subprocess.run([sys.executable, str(SCRIPT), str(selected)], check=True) + text = selected.read_text() + assert text.startswith("#!/usr/bin/env python3\n") + assert "SPDX-License-Identifier: Apache-2.0" in text + assert text.endswith('print("example")\n') + assert selected.stat().st_mode & 0o111 == 0o111 + assert untouched.read_text() == body + subprocess.run([sys.executable, str(SCRIPT), "--check", str(selected)], check=True) + + +def test_directory_names_containing_skip_tokens_do_not_hide_sources(tmp_path: Path) -> None: + # .github is not .git; suffixes such as .pyc in a parent name are not caches. + selected = tmp_path / ".github.pyc-source" / "script.py" + selected.parent.mkdir() + selected.write_text('print("source")\n') + subprocess.run([sys.executable, str(SCRIPT), str(selected)], check=True) + assert "SPDX-License-Identifier:" in selected.read_text()