JSpecify big wave 3 by dondonz · Pull Request #4274 · graphql-java/graphql-java

dondonz · 2026-03-01T00:01:20Z

Another wave of JSpecify changes, this time featuring Claude's agent teams.

66 classes annotated

graphql.analysis (16 classes)

QueryComplexityCalculator, QueryComplexityInfo, QueryDepthInfo, QueryReducer, QueryTransformer, QueryTraversalOptions, QueryVisitor, QueryVisitorFieldArgumentEnvironment, QueryVisitorFieldArgumentInputValue, QueryVisitorFieldArgumentValueEnvironment, QueryVisitorFieldEnvironment, QueryVisitorFragmentDefinitionEnvironment, QueryVisitorFragmentSpreadEnvironment, QueryVisitorInlineFragmentEnvironment, QueryVisitorStub, ValueTraverser

graphql.execution core (26 classes)

AbortExecutionException, AsyncExecutionStrategy, AsyncSerialExecutionStrategy, CoercedVariables, DataFetcherExceptionHandlerParameters, DataFetcherExceptionHandlerResult, DefaultValueUnboxer, ExecutionContext, ExecutionId, ExecutionStepInfo, ExecutionStrategyParameters, FetchedValue, FieldValueInfo, InputMapDefinesTooManyFieldsException, MergedSelectionSet, MissingRootTypeException, NonNullableValueCoercedAsNullException, NormalizedVariables, OneOfNullValueException, OneOfTooManyKeysException, ResultNodesInfo, ResultPath, SimpleDataFetcherExceptionHandler, SubscriptionExecutionStrategy, UnknownOperationException, UnresolvedTypeException

graphql.execution sub-packages (24 classes)

ConditionalNodeDecision, QueryAppliedDirective, QueryAppliedDirectiveArgument, QueryDirectives, FieldValidationInstrumentation, SimpleFieldValidation, InstrumentationCreateStateParameters, InstrumentationExecuteOperationParameters, InstrumentationExecutionParameters, InstrumentationExecutionStrategyParameters, InstrumentationFieldCompleteParameters, InstrumentationFieldFetchParameters, InstrumentationFieldParameters, InstrumentationValidationParameters, TracingInstrumentation, TracingSupport, PreparsedDocumentEntry, ApolloPersistedQuerySupport, InMemoryPersistedQueryCache, PersistedQueryCacheMiss, PersistedQueryIdInvalid, PersistedQueryNotFound, DelegatingSubscription, SubscriptionPublisher

Review passes

./gradlew compileJava passes. The reviewer agent also fixed cascading NullAway errors in calling code (e.g. ResultPath, ExecutionStepInfo, ExecutionContext, TracingSupport) using assertNotNull() where structural invariants guarantee non-null.

https://claude.ai/code/session_01FG3V2VvjSSbUVcjn9FX4yx

dondonz · 2026-03-01T00:02:35Z

 * This Exception indicates that the current execution should be aborted.
 */
 @PublicApi
+@NullMarked


I abruptly ran out of credits here - more is coming

Given it's already 66 classes - the next wave will happen in a separate PR

github-actions · 2026-03-01T00:03:36Z

Test Results

335 files ±0 335 suites ±0 5m 4s ⏱️ -2s
5 378 tests ±0 5 369 ✅ - 1 9 💤 +1 0 ❌ ±0
5 467 runs ±0 5 458 ✅ - 1 9 💤 +1 0 ❌ ±0

Results for commit 0a77532. ± Comparison against base commit 739403f.

This pull request removes 196 and adds 172 tests. Note that renamed tests count towards both.

	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure23@2f2bff16 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure24@75de29c0 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure25@fc807c1 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure20@733aa9d8 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure21@6dcc40f5 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure22@2b680207 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure3@6eb17ec8 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure4@48d293ee delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure5@7a0ef219 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of error args with non null does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_6prov0_closure6@e5cbff2 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
…

This pull request skips 1 test.

graphql.schema.fetching.LambdaFetchingSupportTest ‑ different class loaders induce certain behaviours

♻️ This comment has been updated with latest results.

@NullMarked

All 66 classes annotated in Wave 1 (graphql.analysis, graphql.execution core, and graphql.execution sub-packages) are removed from the exemption list now that they carry @NullMarked. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

dondonz · 2026-03-01T05:01:03Z

