more

Author: trask

.github/agents/code-review-and-fix.agent.md

@@ -162,16 +162,20 @@ Auto-fix boundaries:
     but absent from the current module's check (or vice versa) reveal a version boundary —
     the class was likely added or removed between versions.
     Then determine the comment form for each class:
-    **Floor class** (proves "at least version X"): look up when the class was
-    **introduced** → comment `// added in X.Y`.
-    **Ceiling class** (proves "not yet version Y"): look up when the class was
-    **removed** → comment `// removed in Y.Z` (meaning: its presence here ensures we
-    don't match version Y.Z+ where a different module takes over).
-    **Single class that provides both bounds**: include both facts in one comment,
-    e.g. `// added in X.Y, removed in Y.Z`.
+   **Positive floor class** (proves "at least version X"): look up when the class was
+   **introduced** → comment `// added in X.Y`.
+   **Positive ceiling class** (proves "not yet version Y"): look up when the class was
+   **removed** → comment `// removed in Y.Z` (meaning: its presence here ensures we
+   don't match version Y.Z+ where a different module takes over).
+   **Negated exclusion class** in `not(hasClassesNamed(...))`: look up when the class was
+   **introduced** → comment `// added in Y.Z`, because that first appearance begins the
+   excluded version range.
+   **Single positive class that provides both bounds**: include both facts in one comment,
+   e.g. `// added in X.Y, removed in Y.Z`.
     A ceiling class might have been *introduced* much earlier than the module's target version.
-    Do not use `// added in` for a ceiling class — that is misleading. The relevant fact is
-    when it was **removed**.
+   Do not use `// added in` for a positive ceiling class — that is misleading. The relevant
+   fact is when it was **removed**. But for a negated exclusion class, `// added in` is the
+   correct form because the class's introduction is exactly what starts excluding newer versions.
     Validate the version in the comment before adding or requesting it. Do not guess the
     version from the module name alone; confirm it with repository or upstream evidence.
     Sources: muzzle `versions.set(...)` ranges, sibling module `classLoaderMatcher()` checks,

.github/agents/code-review.agent.md

@@ -96,9 +96,11 @@ For each file in scope:
    allocated on every invocation.
 6. Flag a missing version-boundary comment on a single-class `hasClassesNamed(...)`
    check in `classLoaderMatcher()` only after validating the stated boundary from
-   repository or upstream evidence. Use `// added in X.Y` when the class only provides
-   the lower bound; if the same class also provides the upper bound because it disappears
-   in a newer sibling version, preserve or request `removed in Y.Z` too.
+   repository or upstream evidence. Use `// added in X.Y` for a pure lower bound.
+   If the same positive class also provides the upper bound because it disappears in a
+   newer sibling version, preserve or request `removed in Y.Z` too. For negated checks
+   such as `not(hasClassesNamed(...))`, use `// added in Y.Z` because the class's first
+   appearance is what starts the excluded version range.
 7. Prevent duplicates:
    - If equivalent `REVIEW:` already exists above the same line, do not add another.
 

.github/agents/knowledge/javaagent-module-patterns.md

@@ -82,14 +82,19 @@ class suffices for a simple version boundary — use multiple classes only when:
   present (e.g., Jersey checks both the JAX-RS spec API and the Jersey implementation class;
   AWS SDK Lambda checks both the Lambda module and JSON protocol core).
 
-Place a comment **above each class name string** stating its version role:
-
-- **Floor class** (proves "at least version X"): use `// added in X.Y`.
-- **Ceiling class** (proves "not yet version Y"): use `// removed in Y.Z`.
-
-A single landmark class can sometimes provide **both** bounds: its presence proves the
-module is at least version `X.Y`, and the same class disappears in `Y.Z`, so its presence
-also excludes `Y.Z+`. In that case, a combined comment such as
+Place a comment **above each class name string** stating its version role and matcher shape:
+
+- **Positive floor class** in `hasClassesNamed(...)` (proves "at least version X"):
+  use `// added in X.Y`.
+- **Positive ceiling class** in `hasClassesNamed(...)` (proves "not yet version Y"):
+  use `// removed in Y.Z`.
+- **Negated exclusion class** in `not(hasClassesNamed(...))` or
+  `.and(not(hasClassesNamed(...)))`: use `// added in Y.Z`, because the class's presence
+  starts excluding `Y.Z+`.
+
+A single **positive** landmark class can sometimes provide **both** bounds: its presence
+proves the module is at least version `X.Y`, and the same class disappears in `Y.Z`, so
+its presence also excludes `Y.Z+`. In that case, a combined comment such as
 `// added in X.Y, removed in Y.Z` is appropriate.
 
 **How to identify ceiling classes**: check whether a **newer sibling module** exists for
@@ -152,9 +157,9 @@ OpenTelemetry instrumentation:
 ```java
 @Override
 public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-  // InternalRequest was introduced in 7.0.0
+  // added in 7.0.0
   return hasClassesNamed("org.elasticsearch.client.RestClient$InternalRequest")
-      // Instrumentation class was introduced in 8.10 (native OTel support)
+      // added in 8.10 (native OTel support)
       .and(not(hasClassesNamed(
           "co.elastic.clients.transport.instrumentation.Instrumentation")));
 }
@@ -165,9 +170,9 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
 ```java
 @Override
 public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-  // LibraryTelemetryOptions was introduced in azure-core 1.53
+  // added in azure-core 1.53
   return hasClassesNamed("com.azure.core.util.LibraryTelemetryOptions")
-      // OpenTelemetryTracer was introduced in azure-core-tracing-opentelemetry 1.0.0-beta.47
+      // added in azure-core-tracing-opentelemetry 1.0.0-beta.47
       .and(not(hasClassesNamed("com.azure.core.tracing.opentelemetry.OpenTelemetryTracer")));
 }
 ```
@@ -185,13 +190,17 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
 - **Version comments on landmark classes.** For multi-class checks, `.and(not(...))`
   chains, or cases where the landmark version differs from the module's base version,
   each `hasClassesNamed()` call must have a `//` comment stating the class's **role**:
-  - Floor class → `// added in X.Y` (class introduced in version X.Y).
-  - Ceiling class → `// removed in Y.Z` (class removed in version Y.Z, ensuring the
-    module does not activate on Y.Z+).
-  - Single class that serves as both floor and ceiling → include both boundaries, e.g.
-    `// added in X.Y, removed in Y.Z`.
-  Do not use `// added in` for a ceiling class — that states when the class first appeared,
-  which is irrelevant and misleading; the purpose is the upper bound.
+  - Positive floor class → `// added in X.Y` (class introduced in version X.Y).
+  - Positive ceiling class → `// removed in Y.Z` (class removed in version Y.Z, ensuring
+    the module does not activate on Y.Z+).
+  - Negated exclusion class in `not(hasClassesNamed(...))` → `// added in Y.Z` (class
+    introduced in version Y.Z, so its presence excludes Y.Z+).
+  - Single positive class that serves as both floor and ceiling → include both boundaries,
+    e.g. `// added in X.Y, removed in Y.Z`.
+  Do not use `// added in` for a **positive** ceiling class — that states when the class
+  first appeared, which is irrelevant and misleading; the purpose is the upper bound.
+  Conversely, do use `// added in` for a **negated** exclusion class, because the class's
+  first appearance is exactly what begins the excluded version range.
   To identify ceiling classes, check if a newer sibling module's `classLoaderMatcher()`
   checks a different variant or replacement class.
 - **Single-class landmark comments are required.** When a single-class