Add agent guidance, review rules, and a comment-hygiene gate

.claude/skills/code-comments/SKILL.md

@@ -0,0 +1,75 @@
+---
+name: code-comments
+description: MUST use before writing or editing any comment in this repository - content rules, budget, width.
+---
+
+# Machine code comments
+
+Before you write a comment, delete it. If the code and its names already say it,
+it was noise. What survives explains a contract, an invariant, a compatibility
+requirement, a performance tradeoff, or a non-obvious reason.
+
+Say WHAT the code guarantees and WHY, in the present tense. Do not narrate HOW
+it works - the comment should survive an equivalent rewrite. A member summary
+describes that member's own contract, not its caller's.
+
+Match the file you are editing. Placement, density, and idiom are local here;
+read what is already there before you add to it, and use the terms it uses.
+
+## Never write these
+
+`scripts/comment-hygiene.ps1` fails on them over the lines your branch adds:
+
+1. **Process framing** - `Phase 1`, `later we'll`, `we'll eventually`.
+2. **History** - `it used to`, `previously returned`, `was removed`,
+   `renamed from`, `no longer used`.
+3. **Provenance** - `extracted from`, `shared by X and Y`, `the only caller`.
+4. **Pointers** - to a Markdown file, a numbered section, a review note, or
+   another file's comment.
+5. **Non-ASCII punctuation** - use `--`, `->`, `...`, `-`, `x`, plain quotes.
+   Typography only; comment text may use any script the language data needs.
+
+Present tense about current state is not history: "Returns null when the stratum
+has no rules" is a contract. A compatibility note about behavior that must stay
+true is welcome, as is an issue reference that is part of the contract.
+
+## Fit the budget
+
+One block - a run of whole-line `//` or `#` comments, ended by a blank line,
+code, or a doc comment - gets **200 characters total**, markers and indentation
+excluded. Every line fits **120 display columns**.
+
+`///` blocks and PowerShell block comments are exempt from the budget, not from
+the content rules or the width limit.
+
+Over budget? Shorten it, or let the code express it directly. Do not convert a
+`//` block to `///` to buy the exemption.
+
+The tree already holds comments over budget. They are known debt, not a
+convention: do not copy them, and do not sweep them either. Shorten one when you
+are already changing the code it describes.
+
+## XML documentation
+
+Prefer none. An undocumented public type is the norm here, several projects use
+`///` nowhere at all, and HermitCrab's heavier use is not a model to copy.
+`SIL.Machine.Morphology.HermitCrab`, `SIL.Machine.Tokenization.SentencePiece`,
+and `SIL.Machine.Translation.TensorFlow` do not set `GenerateDocumentationFile`
+at all, so a `///` block there ships nothing to a consumer.
+
+Write one only when a caller needs a contract the signature cannot state: units,
+nullability, ownership, an exception they must handle. Then one `<summary>`
+above the member. Omit `<param>` and `<returns>` that only restate a name or
+type; keep them for real result semantics. Document every parameter or none. No
+file headers, no divider comments, no fact repeated in both the summary and the
+parameters.
+
+A test comment explains a non-obvious fixture or setup constraint. It does not
+restate the test name.
+
+## Run the check
+
+`pwsh ./scripts/comment-hygiene.ps1` scans the lines your branch adds. Add
+`-Full -Advisory` to size existing debt, or `-SelfTest` to check the rules
+themselves. Agents run `./local_check.sh --agent-strict`, which makes the scan
+blocking; the pull request check is advisory, so its green tick proves nothing.

.claude/skills/commit-messages/SKILL.md

@@ -0,0 +1,26 @@
+---
+name: commit-messages
+description: How to write a commit message in sillsdev/machine - imperative subject, short body.
+---
+
+# Commit messages
+
+Name the change in an imperative, sentence-case subject under about 72
+characters, with no terminal punctuation:
+
+- `Fix bug in MergeEquivalentAnalyses (#493)`
+- `Port changes from sillsdev/machine.py#336 (#498)`
+
+A body, when there is one: blank line after the subject, wrapped at about 80
+columns, saying what changed and why. Reference a GitHub issue when one exists.
+
+## Two traps in the history
+
+1. The `(#N)` suffix is added by GitHub when a pull request is squashed. Never
+   type it into a local commit.
+2. Older commits carry Jira identifiers such as `LT-22605`. That is historical;
+   use a GitHub issue reference.
+
+The 72-character limit is not enforced and longer subjects exist. Do not rewrite
+shared history to satisfy it, or to fix a message on a pushed branch - add a
+corrective commit unless the author asks for the rewrite.

.claude/skills/issue-authoring/SKILL.md

@@ -0,0 +1,58 @@
+---
+name: issue-authoring
+description: How to write a GitHub issue in sillsdev/machine - one symptom, short body, real evidence.
+argument-hint: Optional issue type, title, symptoms, acceptance criteria, or source PR
+user-invocable: true
+---
+
+# Writing a machine issue
+
+Search open and recently closed issues first, and say what you searched. Then
+write the title and the three-sentence lede; the form fields hold the rest.
+
+GitHub issues are the tracker here. An `LT-` reference is an external link, and
+only when someone supplied it.
+
+## 1. Title: one symptom
+
+Under about 70 characters, in the reader's words. No "investigate", no
+"improve", no component prefix the labels already carry.
+
+Bad: *Tokenizer improvements*
+Good: *USFM attribute is dropped when the locale is tr-TR*
+
+## 2. Lede: three sentences
+
+1. **The symptom** - what goes wrong.
+2. **The trigger** - the smallest condition that produces it.
+3. **The cost** - who is blocked, or what the caller sees instead.
+
+Under 25 words each. For a feature, the same three: what is missing, when it
+bites, what it costs.
+
+Good: *A USFM attribute is dropped when the tokenizer runs under tr-TR. Any
+marker containing an ASCII `i` splits at the wrong index on a Turkish locale.
+Round-tripping a Turkish project silently loses the attribute.*
+
+## 3. Body: labelled lines, never a wall
+
+Write `Unknown` where you do not know, and say how to find out.
+
+- **Bug** - affected API; version, OS, runtime; the smallest input that shows
+  it; expected vs actual; sanitized log; when it started; the test that catches
+  it.
+- **Feature** - who is blocked and by what; the proposed behavior and its
+  compatibility cost; acceptance criteria an outsider could check; non-goals.
+- **Porting** - the source PR URL, what behavior matters here, what does not.
+
+Sanitize first: no secrets, tokens, customer text, or private project data.
+
+## 4. Check it is ready
+
+Ready means another maintainer can reproduce the bug, judge the acceptance
+criteria, or find the change to port - without asking you a question.
+
+A workflow files the porting issue after a merge, marked `AUTO-GENERATED-ISSUE`.
+Do not write a second one by hand.
+
+Hand back the title, labels, and body. The author decides whether to publish.

.claude/skills/pr-authoring/SKILL.md

@@ -0,0 +1,86 @@
+---
+name: pr-authoring
+description: How to write a pull request in sillsdev/machine - strong lede, short body, honest evidence.
+argument-hint: Optional branch purpose, issue number, or PR number
+user-invocable: true
+---
+
+# Writing a machine PR
+
+Write the body to a file, then `gh pr edit <n> --body-file`. Do not push, open,
+or edit a PR unless the author asked.
+
+What to check and what to run is in `AGENTS.md` and `docs/review/`. This is the
+write-up.
+
+## 1. Write the lede
+
+Three sentences, each with a different job, before anything else:
+
+1. **What it does** - what a caller can now do, or what stopped being broken.
+2. **The first unknown, answered** - usually "what breaks?" or "why so big?"
+3. **The boundary** - what it does not touch.
+
+Under 25 words each. A sentence needing a subordinate clause belongs in the
+body.
+
+Bad: *This PR refactors the tokenizer and adds some tests.*
+
+Good: *USFM markers now split identically under tr-TR, where the attribute used
+to be dropped. No public signature changes - the fix is one comparison, from
+culture-aware to ordinal. Nothing outside `UsfmTokenizer` is touched.*
+
+Invisible to callers? Lead with what it protects: *Agents can no longer land a
+comment that narrates its own history.*
+
+## 2. Fill the top zone
+
+Under 200 words. Drop any section that would be empty.
+
+```markdown
+## Quick summary
+<The lede. Nothing else.>
+
+## Where to look
+- <risk> -- <the test, invariant, or gate that pins it>
+
+## Deliberately not included
+- <deferred path, and what would unblock it>
+
+## Validation
+- <exact command> -- <exact result>
+
+## Issue / porting context
+<Fixes #N only for a real issue. For machine.py work, link the source PR.>
+```
+
+This mirrors `.github/PULL_REQUEST_TEMPLATE.md`. If the two ever differ, the
+template is what contributors actually see; fix this to match it.
+
+## 3. Put the reasoning below the rule
+
+Everything longer goes under a `---`, in closed `<details>` blocks: *Reading
+this a year from now*, *Decisions, and why*, *Paths not taken*, *Deferred, and
+what would unblock it*. Long reasoning is welcome there and nowhere above.
+
+No preamble, no apology, no "should be fine", no recap.
+
+## 4. Check the claims
+
+Every count, path, type, and test name must match the tree. A wrong number in a
+PR body outlives the PR.
+
+Validation lines carry the command and its result, nothing else. Never list a
+command you did not run. Never call a local run CI-equivalent - CI collects
+coverage and `local_check.sh` does not. Name any check you skipped.
+
+## Replying to review comments
+
+Reply in the thread, on the line, in two or three sentences. Classify first:
+
+- **Fix** - sound and unambiguous; make the smallest change.
+- **Clarify** - ask the one specific question.
+- **Reply only** - state the verified behavior; change nothing.
+- **Defer** - name the follow-up and why it is outside this PR.
+
+Resolve only a thread that is fully answered and that you did not dispute.

