Skip to content

Java: Generate typed catalogue candidates - #2532

Merged
SteveSandersonMS merged 3 commits into
mainfrom
gokhanarkan/fix-java-catalogue-unions
Sep 4, 2026
Merged

SteveSandersonMS merged 3 commits into
mainfrom
gokhanarkan/fix-java-catalogue-unions

Conversation

@gokhanarkan

Copy link
Copy Markdown
Member

Summary

  • generate typed Java catalogue candidates instead of List<Object>
  • keep non-catalogue and non-Java generated output unchanged

Validation

  • npm --prefix java/scripts/codegen test
  • repeated Java codegen with zero non-Catalog generated delta
  • cd java && mvn verify

Related to #2106

@gokhanarkan
gokhanarkan requested a review from a team as a code owner September 4, 2026 14:07
Copilot AI balanced review requested due to automatic review settings September 4, 2026 14:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Generated polymorphic candidates can serialize duplicate discriminator properties.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 High severity

New issues introduced by this change (1)
Severity Finding
High severity java/​scripts/​codegen/​java.ts — Returning the promoted union as a polymorphic type also changes serialization:…
What changed in this PR

Generates typed Java catalogue candidate hierarchies from nested discriminated unions.

Changes:

  • Promotes nested catalogue unions into generated Java types.
  • Adds candidate/source classes and typed candidate lists.
  • Adds generator and Jackson coverage.
File Description
CatalogCandidateJacksonTest.java Tests candidate deserialization and round-tripping.
CatalogSearchSucceeded.java Uses typed candidates.
CatalogMcpServerInstallability.java Adds installability enum.
CatalogMcpServerCandidateProvenance.java Adds MCP provenance type.
CatalogMcpServerCandidate.java Adds MCP candidate variant.
CatalogCandidateSourceUrl.java Adds URL source variant.
CatalogCandidateSourceEmbedded.java Adds embedded source variant.
CatalogCandidateSource.java Adds source union base.
CatalogCandidate.java Adds candidate union base.
CatalogAiSkillCandidateProvenance.java Adds skill provenance type.
CatalogAiSkillCandidate.java Adds AI skill candidate variant.
java/​scripts/​codegen/​package.json Adds generator test command.
java/​scripts/​codegen/​java.ts Implements nested-union promotion.
java/​scripts/​codegen/​java.test.ts Tests promoted type resolution.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread java/scripts/codegen/java.ts
@github-actions

This comment has been minimized.

@gokhanarkan

Copy link
Copy Markdown
Member Author

@SteveSandersonMS CI is green. This is the agreed Java-only scope: typed catalogue candidate and source generation, with non-Catalogue and non-Java output unchanged. Would you mind taking a look?

@github-actions

This comment has been minimized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@SteveSandersonMS SteveSandersonMS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I verified this is a minimal Java-only codegen parity fix: other SDK RPC output is unchanged, non-catalog Java output regenerates byte-identically, and the catalog unions now deserialize and serialize with the existing discriminators exactly once. I also resolved the package.json conflict against main while retaining both schema-fetch and codegen tests.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review — PR #2532

Scope: Java-only change (codegen script + generated types), fixing CatalogSearchSucceeded.candidates which was previously untyped (List<Object>) in Java while all other SDKs already used the properly-typed candidate union:

  • TypeScript: candidates: CatalogCandidate[]
  • Python: candidates: list[CatalogCandidate]
  • Go: Candidates []CatalogCandidate
  • Rust: pub candidates: Vec<CatalogCandidate>
  • .NET: uses the analogous typed generated model

The root cause was the Java codegen's handling of nested discriminated unions inside array items — it wasn't promoting CatalogCandidate (and its Catalog*Candidate/Catalog*Provenance/Catalog*Source* variants) to named types. This PR adds collectNestedDiscriminatedUnionTypeNames to java/scripts/codegen/java.ts, regenerates the new Java model classes, and adds targeted unit/Jackson serialization tests.

Assessment: ✅ No consistency issue — this brings Java into parity with the other five SDKs rather than creating a gap. It's a bugfix confined to the Java codegen pipeline and generated output; no behavior or API changed in TS/Python/Go/.NET/Rust, so no cross-language follow-up is needed.

Generated by SDK Consistency Review Agent for #2532 · copilot · sonnet50 · 28.6 AIC · ⌖ 11.9 AIC · ⊞ 9.7K ·

@gokhanarkan
gokhanarkan added this pull request to the merge queue Sep 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 4, 2026
@SteveSandersonMS
SteveSandersonMS added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 1638dca Sep 4, 2026
46 checks passed
@SteveSandersonMS
SteveSandersonMS deleted the gokhanarkan/fix-java-catalogue-unions branch September 4, 2026 15:43
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.

3 participants