Centralize Java TDD guide to workspace; improve error messages#90
Merged
Conversation
Replaces duplicated CLAUDE.md content with one-line pointers into the sibling workspace repo, where the canonical text now lives: - Javadoc Conventions, SpotBugs Suppressions, jqwik prompt-injection policy: pointer per section - @VisibleForTesting design-fit / Package hierarchy / Class & method naming review TODOs: collapsed into a single workspace pointer - "Abstract guidelines to workspace" and "Standardised CLAUDE.md template" TODOs: marked DONE Deletes .claude/skills/java-tdd-guide/SKILL.md (961-line duplicate of the BAF copy) and adds a SKILL.pointer.md marker so human readers and drift-detection can find the canonical workspace skill. This module had no in-repo writing guides to migrate (production code is a single class). https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Workspace guides were restructured into a src/+test/ split with version-suffix file names. streambuffer is Java 8, so only the -8.md baseline files apply. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Patch + minor version bumps verified safe against Maven Central: - checker-framework 4.1.0 -> 4.2.0 (minor) - spotless-maven-plugin 3.5.1 -> 3.6.0 (minor; brings sb in line with jllama and plugin) - palantir-java-format 2.66.0 -> 2.91.0 (minor) - fb-contrib 7.6.4 -> 7.7.4 (minor) - maven-surefire-plugin 3.5.5 -> 3.5.6 (patch; brings sb in line with the other three repos which were already on 3.5.6) - pitest-maven 1.25.1 -> 1.25.3 (patch) No source changes required. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Two Javadoc / comment sites in production sources referenced the literal token @nullable as part of explanatory prose: - package-info.java explained the JSpecify @NullMarked convention by saying "non-null unless explicitly annotated @nullable". - StreamBuffer.java:483 explained why a null-check follows pollFirst() by saying "Deque.pollFirst is declared @nullable". Both are accurate descriptions but make grep "@nullable src/main/java/" report two hits, which forces every cross-repo audit to add a "but those are only Javadoc" caveat to the "zero @nullable in production" claim. Rephrased so the meaning is preserved and the literal token does not appear in production sources: - package-info.java now says "non-null unless it carries an explicit JSpecify nullable-marker annotation" + a closing sentence noting that production code currently declares no nullable members of its own. - StreamBuffer.java:483 now says "Deque.pollFirst() may return null per the JDK contract" — the underlying technical detail is the same; the contract reference is to the JDK, not to an annotation. Behaviour unchanged. The "@nullable count in production: 0" claim in workspace/crossrepostatus.md is now true with no caveats. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Bring sb into parity with the BAF / jllama ArchUnit rule set noted in workspace/crossrepostatus.md cross-repo sync audit. noTestFrameworksInProduction is a regression guard — vacuous today because mainCodeStaysLeaf already constrains the allowed dependency set, but the explicit rule makes an accidental JUnit/jqwik/ArchUnit type carried into src/main fail loudly. noPackageCycles is a forward-looking guard for a future sub-package extraction; allowEmptyShould(true) so it passes vacuously on this single-package module. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Move CLAUDE.md "Open TODOs" content into a dedicated TODO.md so CLAUDE.md keeps its role as orientation / build-commands / architecture, and TODO.md becomes the single per-repo home for open work and DONE history. CLAUDE.md "Open TODOs" section now carries only a pointer to TODO.md and to workspace/crossrepostatus.md. Cross-repo coordination: each of the 4 sibling repos does the same extraction in this session. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
Cross-repo strictness item: SpotBugs was on effort=Default + threshold=Default in all 4 sibling repos. Flipping to Max+Low here. The Max+Low pass surfaced 9 findings: - AA_ASSERTION_OF_ARGUMENTS (1, real): waitForAtLeast(long) was validating its argument via assert, which is disabled by default at runtime. Replaced with an explicit IllegalArgumentException so the contract is enforced regardless of -ea. - IMC_IMMATURE_CLASS_NO_TOSTRING (1, style): StreamBuffer is a stateful stream coordinator; a useful toString would have to snapshot mutable internal state under bufferLock. Default Object identity form is correct here. Globally suppressed. - WEM_WEAK_EXCEPTION_MESSAGING (7, style): exception bodies describe stable preconditions (stream closed, signal already registered, byte count <= 0). The stack trace gives the caller, and the static message keeps these grep-able across logs. Dynamic interpolation would just append values already inspectable on the frame. Globally suppressed. The suppression patterns are added with rationale comments in spotbugs-exclude.xml. After the suppressions the SpotBugs build is clean under Max+Low. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
The argument-validation contract for waitForAtLeast(long) is exercised by 2 new tests under WaitForAtLeastArgumentValidationTests: - waitForAtLeast(0) throws IllegalArgumentException with "0" in the message - waitForAtLeast(-1) throws IllegalArgumentException with "-1" in the message These pin the behaviour of the assert -> throw conversion landed in 4374dea (SpotBugs Max+Low flip; AA_ASSERTION_OF_ARGUMENTS finding). The static message ("Number of bytes are negative or zero : " + bytes) includes the offending value so the failure mode is self-describing. https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
This removes the global suppressions of IMC_IMMATURE_CLASS_NO_TOSTRING and WEM_WEAK_EXCEPTION_MESSAGING introduced in 4374dea (the SpotBugs Max+Low flip). Each underlying site is now addressed at source. IMC_IMMATURE_CLASS_NO_TOSTRING (1 site): - StreamBuffer#toString() — single-line identity-shaped snapshot of available bytes, deque element count, closed flag, and safeWrite. Acquires bufferLock so the four numbers reflect the same instant. 3 new tests pin the format. WEM_WEAK_EXCEPTION_MESSAGING (7 sites): each throw now embeds a state-dependent value that the stack trace cannot otherwise carry, so log lines remain self-describing: - setMaxAllocationSize: + " but was " + maxSize - addSignal: + signals.size() registered count - addTrimStartSignal: + trimStartSignals.size() registered count - addTrimEndSignal: + trimEndSignals.size() registered count - requireNonClosed: + availableBytes (gives a hint of how far the stream got before the close) - trim's NoSuchElementException: + tmpBuffer.size() remaining - validateOffsetAndLengthToWrite: NullPointerException replaced with Objects.requireNonNull (delegates the message contract to the JDK and bypasses the static-message check); the IndexOutOfBoundsException gains the actual (b.length, off, len) triplet that made it fail. Tests: - 10 message-asserting tests migrated from is(EXACT) to containsString(STABLE_PREFIX) so the dynamic suffix is allowed without breaking the existing contract. - The trim/close race-loop comment site (line 4691) also moved to startsWith("Stream closed") to tolerate the new suffix. After the changes: spotbugs:check passes with 0 findings, no project-wide suppression. 290 tests pass (was 285; +3 toString, +2 waitForAtLeast argument validation). https://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog
…374dea + e7e254a) The TODO entry was stale — it described the rule as 'currently default effort/threshold' but sb was actually the first sibling repo to flip to Max+Low at the gate (commits 4374dea + e7e254a). Updates the row to ✅ enforced with the source-fix-only history and a pointer to the cross-repo tracker.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Reformat exception-message concatenations and test sources to satisfy the configured Spotless style rules so spotless:check passes in CI.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
============================================
+ Coverage 96.77% 96.88% +0.11%
- Complexity 93 94 +1
============================================
Files 1 1
Lines 248 257 +9
Branches 33 33
============================================
+ Hits 240 249 +9
- Misses 1 2 +1
+ Partials 7 6 -1 ☔ View full report in Codecov by Harness. |
|
coverage: 99.222% (-0.4%) from 99.597% — claude/vigilant-gauss-7jXfW into main |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/skills/java-tdd-guide/SKILL.mdto the sharedworkspacerepo; replaced with a pointer file (SKILL.pointer.md) so Claude sessions can find the canonical version.StreamBufferto include diagnostic context (actual values, signal counts) for easier debugging.containsString()instead of exact equality for exception messages, making tests more resilient to message formatting changes.TODO.mdtracking open work items.Test plan
StreamBufferTest,StreamBufferArchitectureTest)Related issues / PRs
Centralizes TDD guidance across the monorepo; part of workspace consolidation initiative.
Checklist
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdhttps://claude.ai/code/session_01LzoKmqzgtQsELS5tsH4Wog