.claude/skills/pr-review/SKILL.md

@@ -0,0 +1,70 @@
+---
+name: pr-review
+description: How to write up a code review in sillsdev/machine - short line comments, evidence, severity.
+argument-hint: "[optional PR number, branch, or review focus]"
+user-invocable: true
+---
+
+# Writing a machine review
+
+Post one short comment per finding, anchored on the line it is about, then one
+summary comment. A review is read-only: do not edit, commit, push, or resolve
+threads.
+
+What to look for is in `docs/review/`; `AGENTS.md` maps a changed path to its
+rules file.
+
+## 1. One finding, one comment
+
+Anchor it on the line. Two problems on one line are two comments. A reviewer
+scrolling the diff should meet each point where it applies.
+
+## 2. Lead with the claim
+
+First sentence names the defect. Evidence second, fix third, if it fits.
+
+```
+Ordinal comparison missing: `marker.IndexOf(":")` is culture-sensitive, so
+tr-TR splits this marker differently. Pass `StringComparison.Ordinal`.
+```
+
+Three lines is long. A finding needing more is a design question - raise it in
+the summary instead.
+
+## 3. Label the severity
+
+**Critical** blocks merge, then **Important**, then **Minor**. Critical means
+demonstrated: a failing command, a broken contract, a missing gate. A worry is
+not Critical.
+
+## 4. Carry the evidence
+
+Every comment gets a `path:line` and a consequence. Mark what you did not
+confirm `Unverified`; an unverified concern never blocks a merge.
+
+- Do not report pre-existing issues the diff does not touch.
+- Do not ask for a migration, modernization, or benchmark the diff gave no
+  reason for.
+- A search that found nothing proves absence only if you state what you
+  searched.
+- Name the commands you ran and what they returned. `./local_check.sh` is the
+  full local sequence; an agent-authored branch also needs `--agent-strict`, and
+  a green advisory `Comment hygiene` check does not stand in for it.
+- A coverage percentage is not evidence that a changed line is tested.
+
+## 5. Close with five lines
+
+1. Verdict: approve, approve with fixes, or request changes.
+2. The one thing that matters most, with its `path:line`.
+3. Counts by severity.
+4. What you ran, and its result.
+5. What you could not verify.
+
+Say which public API, target framework, package, or parity contract changed, or
+`None verified`.
+
+Then mark each finding **changed**, **accepted**, or **unverified**. Of 140
+review threads here in three years, 128 have no follow-up, so nobody can tell
+which findings mattered. Leave nothing implicit.
+
+For an adversarial second pass, apply `docs/review/devils-advocate.md`.

.github/ISSUE_TEMPLATE/bug_report.yml

@@ -0,0 +1,68 @@
+name: Bug report
+description: Report a reproducible problem in SIL Machine
+title: "[Bug]: "
+labels:
+  - bug
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Please remove secrets and private project data from examples and logs.
+  - type: textarea
+    id: summary
+    attributes:
+      label: Summary
+      description: >-
+        Three short sentences: the symptom, the smallest trigger, and what it
+        costs. Detail goes in the fields below.
+      placeholder: |
+        A USFM attribute is dropped when the tokenizer runs under tr-TR.
+        Any marker containing an ASCII i splits at the wrong index on a Turkish locale.
+        Round-tripping a Turkish project silently loses the attribute.
+    validations:
+      required: true
+  - type: input
+    id: package
+    attributes:
+      label: Affected package or API
+      description: Name the SIL.Machine package, project, or public API.
+    validations:
+      required: true
+  - type: input
+    id: version
+    attributes:
+      label: Version or commit
+      description: Include the package version or commit SHA.
+    validations:
+      required: true
+  - type: input
+    id: environment
+    attributes:
+      label: Environment
+      description: OS, architecture, and .NET runtime.
+    validations:
+      required: true
+  - type: textarea
+    id: reproduction
+    attributes:
+      label: Reproduction
+      description: Minimal input/fixture and exact steps, including frequency.
+      placeholder: |
+        1. ...
+        2. ...
+        Expected: ...
+        Actual: ...
+    validations:
+      required: true
+  - type: textarea
+    id: logs
+    attributes:
+      label: Logs or exception
+      description: Paste sanitized output, or write None.
+  - type: textarea
+    id: regression
+    attributes:
+      label: Regression and test idea
+      description: State the first known good version, if known, and the smallest regression test.
+    validations:
+      required: true

.github/ISSUE_TEMPLATE/config.yml

@@ -0,0 +1 @@
+blank_issues_enabled: true