Skip to content

Add a copilot-style adversarial review skill for pre-merge checks - #267

Open
rubo77 wants to merge 1 commit into
parawanderer:mainfrom
rubo77:feature/copilot-style-review-skill
Open

rubo77 wants to merge 1 commit into
parawanderer:mainfrom
rubo77:feature/copilot-style-review-skill

Conversation

@rubo77

@rubo77 rubo77 commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Copilot's PR reviews here keep finding bugs that compile and pass
tests - filter/parser disagreements, lifecycle leaks, silent empty
states, contrast failures. This codifies that review as an agent skill
under .devin/skills/:

  • SKILL.md - the generic procedure: mechanism tracing over a
    failure-class checklist, Copilot's dense finding format, verify
    before reporting
  • repo-notes-opentagviewer.md - this repo's conventions (stacked
    branches, 10 locales via add_strings, Java-only, the import checker)
    and the concrete findings Copilot has already made here, kept as the
    bar a review has to meet

The two-file split keeps the method portable; the repo notes load only
when the working tree is this repository.

Test plan

  • Document-only change; no code touched

Generated with Devin

Copilot's PR reviews keep finding bugs that compile and pass tests:
filter/parser disagreements, lifecycle leaks, silent empty states,
contrast failures. This skill codifies that review as a procedure -
mechanism tracing over a checklist of failure classes, findings only
when they can be written as a concrete trigger sequence, reported in
Copilot's own format, and fixed in the commit that introduced them.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 20:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The documented workflow skips instrumented tests and prescribes an add-only command for updating existing translations.

4 open findings
What changed in this PR

Adds a reusable pre-merge review skill, separating the review procedure from OpenTagViewer-specific guidance.

Changes:

  • Defines mechanism tracing, failure checks, and finding verification.
  • Records repository conventions and past review examples.
File Description
.devin/​skills/​copilot-style-review/​SKILL.md Defines the review procedure and reporting format.
.devin/​skills/​copilot-style-review/​repo-notes-opentagviewer.md Adds repository-specific workflows and review precedents.

🧠 Review effort: Balanced


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

Comment on lines +16 to +17
- **10 locales, machine-maintained**: new/changed UI strings go through
`python3 scripts/add_strings.py <json>`; verify with `--check` (all locales covered).
- **10 locales, machine-maintained**: new/changed UI strings go through
`python3 scripts/add_strings.py <json>`; verify with `--check` (all locales covered).
Never pass translations through shell quoting - mojibake history.
- **Verification**: `bash gradlew testAll` (JVM + pytest), `bash gradlew assembleDebug`,
Comment on lines +20 to +21
`python3 scripts/check_java_imports.py` (fully-qualified names in imports are
*blocked* upstream - `pkg.Foo` in code must become a real import).
Comment on lines +33 to +34
- **PR #264**: an empty state reporting "no permission" when the real state was "none
found" - honest-state finds count.

This branch has not been deployed

No deployments
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