S7 review findings

specs/S7-validation.md

@@ -47,9 +47,9 @@ Set JAVA_HOME to JDK 25 and run from the repository root.
 
 | Command/check | Result |
 | --- | --- |
-| `mvn -Dformat.mode=check clean verify` | Passed: 38 backend tests, 17 checker tests, standards checks and the exact backend coverage gate. |
+| `mvn -Dformat.mode=check clean verify` | Passed: 38 backend tests, 19 checker tests, standards checks and the exact backend coverage gate. |
 | `mvn -pl agiledancers-be package -DskipTests` | Passed; Quarkus packaged app successfully produced and subsequently exercised by Playwright. |
-| `mvn -f tooling/standards/pom.xml test exec:java` | 17 checker tests passed; no unwaived blocking violations. |
+| `mvn -f tooling/standards/pom.xml test exec:java` | 19 checker tests passed; no unwaived blocking violations. |
 | `java tooling/coverage/Coverage.java agiledancers-be/target/site/jacoco/jacoco.xml` | Clean backend-only line gate passed: 791 covered / 818 total = 96.69926650366749%. |
 | `PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/google/chrome/chrome npm run e2e` in frontend | Both browser tests passed against the packaged instrumented backend. |
 | Pinned CLI `dump --address 127.0.0.1 --destfile agiledancers-be/target/playwright.exec` | Successfully collected execution data from the agent's TCP server. |
@@ -111,3 +111,15 @@ The maintainer approved both proposals in the S7 implementation conversation:
 
 There are no parameter waivers for the three service methods, blanket suppressions
 or weakened thresholds. The completed specification resides in `specs/in_review`.
+
+## Review fixes
+
+The review fixes enforce finality for assign-once pattern bindings, including
+`instanceof`, switch and record-deconstruction bindings, while allowing reassigned
+bindings and inferred record-pattern types. Trivial-accessor detection now uses
+AST statements and resolved field/parameter references, so comments do not change
+public-method counts. Accessors with calls, extra statements or compound
+assignments still count. Two regression tests cover these cases.
+
+After these fixes, JDK 25 `mvn -B -ntp -Dformat.mode=check verify` passed with
+38 backend tests, 19 checker tests and backend line coverage of 791/818 (96.70%).

tooling/standards/src/main/java/Standards.java

