Skip to content

[skills] Strengthen Java name conversion reviews - #12200

Merged
jonathanpeppers merged 2 commits into
mainfrom
jonathanpeppers-strengthen-java-name-review
Jul 21, 2026
Merged

[skills] Strengthen Java name conversion reviews#12200
jonathanpeppers merged 2 commits into
mainfrom
jonathanpeppers-strengthen-java-name-review

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Description

PR #12187 exposed a reviewer blind spot: replacing an inline JNI-name conversion with a source-name helper silently changed nested Java binary names. The reviewer guidance did not clearly distinguish the representations or require tests at the consuming boundary.

Teach android-reviewer to load interop guidance for trimmable typemap changes, distinguish JNI internal, Java binary, Java source, and managed nested names, and require nested-name assertions on final manifest, map, rule, or generated-source output.

Evaluation

A blinded replay of the original regression commit using the same model and review prompt detected the bug in 9/10 trials with this concise guidance, compared with 7/10 using the original skill. Blocking findings improved from 7/10 to 8/10. The sample is directional rather than statistically conclusive.


Pull Request
title and
description
should follow the
commit-messages.md workflow documentation, and in particular should include:

  • Useful description of why the change is necessary.
  • Links to issues fixed
  • Unit tests

Distinguish JNI, Java binary, Java source, and managed nested type names during review. Load interop guidance for trimmable typemap changes and require nested-name assertions at serialized output boundaries.

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

Copilot-Session: 1a389fd8-0521-4de1-b2d4-ba67eff72ff8
Copilot AI review requested due to automatic review settings July 21, 2026 19:31

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.

Pull request overview

Updates the android-reviewer skill guidance to better catch regressions around Java/JNI name conversions—especially nested types—by clarifying which name representation is required at each consumer boundary and by strengthening test expectations.

Changes:

  • Broadened when interop-rules.md should be loaded (now includes TrimmableTypeMap/name-conversion scenarios).
  • Added explicit guidance distinguishing JNI internal, Java binary, Java source, and managed nested type names.
  • Added testing guidance requiring $ nested-type assertions at the final serialized output boundary (manifest/map/rules/generated source).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
.github/skills/android-reviewer/SKILL.md Updates conditional rule-loading guidance to include Java/JNI name-conversion scenarios.
.github/skills/android-reviewer/references/testing-rules.md Adds a rule requiring nested-name ($) assertions at consumer boundaries.
.github/skills/android-reviewer/references/interop-rules.md Adds a Java type-name representation section to reduce review blind spots for nested types.

Comment thread .github/skills/android-reviewer/SKILL.md
Comment thread .github/skills/android-reviewer/references/interop-rules.md
Describe rule selection as based on changed files and diff content, and update the interop reference introduction for Java/JNI name handling.

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

Copilot-Session: 1a389fd8-0521-4de1-b2d4-ba67eff72ff8
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 21, 2026
@jonathanpeppers
jonathanpeppers merged commit 054ae69 into main Jul 21, 2026
26 of 44 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-strengthen-java-name-review branch July 21, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants