which-fixer skill#19
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new which-fixer skill to standardize “restrictive which → that” fixes in source-code comments and repository documentation, with guidance for bulk vs incremental operation and an agent entrypoint configuration.
Changes:
- Introduces
skills/which-fixer/SKILL.mddefining scope, workflow, and reporting format for the fixer. - Adds
skills/which-fixer/agents/openai.yamlto register the skill with a default prompt and UI metadata.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| skills/which-fixer/SKILL.md | Documents the skill’s grammar rule, scanning constraints, and bulk/incremental workflow including completion recording. |
| skills/which-fixer/agents/openai.yaml | Registers the skill’s display name, short description, and default prompt for the OpenAI agent interface. |
💡 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: 00200e74b2
ℹ️ 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".
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fe189058a
ℹ️ 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".
armiol
left a comment
armiol
left a comment
There was a problem hiding this comment.
@alexander-yevsyukov LGTM in general. I can see a lot of comments from reviewers, and also this is still a draft, so just commenting for now.
Skip parenthetical/dash non-restrictive clauses and hyphenated identifiers; exclude code inside doc comments and indented/AsciiDoc blocks; broaden proto scope and restore .adoc; scan tracked files only (skip submodules); include working-tree edits and drop deleted paths from incremental diffs; soften the pre-pr wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| - `git diff --name-only --diff-filter=ACMR origin/master...HEAD` — committed | ||
| changes since the branch diverged from `origin/master` (three-dot, not a | ||
| plain tip-to-tip diff); and | ||
| - `git diff --name-only --diff-filter=ACMR HEAD` — staged and unstaged | ||
| working-tree edits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 201223e6c0
ℹ️ 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".
| - `git diff --name-only --diff-filter=ACMR HEAD` — staged and unstaged | ||
| working-tree edits. |
There was a problem hiding this comment.
When incremental mode is run before a newly created .md, .kt, or .proto file is staged, both commands in this union omit it: the three-dot diff only covers committed branch changes, and git diff --name-only ... HEAD reports tracked/staged changes but not untracked paths (I checked git status -h, where untracked files are surfaced separately via --untracked-files). Since this workflow explicitly targets agents working before an explicit commit, new documentation/comment files can be skipped entirely until the user stages them; add git ls-files --others --exclude-standard to the union or state that new files must be staged first.
Useful? React with 👍 / 👎.
| `{@code …}` / `{@literal …}` inline tags, `<pre>`/`<code>` HTML blocks, | ||
| `@sample` references, and Markdown backtick spans or fenced blocks inside | ||
| KDoc — so a documented snippet such as `which java` is never rewritten. |
There was a problem hiding this comment.
Fresh evidence after the earlier source-comment code-block fix: this exclusion covers inline tags, HTML blocks, @sample, and KDoc backticks/fences, but not Markdown indented code blocks. Because KDoc uses Markdown, a comment example written as a four-space- or tab-indented snippet inside /** ... */ can still be scanned and a command like which java can be rewritten to that java, corrupting the sample; add the same indented-block exclusion for KDoc comment text before replacements run.
Useful? React with 👍 / 👎.
| Skip every code form, not just the common ones: | ||
| - Markdown — fenced code blocks (triple-backtick and `~~~`), inline code | ||
| spans, and four-space-indented code blocks. | ||
| - AsciiDoc — delimited blocks: listing (`----`), literal (`....`), and | ||
| `[source]` blocks, plus inline code spans. |
There was a problem hiding this comment.
This Markdown/AsciiDoc exclusion list skips code forms but not link or image destinations, so a target such as [guide](which.md) or a reference URL containing /which can pass the later word checks and be rewritten to that, breaking the link even though the occurrence is not prose. Please exclude inline-link destinations, image destinations, and reference definitions from the scan while still allowing visible link text to be fixed.
Useful? React with 👍 / 👎.
|
|
||
| # Which/that fixer | ||
|
|
||
| Fix the common "which/that" confusion in English documentation and code comments. |
There was a problem hiding this comment.
When this skill runs on docs that discuss the word itself, this quoted "which/that" term is not a relative pronoun, but it still passes the later guards and can be rewritten to "that/that"; the same applies to wording like "which" clauses elsewhere in this file. Add a guard for quoted/metalinguistic mentions of the word, or require those mentions to be marked as code before scanning.
Useful? React with 👍 / 👎.
| - **Comma check (mandatory):** the word "which" is *not* immediately preceded | ||
| by a comma (allowing for optional whitespace between the comma and "which"). | ||
| If a comma precedes it, this is non-restrictive — leave it untouched. |
There was a problem hiding this comment.
In Markdown prose, formatting can sit between the comma and which, such as the file, **which** is generated, or a blockquote continuation can insert > before the next-line which; this check only allows whitespace between the comma and the word, so those non-restrictive clauses pass and get rewritten to that. Strip or ignore Markdown emphasis/quote markers when checking the preceding punctuation so valid non-restrictive clauses stay unchanged.
Useful? React with 👍 / 👎.
| Apply changes with the Edit tool, file by file. When a file has multiple | ||
| occurrences, batch them into one edit per file rather than one edit per | ||
| occurrence. |
There was a problem hiding this comment.
AGENTS.md requires shared skills to stay agent-neutral for Claude, Codex, and Junie, but this hard-codes an Edit tool workflow. In runtimes that do not expose a tool with that name, the skill can become ambiguous or fail even though a normal file edit would work; phrase this as editing the file with the current agent's available mechanism instead of naming one runtime's tool.
Useful? React with 👍 / 👎.
What
Adds a new shared skill,
which-fixer, that fixes the restrictive"which/that" grammar error in source-code comments and documentation. It
rewrites a restrictive "which" (no preceding comma) to "that" and leaves
non-restrictive ", which" clauses alone. The mistake recurs across the Spine
codebase because the Russian relative pronoun который covers both English
senses indiscriminately, so it warrants a dedicated, repeatable fixer rather
than ad-hoc cleanup.
How it works
.agents/memory/which-fixer-applied.mdmarker) it sweeps the whole codebase(tracked files only, so submodules and generated paths are skipped), then
records completion. Every later run reads that marker and switches to
incremental mode — only the files changed on the current branch, taking
the union of
git diff --diff-filter=ACMR origin/master...HEADand theworking-tree diff so uncommitted edits are covered too.
rather than printing a list of complaints.
.kt/.kts/.java/.protoit touches commenttext only — never code tokens, string literals, or code embedded in doc
comments (
{@code},<pre>,@sample, KDoc backticks); in.md/.adocitskips fenced, indented, inline, and AsciiDoc delimited code so documented
samples are never altered.
non-restrictive markers (parenthesis- or dash-introduced), any preposition +
"which", interrogative/determiner (including embedded), sentence-initial
"Which", hyphenated identifiers (e.g.
which-fixer), and the fixed phrases"that which" / "which is which". An ambiguous case is left unchanged and
reported under
Skipped[]— a missed fix is preferred over a wrong one.Rollout
Run
which-fixeronce per repo for the initial sweep; the recorded marker —kept permanently in
.agents/memory/— then keeps later invocations inincremental mode. Run it again on each branch afterward to catch new
occurrences in changed comments and docs.
Files
skills/which-fixer/SKILL.md— the skill definition.skills/which-fixer/agents/openai.yaml— Codex/OpenAI interface metadata.Verification
correctness, grammar, US-spelling consistency, false-positive coverage):
HAS FINDINGS → HAS FINDINGS → CLEAN; subsequent Copilot and Codex review
comments were addressed in the
Address review commentscommits.author-skillchecklist passes: directory name matchesname, description583 chars (< 1024), 180 lines (< 500),
$which-fixerprompt present, notask-plan references.
"which".