Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #377 +/- ##
=======================================
Coverage 92.13% 92.13%
=======================================
Files 390 390
Lines 24644 24644
=======================================
Hits 22705 22705
Misses 1939 1939 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Switch from the code-review plugin to the built-in skill at high effort, running on Opus 5. The plugin dropped any finding scoring under 80 on its confidence rubric, which filtered out design and public API issues that don't change current behavior. Install the project dependencies and fetch full history so the reviewer can reproduce a suspected bug instead of only reasoning about it. Add a CLAUDE.md covering what the repo doesn't already record: the C# port relationship, the comment and breaking change conventions, and how to use the environment during review. Also drop EFLOMAL_PATH from the CI build. Nothing has read it since the switch to the native eflomal implementation in #336. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ddaspit
force-pushed
the
improve-claude-code-review
branch
from
September 22, 2026 17:24
da478eb to
0cdd0d0
Compare
The allowlist named `python`, which on the runner is the setup-python interpreter without the project dependencies, not the Poetry venv. Allow `poetry run` and the venv interpreter instead. Also allow the git history commands. The full checkout added in the previous commit was pointless without them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ddaspit
marked this pull request as draft
September 22, 2026 17:35
…rom machine PR #514 Adds AGENTS.md, five Claude skills, path-scoped review rules under docs/review/, issue and pull request templates, and a comment-hygiene gate over the lines a branch adds. The gate is rewritten in Python rather than vendoring the PowerShell module. On this repo's current tree both find the same 16 violations, but the PowerShell rules cannot see Python docstrings, which the code-comments skill holds to the same content rules, and this repo needs no pwsh otherwise. The review rules are rebuilt from this repository's own history rather than carrying the C# commit citations, which do not resolve here. Marker and paragraph handling is the defect class that ships most often, then reference and versification arithmetic, then crashes in punctuation analysis. CLAUDE.md now imports AGENTS.md. The review action restores CLAUDE.md from main but not the file it imports, so the fork gate on the review workflow is what keeps that guidance trusted; the workflow comment says so. Closes #375 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A reply or a follow-up round can then refer to a finding without quoting it. The prefix is F1, F2, ... rather than #1, which GitHub links to issue 1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Drops what an agent can read from the tree: what local_check.sh runs, pyright's
mode, the testutils import path, the extras layout, the directory map in
CLAUDE.md, and restatements of AGENTS.md in the skills. pr-authoring now points
at the PR template instead of copying it.
Also drops claims that did not hold for this repo: a vocabulary list written by
analogy with machine ("segment" never appears in machine/tokenization), a rule
about platform-dependent test skips (the three skipped tests are manual-only),
an "await" in the jobs rules (the jobs are synchronous), and a Jira trap backed
by one commit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The built-in code-review skill still finds and verifies the findings, but now reports back instead of posting, and the pr-review skill sets what is posted: numbered findings, severity labels, and the summary comment. That summary needs gh pr comment, so it joins the allowlist. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
pr-review now gets its findings from the built-in code-review skill unless the caller already has them, so /pr-review works on its own, locally or in CI. The workflow prompt becomes a single /pr-review, and the two-step sequence lives in a skill the action restores from main rather than in the prompt. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Use the opus alias instead of pinning a model ID, so the review picks up new Opus releases without a workflow change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reviewable sets a discussion's disposition from a shorthand keyword at the start of a comment posted on GitHub, so the severity labels become its words: Major blocks, Minor stays open, FYI starts resolved. The summary starts with FYI so it never blocks on its own. A test on #377 confirmed the leading keyword works; a disposition on its own line does not, on a new comment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ddaspit
marked this pull request as ready for review
September 22, 2026 20:32
A top-level PR comment lands in Reviewable's main discussion, which has no dispositions and is always resolved, so the prefix did nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Findings are Critical, Important, or Low. Only a finding comment posted to the PR starts with the Reviewable keyword for its severity, now followed by a colon; the summary, replies, and unposted reviews use the severity names, because Minor reads as trivial and marks an Important finding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does two things: it fixes the automated Claude review, and it ports the agent guidance from sillsdev/machine#514.
Fixing the automated review
The automated review on #371 reported no issues, but a manual review found a few real ones. The workflow used the code-review plugin, which throws away any finding that scores under 80 on a confidence rubric, so design and public API issues got dropped. It also ran on Sonnet, had no Python environment, and checked out a single commit.
pr-reviewskill, which has the built-in code-review skill find and verify at high effort, then posts numbered findings. It runs on theopusalias, so it picks up new Opus releases automatically.poetry.lock.Porting machine#514
AGENTS.mdholds the guidance, andCLAUDE.mdjust imports it, matching machine. It only covers things the repo doesn't already say: where the tree misleads, the defect classes that keep shipping, and the C# port relationship.docs/review/, rebuilt from this repo's own history rather than copied, since machine's commit citations don't exist here.hermitcrab.mdis dropped (no Python counterpart) andjobs.mdis new../local_check.sh --agent-strict.Worth knowing
CLAUDE.mdfrom main but not theAGENTS.mdit imports, so a PR could change what the reviewer is told. That's accepted for now because only branches from this repo are reviewed. The workflow comment says so.EFLOMAL_PATHfrom the CI build. Nothing has read it since Use new machine eflomal implementation #336. The dockerfiles still set it and are left alone.Validation: 847 tests passed (3 skipped), flake8 and pyright clean, black unchanged, comment-hygiene self-test passed and the branch scan is clean.
Closes #375
🤖 Generated with Claude Code
This change is