Skip to content

Add JSpecify annotations to 10 classes in graphql.language package and improve annotation prompt#4216

Draft
Copilot wants to merge 78 commits intomasterfrom
copilot/add-jspecify-annotations-to-classes
Draft

Add JSpecify annotations to 10 classes in graphql.language package and improve annotation prompt#4216
Copilot wants to merge 78 commits intomasterfrom
copilot/add-jspecify-annotations-to-classes

Conversation

Copy link
Contributor

Copilot AI commented Jan 24, 2026

Annotates 10 foundational classes in graphql.language with JSpecify nullability markers, continuing the repository-wide nullability annotation effort. Also updates the JSpecify annotation prompt with effectiveness improvements for future annotation work.

Changes

Interfaces (4): Node, Definition, Type, NodeVisitor

  • Added @NullMarked at class level

Simple classes (4): IgnoredChar, IgnoredChars, NodeVisitorStub, Comment

  • Added @NullMarked at class level
  • Marked Comment.sourceLocation as @Nullable (parser may create comments without location)

Complex classes (2): SourceLocation, NodeChildrenContainer

  • Added @NullMarked at class level
  • Marked SourceLocation.sourceName and getLocation() return as @Nullable
  • Marked NodeChildrenContainer.getChildOrNull() return as @Nullable
  • Used @NullUnmarked for NodeChildrenContainer.Builder per JSpecify builder pattern

Exemption list: Removed all 10 classes from JSpecifyAnnotationsCheck.groovy

Prompt improvements: Enhanced .claude/commands/jspecify-annotate.md with:

  • Batch size guidance (10 classes per batch for optimal context management)
  • Class prioritization strategy (start with interfaces/simple classes first)
  • Exploration phase guidance (use grep to understand null usage patterns)
  • Concrete pattern examples (interface, nullable field, nullable return type, builder)
  • Incremental validation strategy (compile after every 3-5 classes)

All changes validated with NullAway static analysis via ./gradlew compileJava.

Original prompt

Task

Add JSpecify annotations to approximately 10 classes in the graphql.language package, following the established JSpecify annotation pattern used in this repository.

Classes to annotate (first batch of ~10)

From the graphql.language package, annotate these classes:

  1. Node
  2. Definition
  3. Type
  4. Comment
  5. SourceLocation
  6. IgnoredChar
  7. IgnoredChars
  8. NodeChildrenContainer
  9. NodeVisitor
  10. NodeVisitorStub

Instructions

