Refine kotlin-engineer and related skills - #3
Conversation
Also: * Rename `kotlin-review` skill to `spine-code-review`.
There was a problem hiding this comment.
Pull request overview
This PR ports the skill/agent refinements from SpineEventEngine/config#676 into the agents repository, primarily splitting Kotlin general standards (kotlin-engineer) from repo-specific review policy (spine-code-review) and updating all cross-references accordingly.
Changes:
- Rename and reshape the repo-specific reviewer skill as
spine-code-review, explicitly deferring general Kotlin standards tokotlin-engineer. - Expand/refine
kotlin-engineer(usage guidance, faster workflow, and reformatted reference/anti-pattern tables). - Update orchestrators and docs (
pre-pr, TOC, Claude agents/commands, and related skills) to reference the new skill split.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/spine-code-review/SKILL.md | Renames/redefines repo-specific reviewer and clarifies responsibility split vs kotlin-engineer. |
| skills/spine-code-review/agents/openai.yaml | Adds OpenAI interface metadata for the new spine-code-review skill. |
| skills/kotlin-engineer/SKILL.md | Broadens the Kotlin baseline skill and adds “When to Use” + “Fast Path for Agents”. |
| skills/kotlin-engineer/references/coroutines.md | Reformats/updates the coroutine anti-pattern reference table. |
| skills/kotlin-engineer/agents/openai.yaml | Updates OpenAI interface metadata to match the new Kotlin skill framing. |
| skills/pre-pr/SKILL.md | Dispatches kotlin-engineer + spine-code-review instead of kotlin-review and updates aggregation labels. |
| skills/raise-coverage/SKILL.md | Adds explicit dependency on kotlin-engineer as the baseline for tests. |
| skills/java-to-kotlin/SKILL.md | Anchors conversions to kotlin-engineer as the Kotlin baseline. |
| skills/gradle-review/SKILL.md | Updates “do not duplicate” and output-format references to the new reviewer names. |
| guidelines/_TOC.md | Updates the index to include kotlin-engineer and spine-code-review. |
| claude/agents/spine-code-review.md | Adds Claude agent wrapper for spine-code-review. |
| claude/agents/kotlin-engineer.md | Adds Claude agent wrapper for kotlin-engineer. |
| claude/agents/review-docs.md | Points doc reviewers to spine-code-review instead of the removed Kotlin reviewer. |
| claude/commands/pre-pr.md | Updates reviewer dispatch instructions to use kotlin-engineer + spine-code-review. |
| claude/commands/review-docs.md | Updates fallback pointer from kotlin-review to spine-code-review. |
| skills/kotlin-review/agents/openai.yaml | Removes obsolete OpenAI interface metadata for the retired kotlin-review. |
| claude/agents/kotlin-review.md | Removes obsolete Claude agent wrapper for the retired kotlin-review. |
Comments suppressed due to low confidence (1)
skills/spine-code-review/SKILL.md:6
- This skill states it enforces the "AGENTS.md code-review filter" / "AGENTS.md § Code review" filter, but this repository's
AGENTS.mddoes not contain a "Code review" section. As written, the procedure points reviewers at a non-existent source of truth; either add the missing section toAGENTS.mdor make the skill treat the filter as optional when the section is absent (while still supporting consumer repos that do have it).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove stale `Team memory` and `Task plans` TOC entries that pointed to `.agents/memory/MEMORY.md` and `.agents/tasks/README.md`, which do not exist in this repo (it uses the built-in auto-memory dir). - Rename `kotlin-engineer` "Setup Check (run first)" to "Setup Check" and align its intro with the Fast Path guidance to run it only for non-trivial changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- TOC skill links used `.agents/skills/...`, which Markdown resolves relative to `guidelines/` (→ `guidelines/.agents/...`, broken). Switch to `../skills/...`, which resolves correctly from both `guidelines/` and the mirrored `.agents/guidelines/` mount. - Stop tracking `.claude/scheduled_tasks.lock` (a machine-local runtime lock) and add it to `.gitignore` alongside `.claude/worktrees/`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
skills/spine-code-review/SKILL.md:16
- The intro paragraph says this reviewer covers “both Kotlin and Java changes”, but the skill description explicitly includes build changes and the procedure includes build-logic checks (e.g., “No mixing Groovy/Kotlin DSL in build logic”). Align this paragraph with the stated scope so users don’t miss build-related review expectations.
Align the OpenAI skill metadata and the Claude agent description with the `spine-code-review` skill's own scope, which already lists "Kotlin, Java, and build changes". Ensures build-only diffs route to this reviewer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
skills/spine-code-review/SKILL.md:8
- The skill description calls out an “AGENTS.md code-review filter”, but this repo’s
AGENTS.mddoesn’t define any "Code review" section or filter; the filter rules are actually spelled out later in this skill. Consider rewording the description to avoid pointing readers at a non-existent reference (or, alternatively, add the missing section toAGENTS.md, but that’s outside this PR’s changed files).
`spine-code-review`'s scope now includes build changes, but pre-pr only dispatched it under the **code** classifier (.kt/.kts/.java). Add a **build** classifier for non-`.kts` build files (`*.gradle`, `settings.gradle`, `gradle.properties`, `*.versions.toml`) and dispatch `spine-code-review` for it so repo-specific safety and version-gate rules cover build-only diffs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![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: f7645afe21
ℹ️ 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".
…e-pr - kotlin-engineer is dispatched as a pre-PR reviewer, but its review output defined no verdict, so a clean Kotlin run had nothing for pre-pr's PASS aggregation to read. Add the same one-line verdict contract (APPROVE / APPROVE WITH CHANGES / REQUEST CHANGES) used by the other reviewers. - The Claude /pre-pr wrapper only dispatched spine-code-review for .kt/.kts/.java; align it with the skill's new build classifier so build-only diffs (*.gradle, settings.gradle, gradle.properties, *.versions.toml) also route to spine-code-review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review — both review comments are addressed in b12ff42:
Please re-review. |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
kotlin-engineer is Kotlin-only, but the reviewer-dispatch wording read as if it ran for any **code** change (which includes .java). Reword both the pre-pr skill and the Claude /pre-pr wrapper: dispatch by file type — spine-code-review for .kt/.kts/.java, kotlin-engineer only for .kt/.kts — so a Java-only diff routes to spine-code-review alone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
skills/spine-code-review/SKILL.md:16
- The skill’s intro says it covers Kotlin/Java only, but this skill is explicitly scoped to build changes as well (front-matter description and procedure mention build/version-gate). Update this sentence so the scope is consistent within the document.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efc90b7cca
ℹ️ 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".
- pre-pr Step 3: run `./gradlew build` for **build**-only changes too, so build-file edits aren't recorded as skipped while still PASSing (Codex P2). - pre-pr Step 4: add a de-duplication note — a diff matching both **code** and **build** must dispatch `spine-code-review` only once. - spine-code-review agent: "Stay in scope: code and build files" instead of "code only", matching the skill's Kotlin/Java/build scope. - kotlin-engineer TOC label: "Kotlin, Java, and build review". - kotlin-engineer agent: it is a read-only review agent (tools: Read/Grep/Glob/ Bash); drop the "refactoring"/"read-only in review mode" wording and point refactoring use at the skill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review — addressed your P2 in 9200f4e: pre-pr Step 3 now runs |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Applies the skill changes from config#676 to this
agentsrepository, which is the new home for shared agent assets after config#677 moved skill delivery to a floating submodule.Paths were mapped from config's old
.agents/skills/…,.claude/agents/…, and.claude/commands/…layout onto this repo's real source tree (skills/,claude/agents/,claude/commands/,guidelines/_TOC.md), preserving this repo's grouped.agents/guidelines/…reference style.Changes
kotlin-review→spine-code-review— now a repo-specific reviewer (AGENTS.md filter, safety rules, testing policy, version gate) that defers general Kotlin language/API/coroutine/Flow/null-safety standards tokotlin-engineer.kotlin-engineer— broadened description, added "When to Use" and "Fast Path for Agents" sections, trimmed the Setup Check, and reformatted the reference and anti-pattern tables.kotlin-engineeras the Kotlin baseline:gradle-review,java-to-kotlin,raise-coverage,pre-pr(now dispatches both reviewers), andguidelines/_TOC.md.kotlin-engineer.mdandspine-code-review.md, removedkotlin-review.md, and updatedreview-docs+pre-prreferences.Deliberately excluded
config-specific files from #676 that belong to the #677 delivery mechanism and have no counterpart here:
migrate,buildSrc/uber-jar-module.gradle.kts,.github/copilot-instructions.md, config'sREADME.md/AGENTS.mdedits, and.agents/tasks/enforce-max-line-length.md(notasks/directory in this repo).🤖 Generated with Claude Code