From 37a8ed6b8c531fa0105171991a31bfb00f9f563f Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Mon, 19 Oct 2026 21:48:03 +0200 Subject: [PATCH 1/4] Fix the eleven remaining checkstyle warnings - ImageDecoder: rename the kEnd/vStart locals to keyEnd/valueStart and pngZTxtChunk/pngITxtChunk to pngZtxtChunk/pngItxtChunk. - ImageDecoderTest: tRnsOn* test methods become trnsOn*. - HfTokenizerTest: drop the leading "a" from two test method names. - SmolVlmGeometryPlan: document the compact constructor, matching Qwen3VlGeometryPlan. Pure renames plus one Javadoc comment; no behavior change. Co-Authored-By: Claude Opus 5.5 --- .../jmlx/tokenizer/HfTokenizerTest.java | 4 +-- .../se/alipsa/jmlx/vision/ImageDecoder.java | 34 +++++++++---------- .../jmlx/vision/SmolVlmGeometryPlan.java | 1 + .../alipsa/jmlx/vision/ImageDecoderTest.java | 4 +-- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java b/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java index 4d0640fb..a1610629 100644 --- a/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java +++ b/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java @@ -338,7 +338,7 @@ void additionalDefaultTemplateBeatsTheLegacyProcessorChatTemplateFile() throws E } @Test - void aBundleWithoutAnyTemplateSourceStillHasNoTemplates() throws Exception { + void bundleWithoutAnyTemplateSourceStillHasNoTemplates() throws Exception { Path directory = Files.createDirectory(temporaryDirectory.resolve("no-templates")); Files.copy(wordPieceFixture(), directory.resolve("tokenizer.json")); HfTokenizer tokenizer = HfTokenizer.fromDirectory(directory); @@ -351,7 +351,7 @@ void aBundleWithoutAnyTemplateSourceStillHasNoTemplates() throws Exception { } @Test - void aMalformedLegacyChatTemplateFileIsRejected() throws Exception { + void malformedLegacyChatTemplateFileIsRejected() throws Exception { Path missingKey = Files.createDirectory(temporaryDirectory.resolve("legacy-missing-key")); Files.copy(wordPieceFixture(), missingKey.resolve("tokenizer.json")); Files.writeString(missingKey.resolve("chat_template.json"), "{\"other\": \"nope\"}"); diff --git a/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/ImageDecoder.java b/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/ImageDecoder.java index 415c24e2..704b1172 100644 --- a/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/ImageDecoder.java +++ b/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/ImageDecoder.java @@ -844,9 +844,9 @@ private static void pngChunk(byte[] b, int pos, int length, boolean preIdat, Png } else if (cid == 0x74455874) { // "tEXt" pngTextChunk(b, data, length, info); } else if (cid == 0x7A545874) { // "zTXt" - pngZTxtChunk(b, data, length, info); + pngZtxtChunk(b, data, length, info); } else if (cid == 0x69545874) { // "iTXt" - pngITxtChunk(b, data, length, info); + pngItxtChunk(b, data, length, info); } } @@ -860,15 +860,15 @@ private static void pngChunk(byte[] b, int pos, int length, boolean preIdat, Png private static void pngTextChunk(byte[] b, int data, int length, PngInfo info) { int end = data + length; int nul = indexOfByte(b, data, end); - int kEnd = nul >= 0 ? nul : end; - int vStart = nul >= 0 ? nul + 1 : end; - if (kEnd == data) { + int keyEnd = nul >= 0 ? nul : end; + int valueStart = nul >= 0 ? nul + 1 : end; + if (keyEnd == data) { return; } - if (equalsRegion(b, data, kEnd - data, EXIF_TEXT_KEYWORD)) { - info.exifPayload = Arrays.copyOfRange(b, vStart, end); - } else if (equalsRegion(b, data, kEnd - data, XMP_TEXT_KEYWORD)) { - info.xmpText = Arrays.copyOfRange(b, vStart, end); + if (equalsRegion(b, data, keyEnd - data, EXIF_TEXT_KEYWORD)) { + info.exifPayload = Arrays.copyOfRange(b, valueStart, end); + } else if (equalsRegion(b, data, keyEnd - data, XMP_TEXT_KEYWORD)) { + info.xmpText = Arrays.copyOfRange(b, valueStart, end); } } @@ -878,12 +878,12 @@ private static void pngTextChunk(byte[] b, int data, int length, PngInfo info) { * incomplete zlib payload yields the reference's empty value, which is stored (the slot is set, * to a falsy value), like the reference's {@code v = b""} on {@code zlib.error}. */ - private static void pngZTxtChunk(byte[] b, int data, int length, PngInfo info) { + private static void pngZtxtChunk(byte[] b, int data, int length, PngInfo info) { int end = data + length; int nul = indexOfByte(b, data, end); - int kEnd = nul >= 0 ? nul : end; + int keyEnd = nul >= 0 ? nul : end; int restStart = nul >= 0 ? nul + 1 : end; - if (kEnd == data) { + if (keyEnd == data) { return; } if (restStart < end && (b[restStart] & 0xFF) != 0) { @@ -893,9 +893,9 @@ private static void pngZTxtChunk(byte[] b, int data, int length, PngInfo info) { if (value == null) { value = new byte[0]; } - if (equalsRegion(b, data, kEnd - data, EXIF_TEXT_KEYWORD)) { + if (equalsRegion(b, data, keyEnd - data, EXIF_TEXT_KEYWORD)) { info.exifPayload = value; - } else if (equalsRegion(b, data, kEnd - data, XMP_TEXT_KEYWORD)) { + } else if (equalsRegion(b, data, keyEnd - data, XMP_TEXT_KEYWORD)) { info.xmpText = value; } } @@ -910,7 +910,7 @@ private static void pngZTxtChunk(byte[] b, int data, int length, PngInfo info) { * ({@code cf} 1) must use comp method 0, and any inflate failure leaves the slots untouched, like * the reference's early returns. */ - private static void pngITxtChunk(byte[] b, int data, int length, PngInfo info) { + private static void pngItxtChunk(byte[] b, int data, int length, PngInfo info) { int end = data + length; int nul1 = indexOfByte(b, data, end); if (nul1 < 0) { @@ -931,8 +931,8 @@ private static void pngITxtChunk(byte[] b, int data, int length, PngInfo info) { if (tkEnd < 0) { return; } - int vStart = tkEnd + 1; - byte[] value = Arrays.copyOfRange(b, vStart, end); + int valueStart = tkEnd + 1; + byte[] value = Arrays.copyOfRange(b, valueStart, end); if (cf != 0) { if (cm != 0) { return; diff --git a/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/SmolVlmGeometryPlan.java b/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/SmolVlmGeometryPlan.java index 17ca1a7d..c5e90c80 100644 --- a/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/SmolVlmGeometryPlan.java +++ b/jmlx-vision/src/main/java/se/alipsa/jmlx/vision/SmolVlmGeometryPlan.java @@ -25,6 +25,7 @@ public record SmolVlmGeometryPlan( int stage1Height, int stage1Width, int stage2Height, int stage2Width, int rows, int cols) { + /** Validates that the component values are mutually consistent (see the record Javadoc). */ public SmolVlmGeometryPlan { if (stage1Height <= 0 || stage1Width <= 0) { throw new IllegalArgumentException( diff --git a/jmlx-vision/src/test/java/se/alipsa/jmlx/vision/ImageDecoderTest.java b/jmlx-vision/src/test/java/se/alipsa/jmlx/vision/ImageDecoderTest.java index f5d9c02f..ac8a6b9c 100644 --- a/jmlx-vision/src/test/java/se/alipsa/jmlx/vision/ImageDecoderTest.java +++ b/jmlx-vision/src/test/java/se/alipsa/jmlx/vision/ImageDecoderTest.java @@ -154,7 +154,7 @@ void iccProfileAndExifDoNotChangeDecodedPixels() throws Exception { } @Test - void tRnsOnTruecolorAndGrayscaleDecodesAsOpaque() throws Exception { + void trnsOnTruecolorAndGrayscaleDecodesAsOpaque() throws Exception { // A tRNS chunk on truecolor/grayscale is ignored (the reference keeps mode RGB/L and its // RGB conversion drops the transparency): each twin decodes byte-identical to its opaque // counterpart. @@ -169,7 +169,7 @@ void tRnsOnTruecolorAndGrayscaleDecodesAsOpaque() throws Exception { } @Test - void tRnsOnPalettedPngDecodesAsOpaqueTwin() throws Exception { + void trnsOnPalettedPngDecodesAsOpaqueTwin() throws Exception { // Palette tRNS is dropped to match the pinned processor (its palette is rebuilt from // getpalette(), RGB only): each tRNS twin decodes byte-identical to its opaque counterpart. Path fixtures = OracleFixtures.fixtures(); From 3a0ed3a25885a6f550a1b3c972fe03c2b18a4e54 Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Mon, 19 Oct 2026 21:48:03 +0200 Subject: [PATCH 2/4] Fail the build on any checkstyle violation The checkstyle config reports at warning severity and Gradle's default maxWarnings is unlimited, so violations only printed [WARN] lines and check still passed. Set maxWarnings = 0 in the root subprojects block and in buildSrc so every violation fails check, the same as spotlessCheck. AGENTS.md and README.md document the policy. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 8 ++++++-- README.md | 2 +- build.gradle | 4 ++++ buildSrc/build.gradle | 2 ++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 46a63185..6f34d1f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -300,8 +300,12 @@ Hand-written sources are Google Java Style, 2-space indent, 100-column width. Fo enforced by running `googleJavaFormat(...)` directly from root `build.gradle`'s Spotless block (no separate Spotless config file); linting is `config/checkstyle/checkstyle.xml`, derived from checkstyle's own bundled `google_checks.xml` with deviations documented in a comment at the top of -that file. The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are exempt from -both and must stay byte-identical to `scripts/regen-bindings.sh`'s output — never hand-edit them. +that file. Checkstyle runs with `maxWarnings = 0` in both builds: the config reports violations at +`warning` severity, but any violation fails `check`, the same as a Spotless difference. Fix it +(rename, add the Javadoc) rather than leaving it; never raise `maxWarnings` or suppress a rule to +get past it. The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are +exempt from both and must stay byte-identical to `scripts/regen-bindings.sh`'s output — never +hand-edit them. `buildSrc` (shared Gradle build-logic helpers, e.g. `PublishedPomDependencies`) is its own isolated Gradle build, so root `build.gradle`'s `subprojects{}` conventions never reach it -- it applies the same checkstyle/Spotless configuration independently in its own `buildSrc/build.gradle` instead diff --git a/README.md b/README.md index bd671c24..ca639b14 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ on a virtual thread is rejected. ## Code style -Hand-written sources are Google Java Style with 2-space indentation and a 100-column width, enforced by running `google-java-format` directly via Spotless (see `build.gradle`'s `spotless` block and `config/checkstyle/checkstyle.xml`, which derives from Google's own upstream artifact; see its comments for the one remaining documented deviation). The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are exempt from both, since they must stay byte-identical to `scripts/regen-bindings.sh`'s output. +Hand-written sources are Google Java Style with 2-space indentation and a 100-column width, enforced by running `google-java-format` directly via Spotless (see `build.gradle`'s `spotless` block and `config/checkstyle/checkstyle.xml`, which derives from Google's own upstream artifact; see its comments for the one remaining documented deviation). Any checkstyle violation fails `check` (`maxWarnings = 0`), even though the config reports it at `warning` severity. The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are exempt from both, since they must stay byte-identical to `scripts/regen-bindings.sh`'s output. ```sh ./gradlew spotlessCheck # verify formatting diff --git a/build.gradle b/build.gradle index 7dcc137f..f2860096 100644 --- a/build.gradle +++ b/build.gradle @@ -784,6 +784,10 @@ subprojects { checkstyle { toolVersion = checkstyleVersion configFile = rootProject.file('config/checkstyle/checkstyle.xml') + // The config's severity is 'warning', and Gradle's default maxWarnings is unlimited, so + // without this a violation only prints a [WARN] line and check still passes. Zero + // tolerance makes every checkstyle violation fail check, just like spotlessCheck. + maxWarnings = 0 } // jmlx-ffi/src/main/generated/java is committed jextract output, not diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index f9e6f9c5..a6b23898 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -41,6 +41,8 @@ test { checkstyle { toolVersion = '13.10.0' configFile = file('../config/checkstyle/checkstyle.xml') + // Same zero-tolerance policy as the main build's subprojects block. + maxWarnings = 0 } spotless { From 0deb9124879af0777e245be5dfefd0af4eb84407 Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Mon, 19 Oct 2026 22:01:18 +0200 Subject: [PATCH 3/4] Allow targeted checkstyle suppressions and name the buildSrc check The zero-warning note in AGENTS.md forbade suppressions outright, although checkstyle.xml deliberately enables @SuppressWarnings, CHECKSTYLE.SUPPRESS comments and an optional checkstyle-suppressions.xml. Permit a targeted suppression where a name is genuinely externally constrained, with a comment saying why; raising maxWarnings stays forbidden. Also say that a buildSrc violation fails ./gradlew -p buildSrc check, which is separate from the root check. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6f34d1f7..f4cd8903 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -301,11 +301,14 @@ enforced by running `googleJavaFormat(...)` directly from root `build.gradle`'s separate Spotless config file); linting is `config/checkstyle/checkstyle.xml`, derived from checkstyle's own bundled `google_checks.xml` with deviations documented in a comment at the top of that file. Checkstyle runs with `maxWarnings = 0` in both builds: the config reports violations at -`warning` severity, but any violation fails `check`, the same as a Spotless difference. Fix it -(rename, add the Javadoc) rather than leaving it; never raise `maxWarnings` or suppress a rule to -get past it. The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are -exempt from both and must stay byte-identical to `scripts/regen-bindings.sh`'s output — never -hand-edit them. +`warning` severity, but any violation fails `check` (for `buildSrc`, run +`./gradlew -p buildSrc check`; see below), the same as a Spotless difference. Fix a violation +(rename, add the Javadoc) rather than leaving it. Never raise `maxWarnings`. Use a targeted +suppression (the config supports `@SuppressWarnings`, `CHECKSTYLE.SUPPRESS:` comments and an +optional `checkstyle-suppressions.xml`) only where a name is genuinely externally constrained, +with a comment saying why. The generated jextract bindings under +`jmlx-ffi/src/main/generated/java` are exempt from both and must stay byte-identical to +`scripts/regen-bindings.sh`'s output — never hand-edit them. `buildSrc` (shared Gradle build-logic helpers, e.g. `PublishedPomDependencies`) is its own isolated Gradle build, so root `build.gradle`'s `subprojects{}` conventions never reach it -- it applies the same checkstyle/Spotless configuration independently in its own `buildSrc/build.gradle` instead From c813dcdca3b5d037644b489e026f1e5fbbe92b5e Mon Sep 17 00:00:00 2001 From: Per Nyfelt Date: Mon, 19 Oct 2026 22:04:47 +0200 Subject: [PATCH 4/4] Drop the unreachable checkstyle-suppressions.xml from AGENTS.md checkstyle.xml reads the suppressions file from ${org.checkstyle.google.suppressionfilter.config}, defaulting to a relative checkstyle-suppressions.xml, and neither build sets that property. A file in config/checkstyle/, the repo root or a module directory therefore never takes effect. Only the two mechanisms verified to work are listed: @SuppressWarnings("checkstyle:") and the CHECKSTYLE.SUPPRESS: for +N lines comment. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f4cd8903..b967f061 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -304,9 +304,9 @@ that file. Checkstyle runs with `maxWarnings = 0` in both builds: the config rep `warning` severity, but any violation fails `check` (for `buildSrc`, run `./gradlew -p buildSrc check`; see below), the same as a Spotless difference. Fix a violation (rename, add the Javadoc) rather than leaving it. Never raise `maxWarnings`. Use a targeted -suppression (the config supports `@SuppressWarnings`, `CHECKSTYLE.SUPPRESS:` comments and an -optional `checkstyle-suppressions.xml`) only where a name is genuinely externally constrained, -with a comment saying why. The generated jextract bindings under +suppression (`@SuppressWarnings("checkstyle:")` or a +`// CHECKSTYLE.SUPPRESS: for +N lines` comment) only where a name is genuinely externally +constrained, with a comment saying why. The generated jextract bindings under `jmlx-ffi/src/main/generated/java` are exempt from both and must stay byte-identical to `scripts/regen-bindings.sh`'s output — never hand-edit them. `buildSrc` (shared Gradle build-logic helpers, e.g. `PublishedPomDependencies`) is its own isolated