@@ -356,9 +356,9 @@ public class Standards {
   static void variable(ASTVariableId id) {
     if (id.isEnumConstant() && !id.getName().matches("[A-Z][A-Z0-9]*(_[A-Z0-9]+)*"))
       block(id, "Enum constants require UPPER_SNAKE_CASE");
-    if (id.isRecordComponent() || id.isEnumConstant() || id.isPatternBinding() || id.isUnnamed())
+    if (id.isRecordComponent() || id.isEnumConstant() || id.isUnnamed())
       return;
-    if (id.isTypeInferred() && !id.isLambdaParameter()) {
+    if (id.isTypeInferred() && !id.isLambdaParameter() && !id.isPatternBinding()) {
       final ASTExpression initializer = id.getInitializer();
       if (!(initializer instanceof ASTConstructorCall)
           && !(initializer instanceof ASTCastExpression)
@@ -447,12 +447,8 @@ public class Standards {
   }
 
   static boolean trivial(ASTExecutableDeclaration method) {
-    if (method.getBody() == null) return false;
-    final String body = method
-        .getTextDocument()
-        .sliceOriginalText(method.getBody().getTextRegion())
-        .toString()
-        .replaceAll("\\s+", "");
+    if (method.getBody() == null || method.getBody().getNumChildren() != 1) return false;
+    final JavaNode statement = method.getBody().getChild(0);
     final String name = method.getName();
     String property = name;
     if (name.startsWith("get") || name.startsWith("set")) property = name.substring(3);
@@ -463,12 +459,26 @@ public class Standards {
     if (method.getEnclosingType().getSymbol().getDeclaredFields().stream()
         .noneMatch(f -> f.getSimpleName().equals(fieldName))) return false;
     if (method.getArity() == 0)
-      return body.equals("{return" + property + ";}")
-          || body.equals("{returnthis." + property + ";}");
+      return statement instanceof ASTReturnStatement returned
+          && accessorField(returned.getExpr(), method, property);
     if (method.getArity() != 1 || !name.startsWith("set")) return false;
-    final String parameter = method.getFormalParameters().get(0).getVarId().getName();
-    return body.equals("{this." + property + "=" + parameter + ";}")
-        || !property.equals(parameter) && body.equals("{" + property + "=" + parameter + ";}");
+    return statement instanceof ASTExpressionStatement expression
+        && expression.getExpr() instanceof ASTAssignmentExpression assignment
+        && !assignment.isCompound()
+        && accessorField(assignment.getLeftOperand(), method, property)
+        && assignment.getRightOperand() instanceof ASTVariableAccess value
+        && method.getFormalParameters().get(0).getVarId().getSymbol()
+            .equals(value.getReferencedSym());
+  }
+
+  static boolean accessorField(
+      ASTExpression expression, ASTExecutableDeclaration method, String property) {
+    if (!(expression instanceof ASTAssignableExpr.ASTNamedReferenceExpr reference)) return false;
+    if (expression instanceof ASTFieldAccess field
+        && !(field.getQualifier() instanceof ASTThisExpression)) return false;
+    return method.getEnclosingType().getSymbol().getDeclaredFields().stream()
+        .anyMatch(field -> field.getSimpleName().equals(property)
+            && field.equals(reference.getReferencedSym()));
   }
 
   record Dependency(JTypeMirror type, Set<String> qualifiers) {}

tooling/standards/src/test/java/StandardsTest.java

@@ -145,6 +145,35 @@ class StandardsTest {
     assertFalse(findings.stream().anyMatch(s -> s.contains("requires final: count")));
   }
 
+  @Test
+  void patternBindingsRequireFinalOnlyWhenAssignedOnce() throws Exception {
+    var findings = analyze("""
+        record Pair(Object key, Object value) {}
+        class Fixture {
+            public void run(final Object input) {
+                if (input instanceof String unchanged) { System.out.println(unchanged); }
+                if (input instanceof final String protectedValue) { System.out.println(protectedValue); }
+                if (input instanceof String reassigned) { reassigned = reassigned.trim(); System.out.println(reassigned); }
+                switch (input) {
+                    case String switched -> System.out.println(switched);
+                    default -> {}
+                }
+                if (input instanceof Pair(var key, var value)) {
+                    System.out.println(key);
+                    System.out.println(value);
+                }
+            }
+        }
+        """);
+    for (String binding : List.of("unchanged", "switched", "key", "value"))
+      assertTrue(findings.stream().anyMatch(s -> s.contains("requires final: " + binding)),
+          findings.toString());
+    for (String binding : List.of("protectedValue", "reassigned"))
+      assertFalse(findings.stream().anyMatch(s -> s.contains("requires final: " + binding)),
+          findings.toString());
+    assertFalse(findings.stream().anyMatch(s -> s.contains("var requires")), findings.toString());
+  }
+
   @Test
   void inheritanceCountsOnlyProjectSuperclasses() throws Exception {
     var findings = analyze(
@@ -202,6 +231,30 @@ class StandardsTest {
     assertTrue(contains(findings, "METRIC", "publicMethods=3.0"), findings.toString());
   }
 
+  @Test
+  void commentedAccessorsRemainTrivialWhileAdditionalWorkStillCounts() throws Exception {
+    var findings = analyze("""
+        class Fixture {
+            private String name;
+            private String trimmed;
+            private String logged;
+            private String calculated;
+            private String appended;
+            public String getName() { /* Intent comment. */ return name; }
+            public void setName(final String value) { this./* Field comment. */name = value; }
+            public String name() { // Read the stored value.
+                return this.name;
+            }
+            public void setName(final String value, final boolean validate) { this.name = value; }
+            public void setTrimmed(final String value) { this.trimmed = value.trim(); }
+            public String getLogged() { System.out.println(logged); return logged; }
+            public String getCalculated() { return calculated.trim(); }
+            public void setAppended(final String value) { this.appended += value; }
+        }
+        """);
+    assertTrue(contains(findings, "METRIC", "publicMethods=5.0"), findings.toString());
+  }
+
   @Test
   void classLengthBoundaryAndNestedTypeLines() throws Exception {
     assertTrue(contains(