From 9ddc2266fadcfd7826e23f790a77276a0b368af8 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 11:52:19 -0400 Subject: [PATCH 01/10] Add agent instructions functionality This functionality allows agents to see much more detailed instructions for fixing warnings, and is activated if verbose level >= 2. For now, just add agent instructions to `verify-hardcoded-version`, since it's the most prone to false positives and developer confusion. Also add a `write-tests` skill and move general testing guidelines from `add-or-modify-hook`. Issue: https://github.com/rapidsai/pre-commit-hooks/issues/169 --- .agents/skills/add-or-modify-hook/SKILL.md | 24 +- .agents/skills/write-test/SKILL.md | 28 ++ .pre-commit-config.yaml | 2 +- .../hardcoded_version.py | 75 ++++- src/rapids_pre_commit_hooks/lint.py | 94 +++++- tests/rapids_pre_commit_hooks/test_lint.py | 300 ++++++++++++++++++ 6 files changed, 497 insertions(+), 26 deletions(-) create mode 100644 .agents/skills/write-test/SKILL.md diff --git a/.agents/skills/add-or-modify-hook/SKILL.md b/.agents/skills/add-or-modify-hook/SKILL.md index 03f076d..37e978f 100644 --- a/.agents/skills/add-or-modify-hook/SKILL.md +++ b/.agents/skills/add-or-modify-hook/SKILL.md @@ -52,27 +52,11 @@ should be added to `rapids_pre_commit_hooks.dependencies.check_dependencies()`. All tests can be found in the `tests` directory. ## Unit Tests and Integration Tests -Unit tests and some integration tests can be found in -`tests/rapids_pre_commit_hooks`. These tests make heavy use of -`rapids_pre_commit_hooks_test_utils.parse_named_spans()`. This function -provides a DSL that is meant for easily picking spans (`tuple[int, int]`) out -of a string of text rather than hard-coding the integers. All tests that use -spans should use `parse_named_spans()`, except tests that test the testing -infrastructure itself. See the documentation for `parse_named_spans()` for -a description of the DSL, and see the existing tests for examples. +See the [write-test](../write-test/SKILL.md) skill for general unit test +guidelines. -Parameterized tests should generally only have one dimension. Parameters -should be listed as an array rather than as a single string. Each parameterized -entry should be declared with `pytest.param()` and a sensible `id` named in -kebab-case (`name-of-test`). - -In most cases, `zip_expected_warnings()` should be used to construct a list of -expected warnings from the named spans and the parallel warning metadata, both -of which should come from the test parameterization. - -Test files generally do not require type annotations. - -When in doubt, follow the existing code conventions present in the tests. +Unit tests and some integration tests for hooks can be found in +`tests/rapids_pre_commit_hooks`. ### YAML Tests Tests that operate on YAML text should use diff --git a/.agents/skills/write-test/SKILL.md b/.agents/skills/write-test/SKILL.md new file mode 100644 index 0000000..9953ff5 --- /dev/null +++ b/.agents/skills/write-test/SKILL.md @@ -0,0 +1,28 @@ +--- +name: write-test +description: Write tests for rapids-pre-commit-hooks. +--- +# Test Guidelines +All tests can be found in the `tests` directory. + +## Unit Tests and Integration Tests +The tests for rapids-pre-commit-hooks make heavy use of +`rapids_pre_commit_hooks_test_utils.parse_named_spans()`. This function +provides a DSL that is meant for easily picking spans (`tuple[int, int]`) out +of a string of text rather than hard-coding the integers. All tests that use +spans should use `parse_named_spans()`, except tests that test the testing +infrastructure itself. See the documentation for `parse_named_spans()` for +a description of the DSL, and see the existing tests for examples. + +Parameterized tests should generally only have one dimension. Parameters +should be listed as an array rather than as a single string. Each parameterized +entry should be declared with `pytest.param()` and a sensible `id` named in +kebab-case (`name-of-test`). + +In most cases, `zip_expected_warnings()` should be used to construct a list of +expected warnings from the named spans and the parallel warning metadata, both +of which should come from the test parameterization. + +Test files generally do not require type annotations. + +When in doubt, follow the existing code conventions present in the tests. diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8aa3a3b..aa9ba1d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,7 +22,7 @@ repos: args: - --py310-plus - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.14.8 + rev: v0.17.1 hooks: - id: ruff-check args: [--fix] diff --git a/src/rapids_pre_commit_hooks/hardcoded_version.py b/src/rapids_pre_commit_hooks/hardcoded_version.py index 5cd5234..ef65019 100644 --- a/src/rapids_pre_commit_hooks/hardcoded_version.py +++ b/src/rapids_pre_commit_hooks/hardcoded_version.py @@ -4,12 +4,13 @@ import bisect import re from enum import Enum +from textwrap import dedent from typing import TYPE_CHECKING import tomlkit import tomlkit.exceptions -from .lint import LintMain, Linter +from .lint import LintMain, Linter, agent_instructions from .utils.toml import find_value_span if TYPE_CHECKING: @@ -219,6 +220,68 @@ def read_version_file( ) +@agent_instructions +def agent_instructions_hardcoded_version(*, version_file: str) -> str: + return dedent( + f'''\ + This check exists to ensure that RAPIDS packages do not hard-code the + RAPIDS version in a script when programmatically reading from the + {version_file} file would suffice. Doing so reduces maintenance + burden and reduces the possibility of hard-coded versions in scripts + neglecting to be updated in a version bump. + + This check is very prone to false positives. The following conditions + can trigger a false positive: + + 1. Floating-point numbers in code that incidentally happen to equal the + version number even with no relation. + + 2. Code documentation that states when a function was added, + deprecated, etc. + + 3. Files that genuinely use the version but are incapable of + programmatically reading from the {version_file} file. + + This check has some heuristics to catch most of these false positives, + but due to the nature of the problem, we can't catch everything. Choose + one of the following options to resolve this problem (in order of + preference): + + 1. If you are writing a script that already reads from the + {version_file} file, use the stored result from that read. + + 2. If you are writing a script that is capable of doing so, but does + not yet do so, programmatically read from the {version_file} file. + + 3. If you are writing documentation, comments, etc., you have likely + encountered a false positive. You may optionally file an issue or PR + at https://github.com/rapidsai/pre-commit-hooks to add a heuristic to + suppress the false positive. + + 4. Suppress the false positive with a next-line directive. For example: + + // rapids-pre-commit-hooks: disable-next-line[verify-hardcoded-version] + // This function was added to the library as of 26.10 + void new_function(); + + Or use a block directive: + + def new_function(): + # rapids-pre-commit-hooks: disable[verify-hardcoded-version] + """New function that does stuff. + + Added to the library as of 26.10 + """ + # rapids-pre-commit-hooks: enable[verify-hardcoded-version] + pass + + 5. If you are writing a file in a format that has no mechanism for + writing comments, or if false positives are extremely common in the + file format, add an exclusion to .pre-commit-config.yaml. + ''' # noqa: E501 + ) + + def check_hardcoded_version( linter: Linter, args: "argparse.Namespace" ) -> None: @@ -244,11 +307,12 @@ def check_hardcoded_version( if skip_heuristics(linter.lines, match): continue - linter.add_warning( + w = linter.add_warning( match.span("full"), f"do not hard-code version, read from {args.version_file} " "file instead", - ).add_note( + ) + w.add_note( match.span("full"), "if this is intentional (as part of a docstring or deprecation " "notice), suppress it with rapids-pre-commit-hooks: " @@ -256,6 +320,11 @@ def check_hardcoded_version( "https://github.com/rapidsai/pre-commit-hooks/blob/main/README.md" "#suppressing-false-positives for details", ) + w.add_agent_instructions( + agent_instructions_hardcoded_version( + version_file=args.version_file + ) + ) def main() -> None: diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index ad32c89..6dff781 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -8,7 +8,8 @@ import functools import re import warnings -from typing import TYPE_CHECKING +from collections.abc import Callable +from typing import ParamSpec, TYPE_CHECKING from rich.console import Console from rich.markup import escape @@ -16,12 +17,27 @@ from . import agents if TYPE_CHECKING: - from collections.abc import Callable, Generator, Iterator + from collections.abc import Generator, Iterator from typing import Optional Span = tuple[int, int] +Args = ParamSpec("Args") + + +def agent_instructions( + func: "Callable[Args, str]", +) -> "Callable[Args, Callable[[], str]]": + @functools.wraps(func) + def instructions( + *args: Args.args, **kwargs: Args.kwargs + ) -> "Callable[[], str]": + return lambda: func(*args, **kwargs) + + return instructions + + class BinaryFileWarning(Warning): pass @@ -47,6 +63,9 @@ class LintWarning: default_factory=list, kw_only=True ) notes: list[Note] = dataclasses.field(default_factory=list, kw_only=True) + agent_instructions: "Optional[Callable[[], str]]" = dataclasses.field( + default=None, compare=False + ) def add_replacement(self, span: Span, newtext: str) -> None: self.replacements.append(Replacement(span, newtext)) @@ -54,6 +73,11 @@ def add_replacement(self, span: Span, newtext: str) -> None: def add_note(self, span: Span, msg: str) -> None: self.notes.append(Note(span, msg)) + def add_agent_instructions( + self, instructions: "Callable[[], str]" + ) -> None: + self.agent_instructions = instructions + class Lines: @functools.total_ordering @@ -154,6 +178,7 @@ def __init__(self, filename: str, content: str, warning_name: str) -> None: Linter.get_disabled_enabled_boundaries(self.lines, warning_name) ) self.sorted_replacements: "Optional[list[Replacement]]" = None + self.has_suppressed_agent_instructions: bool = False def add_warning(self, span: Span, msg: str) -> LintWarning: w = LintWarning(span, msg) @@ -254,6 +279,11 @@ def print_warnings(self, args: "argparse.Namespace") -> None: verbose, ) + if warning.agent_instructions is not None: + self.print_agent_instructions(args, warning.agent_instructions) + + self.print_suppressed_agent_instrutions() + def _print_highlighted_code( self, span: Span, @@ -436,6 +466,66 @@ def sort_and_check_replacements_for_conflicts(self) -> None: else: break + @classmethod + def format_agent_instructions(cls, instructions: str) -> list[str]: + MAX_LINE_LENGTH = 79 + + lines: list[str] = [] + for paragraph in filter( + lambda p: p != "", re.split(r"\n\n+", instructions) + ): + if lines: + lines.append("") + if paragraph.startswith(" "): + for line in paragraph.split("\n"): + assert line.startswith(" "), ( + "Do not mix formatted and unformatted lines in a " + "paragraph" + ) + lines.append(f" {line}") + else: + assert "\n " not in paragraph, ( + "Do not mix formatted and unformatted lines in a paragraph" + ) + line = "" + sep = "" + for word in filter( + lambda w: w != "", re.split(r"[ \n]", paragraph) + ): + assert len(word) <= MAX_LINE_LENGTH, ( + f'Word "{word}" is too long' + ) + if len(f" {line}{sep}{word}") > MAX_LINE_LENGTH: + lines.append(f" {line}") + line = f"{word}" + else: + line += f"{sep}{word}" + sep = " " + if line != "": + lines.append(f" {line}") + return lines + + def _print_agent_instructions(self, instructions: str) -> None: + self.console.print("[bold]instructions for agents:[/bold]") + for line in self.format_agent_instructions(instructions): + self.console.print(escape(line)) + self.console.print() + + def print_agent_instructions( + self, args: "argparse.Namespace", instructions: "Callable[[], str]" + ) -> None: + if self.verbose_output(args, 2): + self._print_agent_instructions(instructions()) + else: + self.has_suppressed_agent_instructions = True + + def print_suppressed_agent_instrutions(self) -> None: + if self.has_suppressed_agent_instructions: + self._print_agent_instructions( + "Run this tool again with AI_AGENT=1 or -vv to see detailed " + "instructions for agents." + ) + class ExecutionContext(contextlib.AbstractContextManager): def __init__(self, warning_name: str, args: argparse.Namespace) -> None: diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index a6a0225..5d8266c 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -378,6 +378,122 @@ def test_print_warnings_verbose(self, verbose, fix, conflict): "too long to display" in linter.console.mock_calls[-2].args[0] ) + @pytest.mark.parametrize( + ["verbose"], + [ + pytest.param(False, id="suppressed"), + pytest.param(True, id="shown"), + ], + ) + def test_print_warnings_agent_instructions(self, verbose): + content, spans = parse_named_spans( + """\ + + one + : ~~~first + + two + : ~~~second + """ + ) + linter = Linter("test.txt", content, "test") + linter.console = Mock() + first = linter.add_warning(spans["first"], "first warning") + assert first.agent_instructions is None + first.add_agent_instructions( + Mock(return_value="Use [bold]literal[/bold] text.\n\n code()") + ) + second = linter.add_warning(spans["second"], "second warning") + second.add_agent_instructions( + Mock(return_value="Read the version file.") + ) + + with patch.object(Linter, "verbose_output", return_value=verbose): + linter.print_warnings(Mock(fix=False)) + + expected = [ + call.print("In file [bold]test.txt:1:1[/bold]:"), + call.print(" [bold]one[/bold]"), + call.print("[bold]warning:[/bold] first warning"), + call.print(), + ] + if verbose: + expected.extend( + [ + call.print("[bold]instructions for agents:[/bold]"), + call.print(r" Use \[bold]literal\[/bold] text."), + call.print(""), + call.print(" code()"), + call.print(), + ] + ) + expected.extend( + [ + call.print("In file [bold]test.txt:2:1[/bold]:"), + call.print(" [bold]two[/bold]"), + call.print("[bold]warning:[/bold] second warning"), + call.print(), + call.print("[bold]instructions for agents:[/bold]"), + ] + ) + if verbose: + expected.append(call.print(" Read the version file.")) + else: + expected.extend( + [ + call.print( + " Run this tool again with AI_AGENT=1 or -vv to see " + "detailed instructions for" + ), + call.print(" agents."), + ] + ) + expected.append(call.print()) + assert linter.console.mock_calls == expected + + @pytest.mark.parametrize( + ["has_warning", "instructions", "disabled"], + [ + pytest.param(False, None, False, id="no-warnings"), + pytest.param(True, None, False, id="no-instructions"), + pytest.param(True, "", False, id="empty-instructions"), + pytest.param(True, "Hidden instructions", True, id="disabled"), + ], + ) + def test_print_warnings_without_agent_instructions( + self, has_warning, instructions, disabled + ): + content, spans = parse_named_spans( + """\ + + # rapids-pre-commit-hooks: disable-next-line[test] + + one + : ~~~disabled + + two + : ~~~enabled + """ + ) + linter = Linter("test.txt", content, "test") + linter.console = Mock() + if has_warning: + span = spans["disabled" if disabled else "enabled"] + warning = linter.add_warning(span, "warning message") + if instructions is not None: + warning.add_agent_instructions(Mock(return_value=instructions)) + + with patch.object(Linter, "verbose_output", return_value=True): + linter.print_warnings(Mock(fix=False)) + + if instructions == "": + assert linter.console.mock_calls[-2:] == [ + call.print("[bold]instructions for agents:[/bold]"), + call.print(), + ] + else: + assert ( + call.print("[bold]instructions for agents:[/bold]") + not in linter.console.mock_calls + ) + if not has_warning or disabled: + assert linter.console.mock_calls == [] + def test_fix(self): content, spans = parse_named_spans( """\ @@ -787,6 +903,190 @@ def test_is_warning_span_enabled(self, content, expected_enabled): == expected_enabled ) + # ruff: disable[E501] + @pytest.mark.parametrize( + ["content", "context", "expected"], + [ + pytest.param( + "", + contextlib.nullcontext(), + [], + id="empty", + ), + pytest.param( + "Hello world", + contextlib.nullcontext(), + [" Hello world"], + id="single-line", + ), + pytest.param( + "Hello\nworld", + contextlib.nullcontext(), + [" Hello world"], + id="two-lines", + ), + pytest.param( + "Hello world", + contextlib.nullcontext(), + [" Hello world"], + id="two-spaces", + ), + pytest.param( + "Joy\nto the\nworld", + contextlib.nullcontext(), + [" Joy to the world"], + id="mixed-newlines-spaces", + ), + pytest.param( + "Hello\nworld\n", + contextlib.nullcontext(), + [" Hello world"], + id="trailing-newline", + ), + pytest.param( + "Hello\n\nworld", + contextlib.nullcontext(), + [" Hello", "", " world"], + id="paragraphs", + ), + pytest.param( + dedent("""\ + Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque + faucibus ex sapien vitae pellentesque sem placerat. In id + cursus mi pretium tellus duis convallis. Tempus leo eu aenean + sed diam urna tempor. Pulvinar vivamus fringilla lacus nec + metus bibendum egestas. Iaculis massa nisl malesuada lacinia + integer nunc posuere. Ut hendrerit semper vel class aptent + taciti sociosqu. Ad litora torquent per conubia nostra inceptos + himenaeos. + + Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque + faucibus ex sapien vitae pellentesque sem placerat. In id + cursus mi pretium tellus duis convallis. Tempus leo eu aenean + sed diam urna tempor. Pulvinar vivamus fringilla lacus nec + metus bibendum egestas. Iaculis massa nisl malesuada lacinia + integer nunc posuere. Ut hendrerit semper vel class aptent + taciti sociosqu. Ad litora torquent per conubia nostra inceptos + himenaeos. + """), + contextlib.nullcontext(), + [ + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex", + " sapien vitae pellentesque sem placerat. In id cursus mi pretium tellus duis", + " convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus", + " fringilla lacus nec metus bibendum egestas. Iaculis massa nisl malesuada", + " lacinia integer nunc posuere. Ut hendrerit semper vel class aptent taciti", + " sociosqu. Ad litora torquent per conubia nostra inceptos himenaeos.", + "", + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex", + " sapien vitae pellentesque sem placerat. In id cursus mi pretium tellus duis", + " convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus", + " fringilla lacus nec metus bibendum egestas. Iaculis massa nisl malesuada", + " lacinia integer nunc posuere. Ut hendrerit semper vel class aptent taciti", + " sociosqu. Ad litora torquent per conubia nostra inceptos himenaeos.", + ], + id="long", + ), + pytest.param( + "Hello\n\n\nworld", + contextlib.nullcontext(), + [" Hello", "", " world"], + id="three-line-separators", + ), + pytest.param( + " Lorem ipsum dolor sit amet consectetur adipiscing elit. " + "Quisque faucibus ex sapien vitae pellentesque sem placerat.", + contextlib.nullcontext(), + [ + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex sapien vitae pellentesque sem placerat." + ], + id="unformatted", + ), + pytest.param( + dedent("""\ + Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque + faucibus ex sapien vitae pellentesque sem placerat. In id + cursus mi pretium tellus duis convallis. Tempus leo eu aenean + sed diam urna tempor. Pulvinar vivamus fringilla lacus nec + metus bibendum egestas. Iaculis massa nisl malesuada lacinia + integer nunc posuere. Ut hendrerit semper vel class aptent + taciti sociosqu. Ad litora torquent per conubia nostra inceptos + himenaeos. + + Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex sapien vitae pellentesque sem placerat. + In id cursus mi pretium tellus duis convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus fringilla lacus nec metus bibendum egestas. + + Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque + faucibus ex sapien vitae pellentesque sem placerat. In id + cursus mi pretium tellus duis convallis. Tempus leo eu aenean + sed diam urna tempor. Pulvinar vivamus fringilla lacus nec + metus bibendum egestas. Iaculis massa nisl malesuada lacinia + integer nunc posuere. Ut hendrerit semper vel class aptent + taciti sociosqu. Ad litora torquent per conubia nostra inceptos + himenaeos. + """), + contextlib.nullcontext(), + [ + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex", + " sapien vitae pellentesque sem placerat. In id cursus mi pretium tellus duis", + " convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus", + " fringilla lacus nec metus bibendum egestas. Iaculis massa nisl malesuada", + " lacinia integer nunc posuere. Ut hendrerit semper vel class aptent taciti", + " sociosqu. Ad litora torquent per conubia nostra inceptos himenaeos.", + "", + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex sapien vitae pellentesque sem placerat.", + " In id cursus mi pretium tellus duis convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus fringilla lacus nec metus bibendum egestas.", + "", + " Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex", + " sapien vitae pellentesque sem placerat. In id cursus mi pretium tellus duis", + " convallis. Tempus leo eu aenean sed diam urna tempor. Pulvinar vivamus", + " fringilla lacus nec metus bibendum egestas. Iaculis massa nisl malesuada", + " lacinia integer nunc posuere. Ut hendrerit semper vel class aptent taciti", + " sociosqu. Ad litora torquent per conubia nostra inceptos himenaeos.", + ], + id="long-with-unformatted", + ), + pytest.param( + "A" * 90, + pytest.raises( + AssertionError, match=rf'^Word "{"A" * 90}" is too long$' + ), + None, + id="long-word", + ), + pytest.param( + " asdf\nfdsa", + pytest.raises( + AssertionError, + match=r"^Do not mix formatted and unformatted lines in a paragraph$", + ), + None, + id="invalid-unformatted-paragraph", + ), + pytest.param( + "asdf\n fdsa", + pytest.raises( + AssertionError, + match=r"^Do not mix formatted and unformatted lines in a paragraph$", + ), + None, + id="invalid-formatted-paragraph", + ), + pytest.param( + "[bold]asdf[/bold]", + contextlib.nullcontext(), + [" [bold]asdf[/bold]"], + id="unescaped", # _print_agent_instructions() will escape + ), + ], + ) + # ruff: enable[E501] + def test_format_agent_instructions(self, content, context, expected): + result = None + with context: + result = Linter.format_agent_instructions(content) + assert result == expected + class TestLintMain: @pytest.fixture(autouse=True) From 7e46cebd734a41916afbc89b44c52bf3e8c7e7f3 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 11:57:37 -0400 Subject: [PATCH 02/10] Annotation --- src/rapids_pre_commit_hooks/lint.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index 6dff781..8b63111 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -8,7 +8,6 @@ import functools import re import warnings -from collections.abc import Callable from typing import ParamSpec, TYPE_CHECKING from rich.console import Console @@ -17,7 +16,7 @@ from . import agents if TYPE_CHECKING: - from collections.abc import Generator, Iterator + from collections.abc import Callable, Generator, Iterator from typing import Optional Span = tuple[int, int] From 92f86059de8389571aeb26b8704e2226e2fb7a19 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 11:59:40 -0400 Subject: [PATCH 03/10] Allow straight strings too --- src/rapids_pre_commit_hooks/hardcoded_version.py | 3 +-- src/rapids_pre_commit_hooks/lint.py | 7 +++++-- tests/rapids_pre_commit_hooks/test_lint.py | 4 +--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rapids_pre_commit_hooks/hardcoded_version.py b/src/rapids_pre_commit_hooks/hardcoded_version.py index ef65019..b96343b 100644 --- a/src/rapids_pre_commit_hooks/hardcoded_version.py +++ b/src/rapids_pre_commit_hooks/hardcoded_version.py @@ -10,7 +10,7 @@ import tomlkit import tomlkit.exceptions -from .lint import LintMain, Linter, agent_instructions +from .lint import LintMain, Linter from .utils.toml import find_value_span if TYPE_CHECKING: @@ -220,7 +220,6 @@ def read_version_file( ) -@agent_instructions def agent_instructions_hardcoded_version(*, version_file: str) -> str: return dedent( f'''\ diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index 8b63111..7fc8ec8 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -73,9 +73,12 @@ def add_note(self, span: Span, msg: str) -> None: self.notes.append(Note(span, msg)) def add_agent_instructions( - self, instructions: "Callable[[], str]" + self, instructions: "str | Callable[[], str]" ) -> None: - self.agent_instructions = instructions + if isinstance(instructions, str): + self.agent_instructions = lambda: instructions + else: + self.agent_instructions = instructions class Lines: diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index 5d8266c..69ebf63 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -402,9 +402,7 @@ def test_print_warnings_agent_instructions(self, verbose): Mock(return_value="Use [bold]literal[/bold] text.\n\n code()") ) second = linter.add_warning(spans["second"], "second warning") - second.add_agent_instructions( - Mock(return_value="Read the version file.") - ) + second.add_agent_instructions("Read the version file.") with patch.object(Linter, "verbose_output", return_value=verbose): linter.print_warnings(Mock(fix=False)) From a7f3ac01bb01661d761660bd5950c8f28312d330 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 12:09:21 -0400 Subject: [PATCH 04/10] Add test and doc --- src/rapids_pre_commit_hooks/lint.py | 6 ++++++ tests/rapids_pre_commit_hooks/test_lint.py | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index 7fc8ec8..9ccde4d 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -28,6 +28,12 @@ def agent_instructions( func: "Callable[Args, str]", ) -> "Callable[Args, Callable[[], str]]": + """Decorator for agent instructions functions that might be costly to run. + + This function exists because some agent instructions might be costly to + run, and should only be run if their results will actually be printed. + """ + @functools.wraps(func) def instructions( *args: Args.args, **kwargs: Args.kwargs diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index 69ebf63..c798849 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -14,10 +14,22 @@ Linter, LintMain, Replacement, + agent_instructions, ) from rapids_pre_commit_hooks_test_utils import parse_named_spans +def test_agent_instructions(): + m = Mock(return_value="Agent instructions") + instructions_func = agent_instructions(m) + m.assert_not_called() + + instructions_lambda = instructions_func(123) + m.assert_not_called() + assert instructions_lambda() == "Agent instructions" + m.assert_called_once_with(123) + + class TestLines: LONG_CONTENTS = ( "line 1\nline 2\rline 3\r\nline 4\r\n\nline 6\r\n\r\nline 8\n\r\n" From a62189f7d117ff1726eedb4e8811d2e7373dfcdc Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 12:18:47 -0400 Subject: [PATCH 05/10] Update skills --- .agents/skills/add-or-modify-hook/SKILL.md | 21 +++++++++++++++++++++ .agents/skills/write-test/SKILL.md | 4 ---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/.agents/skills/add-or-modify-hook/SKILL.md b/.agents/skills/add-or-modify-hook/SKILL.md index 37e978f..878e5e4 100644 --- a/.agents/skills/add-or-modify-hook/SKILL.md +++ b/.agents/skills/add-or-modify-hook/SKILL.md @@ -48,6 +48,22 @@ New sub-checks that get added to `verify-dependencies` should have new modules created in `rapids_pre_commit_hooks.dependencies`, and the check functions should be added to `rapids_pre_commit_hooks.dependencies.check_dependencies()`. +## Agent Instructions +In cases where an automatic fix is not possible, hooks should generally provide +instructions to agents with the `add_agent_instructions()` method. These +instructions should give the agents as much context about the problem as is +needed to solve it. + +Natural language text should have no indentation in the string, and code blocks +should be indented with two spaces. Agent instructions should generally be +returned from a function named `agent_instructions_*()` that has no positional +arguments. The function may optionally have required keyword arguments. + +If running the function is computationally expensive, requires internet or file +access, or is otherwise nontrivial to run, it should be decorated with the +`@agent_instructions` decorator. This decorator causes the function to instead +return a no-argument lambda that invokes the function lazily. + # Tests All tests can be found in the `tests` directory. @@ -58,6 +74,11 @@ guidelines. Unit tests and some integration tests for hooks can be found in `tests/rapids_pre_commit_hooks`. +In most cases, `zip_expected_warnings()` should be used to construct a list of +expected warnings from the named spans and the parallel warning metadata, both +of which should come from the test parameterization. Agent instructions do not +need to be included in these test parameterizations. + ### YAML Tests Tests that operate on YAML text should use `rapids_pre_commit_hooks_test_utils.find_yaml_node_for_span()` rather than diff --git a/.agents/skills/write-test/SKILL.md b/.agents/skills/write-test/SKILL.md index 9953ff5..6d7bb79 100644 --- a/.agents/skills/write-test/SKILL.md +++ b/.agents/skills/write-test/SKILL.md @@ -19,10 +19,6 @@ should be listed as an array rather than as a single string. Each parameterized entry should be declared with `pytest.param()` and a sensible `id` named in kebab-case (`name-of-test`). -In most cases, `zip_expected_warnings()` should be used to construct a list of -expected warnings from the named spans and the parallel warning metadata, both -of which should come from the test parameterization. - Test files generally do not require type annotations. When in doubt, follow the existing code conventions present in the tests. From 8f94e0c2adc0bfaf4d5907dce8d4e07f07082aeb Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 12:22:57 -0400 Subject: [PATCH 06/10] Formatting --- src/rapids_pre_commit_hooks/codeowners.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/rapids_pre_commit_hooks/codeowners.py b/src/rapids_pre_commit_hooks/codeowners.py index 3d8c076..c19946b 100644 --- a/src/rapids_pre_commit_hooks/codeowners.py +++ b/src/rapids_pre_commit_hooks/codeowners.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION. +# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 import argparse @@ -54,10 +54,8 @@ def hard_coded_codeowners(owners: dict[str, str]) -> CodeownersTransform: def project_codeowners(category: str) -> CodeownersTransform: - return ( - lambda *, - org, - project_prefix: f"@{org}/{project_prefix}-{category}-codeowners" + return lambda *, org, project_prefix: ( + f"@{org}/{project_prefix}-{category}-codeowners" ) From b157f608bd7c060a761d6c4961488d3652d052a4 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 19 Oct 2026 12:31:05 -0400 Subject: [PATCH 07/10] Named arg --- tests/rapids_pre_commit_hooks/test_lint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index c798849..0cc5d3b 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -24,10 +24,10 @@ def test_agent_instructions(): instructions_func = agent_instructions(m) m.assert_not_called() - instructions_lambda = instructions_func(123) + instructions_lambda = instructions_func(num=123) m.assert_not_called() assert instructions_lambda() == "Agent instructions" - m.assert_called_once_with(123) + m.assert_called_once_with(num=123) class TestLines: From 813114f80b24a1c1cb5f98d08feca284ff46e64b Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 20 Oct 2026 09:48:20 -0400 Subject: [PATCH 08/10] Fix Codex environment variable --- src/rapids_pre_commit_hooks/agents.py | 2 +- tests/rapids_pre_commit_hooks/test_agents.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rapids_pre_commit_hooks/agents.py b/src/rapids_pre_commit_hooks/agents.py index a0e6ab9..8bc54f5 100644 --- a/src/rapids_pre_commit_hooks/agents.py +++ b/src/rapids_pre_commit_hooks/agents.py @@ -32,7 +32,7 @@ def is_agent() -> bool: @_agent_test def is_codex() -> bool: - return os.getenv("CODEX_CLI") == "1" + return os.getenv("CODEX_CI") == "1" def is_agent_environment() -> bool: diff --git a/tests/rapids_pre_commit_hooks/test_agents.py b/tests/rapids_pre_commit_hooks/test_agents.py index e36f206..f74ef0f 100644 --- a/tests/rapids_pre_commit_hooks/test_agents.py +++ b/tests/rapids_pre_commit_hooks/test_agents.py @@ -67,7 +67,7 @@ def test_is_agent(env, result): ], ) def test_is_codex(env, result): - with patch_environment({"CODEX_CLI": env}): + with patch_environment({"CODEX_CI": env}): assert agents.is_codex() == result From cb9aed9718672b8192e8271ce5608d5afa4a87e5 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 20 Oct 2026 09:50:01 -0400 Subject: [PATCH 09/10] Move agents to utils --- src/rapids_pre_commit_hooks/lint.py | 2 +- src/rapids_pre_commit_hooks/{ => utils}/agents.py | 0 tests/rapids_pre_commit_hooks/test_lint.py | 4 ++-- tests/rapids_pre_commit_hooks/{ => utils}/test_agents.py | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) rename src/rapids_pre_commit_hooks/{ => utils}/agents.py (100%) rename tests/rapids_pre_commit_hooks/{ => utils}/test_agents.py (93%) diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index 9ccde4d..48b63f4 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -13,7 +13,7 @@ from rich.console import Console from rich.markup import escape -from . import agents +from .utils import agents if TYPE_CHECKING: from collections.abc import Callable, Generator, Iterator diff --git a/src/rapids_pre_commit_hooks/agents.py b/src/rapids_pre_commit_hooks/utils/agents.py similarity index 100% rename from src/rapids_pre_commit_hooks/agents.py rename to src/rapids_pre_commit_hooks/utils/agents.py diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index 0cc5d3b..ecee7a3 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -267,7 +267,7 @@ def test_verbose_output( ): args = Mock(verbose=verbose) with patch( - "rapids_pre_commit_hooks.agents.is_agent_environment", + "rapids_pre_commit_hooks.utils.agents.is_agent_environment", return_value=agent_environment, ) as check: assert Linter.verbose_output(args, *level_args) == expected @@ -1102,7 +1102,7 @@ class TestLintMain: @pytest.fixture(autouse=True) def non_agent_environment(self): with patch( - "rapids_pre_commit_hooks.agents.is_agent_environment", + "rapids_pre_commit_hooks.utils.agents.is_agent_environment", return_value=False, ): yield diff --git a/tests/rapids_pre_commit_hooks/test_agents.py b/tests/rapids_pre_commit_hooks/utils/test_agents.py similarity index 93% rename from tests/rapids_pre_commit_hooks/test_agents.py rename to tests/rapids_pre_commit_hooks/utils/test_agents.py index f74ef0f..34511d4 100644 --- a/tests/rapids_pre_commit_hooks/test_agents.py +++ b/tests/rapids_pre_commit_hooks/utils/test_agents.py @@ -4,13 +4,13 @@ import pytest from unittest.mock import Mock, patch -from rapids_pre_commit_hooks import agents +from rapids_pre_commit_hooks.utils import agents from rapids_pre_commit_hooks_test_utils import patch_environment def test_agent_test_decorator(): with patch( - "rapids_pre_commit_hooks.agents._AGENT_TESTS", [] + "rapids_pre_commit_hooks.utils.agents._AGENT_TESTS", [] ) as mock_tests: @agents._agent_test @@ -82,7 +82,7 @@ def test_is_codex(env, result): ) def test_is_agent_environment(values, called, result): with patch( - "rapids_pre_commit_hooks.agents._AGENT_TESTS", + "rapids_pre_commit_hooks.utils.agents._AGENT_TESTS", [Mock(return_value=v) for v in values], ) as tests: assert agents.is_agent_environment() == result From 2bd938211a4ff10021edf0f19429c91542baf1cc Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 20 Oct 2026 11:08:25 -0400 Subject: [PATCH 10/10] Fix word length handling --- src/rapids_pre_commit_hooks/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index 48b63f4..59afad9 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -500,7 +500,7 @@ def format_agent_instructions(cls, instructions: str) -> list[str]: for word in filter( lambda w: w != "", re.split(r"[ \n]", paragraph) ): - assert len(word) <= MAX_LINE_LENGTH, ( + assert len(word) <= MAX_LINE_LENGTH - 2, ( f'Word "{word}" is too long' ) if len(f" {line}{sep}{word}") > MAX_LINE_LENGTH: