Skip to content

Commit 49fdb2a

Browse files
committed
Merge branch 'main' into akaufman/usd-2608-allow
2 parents bec53ee + d37f4d3 commit 49fdb2a

517 files changed

Lines changed: 55522 additions & 22594 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
name: code-review-newton
3+
description: Use when reviewing a Newton pull request, branch, commit range, work-in-progress change, or design proposal for project fit, requirements fidelity, and coding and repository standards.
4+
---
5+
6+
# Review Newton Changes
7+
8+
Read `REVIEW_GUIDELINES.rst` and `CODING_GUIDELINES.rst` in full. Treat the
9+
former as the explicit definitions of the Fit, Requirements, and Standards
10+
axes, and enforce the latter during Standards review.
11+
12+
1. Resolve the exact merge base and head. Read the complete
13+
`merge-base..head` diff and commit list.
14+
2. Read the pull-request description, linked issue or specification, relevant
15+
primary sources, and `CODING_GUIDELINES.rst`. If no separate specification
16+
exists, use the pull-request description as the stated intent. Do not read
17+
existing review comments yet.
18+
3. Perform an adversarial correctness-first Requirements pass. Trace changed
19+
inputs, state, counts, offsets, ownership, and outputs through every relevant
20+
supported execution mode. Check applicable zero/one/many and capacity
21+
boundaries, heterogeneous inputs, toggle/reset/reuse and partial failures,
22+
CPU/CUDA and backend differences, autodiff, determinism, graph capture, and
23+
numerical invariants. For performance changes, examine setup and steady-state
24+
complexity, allocations, synchronization, transfers, and whether benchmarks
25+
exercise the changed path. Run a minimal adversarial probe when feasible.
26+
4. Freeze candidate findings before reading existing reviews. Record the
27+
location, triggering input or condition, mechanism, observable impact,
28+
evidence, and confidence for each candidate.
29+
5. Read the top-level discussion and review threads. Refine or deduplicate the
30+
independently discovered concerns; acknowledge existing concerns when they
31+
affect the verdict.
32+
6. Review Fit and Standards independently using their definitions in
33+
`REVIEW_GUIDELINES.rst`. When independent agent contexts are available, use
34+
separate passes so one conclusion does not bias another.
35+
7. Report Fit and architectural concerns first, but retain every supported
36+
material behavioral, correctness, compatibility, and performance finding
37+
even when Fit fails. Do not let missing tests, documentation, or measurements
38+
replace the search for an underlying behavioral defect.
39+
8. For each remaining concern, state its priority, location, problem, impact,
40+
and evidence. Distinguish demonstrated defects from questions, evidence
41+
requests, requirements, and judgment calls. Do not prescribe a correction
42+
unless the user asks for one.
43+
44+
Aggregate the three axes without suppressing a finding merely because another
45+
axis passes. Give the explicit Fit verdict defined by the review guide.

.claude/skills/release-audit/SKILL.md

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ allowed-tools: Bash(git log *) Bash(git show *) Bash(git grep *) Bash(git tag *)
1010

1111
Generates a markdown audit of a Newton release for keep/defer decisions (or, in retrospective mode, for skill calibration). Three modes, auto-detected in Phase 1:
1212

13+
Read `CODING_GUIDELINES.rst` and `REVIEW_GUIDELINES.rst` from the
14+
repository root before auditing. They define the canonical coding, API, and
15+
review policies. This skill performs the final cross-release reconciliation;
16+
it does not replace pull-request review, and a prior review pass does not make
17+
any audit phase optional. In retrospective mode, using the current guides is
18+
intentional: calibrate the current audit policy against evidence pinned to the
19+
target tag. Do not claim historical noncompliance unless the requirement also
20+
existed at that tag.
21+
1322
- **Pre-release**: spot-check while work is still landing on main. No release branch cut. Version string is `X.Y.Z.devN`.
1423
- **Release-candidate**: readiness review after the release branch is cut. Version string is `X.Y.ZrcN` or head is `release-X.Y`.
1524
- **Retrospective**: audit an already-shipped release (e.g., `v1.1.0`) against its predecessor, with a Calibration Notes section (Phase 7) that checks Claude's flags against what subsequent patch/minor releases actually did. Triggered by passing a bare released-version argument that matches an existing git tag.
@@ -244,7 +253,25 @@ Classification:
244253
245254
**If an entry mentions multiple symbols where some are new and some pre-existed** (e.g., "Add `newton.geometry.compute_offset_mesh()` and a viewer toggle"), split: the genuinely new symbols each get a New API entry; the extensions to existing symbols each get a Changes entry.
246255
247-
**Public-API exposure check.** For every symbol that passes the "genuinely new" test, also verify at HEAD that it is reachable via one of the public re-export modules listed above. If the symbol only exists under `newton._src.*` and is not re-exported through a public module, flag it in the report (Section "CHANGELOG Review Notes" → 🕵️ Private-only) — AGENTS.md requires user-facing symbols to be re-exported and forbids examples/docs from importing `newton._src`. Do not treat this as a hard block on the entry; surface it so the release manager can confirm the symbol was intended to be public.
256+
**Public-API declaration and exposure check.** At HEAD, inspect every public
257+
module discovered by `api_modules()` and `solver_submodule_pages()`. Each module
258+
must define `__all__` as a list or tuple containing only strings; a missing or
259+
invalid declaration is a policy finding in "CHANGELOG Review Notes". For every
260+
symbol that passes the "genuinely new" test, collect its memberships across
261+
those declarations:
262+
263+
- No memberships: report 🕵️ Private-only. The symbol exists only under
264+
`newton._src.*` and was not re-exported through a public module.
265+
- One membership: the symbol has a canonical public import path.
266+
- More than one membership: report a duplicate-public-export policy finding
267+
and list every public module that exports it.
268+
269+
`CODING_GUIDELINES.rst` requires each public symbol to appear in exactly one
270+
public module's `__all__` and forbids examples/docs from importing
271+
`newton._src`. Do not treat the private-only finding as a hard block on the
272+
entry; surface it so the release manager can confirm the symbol was intended
273+
to be public. Treat missing or invalid declarations and duplicate exports as
274+
policy findings that require an explicit release decision.
248275
249276
### 4b — Resolve New API signatures + docstrings
250277
@@ -273,7 +300,7 @@ For each CHANGELOG entry in Changed / Removed / Deprecated (plus any "capability
273300
- For semantic-only changes (no signature shift) where the prose describes a rename / reorder / behavioral flip: skip the diff block; include the backing commit's URL and the full CHANGELOG text.
274301
- For Removed entries: show the old signature on a `-` line; omit `+`.
275302
276-
**Deprecation-window lookup for Removed entries.** Newton's policy (per AGENTS.md) is: *breaking changes require a deprecation first*. Every Removed entry needs evidence of a deprecation in a prior release. Start with the released CHANGELOG, then fall back to code-level runtime-warning evidence at the base ref. For every Removed entry (and every Changed entry whose prose describes a removal), search CHANGELOG.md for the matching prior Deprecated entry:
303+
**Deprecation-window lookup for Removed entries.** Newton's policy (per `CODING_GUIDELINES.rst`) is: *breaking changes require a deprecation first*. Every Removed entry needs evidence of a deprecation in a prior release. Start with the released CHANGELOG, then fall back to code-level runtime-warning evidence at the base ref. For every Removed entry (and every Changed entry whose prose describes a removal), search CHANGELOG.md for the matching prior Deprecated entry:
277304
278305
1. Extract distinctive tokens from the Removed entry: the named symbol(s) in backticks and, if the entry carries a GH ref, that ref number.
279306
2. Scan the appropriate released-version sections of CHANGELOG.md for a `### Deprecated` bullet that names the same symbol(s) OR the same GH ref. The search scope depends on mode:
@@ -367,8 +394,8 @@ Read `references/language-review-examples.md`. For EACH CHANGELOG entry, apply L
367394
- **🗣️ Internal language**: internal module paths (`newton._src.*`), private identifiers with a leading underscore, Warp-internal types (`wp.types.*` that are not documented user types), implementation-detail verbs ("refactor", "reorganize", "rewrite") without a user-visible outcome.
368395
- **📝 Too terse**: under ~10 words with no context, or missing migration guidance in a Deprecated / Changed entry that names a rename or removal.
369396
- **🕵️ Private-only symbol**: the CHANGELOG `### Added` entry names a symbol that exists only in `newton._src.*` at HEAD and is not re-exported through a public module. See Phase 4a.
370-
- **📐 Missing migration guidance** (Newton-specific): entries in `### Deprecated`, `### Removed`, or `### Changed` (where the prose indicates rename / reorder / removal) MUST include migration guidance per AGENTS.md ("Use `Y` instead", "in favor of `Y`", "switch to `Y`"). Flag entries that rename or remove symbols without pointing to the replacement.
371-
- **🏷️ Naming-convention drift** (Newton-specific): new public symbols in `### Added` whose names violate Newton's prefix-first convention (e.g., `PDActuator` should be `ActuatorPD`; `add_sphere_shape()` should be `add_shape_sphere()`). See AGENTS.md.
397+
- **⬆️ Missing migration guidance** (Newton-specific): entries in `### Deprecated`, `### Removed`, or `### Changed` (where the prose indicates rename / reorder / removal) MUST include migration guidance per `changelog/README.md` ("Use `Y` instead", "in favor of `Y`", "switch to `Y`"). Flag entries that rename or remove symbols without pointing to the replacement.
398+
- **🏷️ Naming-convention drift** (Newton-specific): new public symbols in `### Added` whose names violate Newton's prefix-first convention (e.g., `PDActuator` should be `ActuatorPD`; `add_sphere_shape()` should be `add_shape_sphere()`). See `CODING_GUIDELINES.rst`.
372399
373400
Record flagged entries. Keep the FULL entry text in the audit table — do not truncate.
374401
@@ -434,7 +461,7 @@ The calibration section groups results by flag class and, within each, by outcom
434461
- Invalidated if: the symbol stays reachable only via `_src` in every post-target release AND no issues reference it — it was probably intentional internal-only despite its position in the Added section. Note this as "intended internal; CHANGELOG language could have been clearer".
435462
- Unresolved if: the symbol was removed / renamed before re-export resolution.
436463
437-
**📐 Missing-migration-guidance:**
464+
**⬆️ Missing-migration-guidance:**
438465
- Validated if: a later release's CHANGELOG bullet (in `### Changed` or `### Fixed`) retroactively points to the replacement, OR a subsequent issue references users stuck on the migration. The second signal is only available if `gh issue list` returns matches for the symbol name; probe with `gh issue list --search "<symbol>" --state all --json number,title`.
439466
- Invalidated if: the migration was handled by a runtime `DeprecationWarning` with a clear message (grep the code at `v<target>` for the warning text).
440467
- Unresolved: default.
@@ -596,7 +623,7 @@ Never pass `--public`. Never file a destination the user did not choose.
596623
- Towncrier insertion marker: `<!-- towncrier release notes start -->`.
597624
- CHANGELOG subsection headers: `### Added`, `### Changed`, `### Deprecated`, `### Removed`, `### Fixed`.
598625
- Symbol extraction from entry text: backtick-quoted `newton.X`, `newton.X.Y`, `ClassName.method`, bare `ClassName` (capitalized identifier), bare `snake_case_name()`. The FIRST backtick-quoted symbol in the bullet is usually the primary subject.
599-
- Migration-guidance regex (Phase 5a 📐): case-insensitive search for `use\s+\x60`, `in favor of`, `renamed? to`, `replaced? by`, `switch to`, `migrate to`, `prefer\s+\x60` (where `\x60` matches a backtick).
626+
- Migration-guidance regex (Phase 5a ⬆️): case-insensitive search for `use\s+\x60[^\x60]+\x60`, `in favor of\s+\x60[^\x60]+\x60`, `rename(?:d)?(?:\s+\x60[^\x60]+\x60)?\s+to\s+\x60[^\x60]+\x60`, `replace(?:d)?(?:\s+\x60[^\x60]+\x60)?\s+by\s+\x60[^\x60]+\x60`, `switch to\s+\x60[^\x60]+\x60`, `migrate to\s+\x60[^\x60]+\x60`, `prefer\s+\x60[^\x60]+\x60` (where `\x60` matches a backtick).
600627
601628
## Failure modes
602629

.claude/skills/release-audit/references/classification-rules.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ Used to decide whether a symbol is "genuinely new" vs. "pre-existed and got exte
99
Newton exposes its public surface through per-topic re-export modules discovered dynamically by `docs/generate_api.py`:
1010

1111
- `api_modules()` imports `newton`, starts with the top-level module, and adds every module-valued name exported through `newton.__all__`.
12-
- Each discovered module's own `__all__` defines its public symbols; when `__all__` is absent, `public_symbols()` falls back to non-private, non-module attributes.
12+
- Each discovered module's own `__all__` list or tuple defines its public symbols. `public_symbols()` rejects missing declarations, invalid containers, and non-string entries.
1313
- `solver_submodule_pages()` adds public solver submodules and recursively exposed module trees under `newton.solvers`.
1414
- There is no fixed `MODULES` constant. Inspect `docs/generate_api.py`, `newton/__init__.py`, and `newton/solvers.py` at both refs so additions such as a new top-level public module or nested experimental solver namespace are included.
1515

16-
Representative modules include `newton.geometry`, `newton.solvers`, and `newton.viewer`. Each public module re-exports from `newton/_src/<topic>/...`. `newton._src` is internal (AGENTS.md: "Examples and docs must not import from `newton._src`").
16+
Representative modules include `newton.geometry`, `newton.solvers`, and `newton.viewer`. Each public module re-exports from `newton/_src/<topic>/...`. `newton._src` is internal (`CODING_GUIDELINES.rst`: examples and docs must not import from `newton._src`).
1717

1818
**To determine if `newton.X` existed at base**:
1919
- Inspect module-valued exports in `newton.__all__` at base and target using the `api_modules()` rules above.
@@ -22,7 +22,7 @@ Representative modules include `newton.geometry`, `newton.solvers`, and `newton.
2222
- For nested solver modules: apply `solver_submodule_pages()` reachability rules to `newton.solvers` at the relevant ref.
2323
- For method additions on an existing class (e.g., `SolverXPBD.update_contacts`): resolve the class's real source file (e.g., `newton/_src/solvers/xpbd.py`) and `ast`-walk it at base.
2424

25-
**Public-API exposure check (Phase 4a addition)**: for every symbol that is genuinely new, verify at HEAD that it is reachable via at least one public module. If the symbol lives only in `newton._src.<path>` and is not re-exported, raise a 🕵️ Private-only flag. Reason: AGENTS.md forbids examples/docs from importing `newton._src`, so a user-facing symbol that is not re-exported is unusable by Newton's own examples and will churn.
25+
**Public-API declaration and exposure check (Phase 4a addition)**: inspect every discovered public module at HEAD. Its `__all__` must be a list or tuple containing only strings. For every genuinely new symbol, collect all public-module `__all__` declarations that contain it. Zero memberships raise a 🕵️ Private-only flag; one establishes the canonical public import path; more than one raises a duplicate-public-export policy finding listing all paths. Reason: `CODING_GUIDELINES.rst` requires each public symbol to appear in exactly one public module's `__all__` and forbids examples/docs from importing `newton._src`.
2626

2727
## No kernel-scope builtin registry
2828

@@ -75,11 +75,11 @@ Newton's CHANGELOG does NOT use the `**Breaking:**` literal marker. Instead, mig
7575
- "Reorder `X()` parameters so `a` precedes `b`."
7676
- "Migrate all Y logic to Z, all Y functions now return ..."
7777

78-
When Phase 4d / 5a encounter these patterns, treat them as migration-required changes (Kind `rename` or `parameter reorder` in the Changes-to-Existing-API table). Check that the entry includes migration guidance (`Use ...`, `in favor of ...`, `prefer ...`). If guidance is missing, raise a 📐 flag in the language review.
78+
When Phase 4d / 5a encounter these patterns, treat them as migration-required changes (Kind `rename` or `parameter reorder` in the Changes-to-Existing-API table). Check that the entry includes migration guidance (`Use ...`, `in favor of ...`, `prefer ...`). If guidance is missing, raise a ⬆️ flag in the language review.
7979

8080
## Deprecation policy (Phase 4d)
8181

82-
AGENTS.md: "Breaking changes require a deprecation first." A prior released `### Deprecated` entry is the preferred evidence. A matching runtime `DeprecationWarning` at the base ref also proves that users received a deprecation window, even if the released CHANGELOG omitted it. The warning may be emitted directly or by a shared helper / decorator that clearly applies to the removed API or behavior.
82+
`CODING_GUIDELINES.rst`: breaking changes require a deprecation first. A prior released `### Deprecated` entry is the preferred evidence. A matching runtime `DeprecationWarning` at the base ref also proves that users received a deprecation window, even if the released CHANGELOG omitted it. The warning may be emitted directly or by a shared helper / decorator that clearly applies to the removed API or behavior.
8383

8484
When Phase 4d cannot find the prior Deprecated entry:
8585
- Resolve the symbol or legacy behavior in code at the base ref.

0 commit comments

Comments
 (0)