Follow the JSpecify annotation process documented in .claude/commands/jspecify-annotate.md:

  1. Set the class to be @NullMarked at the class level
  2. Remove all redundant @NonNull annotations that IntelliJ may have added
  3. Check Javadoc for @param tags mentioning "null", "nullable", "may be null"
  4. Check Javadoc @return tags mentioning "null", "optional", "if available"
  5. Inspect method implementations that return null or check for null
  6. Consider GraphQL specification details when determining nullability (see https://spec.graphql.org/draft/)
  7. For Builder static classes, label them @NullUnmarked and no further annotations needed for that static class

Validation

After making changes, run the NullAway compile check:

./gradlew compileJava

If you find NullAway errors, make the smallest possible change to fix them. You can use assertNotNull with a message if needed.

Cleanup

  1. Remove each completed class from the exemption list in src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy
  2. Delete all unused imports from the classes you've annotated
  3. Do not make spacing or formatting changes - only make minimal changes necessary for the annotations

Important Notes

  • JSpecify is already imported in this repository (org.jspecify.annotations.NullMarked, org.jspecify.annotations.Nullable, org.jspecify.annotations.NullUnmarked)
  • Use @Nullable for fields, parameters, and return types that can be null
  • The base branch is infer-nullity-3 which already has some JSpecify work done
  • Look at existing annotated classes like IntValue.java, StringValue.java, AbstractNode.java in the same package for reference patterns

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add JSpecify annotations to classes in graphql.language package Add JSpecify annotations to 10 classes in graphql.language package Jan 24, 2026
Copilot AI requested a review from dondonz January 24, 2026 20:44
@dondonz
Copy link
Member

dondonz commented Jan 24, 2026

This is from me, I'm experimenting with Copilot agents running in parallel

@dondonz dondonz marked this pull request as ready for review January 24, 2026 20:52
@dondonz dondonz marked this pull request as draft January 24, 2026 20:52
@dondonz
Copy link
Member

dondonz commented Jan 24, 2026

builds didn't run because I am targeting an inprogress JSpecify branch

@dondonz
Copy link
Member

dondonz commented Jan 24, 2026

I will later change this branch to target master for merging. The other JSpecify PR should be merged in first

I ran the NullAway checks, and the full test suite locally, and it's passed.

Very impressed with Copilot agents for parallelising this work. This was made with Claude Sonnet 4.5

Other learnings - I tried 40-50 classes at a time with Opus 4.5 via Antigravity. It couldn't cope with too much context. Much better results with doing 10 at a time, as done here

@dondonz
Copy link
Member

dondonz commented Jan 24, 2026

@copilot Excellent work. Coming back to the original JSpecify prompt, what would you change to make the next agent run even more effective?

Copy link
Contributor Author

Copilot AI commented Jan 24, 2026

@copilot Excellent work. Coming back to the original JSpecify prompt, what would you change to make the next agent run even more effective?

Based on this run, here are suggestions to make the prompt even more effective:

Add to the prompt:

  1. Explicit class prioritization: "Start with interface/simple classes first, then tackle complex classes with builders. This helps identify patterns early."

  2. Exploration phase guidance: "Before annotating, use grep to search for how each class is instantiated (e.g., grep -r "new Comment") to understand which parameters can be null."

  3. Pattern examples: Add 2-3 concrete before/after examples showing:

    • Interface with just @NullMarked
    • Class with nullable field
    • Class with nullable return type
    • Builder with @NullUnmarked
  4. Validation strategy: "Run ./gradlew compileJava after every 3-5 classes annotated, not just at the end. This catches issues early and makes debugging easier."

  5. Batch size confirmation: "You mentioned 10 classes works well - add this explicitly: 'Annotate approximately 10 classes per batch for optimal context management.'"

Already working well:

  • The builder pattern guidance (@NullUnmarked)
  • GraphQL spec compliance section
  • Minimal changes emphasis
  • Exemption list cleanup reminder

The current prompt is solid - these additions would just reduce exploration time and catch issues faster.

@dondonz
Copy link
Member

dondonz commented Jan 24, 2026

@copilot Make these changes to the prompt in this pull request

Copy link
Contributor Author

Copilot AI commented Jan 24, 2026

@copilot Make these changes to the prompt in this pull request

Updated the prompt in .claude/commands/jspecify-annotate.md with the suggested improvements (commit 3db2dde):

Added sections:

  • Batch Size and Prioritization: 10 classes per batch, prioritize simple classes first
  • Exploration Phase: Use grep to understand null usage patterns
  • Pattern Examples: 4 concrete before/after examples (interface, nullable field, nullable return, builder)
  • Validation Strategy: Compile after every 3-5 classes for early error detection

The prompt now provides clearer guidance on workflow and includes practical examples to reduce exploration time.

Copilot AI changed the title Add JSpecify annotations to 10 classes in graphql.language package Add JSpecify annotations to 10 classes in graphql.language package and improve annotation prompt Jan 24, 2026
@dondonz dondonz changed the base branch from infer-nullity-3 to master February 8, 2026 07:17
@dondonz dondonz changed the base branch from master to infer-nullity-3 February 8, 2026 21:37
def sanitizedBranchName = branchName.replace('/', '-')

return makeDevelopmentVersion(["0.0.0", branchName, "SNAPSHOT"])
return makeDevelopmentVersion(["0.0.0", sanitizedBranchName, "SNAPSHOT"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copliot PRs always have a / in the name, which makes it appear like the jar is in another directory when developing locally. This sanitises the name

@dondonz dondonz changed the base branch from infer-nullity-3 to master February 8, 2026 21:59
@dondonz
Copy link
Member

dondonz commented Feb 8, 2026

This diff looks unusual because it depends on the first large jspecify pr to go in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants