Skip to content

Commit ac544b4

Browse files
authored
Merge pull request #6040 from hjmjohnson/doc-ai-skill-updates-clean
DOC: Capture recent CI lessons in Documentation/AI/ skill files
2 parents 0ed533c + 5626cc7 commit ac544b4

File tree

10 files changed

+706
-47
lines changed

10 files changed

+706
-47
lines changed

AGENTS.md

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,24 @@ Apache 2.0 licensed. Build tool: **Pixi** (wraps CMake + Ninja).
66

77
## Context to Load on Demand
88

9-
Load only what your task requires:
9+
Load only what your task requires. Files are small and focused — load
10+
the minimum set for the task at hand.
1011

1112
| Task | Read |
1213
|------|------|
13-
| Understanding the codebase layout | [./Documentation/AI/architecture.md](./Documentation/AI/architecture.md) |
14-
| Building or configuring ITK | [./Documentation/AI/building.md](./Documentation/AI/building.md) |
15-
| Writing or running tests | [./Documentation/AI/testing.md](./Documentation/AI/testing.md) |
16-
| Code style, naming conventions | [./Documentation/AI/style.md](./Documentation/AI/style.md) |
17-
| Writing ITK C++ classes, CMake build configuration, and Python wrapping configuration | [./Documentation/AI/conventions.md](./Documentation/AI/conventions.md) |
18-
| Creating a git commit with C++ changes | [./Documentation/AI/git-commits.md](./Documentation/AI/git-commits.md), [./Documentation/AI/compiler-cautions.md](./Documentation/AI/compiler-cautions.md), [./Documentation/AI/building.md](./Documentation/AI/building.md), [./Documentation/AI/testing.md](./Documentation/AI/testing.md) |
19-
| Opening or updating a pull request | [./Documentation/AI/pull-requests.md](./Documentation/AI/pull-requests.md) |
20-
| Avoiding compiler-specific pitfalls and refactoring hazards | [./Documentation/AI/compiler-cautions.md](./Documentation/AI/compiler-cautions.md) |
14+
| Understanding the codebase layout | [architecture.md](./Documentation/AI/architecture.md) |
15+
| Building or configuring ITK | [building.md](./Documentation/AI/building.md) |
16+
| Writing or running tests | [testing.md](./Documentation/AI/testing.md) |
17+
| Code style, formatting, naming | [enforced-code-style.md](./Documentation/AI/enforced-code-style.md) |
18+
| Writing or reviewing C++ code | [code-review-lessons.md](./Documentation/AI/code-review-lessons.md) |
19+
| Writing ITK C++ classes, CMake, Python wrapping | [conventions.md](./Documentation/AI/conventions.md) |
20+
| Avoiding compiler pitfalls and refactoring hazards | [compiler-cautions.md](./Documentation/AI/compiler-cautions.md) |
21+
| Creating a **DOC:** commit | [git-commits.md](./Documentation/AI/git-commits.md) |
22+
| Creating a **STYLE:** commit | [git-commits.md](./Documentation/AI/git-commits.md), [enforced-code-style.md](./Documentation/AI/enforced-code-style.md) |
23+
| Creating a **BUG:** or **ENH:** commit | [git-commits.md](./Documentation/AI/git-commits.md), [compiler-cautions.md](./Documentation/AI/compiler-cautions.md), [testing.md](./Documentation/AI/testing.md) |
24+
| Creating a **COMP:** commit | [git-commits.md](./Documentation/AI/git-commits.md), [compiler-cautions.md](./Documentation/AI/compiler-cautions.md) |
25+
| Commit or PR attribution | [attribution.md](./Documentation/AI/attribution.md) |
26+
| Opening or updating a pull request | [pull-requests.md](./Documentation/AI/pull-requests.md), [attribution.md](./Documentation/AI/attribution.md) |
2127

2228
## Critical Pitfalls
2329

Documentation/AI/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Add file to table in AGENTS.md for routing:
2424
| File | When to load |
2525
|------|-------------|
2626
| `building.md` | Configuring or building ITK with Pixi or CMake |
27+
| `code-review-lessons.md` | Writing or reviewing C++ code — recurring reviewer concerns |
2728
| `testing.md` | Writing, running, or converting tests |
2829

2930
## Adding a New Context File

Documentation/AI/attribution.md

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Commit and PR Attribution Rules
2+
3+
## Commits: what and why only
4+
5+
Commit messages describe **what** changed and **why**. They do not
6+
describe how the change was produced. AI tool details, model names, and
7+
tool-specific trailers (`Tool-Assisted:`, `Assisted-by:`) do **not**
8+
belong in commit messages.
9+
10+
Keep commit messages concise — a 1-line subject (≤ 78 chars) plus a
11+
short body describing intent. Long rationale goes in the PR description,
12+
not the commit body.
13+
14+
## Co-Authored-By
15+
16+
`Co-Authored-By:` is for contributors whose intellectual contribution
17+
materially shaped the commit — typically a co-developer or a reviewer
18+
whose feedback drove a design change.
19+
20+
**For AI tools: never.** The human who prompted, reviewed, and
21+
committed the code bears authorship responsibility. `Co-Authored-By:`
22+
implies accountability — AI tools cannot be paged when the code breaks,
23+
cannot defend design decisions in review, and cannot maintain the code
24+
going forward. AI assistance is disclosed in the PR description instead
25+
(see below).
26+
27+
**For reviewers:** use `Co-Authored-By:` only when a reviewer's
28+
feedback **materially shaped the design** of the commit (e.g., "keep
29+
v142 but migrate to windows-2022" led to a different implementation
30+
approach). A reviewer who caught a typo or requested a rename is a
31+
reviewer, not a co-author. For review acknowledgment without
32+
co-authorship, mention the reviewer in prose in the commit body or PR
33+
description:
34+
35+
```
36+
Addresses dzenanz's review: restored v142 toolset entry with updated
37+
generator and runner image.
38+
```
39+
40+
**The responsibility test:** if this code has a bug, who gets paged?
41+
If the answer is a human, that human may be a Co-Author. If the answer
42+
is "whoever committed it," the committer is the sole author.
43+
44+
## AI disclosure: PR description only
45+
46+
The PR description is the **sole location** for AI tool disclosure.
47+
Commits are clean.
48+
49+
When AI tools made a substantive contribution (root-cause analysis,
50+
algorithm design, hypothesis testing), disclose in the PR body:
51+
52+
```markdown
53+
Short visible summary of what changed and why.
54+
55+
<details>
56+
<summary>AI assistance</summary>
57+
58+
- Tool: Claude Code (claude-opus-4-6)
59+
- Role: root-cause analysis of ccache hit rate regression
60+
- Contribution: identified CCACHE_NODIRECT=1 as sole cause by
61+
comparing ARM CI with Azure DevOps pipeline configurations
62+
- All code was reviewed and tested locally before committing
63+
64+
</details>
65+
```
66+
67+
**No disclosure needed** for mechanical changes: reformatting, rename
68+
refactoring, boilerplate generation, applying a well-known pattern the
69+
human specified.
70+
71+
## PR body format: concise by default, details on request
72+
73+
PR descriptions must be **short and reviewer-friendly by default**.
74+
A reviewer should understand the PR's purpose in under 10 seconds
75+
from the visible text.
76+
77+
- **Lead with a 1-3 line summary** — what changed and why.
78+
- **Sequester longer analysis inside `<details>` blocks** — root cause
79+
analysis, AI assistance details, test output, failed approaches,
80+
status tables, and dependency discussion.
81+
- **Keep visible text to actionable items** — if the reviewer must read
82+
it to know what to do, keep it visible. If it's background context
83+
for those who want the deep dive, collapse it.
84+
85+
```markdown
86+
Fix float-precision division in BresenhamLine::BuildLine. Closes #6054.
87+
88+
<details>
89+
<summary>Root cause</summary>
90+
91+
The integer division `abs(dx)/abs(dy)` truncated to zero for
92+
shallow angles, producing incorrect line segments...
93+
94+
</details>
95+
96+
<details>
97+
<summary>AI assistance</summary>
98+
99+
Claude Code identified the truncation pattern and suggested the
100+
`static_cast<double>` fix. Verified locally with the GTest suite.
101+
102+
</details>
103+
```
104+
105+
## External context
106+
107+
When Discourse threads, issues, or other sources informed a commit,
108+
attribute in prose with stable links. Replace transient URLs (CI build
109+
links, Azure DevOps runs, CDash build URLs) with extracted context:
110+
111+
```
112+
# BAD — transient links that will expire:
113+
See https://dev.azure.com/itkrobot/.../buildId=14265
114+
115+
# GOOD — context preserved:
116+
Based on discussion in https://discourse.itk.org/t/7745 (patches
117+
for 5.4.6). Azure DevOps ITK.macOS.Python failed with exit code
118+
255 (dashboard script treats warnings as fatal; CDash shows
119+
0 compile errors and 0 test failures).
120+
```
121+
122+
[attribution-issue]: https://github.com/InsightSoftwareConsortium/ITK/issues/6055

0 commit comments

Comments
 (0)