Promote core-jvm proto-stub memory into shared skills#17
Conversation
Encode two test-code conventions captured in core-jvm's agents memory
into the org-wide skill definitions, so they ship to every Spine repo:
- kotlin-jvm-tester: proto stubs use the Kotlin Protobuf DSL only for
complete `.build()` messages; a `buildPartial()` stub is a plain Java
builder chain (never `.apply { }`), and a function returning a
`buildPartial()` message annotates its return type `@NonValidated`.
- spine-code-review: refine the Protobuf-DSL checklist item so a
reviewer does not flag a `buildPartial()` test chain and does flag a
missing `@NonValidated` annotation.
https://claude.ai/code/session_013wn51MjLUzds4YS2wp3GsV
There was a problem hiding this comment.
Pull request overview
This PR promotes two proto-stub testing conventions (previously captured as core-jvm repo-local agent memory) into the shared skill documentation so they apply consistently across Spine repositories.
Changes:
- Refines
spine-code-reviewguidance so reviewers prefer the Kotlin Protobuf DSL only for fully-built (.build()) messages and explicitly allowbuildPartial()stubs as Java builder chains. - Adds
kotlin-jvm-testerguidance covering (1) DSL vsbuildPartial()usage in tests and (2) requiring@NonValidatedon return types for helpers that returnbuildPartial()messages.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| skills/spine-code-review/SKILL.md | Updates the review checklist to avoid mis-flagging buildPartial() chains and to require @NonValidated on buildPartial() stub helpers. |
| skills/kotlin-jvm-tester/SKILL.md | Documents the authoritative conventions for building complete vs partial proto messages in tests, and when to annotate with @NonValidated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2af59fa8e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `@NonValidated` (`io.spine.validation.NonValidated`) — e.g. | ||
| `private fun stubEvent(): @NonValidated Event = Event.newBuilder().setId(id).buildPartial()`. | ||
| This holds for every message type. A `.build()` return needs no annotation, nor does a | ||
| function that only delegates to an already-annotated helper. |
There was a problem hiding this comment.
For a wrapper that only calls an annotated partial-stub helper but declares its own return type, this exception lets the wrapper expose a plain Event/message return even though the value is still partial; callers and reviewers then lose the @NonValidated contract at the new API boundary. Please require delegating functions to keep @NonValidated on their own return type unless they validate before returning.
Useful? React with 👍 / 👎.
| **Proto stubs — DSL vs. `buildPartial()`.** Build a *complete* message with the | ||
| Kotlin Protobuf DSL (`message { field = value }`) — the default for anything fully |
There was a problem hiding this comment.
When an XJavaSpec needs a complete proto fixture, this skill still governs that Java test, but the new absolute instruction to use the Kotlin Protobuf DSL cannot be followed in src/test/java. Please scope this rule to Kotlin specs and state that Java bridge suites should keep using the Java builder API, otherwise agents may generate non-compiling Java tests.
Useful? React with 👍 / 👎.
What
Applies two test-code conventions captured in
core-jvm's agents memory(
.agents/memory/) to the shared skill definitions so they ship to everySpine repository on the next pull, instead of living only as repo-local memory.
Source memory entries:
.build();plain Java chains for
buildPartial(); never.apply { }on a proto builder.@NonValidatedonbuildPartial()stubs — annotate the return type of anyfunction returning a
buildPartial()proto with@NonValidated.Changes
kotlin-jvm-tester/SKILL.md(the test-writing authority) — new "Protostubs" guidance next to the existing "Proto assertions" note:
message { … }) for complete (.build()) messages;buildPartial()stub is a plain Java builder chain, never.apply { };buildPartial()message annotates its return type@NonValidated(io.spine.validation.NonValidated);.build()returns andpure delegators do not.
spine-code-review/SKILL.md— refines the blanket "prefer Protobuf DSLover Java builders" checklist item so a reviewer does not flag a
buildPartial()test chain and does flag a missing@NonValidated,pointing to
kotlin-jvm-testerfor detail.Why here
Both lessons are about proto-message construction in test/stub code, so the
authoritative home is
kotlin-jvm-tester. The blanket DSL rule echoed inspine-code-reviewwould otherwise mis-flag a legitimatebuildPartial()chain,so it's refined in lockstep.
core-jvm's local memory is left untouched (sourceof the lesson); maintainers may prune it separately now that it's shared.
Validation
name;SKILL.mdfiles well under the sizecap (284 / 132 lines).
max-line-length..agents/skills/kotlin-jvm-tester/SKILL.mdreference resolves via thein-repo symlinks.
https://claude.ai/code/session_013wn51MjLUzds4YS2wp3GsV
Generated by Claude Code