+graphql.execution.reactive.DelegatingSubscription
+graphql.execution.reactive.SubscriptionPublisher
+```
+


Worker 4 got removed because it was covered by previous PRs.

Work from worker 5 onwards will be in another PR. This PR is already quite large

- Explicitly state that @internal classes must not be annotated - Tighten exemption list cleanup instruction to remove only the annotated class Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

bbakerman · 2026-03-07T04:45:13Z

         * @return this builder
         */
-        public Builder valueLiteral(@NonNull Value<?> value) {
+        public Builder valueLiteral(Value<?> value) {


So you tuned OFF null marked but this is no less precise than it was. Perhaps a Object.requireNonNull ??

That's a good idea

bbakerman · 2026-03-07T04:48:35Z


+    @Nullable
    public List<? extends GraphQLError> getErrors() {
        return errors;


Maybe we should change this so that errors is always non null but some times empty?

Document has to nullable of course. Quasi breaking change but nicer - eg less nullable things

That's a good idea, you mentioned this in another class too. I think better to make it an empty list by default, like DataFetcherResult

I will add a breaking change label

bbakerman

dondonz · 2026-03-07T07:24:26Z

I'm impressed, I have the correct number of fingers on both hands. Models have come a long way.

But I'm going to deduct marks for Python references creeping in!

github-actions · 2026-03-07T20:43:04Z

Test Report

Test Results

Java Version	Total	Passed	Failed	Errors	Skipped
Java 11	5733 (±0)	5677 (±0)	0 (±0)	0 (±0)	56 (±0)
Java 17	5733 (±0)	5676 (±0)	0 (±0)	0 (±0)	57 (±0)
Java 21	5733 (±0)	5676 (±0)	0 (±0)	0 (±0)	57 (±0)
Java 25	5733 (±0)	5676 (±0)	0 (±0)	0 (±0)	57 (±0)
jcstress	32 (±0)	32 (±0)	0 (±0)	0 (±0)	0 (±0)
Total	22964 (±0)	22737 (±0)	0 (±0)	0 (±0)	227 (±0)

Code Coverage (Java 25)

Metric	Covered	Missed	Coverage	vs Master
Lines	28903	3119	90.3%	±0.0%
Branches	8418	1504	84.8%	±0.0%
Methods	7730	1222	86.3%	±0.0%

Changed Class Coverage (1 class)

Class	Line	Branch	Method
g.e.p.PreparsedDocumentEntry	±0.0%	+25.0% 🟢	±0.0%

Full HTML report: build artifact jacoco-html-report

Updated: 2026-04-23 00:34:55 UTC

# Conflicts: # .claude/commands/jspecify-annotate.md # src/main/java/graphql/execution/ExecutionContext.java # src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy

…tated classes from exemption list - Update PreparsedDocumentEntryTest to expect empty list instead of null for errors - Remove 10 classes from JSpecify exemption list that are already annotated Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

….parseAndValidate Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

dondonz · 2026-03-31T19:35:01Z

    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {
        return null;


TODO open question if we want null or an empty list here?

as a general rule we always do empty list instead of null list

dondonz · 2026-04-04T20:38:12Z

    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {
        return sourceLocations;


Also a TODO question - should we have any lists returning null?

same as above ... in general we always to return empty lists instead of null

dondonz · 2026-04-04T20:38:33Z


    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {


TODO Should any list (not only errors) be nullable?

same as above

andimarek

There are some comments about empty list vs null ... I actually think we should create a todo and fix that up later ... lets focus on nullable stuff for now.

Looks good.

andimarek · 2026-04-19T22:03:43Z

    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {
        return null;


as a general rule we always do empty list instead of null list

andimarek · 2026-04-19T22:05:46Z

    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {
        return sourceLocations;


same as above ... in general we always to return empty lists instead of null

andimarek · 2026-04-19T22:06:05Z


    @Override
-    public List<SourceLocation> getLocations() {
+    public @Nullable List<SourceLocation> getLocations() {


same as above

andimarek · 2026-04-19T22:06:22Z

    @Override
-    public List<Object> getPath() {
+    public @Nullable List<Object> getPath() {
        return path;


same hear ... about not return null for lists.

claude and others added 30 commits February 28, 2026 23:21

Add JSpecify agent team orchestrator prompt

7a3d03f

https://claude.ai/code/session_01FG3V2VvjSSbUVcjn9FX4yx

Move to .claude folder

b30585a

Add JSpecify annotations to QueryComplexityCalculator

18cd241

Add JSpecify annotations to AbortExecutionException

fc88122

Add JSpecify annotations to ConditionalNodeDecision

d71b245

Add JSpecify annotations to AsyncExecutionStrategy

f79e9ea

Add JSpecify annotations to QueryComplexityInfo

3194a43

Add JSpecify annotations to AsyncSerialExecutionStrategy

119d6dd

Add JSpecify annotations to QueryDepthInfo

ce144ba

Add JSpecify annotations to CoercedVariables

7765059

Add JSpecify annotations to QueryReducer

7c8569a

Add JSpecify annotations to QueryAppliedDirective

6801331

Add JSpecify annotations to DataFetcherExceptionHandlerParameters

6035167

Add JSpecify annotations to DataFetcherExceptionHandlerResult

dfae148

Add JSpecify annotations to QueryTransformer

460ee6b

Add JSpecify annotations to DefaultValueUnboxer

4427846

Add JSpecify annotations to QueryAppliedDirectiveArgument

32ecef7

Add JSpecify annotations to QueryTraversalOptions

f223706

Add JSpecify annotations to QueryVisitor

bfe03d0

Add JSpecify annotations to ExecutionContext

8b7d7a2

Add JSpecify annotations to ExecutionId

1d3a502

Add JSpecify annotations to QueryDirectives

ed60978

Add JSpecify annotations to QueryVisitorFieldArgumentEnvironment

24aa942

Add JSpecify annotations to QueryVisitorFieldArgumentInputValue

d92048b

Add JSpecify annotations to FieldValidationInstrumentation

1ebd551

Add JSpecify annotations to QueryVisitorFieldArgumentValueEnvironment

c3bda94

Add JSpecify annotations to SimpleFieldValidation

560b28d

Add JSpecify annotations to InstrumentationCreateStateParameters

b540dbb

Add JSpecify annotations to ExecutionStepInfo

e54951c

Add JSpecify annotations to QueryVisitorFieldEnvironment

f9c46df

dondonz commented Mar 1, 2026

View reviewed changes

dondonz and others added 2 commits March 1, 2026 16:18

Clarify jspecify-annotate prompt: public API only, fix exemption cleanup

bb25aed

- Explicitly state that @internal classes must not be annotated - Tighten exemption list cleanup instruction to remove only the annotated class Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Merge branch 'master' into claude/agent-team-jspecify-Frd74

0a77532

bbakerman reviewed Mar 7, 2026

View reviewed changes

bbakerman approved these changes Mar 7, 2026

View reviewed changes

Merge branch 'master' into claude/agent-team-jspecify-Frd74

d018b2d

dondonz added this to the 26.x is a breaking change release (other fixes as well) milestone Mar 21, 2026

Address comments

fbe0cac

dondonz added the breaking change requires a new major version to be relased label Mar 22, 2026

dondonz added 2 commits March 22, 2026 17:53

Merge branch 'master' into claude/agent-team-jspecify-Frd74

9dfc436

# Conflicts: # .claude/commands/jspecify-annotate.md # src/main/java/graphql/execution/ExecutionContext.java # src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy

Adjust for new operation directives

0c5cf94

github-actions Bot mentioned this pull request Mar 22, 2026

CI Failure DoctorCI Failure - Run #23398087408 - JSpecifyAnnotationsCheck exemption list conflict #4351

Closed

1 task

dondonz mentioned this pull request Mar 24, 2026

Fix build.gradle variable name breaking CI #4352

Closed

1 task

dondonz and others added 2 commits March 24, 2026 19:59

Merge branch 'master' into claude/agent-team-jspecify-Frd74

cb36b21

Fix NullAway error: assert document non-null at assignment in GraphQL…

02401d5

….parseAndValidate Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

dondonz marked this pull request as ready for review March 24, 2026 09:18

dondonz commented Mar 31, 2026

View reviewed changes

dondonz commented Apr 4, 2026

View reviewed changes

andimarek approved these changes Apr 19, 2026

View reviewed changes

Merge branch 'master' into claude/agent-team-jspecify-Frd74

9735fa1

andimarek enabled auto-merge (squash) April 23, 2026 00:25

andimarek merged commit a6cc631 into master Apr 23, 2026
11 checks passed

Conversation

dondonz commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

66 classes annotated

graphql.analysis (16 classes)

graphql.execution core (26 classes)

graphql.execution sub-packages (24 classes)

Review passes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbakerman left a comment

Choose a reason for hiding this comment

Uh oh!

dondonz commented Mar 7, 2026

Uh oh!

github-actions Bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Report

Test Results

Code Coverage (Java 25)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andimarek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dondonz commented Mar 1, 2026 •

edited

Loading

github-actions Bot commented Mar 1, 2026 •

edited

Loading

github-actions Bot commented Mar 7, 2026 •

edited

Loading